Skip to content

Commit 13a0dee

Browse files
committed
Merge origin/main into fix/domain-critic-loads-agent-definition
Resolve CHANGELOG.md: main released code-review v3.7.1 (#194), so this branch's entry moves to v3.7.2 above it and plugin.json bumps to match. Without the bump the plugin-version-bump CI job fails against the new merge base.
2 parents 0bb4f9e + c50e3a4 commit 13a0dee

12 files changed

Lines changed: 360 additions & 37 deletions

File tree

CHANGELOG.md

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,30 @@ All notable changes to the claude-plugins project will be documented in this fil
44

55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`.
66

7-
### code-review v3.7.1
7+
### code-review v3.7.2
88

99
#### Fixed
1010
- **A project-declared domain critic now loads its own `.claude/agents/<critic-name>.md`.** Domain critics spawn as the generic `code-review:code-review-worker` and receive only their name as a quoted `CRITIC_DOMAIN` string, so a project that defines the critic's entire method in an agent file of the same name got none of it — the only context-loading line in the domain critic prompt was the unranked "Read the repository CLAUDE.md for project context", which sits after the hard `FIRST…THEN…` block. Across three real `/code-review` runs in a consuming repo (`cr-51875`, `cr-95074`, `cr-97905`), zero of twelve spawned workers obeyed that line; the one critic that did read project doctrine got there by spontaneously grepping its own domain token, and self-describing critic names (`api-architect`, `auth-security-expert`) never self-grep at all. The critic still ran and still emitted plausible findings, with nothing in the output artifact recording that its definition was never loaded. `derive-spawn-spec` now resolves `.claude/agents/<critic-name>.md` and, when the file exists, puts its path on the domain critic descriptor as `agent_definition_file`; `cmd_route` does the same for the fast path's PASS 3 under `route.domain_critic_definitions` (both keys are omitted entirely when no such file exists). The `spawn-reviewers` skill turns that path into a non-negotiable first step in the critic's prompt — read the definition before the patches file, follow it in full, and say so in the output if the read fails. A critic with no agent file, which is the common case, produces a byte-identical descriptor, routing payload, and prompt to before. Filename convention only: a definition whose frontmatter `name` differs from its filename is not resolved, and symlinks and non-regular files are refused (the pipeline reviews untrusted checkouts).
1111

12+
### code-review v3.7.1
13+
14+
#### Fixed
15+
- Override-cache tests no longer depend on the wall-clock date. Three tests pinned an override's `asserted_at` to a fixed `2026-05-29` timestamp and asserted the override was honored — by `verify-prepare` short-circuiting into `override_hits`, and by the prepare-then-consolidate paths that route an override to `RE_ASSERTED`. Once that fixed timestamp aged past the 90-day `overrides` cache TTL, the overrides were correctly treated as expired and the three tests began failing with no accompanying code change. They now derive `asserted_at` relative to the current time, so they exercise the honored-override path regardless of when the suite runs.
16+
17+
#### Changed
18+
- Test helper `_stale_cached_at()` renamed to `_iso_days_ago()` so the name matches its documented behavior (an ISO-8601 timestamp N days in the past, used for both within-TTL and past-TTL fixtures); all call sites updated, and the two override TTL boundary tests now delegate to it instead of inlining the same `datetime.now(timezone.utc) - timedelta(...)` expression.
19+
20+
### code v1.14.10
21+
22+
#### Added
23+
- Contract test suite `test_decision_table_skill_contract.py` pinning the new decision-table skill guarantees: `Not aligned` as a terminal workflow stop, required coexisting-source interaction and precedence rows, shared-corpus production-boundary parity for executable twins, and row-ID-to-test traceability in `Required Tests`.
24+
25+
#### Changed
26+
- Hardened the `decision-table` skill's alignment gate: `Final Alignment Status: Not aligned` is now a terminal workflow stop, not a report-only status. No PR creation, merge, completion signal, or success closeout may follow until the artifact is re-verified as `Aligned`; fixable repo-local findings remain unresolved work and cannot be normalized into a successful handoff. The human-facing closeout for a `Not aligned` run must state that downstream PR/merge/completion is blocked, name every unresolved blocker, and give the exact next action and owner. A matching common-miss item ("Not-aligned status treated as success") and a contract-heavy review bullet enforce the stop during review-prevention passes.
27+
- Decision rows now carry stable row IDs, and `Required Tests` is a traceability table instead of a prose list. The `Current Code` and `Intended Change` tables gain a `Row ID` column, and each required test maps to its decision-row IDs with a positive control, a wrong-input or mixed-state negative case, the real production boundary exercised, and expected evidence. Every material row must be covered by at least one mapped test or carried into `Not aligned` with a blocker; a test listed without row IDs or without the negative case needed to distinguish the row's branch is a named review-prevention anti-pattern.
28+
- New "Executable policy twins and parity" edge-case category: when one behavior or policy has multiple executable twins (a pure helper, SQL predicate, route, worker, producer, batch path, or recovery path), one shared scenario corpus must exercise every twin through its real production boundary and assert identical decisions. Source-string, AST-presence, and SQL-shape assertions are supplemental only and never establish parity, and a negative shape assertion that requires a policy predicate, identity term, join, or branch to be absent is flagged as pinning missing policy: if it permits divergence, the row is `Not aligned` until corrected. Two new review-prevention anti-patterns cover parity inferred from separate tests and shape assertions that pin missing policy.
29+
- New "Coexisting sources and precedence" edge-case category: when multiple evidence, authority, history, cache, or fallback sources can coexist, singleton rows are insufficient. The table must add a bounded interaction set (pairwise plus code-identified high-risk intersections, never an unbounded Cartesian product) covering legacy or absent evidence alongside fresh valid evidence, corrupt or undated evidence alongside fresh valid evidence, irrelevant historical evidence alongside a current authoritative record, tied or conflicting current records, and source/state precedence, stating which source wins and why. Singleton-only source coverage is a named review-prevention anti-pattern.
30+
1231
### platform v1.1.4
1332

1433
#### Fixed

plugins/code-review/.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code-review",
33
"description": "Code review plugin",
4-
"version": "3.7.1",
4+
"version": "3.7.2",
55
"author": {
66
"name": "ClosedLoop",
77
"email": "support@closedloop.ai"

plugins/code-review/README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,14 @@ plugins/code-review/
4848
python/code_review_schema.py Canonical Finding + ResultEnvelope schema + validators (PLN-719)
4949
python/test_code_review_schema.py Schema tests + round-trips
5050
python/code_review_helpers.py Deterministic helper CLI (parse-diff, hygiene, partition, route, validate, cache, finalize-result, arbitrate-budget, prepare-run, etc.)
51+
python/config/cli.json Declarative argparse spec the helper CLI builds its subparsers from
52+
python/config/stages.json Declarative stage table backing `prepare-run`'s `run_plan.json` and `run-prefix`
53+
python/signal_taxonomy.json Signal taxonomy loaded by signal extraction; its bytes are hashed into the extraction cache key
54+
python/conftest.py Shared pytest fixtures and finding factories for the co-located tests
5155
python/test_code_review_helpers.py Unit tests for the helper CLI
5256
python/golden_fixture_harness.py Golden fixture harness: replays canonical inputs through helper subcommands and diffs against expected envelopes (PLN-719 Phase 8)
5357
python/test_golden_fixtures.py Pytest driver that runs every fixture under tools/python/fixtures/
54-
python/fixtures/<name>/ Per-fixture directory (config.yaml + inputs/ + expected/); 3 full scenarios + 6 README-stubs for future coverage
58+
python/fixtures/<name>/ Per-fixture directory (config.yaml + inputs/ + expected/); 4 full scenarios + 3 README-stubs for future coverage
5559
python/prefix_golden_harness.py Prefix golden harness + subprocess A/B parity oracle: walks the deterministic prefix against real git fixtures — in-process for golden snapshots, and per-stage-subprocess vs `run-prefix` for byte-equal parity (PLN-1229 Phase 0/1)
5660
python/test_prefix_golden.py Pytest driver for the prefix harness: determinism oracle + golden diff across the prefix_fixtures/ matrix
5761
python/prefix_fixtures/<name>/ Per-fixture directory (expected/ golden snapshots); 7 branch scenarios (standard, fast-path, hygiene-only, empty-diff, cache-hit, since-last-review, coverage-critic)

plugins/code-review/tools/python/test_code_review_helpers.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@
108108
# A `cached_at` timestamp always within the BHA cache TTL (30 days).
109109
# PLN-719 Phase 7 added sweep-on-read TTL eviction; fixtures that want a hit
110110
# must use a fresh timestamp. Tests that want eviction behavior should use
111-
# an explicitly-stale timestamp via ``_stale_cached_at()``.
111+
# an explicitly-stale timestamp via ``_iso_days_ago()``.
112112
_FRESH_CACHED_AT = datetime.now(timezone.utc).isoformat()
113113

114114

115-
def _stale_cached_at(days_ago: int = 365) -> str:
115+
def _iso_days_ago(days_ago: int = 365) -> str:
116116
"""Return an ISO timestamp ``days_ago`` days in the past (default: 1 year)."""
117117
return (datetime.now(timezone.utc) - timedelta(days=days_ago)).isoformat()
118118

@@ -3385,14 +3385,14 @@ def test_stale_entry_within_ttl_hits(self, tmp_path: Path) -> None:
33853385
from code_review_helpers import _is_entry_fresh, CACHE_NAMESPACE_BHA
33863386

33873387
# 29 days old: under the 30-day BHA TTL.
3388-
entry = {"cached_at": _stale_cached_at(days_ago=29)}
3388+
entry = {"cached_at": _iso_days_ago(days_ago=29)}
33893389
assert _is_entry_fresh(entry, CACHE_NAMESPACE_BHA) is True
33903390

33913391
def test_stale_entry_past_ttl_misses(self, tmp_path: Path) -> None:
33923392
from code_review_helpers import _is_entry_fresh, CACHE_NAMESPACE_BHA
33933393

33943394
# 31 days old: past the 30-day BHA TTL.
3395-
entry = {"cached_at": _stale_cached_at(days_ago=31)}
3395+
entry = {"cached_at": _iso_days_ago(days_ago=31)}
33963396
assert _is_entry_fresh(entry, CACHE_NAMESPACE_BHA) is False
33973397

33983398
def test_missing_cached_at_treated_as_fresh(self, tmp_path: Path) -> None:
@@ -3406,7 +3406,7 @@ def test_missing_cached_at_treated_as_fresh(self, tmp_path: Path) -> None:
34063406
def test_unknown_namespace_skips_ttl_check(self, tmp_path: Path) -> None:
34073407
from code_review_helpers import _is_entry_fresh
34083408

3409-
entry = {"cached_at": _stale_cached_at(days_ago=365 * 10)}
3409+
entry = {"cached_at": _iso_days_ago(days_ago=365 * 10)}
34103410
assert _is_entry_fresh(entry, "future-namespace") is True
34113411

34123412
def test_v1_cache_check_evicts_stale_entry(self, tmp_path: Path) -> None:
@@ -3425,7 +3425,7 @@ def test_v1_cache_check_evicts_stale_entry(self, tmp_path: Path) -> None:
34253425
"prompt_hash": "abc123",
34263426
"patch_hash": patch_hash,
34273427
"findings": [{"file": "a.ts", "line": 1, "issue": "stale"}],
3428-
"cached_at": _stale_cached_at(days_ago=45),
3428+
"cached_at": _iso_days_ago(days_ago=45),
34293429
}
34303430
}
34313431
_write_manifest(cache_dir, manifest)
@@ -3443,7 +3443,7 @@ def test_v2_cache_check_evicts_stale_entry(self, tmp_path: Path) -> None:
34433443
patch_hash = _compute_patch_hash("a.ts", diff_data["patch_lines"]["a.ts"])
34443444
composite = _compute_composite_key("opus", "abc123", patch_hash, "ctx")
34453445

3446-
stale = _stale_cached_at(days_ago=45)
3446+
stale = _iso_days_ago(days_ago=45)
34473447
v2_manifest = {
34483448
"a.ts": {
34493449
composite: {
@@ -10133,7 +10133,7 @@ def test_verify_prepare_short_circuits_on_valid_override(
1013310133
"finding_id": "bha_p0_f0",
1013410134
"file_content_hash": _file_content_hash(cr, "src/x.py", 3),
1013510135
"override": "RE_ASSERT",
10136-
"asserted_at": "2026-05-29T22:00:00+00:00",
10136+
"asserted_at": _iso_days_ago(days_ago=1),
1013710137
})
1013810138
# PR #114 review fix — delegate to the shared helper with an
1013910139
# explicit cr_dir override so the per-test stdout/Namespace dance
@@ -11555,7 +11555,7 @@ def test_prepare_then_consolidate_routes_override_to_verified(
1155511555
"finding_id": "bha_p0_f0",
1155611556
"file_content_hash": _file_content_hash(cr, "src/x.py", 3),
1155711557
"override": "RE_ASSERT",
11558-
"asserted_at": "2026-05-29T22:00:00+00:00",
11558+
"asserted_at": _iso_days_ago(days_ago=1),
1155911559
})
1156011560

1156111561
# Phase 1 — prepare. Should record the fid in override_hits and
@@ -11604,7 +11604,7 @@ def test_prepare_then_consolidate_writes_re_asserted_to_stats(
1160411604
"finding_id": "bha_p0_f0",
1160511605
"file_content_hash": _file_content_hash(cr, "src/x.py", 3),
1160611606
"override": "RE_ASSERT",
11607-
"asserted_at": "2026-05-29T22:00:00+00:00",
11607+
"asserted_at": _iso_days_ago(days_ago=1),
1160811608
})
1160911609

1161011610
_, manifest = _run_verify_prepare(
@@ -11736,7 +11736,7 @@ def test_override_invalidated_when_ttl_expired(
1173611736
self._write_target_file(tmp_path, "src/x.py", "a\nb\nc\nd\ne\n")
1173711737
cache = tmp_path / "cache"
1173811738
cache.mkdir()
11739-
old_ts = (datetime.now(timezone.utc) - timedelta(days=120)).isoformat()
11739+
old_ts = _iso_days_ago(days_ago=120)
1174011740
_write_override(cache, {
1174111741
"finding_id": "bha_p0_f0",
1174211742
"file_content_hash": _file_content_hash(cr, "src/x.py", 3),
@@ -11762,7 +11762,7 @@ def test_override_honored_when_ttl_within_bounds(
1176211762
self._write_target_file(tmp_path, "src/x.py", "a\nb\nc\nd\ne\n")
1176311763
cache = tmp_path / "cache"
1176411764
cache.mkdir()
11765-
recent_ts = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat()
11765+
recent_ts = _iso_days_ago(days_ago=30)
1176611766
_write_override(cache, {
1176711767
"finding_id": "bha_p0_f0",
1176811768
"file_content_hash": _file_content_hash(cr, "src/x.py", 3),

plugins/code/.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code",
33
"description": "Code and planning framework plugin",
4-
"version": "1.14.8",
4+
"version": "1.14.10",
55
"author": {
66
"name": "ClosedLoop",
77
"email": "support@closedloop.ai"

0 commit comments

Comments
 (0)