diff --git a/.claude/agents/engine-implementation-executor.md b/.claude/agents/engine-implementation-executor.md index 8c356e28e6..7d3e4894c8 100644 --- a/.claude/agents/engine-implementation-executor.md +++ b/.claude/agents/engine-implementation-executor.md @@ -17,14 +17,13 @@ The orchestrator gives you: 2. The reviewed plan (every section: Pattern Coverage, Building Blocks, Logic Placement, Rust Idioms, Nom Compliance, Extension vs Creation, Analogous Trace, step-by-step file changes). 3. `BASE_SHA`; for `implementation/fix`, named `START_SHA` and `IMPLEMENTATION_WORKTREE`; for `measurement-only`, immutable `CANDIDATE_SHA` and the named `IMPLEMENTATION_WORKTREE` too. 4. Frozen in-/out-of-bounds scope paths as a duplicate-free `LC_ALL=C sort -z` NUL-delimited representation and its SHA256; for measurement-only, clean detached base/candidate projection worktrees. -5. The externally owned canonical receipt path, read-only pinned data root containing `AtomicCards.json`, and completion evidence destinations. The receipt is the only provenance contract; do not create a manifest, seal, provenance envelope, replica/quorum report, or parallel parser ledger. 6. For an implementation/fix round, any reviewer findings as constraints. Mode is a hard boundary: -- **`implementation/fix`:** First verify and report `IMPLEMENTATION_WORKTREE` as clean with `HEAD == START_SHA` and no staged entries. The initial executor has `START_SHA == BASE_SHA`; every fix executor has the prior reviewed `CANDIDATE_SHA` as `START_SHA`, never a moving head. After surgical edits, report only **PREPARATORY** checks and the required end-of-edit stable-HEAD check (`HEAD == START_SHA`, no executor staging, exact authorized unstaged delta). Do not create a candidate commit, receipt, or completion claim. -- **`measurement-only`:** Make no source edits, formatting edits, or commits. Its only successful result is `MEASURED`; every non-success is `CANNOT_ANSWER`. In particular, an identity, detached-state, clean-state, command, source-hash, authority-diff, projection, receipt, or artifact/digest failure must end the run as `CANNOT_ANSWER`; retain any completed records but do not claim parser evidence. First prove the supplied base/candidate worktrees are detached, clean, and at their expected SHAs. Execute `scripts/engine-source-hash.sh` in each respective worktree and the exact NUL-safe four-path `BASE_SHA..CANDIDATE_SHA` authority diff from the receipt contract, then write the bound records. Skip projection only when source hashes are equal **and** that authority-diff artifact is empty; otherwise record the canonical non-`NONE` `projection_forced_reason` and produce exactly one direct base projection and one direct candidate projection against the same pinned `AtomicCards.json`, followed by one base-built comparator invocation. Record every action in the canonical receipt. +- **`implementation/fix`:** First verify and report `IMPLEMENTATION_WORKTREE` as clean with `HEAD == START_SHA` and no staged entries. The initial executor has `START_SHA == BASE_SHA`; every fix executor has the prior reviewed `CANDIDATE_SHA` as `START_SHA`, never a moving head. After surgical edits, report only **PREPARATORY** checks and the required end-of-edit stable-HEAD check (`HEAD == START_SHA`, no executor staging, exact authorized unstaged delta). Do not create a candidate commit or a completion claim. +- **`measurement-only`:** Make no source edits, formatting edits, or commits. Answer one question: does this change move parser output? First confirm the supplied base/candidate worktrees are detached, clean, and at their expected SHAs. Run `scripts/engine-source-hash.sh` in each. If the hashes match and nothing in `Cargo.toml`, `.cargo/config.toml`, `rust-toolchain.toml`, or `scripts/engine-source-hash.sh` changed between the two, there is no parse-affecting change and you are done. Otherwise build the tooling on each side, generate card data from each against the same pinned data root, run the comparator, and report what actually differs. If you cannot complete the measurement, say so plainly and do not claim parser evidence. ### Phase mode (spawn-input overlay on `implementation/fix`) @@ -35,17 +34,7 @@ When the orchestrator's spawn inputs include a phase charter, a phase index, and 3. **Discriminating-test gate:** a changed behavioral seam whose discriminating test is charter-deferred (the defining property of an infrastructure→consumer seam) records `DEFERRED(phase n)` in the production-path coverage map instead of stop-and-returning. The phase's *own* chartered discriminating test(s) and structural verification (green tree, existing suites, unit-level assertions) remain mandatory, and "if any changed behavioral seam has no mapped production-path test, add one or return it as a stop-and-return item" still applies to every seam that is **not** charter-deferred. 4. **New-field threading sweep:** needs no new vocabulary — its existing `defaults intentionally because ` status absorbs chartered deferral with the charter as the reason: `defaults intentionally because DEFERRED(phase n)`. -Everything else — the receipt contract, multi-agent safety, nom mandate, CR verification, preparatory verification blocks, and the stable-HEAD rules — is unchanged in phase mode. - -### Canonical receipt contract - -The un-hashed receipt is UTF-8/LF with exactly one final LF and no CR/NUL. Percent-encode every UTF-8 byte except `[A-Za-z0-9._~-]` with uppercase hex. The frozen scope representation is duplicate-free and made exactly by `printf '%s\0' "${SCOPE_PATHS[@]}" | LC_ALL=C sort -z`; record its path and the SHA256 of its exact bytes. Record the exact canonical scoped diff command: - -```bash -git -C "$IMPLEMENTATION_WORKTREE" -c color.ui=false -c diff.noprefix=false -c core.quotepath=true -c diff.orderFile=/dev/null -c diff.interHunkContext=0 -c diff.suppressBlankEmpty=false diff --no-color --no-ext-diff --no-textconv --no-renames --diff-algorithm=myers --no-indent-heuristic --full-index --binary --src-prefix=a/ --dst-prefix=b/ --unified=3 "$BASE_SHA" "$CANDIDATE_SHA" -- "${SCOPE_PATHS[@]}" -``` - -Write the fixed fields, then the mandatory `source_hash_record.base.*` and `source_hash_record.candidate.*` groups, and then all indexed groups in the exact order defined by `/engine-implementer`: no duplicate/missing keys, unordered index, or partial group is valid. Each source-hash group contains, in order, `command`, `expected_sha`, `head_before`, `detached_before`, `clean_before`, `head_after`, `detached_after`, `clean_after`, `exit`, `stdout_path`, `stdout_sha256`, `stderr_path`, `stderr_sha256`, `artifact_path`, `artifact_size`, and `artifact_sha256`. Each group records its exact `scripts/engine-source-hash.sh` command and SHA-bound output artifact. `detached_before` and `detached_after` each prove `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; any other result makes the measurement result `CANNOT_ANSWER`, not detached-state evidence. The source-hash groups and `projection_authority_diff_*` fields are mandatory even when the hashes are equal. Run exactly `git -C "$IMPLEMENTATION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- Cargo.toml .cargo/config.toml rust-toolchain.toml scripts/engine-source-hash.sh`, capture its NUL-delimited stdout as the recorded path/size/SHA256 artifact, and set `projection_forced_reason` to `NONE`, `SOURCE_HASH_DIFFERENCE`, `EXCLUDED_PROJECTION_AUTHORITY_INPUT_CHANGED`, or `SOURCE_HASH_DIFFERENCE_AND_EXCLUDED_PROJECTION_AUTHORITY_INPUT_CHANGED` according to the two results. Only `NONE` permits no projection section. Record every artifact path, size, and SHA256, but never record or hash the receipt itself; receipt validation is an external reviewer/final-acceptance gate, never a `completion_check` row or receipt artifact. Any malformed receipt or failed identity/digest check is `CANNOT_ANSWER`. +Everything else — multi-agent safety, nom mandate, CR verification, preparatory verification blocks, and the stable-HEAD rules — is unchanged in phase mode. ## Hard Rules @@ -184,11 +173,10 @@ For parser changes always run additionally as preparatory checks: ### Measurement-only mode: parser evidence -Execute `scripts/engine-source-hash.sh "$BASE_SHA"` in the detached base projection worktree and `scripts/engine-source-hash.sh "$CANDIDATE_SHA"` in the detached candidate projection worktree, binding each output to its SHA through the required `source_hash_record.base` / `.candidate` receipt groups, then run and record exactly `git -C "$IMPLEMENTATION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- Cargo.toml .cargo/config.toml rust-toolchain.toml scripts/engine-source-hash.sh`. This NUL-delimited artifact is the excluded projection-authority input check. The source-hash groups and authority-diff record remain mandatory when values are equal; only equal hashes with an empty authority-diff artifact set `projection_forced_reason=NONE`, write `NO_PARSE_AFFECTING_CHANGE`, omit the projection section, and never invoke a parser tool. A source-hash difference and/or any authority-diff path sets its canonical non-`NONE` reason and forces `PROJECTED_PARSE_DIFF`. +Run `scripts/engine-source-hash.sh "$BASE_SHA"` in the detached base worktree and `scripts/engine-source-hash.sh "$CANDIDATE_SHA"` in the detached candidate worktree, then `git -C "$IMPLEMENTATION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- Cargo.toml .cargo/config.toml rust-toolchain.toml scripts/engine-source-hash.sh`. Equal hashes with an empty authority diff mean no parse-affecting change: report that and run no parser tool. Otherwise project both sides. -For any non-`NONE` `projection_forced_reason`, pin the read-only `AtomicCards.json` input once and use it directly for both detached worktrees. For each side, execute in that side's detached worktree, set its own receipt-owned `CARGO_TARGET_DIR`, and run exactly `cargo build --profile tool --features cli --bin oracle-gen --bin coverage-report --bin coverage-parse-diff`. Run that side's `oracle-gen` and `coverage-report` directly in that worktree to create exactly one projection. Then invoke only the base-built comparator with both `--base-sha "$BASE_SHA"` and `--head-sha "$CANDIDATE_SHA"`. Each build, generator, report, and comparator has a complete indexed receipt group: command/environment, worktree and target, expected SHA, `head_before`, `detached_before`, `head_after`, `detached_after`, `clean_before`, `clean_after`, exit result, stdout/stderr paths and SHA256s, and produced artifacts. `detached_before` and `detached_after` each prove that `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; any other result is `CANNOT_ANSWER`. The artifact groups enumerate every input/output with path, size, and SHA256. A source-hash difference or excluded-authority-input change is valid; any missing/mismatched required source-hash, authority-diff, or projection record, extra projection, or operational failure is `CANNOT_ANSWER`. +When projecting, pin the read-only `AtomicCards.json` once and use it for both sides. For each side, work in that side's detached worktree with its own `CARGO_TARGET_DIR` and run `CARGO_INCREMENTAL=0 cargo build --profile tool --features cli --bin oracle-gen --bin coverage-report --bin coverage-parse-diff` (these targets are build-once; incremental state is pure disk cost — measured 17 of 28 GB on one such directory). Run that side's `oracle-gen` and `coverage-report` in that worktree, then invoke the base-built comparator with both `--base-sha "$BASE_SHA"` and `--head-sha "$CANDIDATE_SHA"`. Report what the comparator found. -On `MEASURED`, the receipt names `BASE_SHA` and `CANDIDATE_SHA`, records raw artifacts and their digests, and sets either `NO_PARSE_AFFECTING_CHANGE` or `PROJECTED_PARSE_DIFF`. On `CANNOT_ANSWER`, retain only completed records and do not claim parser evidence or completion verification; completion belongs to the orchestrator after this mode returns. ### Discriminating-test gate @@ -261,12 +249,12 @@ Any `UNVERIFIED:` line is a hard stop — the rule number does not exist in the ## Output -Return a structured report to the orchestrator. This structured report is your return value and is the contract — always emit it as your final text. It must begin with `Mode`, `BASE_SHA`, and, in implementation/fix mode, `START_SHA` and `IMPLEMENTATION_WORKTREE`; measurement-only mode also names `CANDIDATE_SHA`. Record each canonical-receipt field and artifact at creation. You also have the `SendMessage` teammate tool: use it to send the lead a brief progress update or completion notice while you work, and to acknowledge a `shutdown_request` so you can be culled gracefully instead of being tmux-pane-killed. `SendMessage` is purely additive — it never replaces this final structured report. +Return a structured report to the orchestrator. This structured report is your return value and is the contract — always emit it as your final text. It must begin with `Mode`, `BASE_SHA`, and, in implementation/fix mode, `START_SHA` and `IMPLEMENTATION_WORKTREE`; measurement-only mode also names `CANDIDATE_SHA`. You also have the `SendMessage` teammate tool: use it to send the lead a brief progress update or completion notice while you work, and to acknowledge a `shutdown_request` so you can be culled gracefully instead of being tmux-pane-killed. `SendMessage` is purely additive — it never replaces this final structured report. ### Implementation/fix output 1. **Diff summary** — files touched, grouped by subsystem, with a one-line purpose per file. -2. **Worktree record** — `START_SHA`, `IMPLEMENTATION_WORKTREE`, clean-start and stable-HEAD/end-of-edit attestations, frozen scope-path SHA256, and the canonical receipt path. State explicitly that preparatory evidence is not completion evidence. +2. **Worktree record** — `START_SHA`, `IMPLEMENTATION_WORKTREE`, clean-start and stable-HEAD/end-of-edit attestations. State explicitly that preparatory evidence is not completion evidence. 3. **PREPARATORY verification results** — which Tilt resources are green; any failures with `tilt logs` excerpts (own vs unrelated). State explicitly that this is not completion evidence. 4. **Parser preparatory gate** — pass/fail with offending lines if any. 5. **Discriminating-test gate** — the existing full production-path coverage map for every behavioral claim, including changed seam/function, production entry point, test name, revert-failing assertion, and sibling/negative cases. Explicitly list any unmapped seam as a stop-and-return item. Confirm no production-reachable arm is left covered only by a degenerate fixture. State if any test is shape-only and whether that is acceptable because semantics remain unsupported/red. @@ -280,9 +268,9 @@ Return a structured report to the orchestrator. This structured report is your r ### Measurement-only output -1. **Identity and worktree record** — `BASE_SHA`, `CANDIDATE_SHA`, `IMPLEMENTATION_WORKTREE`, detached/clean start and end checks, frozen scope-path SHA256, canonical receipt path, and all artifact paths. +1. **Identity** — `BASE_SHA`, `CANDIDATE_SHA`, and `IMPLEMENTATION_WORKTREE`. 2. **Source-hash records** — both `engine-source-hash.sh` outputs bound to their SHAs and their equality/difference result. -3. **Parser evidence** — the two SHA-bound source-hash outputs, the exact authority-diff artifact and `projection_forced_reason`, and either `NO_PARSE_AFFECTING_CHANGE` with no projection section, or the pinned `AtomicCards.json`, exact isolated tool-build records, base/candidate direct projections, base-built comparator with both SHA flags, and every indexed receipt/artifact record. +3. **Parser evidence** — whether the change moves parser output, and if so what the comparator showed. 4. **No-edit/no-commit attestation** — confirm source diff and `HEAD` did not change during measurement. 5. **Stop-and-return items, deviations, and risks** — especially any condition that makes measurement `CANNOT_ANSWER`. diff --git a/.claude/skills/engine-implementer/SKILL.md b/.claude/skills/engine-implementer/SKILL.md index aab0a8576d..4af15e8c91 100644 --- a/.claude/skills/engine-implementer/SKILL.md +++ b/.claude/skills/engine-implementer/SKILL.md @@ -23,33 +23,11 @@ This is the orchestrator for the phase.rs implementation pipeline. It runs as a The orchestrator never authors content itself. Its only jobs are: spawn agents, route their output to the next step, loop review steps until clean, own the commit, and gracefully cull each spawned agent once its output is consumed (send a `shutdown_request` and wait for the `shutdown_response` ack — spawned agents now carry `SendMessage`, so they cull gracefully instead of being pane-killed). The structured report each agent returns stays the authoritative step handoff; SendMessage is an additive progress/acknowledgment channel, not a replacement. -## Run ownership, checkpoint identity, and the canonical receipt +## Run ownership and checkpoint identity -Before dispatching an executor, the orchestrator records `BASE_SHA`, a frozen in-scope path representation, its SHA256, and an externally owned run directory. The representation is a file made exactly by `printf '%s\0' "${SCOPE_PATHS[@]}" | LC_ALL=C sort -z`, with no duplicate path records; `scope_paths_sha256` is the SHA256 of those exact NUL-delimited bytes. Run-level `BASE_SHA` never changes; in a chartered run each phase's `PHASE_BASE_SHA` is determined at phase start — it is the prior phase's accepted candidate (phase 1: run `BASE_SHA`) — and never changes, while the phase's frozen *scope* is fixed later, at that phase's scope freeze (see "Phase-fit gate and chartered runs"). The orchestrator alone stages or commits scope paths; executors never stage, commit, amend, or move `HEAD`. +Before dispatching an executor, fix `BASE_SHA` and the in-scope paths for the run; in a chartered run each phase fixes its own `PHASE_BASE_SHA` at phase start. Every implementation or fix dispatch has a named `START_SHA` and `IMPLEMENTATION_WORKTREE` — the first round starts at `BASE_SHA`, a fix round at the prior reviewed `CANDIDATE_SHA`. Check `HEAD == START_SHA` and a clean tree before edits and again before the checkpoint, and stop if the diff has escaped the intended scope. -Every implementation/fix dispatch has a named `START_SHA` and `IMPLEMENTATION_WORKTREE`. The first round has `START_SHA == BASE_SHA`; a fix round starts from the prior reviewed `CANDIDATE_SHA` in a fresh implementation worktree. Before edits and immediately before checkpoint, record that `HEAD == START_SHA`, the index has no executor staging, and the authorized unstaged delta is exactly the frozen scope. A changed `HEAD`, staged entry, unexpected path, or changed diff digest stops the run. The checkpoint commit is the sole candidate identity: explicitly stage only the frozen paths, commit only those paths, record `CANDIDATE_SHA`, and immediately prove `rev-parse HEAD == CANDIDATE_SHA`. Never measure an uncommitted tree or a moving `HEAD`. - -Keep artifacts outside all worktrees. Each checkpoint owns one un-hashed canonical receipt, for example `/engine-implementer-runs//candidates//receipt`. It is the only provenance contract: do not create manifests, seals, provenance envelopes, replica/quorum records, or a parallel ledger. - -The receipt is UTF-8 with LF line endings and exactly one final LF. It has one `key=value` line per field, no CR or NUL, and percent-encodes every UTF-8 byte except `[A-Za-z0-9._~-]` using uppercase hex. Keys appear exactly once in this fixed order: `format=engine-implementer-receipt-v1`, `base_sha`, `start_sha`, `candidate_sha`, `head_sha`, `scope_paths_path`, `scope_paths_sha256`, `scoped_diff_command`, `scoped_diff_path`, `scoped_diff_size`, `scoped_diff_sha256`, `base_source_hash`, `candidate_source_hash`, `projection_authority_diff_command`, `projection_authority_diff_path`, `projection_authority_diff_size`, `projection_authority_diff_sha256`, `projection_forced_reason`, and `parser_evidence`. `scope_paths_path` names the frozen NUL-delimited, `LC_ALL=C sort -z` representation above. `scoped_diff_command` is exactly: - -```bash -git -C "$IMPLEMENTATION_WORKTREE" -c color.ui=false -c diff.noprefix=false -c core.quotepath=true -c diff.orderFile=/dev/null -c diff.interHunkContext=0 -c diff.suppressBlankEmpty=false diff --no-color --no-ext-diff --no-textconv --no-renames --diff-algorithm=myers --no-indent-heuristic --full-index --binary --src-prefix=a/ --dst-prefix=b/ --unified=3 "$BASE_SHA" "$CANDIDATE_SHA" -- "${SCOPE_PATHS[@]}" -``` - -Next come the mandatory `source_hash_record.base.*` and `source_hash_record.candidate.*` groups, each in this order: `command`, `expected_sha`, `head_before`, `detached_before`, `clean_before`, `head_after`, `detached_after`, `clean_after`, `exit`, `stdout_path`, `stdout_sha256`, `stderr_path`, `stderr_sha256`, `artifact_path`, `artifact_size`, `artifact_sha256`. Each group records the exact `scripts/engine-source-hash.sh` invocation and its SHA-bound output artifact. `detached_before` and `detached_after` prove `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; any other result is `CANNOT_ANSWER` during measurement, not detached-state evidence. These source-hash groups and the `projection_authority_diff_*` record are mandatory even when hashes are equal. `projection_authority_diff_command` is exactly `git -C "$IMPLEMENTATION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- Cargo.toml .cargo/config.toml rust-toolchain.toml scripts/engine-source-hash.sh`; capture its NUL-delimited stdout as the recorded path/size/SHA256 artifact. `projection_forced_reason` is exactly one of `NONE`, `SOURCE_HASH_DIFFERENCE`, `EXCLUDED_PROJECTION_AUTHORITY_INPUT_CHANGED`, or `SOURCE_HASH_DIFFERENCE_AND_EXCLUDED_PROJECTION_AUTHORITY_INPUT_CHANGED`. Next comes no projection section only when `projection_forced_reason=NONE`; otherwise write `projection_step_count` followed by every `projection_step..*` group in strictly ascending zero-based `N`; each group contains, in this order, `side`, `command`, `env`, `worktree`, `target`, `expected_sha`, `head_before`, `detached_before`, `head_after`, `detached_after`, `clean_before`, `clean_after`, `exit`, `stdout_path`, `stdout_sha256`, `stderr_path`, `stderr_sha256`, `produced_artifacts`. The detached fields use the same exact exit-`1` / empty-stdout-and-stderr proof. Then write `completion_check_count` and each strictly ascending `completion_check..*` group with the same record fields, `artifact_count` and each strictly ascending `artifact..path`, `.size`, `.sha256`, then `declared_absent_input_count` and each strictly ascending `declared_absent_input..path`. No indexed member may be omitted; a conditional section is omitted only as stated. Artifact rows cover every source-hash output, authority-diff output, projection output, command stdout/stderr capture, scoped diff, and completion artifact. The receipt itself is never listed or hashed; receipt validation is an external reviewer/final-acceptance gate and never a `completion_check` row or receipt artifact. A malformed, unordered, duplicate, missing, or digest-mismatched receipt is `CANNOT_ANSWER`. - -The parser-impact decision happens only after the checkpoint. Measurement-only has exactly two outcomes: `MEASURED` or `CANNOT_ANSWER`. Any identity, detached/clean-state, command, source-hash, authority-diff, projection, receipt, artifact, or digest failure is `CANNOT_ANSWER`; retain completed records but do not claim parser evidence. On `MEASURED`, record `scripts/engine-source-hash.sh "$BASE_SHA"` and `scripts/engine-source-hash.sh "$CANDIDATE_SHA"` in the mandatory `source_hash_record.base` / `.candidate` receipt groups, including their SHA-bound output artifacts. Also run and record the exact `projection_authority_diff_command`. Set `projection_forced_reason=NONE` only when the source hashes are equal and that NUL-delimited path artifact is empty. A source-hash difference and/or any path in that artifact forces `parser_evidence=PROJECTED_PARSE_DIFF`, exactly one direct base projection, and exactly one direct candidate projection from the same read-only, pinned `AtomicCards.json`; choose the canonical reason that names both causes when both apply. Only `NONE` permits `parser_evidence=NO_PARSE_AFFECTING_CHANGE` and no projection inputs, projection records, or comparator outputs. Record the pinned `AtomicCards.json` path and SHA256 as artifacts whenever projection is forced. Do not download, regenerate, copy, sample, or compare replicas. - -For each projection use its detached clean worktree and an isolated target directory. Build exactly with `CARGO_TARGET_DIR= cargo build --profile tool --features cli --bin oracle-gen --bin coverage-report --bin coverage-parse-diff`. Run that side's `oracle-gen` directly against the pinned data root, write its `card-data.json` and `card-names.json` under the projection directory, then run that side's `coverage-report` directly against the projection directory to write `coverage-data.json`. Capture the exact command, environment, worktree identity/clean checks, binary target, exit status, stdout/stderr, and every produced artifact in a projection-step receipt group. Each group's ordered `detached_before` and `detached_after` fields must prove `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; any other result is `CANNOT_ANSWER`. Invoke the **base-built** comparator directly once against the two projected coverage files: - -```bash -"$BASE_TARGET/tool/coverage-parse-diff" "$BASE_PROJECTION/coverage-data.json" "$CANDIDATE_PROJECTION/coverage-data.json" \ - --base-sha "$BASE_SHA" --head-sha "$CANDIDATE_SHA" \ - --markdown "$RUN_ROOT/parse-diff.md" --json "$RUN_ROOT/parse-diff.json" --max-clusters -``` - -Record that comparator invocation and both outputs as projection steps/artifacts. `parser_evidence=PROJECTED_PARSE_DIFF` only after all required base, candidate, and comparator records verify. Do not put these operational artifacts in the implementation diff unless the reviewed plan explicitly scopes them. +That is the whole provenance contract. **Do not build receipts, evidence records, manifests, digests, seals, ledgers, or any other artifact whose purpose is to prove to a later reader that these steps happened.** Git already records what changed and at which commit, and the reviewer reads the diff. Every step below is something you do and then act on, never something you notarize. ## Inputs @@ -58,7 +36,9 @@ Either: 1. A task description (cards, CR rules, Oracle text patterns, affected subsystems, expected behavior), or 2. A pre-existing plan — treat as a draft unless it has already passed `/review-engine-plan` to clean. -Before Step 3, prepare and verify a clean `IMPLEMENTATION_WORKTREE` at `START_SHA`. After its checkpoint, prepare clean detached base and candidate projection worktrees at `BASE_SHA` and `CANDIDATE_SHA` (in a chartered phase these are `PHASE_BASE_SHA` and the phase's `CANDIDATE_SHA` — measurement must match the receipt's `base_sha`), and a distinct clean detached `COMPLETION_WORKTREE` at `CANDIDATE_SHA`; no projection or completion worktree is used for implementation. Per `feedback_session_default_no_worktree`, do not re-ask about worktrees during an active pipeline session — use the session default. +Before Step 3, prepare and verify a clean `IMPLEMENTATION_WORKTREE` at `START_SHA`. After its checkpoint, prepare clean detached base and candidate projection worktrees at `BASE_SHA` and `CANDIDATE_SHA` (in a chartered phase these are `PHASE_BASE_SHA` and the phase's `CANDIDATE_SHA`), and a distinct clean detached `COMPLETION_WORKTREE` at `CANDIDATE_SHA`; no projection or completion worktree is used for implementation. Per `feedback_session_default_no_worktree`, do not re-ask about worktrees during an active pipeline session — use the session default. + +Two measured build-economy rules govern measurement worktrees. **Build-once directories never pay for incremental state:** every measurement build — projection, completion, any target directory built once and never rebuilt — runs with `CARGO_INCREMENTAL=0` (measured: dead incremental state was 17 of 28 GB on one completion target). **Completion allocation is per run-segment, not per candidate:** one completion worktree and one isolated completion target directory per phase (per run when unphased), reset to each round's `CANDIDATE_SHA`, instead of fresh ones per candidate (measured per-candidate cost: seven cold builds, 130 GB, one phase). Reuse is strictly sequential within the owning run — the metadata-hash collision hazard is concurrent writers on one target path, which sequential reuse never creates; never share a reused directory with any concurrently running agent or lane. A reused completion directory is no longer build-once, so the first rule's rationale does not carry to it automatically; it still runs `CARGO_INCREMENTAL=0`, since the 17 GB disk cost is measured and the warm-rebuild speedup is not. Re-measure if warm completion rebuilds start dominating wall clock. **Sizing for pre-existing plans:** Step 1a requires a Sizing section regardless of how the plan arrived. A pre-existing plan lacking one — whether already `/review-engine-plan`-clean (which bypasses Step 1 entirely) or a draft (which reaches Step 1a before its first Step 2 round) — gets a **sizing addendum** from a spawned planner in `engine-planner` sizing-only mode, followed by a review loop in `review-engine-plan` sizing-audit mode: findings → a fresh planner revises the addendum → fresh sizing-audit re-review, inheriting Step 2's stop conditions plus the 4-round charter-loop backstop (T4 is inactive here; its axis is undefined for a one-section artifact). Without this audit the addendum would be the only Sizing section no reviewer ever checks. The addendum, each round's result, and the adjudication are recorded in the phase-fit record. When Step 1a then fires multi-phase on an already-clean plan, the charter-mode planner partitions rather than re-plans (its review-clean input case). @@ -87,7 +67,7 @@ Size without split structure (one large unit) and count without size (trivial mu T4 fires regardless of unit count — it overrides the one-unit anchor, because the anchor is a prediction while T4's three conditions together are an observation of non-convergence. T4 fires when rounds k−1 and k satisfy **all three**: (i) k ≥ 3 — never the first round pair; a fresh artifact's first review is routinely broad and breadth alone is not non-convergence; (ii) each round contains blocking findings classified into ≥3 distinct layers of the axis; (iii) round k's classified blocking count ≥ round k−1's — a shrinking count is a converging loop. - **Axis:** the lockstep registration layer list — types / parser / resolver / targeting / frontend / AI / tests. -- **Severity mapping (exhaustive):** Step 2 rounds (`/review-engine-plan`: blockers and material gaps) — both count; Step 6 rounds (`/review-impl`: HIGH/MED/LOW) — HIGH and MED count, LOW does not, a checkpoint-mode clean verdict contributes zero; checkpoint mode's untagged receipt/gate "blocking findings" are process findings — always layer-unclassified, counting toward no layer. +- **Severity mapping (exhaustive):** Step 2 rounds (`/review-engine-plan`: blockers and material gaps) — both count; Step 6 rounds (`/review-impl`: HIGH/MED/LOW) — HIGH and MED count, LOW does not, a checkpoint-mode clean verdict contributes zero; checkpoint mode's untagged gate "blocking findings" are process findings — always layer-unclassified, counting toward no layer. - **Classification:** a finding is assigned to the layer(s) of the file(s)/plan-sections it names; multi-layer findings count toward each; findings naming nothing on the axis (process, CR-citation, cross-cutting) count toward none. - **Spot-round exclusion (Step 2 loops only):** a round in which every finding is *spot* per the surgical-mode classification contributes to no T4 pair — spot findings are cheap check-and-replace and surgical mode takes precedence. No such exclusion in Step 6 loops, where surgical mode never operates; impl-loop spot-grade findings map to LOW, which already doesn't count. - **At most once per run:** a T4 firing when the phase-fit record already contains any prior T4 firing or feasibility exit **stops the run and surfaces to the user**. After a branch-(b) return to Step 1, the fresh Step 1a re-measures freely (a redesigned plan may honestly size single-phase), but the persisted T4 entry makes a second observed non-convergence terminal. @@ -95,31 +75,31 @@ T4 fires regardless of unit count — it overrides the one-unit anchor, because These two terminal stops join the unbounded loops' enumerated stop-condition list alongside the existing three. -**Routes.** *Step 2 loop, unphased run:* exit the loop, spawn a charter-mode planner with the current draft plan and accumulated findings, run the charter review loop, proceed per-phase. *Step 6 loop, unphased run:* two branches, and in both the charter exists **before** any acceptance — (a) if the candidate can plausibly be stabilized green-and-coherent: first spawn the charter-mode planner (phase 1 = the stabilized current candidate + its deferral list; phases 2..n = the remainder — the planner's third input case), run the charter review loop, then one stabilization fix round, then the normal checkpoint → measure → completion → review sequence under `/review-impl` phase mode with phase 1's allowlist; zero findings → accept as phase 1; findings → resume normal fix rounds within phase 1; (b) if it cannot be made coherent, return to Step 1 with a decomposition directive — the abandoned receipted candidates stay outside every accepted interval and are listed in the Final Report. *Inside a chartered phase:* charter revision — before executor dispatch, split the phase; in the impl loop, the truncate/restart branches below. *Second-level firing of either kind* — T4 or predictive, inside a phase that a charter revision or T4 stabilization produced — stops the run and surfaces to the user. +**Routes.** *Step 2 loop, unphased run:* exit the loop, spawn a charter-mode planner with the current draft plan and accumulated findings, run the charter review loop, proceed per-phase. *Step 6 loop, unphased run:* two branches, and in both the charter exists **before** any acceptance — (a) if the candidate can plausibly be stabilized green-and-coherent: first spawn the charter-mode planner (phase 1 = the stabilized current candidate + its deferral list; phases 2..n = the remainder — the planner's third input case), run the charter review loop, then one stabilization fix round, then the normal checkpoint → measure → completion → review sequence under `/review-impl` phase mode with phase 1's allowlist; zero findings → accept as phase 1; findings → resume normal fix rounds within phase 1; (b) if it cannot be made coherent, return to Step 1 with a decomposition directive — the abandoned candidates stay outside every accepted interval and are listed in the Final Report. *Inside a chartered phase:* charter revision — before executor dispatch, split the phase; in the impl loop, the truncate/restart branches below. *Second-level firing of either kind* — T4 or predictive, inside a phase that a charter revision or T4 stabilization produced — stops the run and surfaces to the user. ### Process records (append-only, by phase index only, never a commit SHA) -`/engine-implementer-runs//phase-fit` and `/phase-charter`. The phase-fit record gets one numbered entry per adjudication — initial, each re-check, each T4 firing, each feasibility exit — carrying the Sizing values used, per-trigger measured results, the T2 groups used, the verdict, and for feasibility exits the named-seam evidence. The no-SHA rule keeps both records inside the `surgical-mode-switch` carve-out ("carries no candidate identity and duplicates no receipt field") and outside the canonical-receipt prohibition on parallel provenance. **There is no phase ledger** — a SHA-bearing acceptance record would be the prohibited parallel ledger; chain integrity is recomputed at run-level acceptance instead. In multi-phase runs, each `surgical-mode-switch` entry is additionally tagged with its phase index (index only), keeping interleaved entries from different phases' plan loops auditable. +`/engine-implementer-runs//phase-fit` and `/phase-charter`. The phase-fit record gets one numbered entry per adjudication — initial, each re-check, each T4 firing, each feasibility exit — carrying the Sizing values used, per-trigger measured results, the T2 groups used, the verdict, and for feasibility exits the named-seam evidence. The no-SHA rule keeps both records inside the `surgical-mode-switch` carve-out. **There is no phase ledger** — a SHA-bearing acceptance record would be the prohibited parallel ledger; chain integrity is recomputed at run-level acceptance instead. In multi-phase runs, each `surgical-mode-switch` entry is additionally tagged with its phase index (index only), keeping interleaved entries from different phases' plan loops auditable. ### The charter Authored by a freshly spawned planner in `engine-planner` **charter mode** (the orchestrator never authors), reviewed through `review-engine-plan` **charter mode** in its own loop. The loop inherits Step 2's enumerated stop conditions plus a dedicated backstop: T4's axis is undefined for charter-shaped findings (T4 inactive, recorded as such), so a charter-review or sizing-audit loop exceeding **4 rounds** without converging stops and surfaces to the user. Once clean, the charter is frozen. -**Charter revision** may only add, split, merge, or re-scope **remaining** phases. Accepted phases are never reworked in place: a finding that invalidates accepted content becomes a **fix phase** — a later phase whose scope overlaps the earlier files. In a chartered phase's impl loop, T4 runs two branches executed as charter revision: *(a) truncate* — the revision truncates phase k (its deferral list grows by the split-off remainder, attributed to new successor phases) and passes charter review; then one stabilization fix round, then the normal checkpoint → measure → completion → review sequence under phase mode with the truncated allowlist; zero findings → accepted (successors base on the truncated phase's accepted candidate); findings → resume normal fix rounds. Already-landed receipted candidates remain the phase's rounds. *(b) restart* — restart phase k from its own `PHASE_BASE_SHA` in a fresh implementation worktree under the revised charter; the abandoned candidates fork off the accepted chain, never appear in any chain-integrity interval, and are listed as abandoned in the Final Report. +**Charter revision** may only add, split, merge, or re-scope **remaining** phases. Accepted phases are never reworked in place: a finding that invalidates accepted content becomes a **fix phase** — a later phase whose scope overlaps the earlier files. In a chartered phase's impl loop, T4 runs two branches executed as charter revision: *(a) truncate* — the revision truncates phase k (its deferral list grows by the split-off remainder, attributed to new successor phases) and passes charter review; then one stabilization fix round, then the normal checkpoint → measure → completion → review sequence under phase mode with the truncated allowlist; zero findings → accepted (successors base on the truncated phase's accepted candidate); findings → resume normal fix rounds. Already-landed candidates remain the phase's rounds. *(b) restart* — restart phase k from its own `PHASE_BASE_SHA` in a fresh implementation worktree under the revised charter; the abandoned candidates fork off the accepted chain, never appear in any chain-integrity interval, and are listed as abandoned in the Final Report. ### Per-phase identity and the substitution rule -Each phase k is a self-contained checkpoint pipeline with `PHASE_BASE_SHA` — phase 1: run-level `BASE_SHA`; phase k>1: phase k−1's accepted `CANDIDATE_SHA` — and its own frozen scope, frozen at the phase's scope-freeze moment (after its plan loop, before executor dispatch; never before the phase plan exists). Per-phase scopes **may overlap** on shared registration files (`effects/mod.rs` and kin); sequential execution makes that safe — there is no global-partition requirement. **Within a phase, every occurrence of `BASE_SHA` in the Inputs worktree preparation and Steps 3–7 (checkpoint delta, `scoped_diff_command`, projection worktrees, completion parser-gate range, Step 6 review span) means `PHASE_BASE_SHA`, including the receipt's `base_sha` field** — each phase's receipt is an ordinary receipt-v1; the literal `"$BASE_SHA"` command templates stay byte-identical while the shell variable carries the phase base, so checkpoint-mode validation needs no changes. Run-level `BASE_SHA` is retained for the final integration span only. Zero receipt-format changes; no receipt-v2 exists or may be invented. +Each phase k is a self-contained checkpoint pipeline with `PHASE_BASE_SHA` — phase 1: run-level `BASE_SHA`; phase k>1: phase k−1's accepted `CANDIDATE_SHA` — and its own frozen scope, frozen at the phase's scope-freeze moment (after its plan loop, before executor dispatch; never before the phase plan exists). Per-phase scopes **may overlap** on shared registration files (`effects/mod.rs` and kin); sequential execution makes that safe — there is no global-partition requirement. **Within a phase, every occurrence of `BASE_SHA` in the Inputs worktree preparation and Steps 3–7 (checkpoint delta, `scoped_diff_command`, projection worktrees, completion parser-gate range, Step 6 review span) means `PHASE_BASE_SHA`,** — the literal `"$BASE_SHA"` command templates stay byte-identical while the shell variable carries the phase base, so checkpoint-mode validation needs no changes. Run-level `BASE_SHA` is retained for the final integration span only. -**Per-phase spawn inputs:** Step 1 planners run in `engine-planner` **phase-plan mode** with the charter, the phase's entry, its deferral allowlist, and prior phases' accepted summaries (never their debates). Step 2 reviewers run in `review-engine-plan` **phase-plan mode** with the phase plan, the original task, the charter, the phase index, and the allowlist — and *all* Step 2 reviews in this pipeline, unphased and per-phase alike, declare the phase-fit context so the Sizing consistency check is blocking here. Step 3 executors run in the executor's **phase mode** with the charter, phase index, and allowlist, so the matrix and test map they author use the same `DEFERRED(phase n)` vocabulary their reviewers audit. Step 6 reviewers run in `/review-impl` **phase mode** with the charter, phase index, and allowlist. +**Per-phase spawn inputs:** Step 1 planners run in `engine-planner` **phase-plan mode** with the charter, the phase's entry, its deferral allowlist, and prior phases' accepted summaries (never their debates). Step 2 reviewers run in `review-engine-plan` **phase-plan mode** with the phase plan, the original task, the charter, the phase index, and the allowlist — and *all* Step 2 reviews in this pipeline, unphased and per-phase alike, declare the phase-fit context so the Sizing consistency check is blocking here. Step 3 executors run in the executor's **phase mode** with the charter, phase index, and allowlist, so the matrix and test map they author use the same `DEFERRED(phase n)` vocabulary their reviewers audit. Step 6 reviewers run in `/review-impl` **phase mode** with the charter, phase index, allowlist, and the phase's frozen scope. ### Run-level acceptance (after the last phase) -Per-phase acceptance is today's Step 7 applied to the phase — receipt revalidation, reviewer-emitted external receipt SHA256, `rev-parse HEAD == CANDIDATE_SHA` — and **emits no Final Report snapshot and no PR-handoff block**; those are run-level only. Run-level final acceptance requires all of: +Per-phase acceptance is today's Step 7 applied to the phase — a clean review, completion checks passing at the candidate, and `rev-parse HEAD == CANDIDATE_SHA` — and **emits no Final Report snapshot and no PR-handoff block**; those are run-level only. Run-level final acceptance requires all of: -1. **Every phase accepted** with its own receipt evidence. -2. **Chain integrity, recomputed fresh by the orchestrator** from the receipts plus its acceptance inputs. Attribution rule: a receipt belongs to phase k iff its `base_sha` equals phase k's `PHASE_BASE_SHA`; within a group, rounds chain by `start_sha` (first round `start_sha == base_sha`; later rounds chain from a prior candidate of the group). Accepted-candidate identification is keyed on the acceptance inputs, not receipts alone — interior phases via phase k+1's `base_sha`, the final phase via the acceptance input itself (necessary: a §restart leaves two `start_sha` chains under one base, and receipts cannot name the final accepted candidate). Acceptance inputs live in orchestrator session state, durably reflected only in the Final Report — the same trust model as single-phase Step 7; the no-SHA process-record rule forecloses any earlier durable home, a trade accepted explicitly. Checks: (i) each phase's accepted chain starts at the prior phase's accepted candidate (phase 1: run `BASE_SHA`); (ii) every fix-round `start_sha` resolves within its own group; (iii) `git rev-list ..` contains **only** commits with a receipt from that phase — containment, not equality: restart-abandoned and branch-(b)-abandoned receipted candidates legitimately sit outside the interval and are listed in the Final Report. All three are mechanical `rev-parse`/`rev-list`/receipt-field comparisons. -3. **The integration review returns zero findings**, run in `/review-impl` **integration mode** (findings-only; scoped to cross-phase seams and charter completeness; no run-span receipt exists and none is created — the per-phase receipts, each validated at per-phase acceptance, cover the span because the chain check proves the phases tile it; the per-phase external receipt SHA256s satisfy the acceptance criterion, never a run-span receipt). Reviewer inputs: the run-span `BASE_SHA..final CANDIDATE_SHA` diff, the charter, and the per-phase receipt paths for reference. Findings dispatch a fix phase via charter revision. **Bound:** at most one fix phase per integration round; findings still present after two fix phases → stop and surface to the user. +1. **Every phase accepted** on its own review and checks. +2. **The phases actually tile the run**, checked against git: each phase's accepted candidate is the next phase's base, and `git rev-list ..` contains only that phase's own commits. Restarted and abandoned candidates legitimately sit outside those intervals — list them in the Final Report rather than forcing the intervals to match. +3. **The integration review returns zero findings**, run in `/review-impl` **integration mode** (findings-only; scoped to cross-phase seams and charter completeness). Reviewer inputs: the run-span `BASE_SHA..final CANDIDATE_SHA` diff and the charter. Findings dispatch a fix phase via charter revision. **Bound:** at most one fix phase per integration round; findings still present after two fix phases → stop and surface to the user. ## Pipeline @@ -175,17 +155,17 @@ The loop above assumes findings move the **design**. Once they stop doing that, - **State the sweep's boundary.** A changelog entry that quotes the struck text will match your own grep for it. Population, predicate, scan direction, and whether the matched line counts — write them down; every enumeration defect is an unstated predicate rather than a bad measurement. - **Fix the neighbours the fix breaks.** A finding's repair frequently contradicts a section that classified the old form. Sweep by mechanism, not by coordinate. - **Then re-review the WHOLE artifact**, fresh context — not just the repaired sections, per `$bug-triage`'s targeted-re-review rule. Repeat apply → whole-artifact re-review until a round returns zero gaps; any finding that requires *deciding* something ends surgical mode and returns to the unbounded loop above. Surgical mode replaces the planner-rewrite rounds, never the final independent check. -- **Record the mode switch, its three measurements, the spot-vs-design classification of each round's findings, each attempted edit's two-sided verification result (pass or fail) and sweep boundary, and why the mode ends** in `/engine-implementer-runs//surgical-mode-switch` (in multi-phase runs, tag each entry with its phase index — index only, no SHA), never in the plan text the fresh re-reviewer and the executor read — recording it there hands the one remaining independent check a prior verdict. It is a process record of this loop, not provenance: it carries no candidate identity and duplicates no receipt field, so the canonical-receipt rule above does not reach it. Append one numbered entry per round, never overwrite: ending surgical mode and re-entering it later continues the same numbered sequence, and clobbering an earlier entry loses the exit that entry recorded. Every exit is then auditable rather than asserted. Each entry carries that round's classification, edit results, and sweep boundaries; only a round that enters the mode records the switch and its three measurements, and only a round that ends the mode records why. +- **Record the mode switch, its three measurements, the spot-vs-design classification of each round's findings, each attempted edit's two-sided verification result (pass or fail) and sweep boundary, and why the mode ends** in `/engine-implementer-runs//surgical-mode-switch` (in multi-phase runs, tag each entry with its phase index — index only, no SHA), never in the plan text the fresh re-reviewer and the executor read — recording it there hands the one remaining independent check a prior verdict. It is a working note for this loop, not a provenance record. Append one numbered entry per round, never overwrite: ending surgical mode and re-entering it later continues the same numbered sequence, and clobbering an earlier entry loses the exit that entry recorded. Every exit is then auditable rather than asserted. Each entry carries that round's classification, edit results, and sweep boundaries; only a round that enters the mode records the switch and its three measurements, and only a round that ends the mode records why. **This does not contradict `$bug-triage`'s fixpoint gate.** That gate requires whole-plan re-review because *"revisions routinely INTRODUCE new gaps in untouched-looking areas"* — planner **rewrites** do. A check-and-replace at a named coordinate does not rewrite, which is why it is the safe tool once the design has stopped moving. `$review-engine-plan` ends its loop with *"or the caller stops the process"* and states no criteria; this section is those criteria, and it lives here because the orchestrator is that caller. -This is not a licence for "two rounds and ship". The unbounded loop remains the default and the burden of proof is on leaving it: no measurement, no switch. Surgical mode is scoped to this Step 2 plan-review loop only — Step 6's implementation-review loop never uses it, because there the artifact is a committed candidate that only an executor may edit under the frozen-scope and receipt contract. +This is not a licence for "two rounds and ship". The unbounded loop remains the default and the burden of proof is on leaving it: no measurement, no switch. Surgical mode is scoped to this Step 2 plan-review loop only — Step 6's implementation-review loop never uses it, because there the artifact is a committed candidate that only an executor may edit under the frozen-scope contract. ### Step 3 — Dispatch implementation Spawn the `engine-implementation-executor` agent. -**Spawn inputs:** mode `implementation/fix`; the reviewed clean plan in full; `BASE_SHA`; named `START_SHA`; frozen in-bounds / out-of-bounds path list and its SHA256; named `IMPLEMENTATION_WORKTREE`; the canonical receipt path; any prior reviewer findings (none on first round); in chartered runs additionally the charter, phase index, and deferral allowlist (the executor's phase mode). First round: `START_SHA == BASE_SHA`. Fix round: `START_SHA` is the previously reviewed `CANDIDATE_SHA`, never a moving branch head. +**Spawn inputs:** mode `implementation/fix`; the reviewed clean plan in full; `BASE_SHA`; named `START_SHA`; the in-bounds / out-of-bounds path list; named `IMPLEMENTATION_WORKTREE`; any prior reviewer findings (none on first round); in chartered runs additionally the charter, phase index, and deferral allowlist (the executor's phase mode). First round: `START_SHA == BASE_SHA`. Fix round: `START_SHA` is the previously reviewed `CANDIDATE_SHA`, never a moving branch head. The implementation executor edits only its frozen scope and runs **preparatory** checks. Preparatory success is not completion evidence. Its existing discriminating-test, selected-authority, coverage-honesty, maintainer-simulation, and CR-annotation gates remain the authoritative gates; do not restate or replace them here. @@ -193,42 +173,30 @@ If the executor returns "stop and return" items (plan contradicts current code, **Large JSON fixture constraint.** Any repository-bound JSON fixture ≳100KB (test fixtures, game-state dumps, generated maps — not runtime/config JSON whose consumers read plain `.json`) gets `gzip -9 -n` (`-n` keeps the archive byte-reproducible) and loads via the established inflate pattern: `include_bytes!("….json.gz")` + a test-local `gunzip` helper using `flate2::read::GzDecoder` (examples: `tests/integration/combo_infinite_pile.rs`, `cr733_resolved_commands_p0.rs`). Never commit the uncompressed twin alongside the `.json.gz`. If a fixture is regenerated by a script, note in the reading test that regeneration requires re-gzipping. -### Step 4 — Checkpoint, then measure the committed candidate +### Step 4 — Checkpoint the candidate -Before Step 3, the orchestrator records `IMPLEMENTATION_WORKTREE`'s staged/unstaged path snapshot and clean `HEAD == START_SHA` attestation. Before staging, it repeats the stable-HEAD check and records its exact implementation delta: the approved path list and the SHA256 of each approved path's `START_SHA..working-tree` diff. It must first prove, from that snapshot, that no pre-existing staged or unstaged change overlaps an approved path; if attribution is ambiguous, stop and return rather than unstage, sweep in, or overwrite another agent's work. The checkpoint is the candidate commit: stage each approved path by explicit pathspec — never `git add -A` — and never commit without explicit pathspec because the shared index can sweep in other agents' staged files (`feedback_git_add_file_bundles_concurrent_work`, `feedback_shared_index_commit_pathspec`). Stage and commit only the explicit approved paths, never alter unrelated index entries, then record the full `CANDIDATE_SHA`. Immediately run `git -C "$IMPLEMENTATION_WORKTREE" rev-parse HEAD` and compare its output to `CANDIDATE_SHA`; a mismatch stops the run. Verify that `START_SHA..CANDIDATE_SHA` contains only the recorded authorized delta (paths and diff digests), and retain the original `BASE_SHA..CANDIDATE_SHA` diff for final review; otherwise stop and return. This is an orchestrator-only commit; the executor never performs it. Do not measure an uncommitted tree or use a moving `HEAD` as the candidate identity. Verify HEAD is on a branch before any explicitly requested push (`feedback_verify_head_attached_before_push`), never pipe `git push` into `tail`/`head` (`feedback_git_push_no_pipe`), and never push unless explicitly requested. +The checkpoint is the candidate commit, and it is the orchestrator's to make — never the executor's. Stage each approved path by explicit pathspec: never `git add -A`, and never commit without a pathspec, because the shared index can sweep in another agent's staged files (`feedback_git_add_file_bundles_concurrent_work`, `feedback_shared_index_commit_pathspec`). Before staging, confirm no pre-existing change overlaps an approved path; if attribution is ambiguous, stop and return rather than unstage, sweep in, or overwrite another agent's work. Commit, then confirm `git -C "$IMPLEMENTATION_WORKTREE" rev-parse HEAD` equals the `CANDIDATE_SHA` you recorded, and that `START_SHA..CANDIDATE_SHA` contains only the intended paths. Never measure an uncommitted tree or use a moving `HEAD` as the candidate. Verify `HEAD` is attached before any explicitly requested push (`feedback_verify_head_attached_before_push`), never pipe `git push` into `tail`/`head` (`feedback_git_push_no_pipe`), and never push unless asked. -Spawn a **fresh** `engine-implementation-executor` in mode `measurement-only` with `BASE_SHA`, `CANDIDATE_SHA`, named `IMPLEMENTATION_WORKTREE`, frozen scope paths, the canonical receipt path, a detached clean base worktree, a detached clean candidate worktree, and the read-only pinned data root containing `AtomicCards.json`. This executor makes no source edits and no commits. +If the change touches the parser, find out whether it moves parser output: build the tooling from the base and the candidate, generate card data from each against the same pinned data root, and diff the two. Report what changed. `./scripts/gen-card-data.sh` and `cargo coverage` do not answer this question. -The measurement-only executor runs `scripts/engine-source-hash.sh` in the respective detached base/candidate projection worktrees at `BASE_SHA` and `CANDIDATE_SHA`, stores both SHA-bound outputs in the mandatory `source_hash_record.base` / `.candidate` receipt groups, and runs the exact four-path NUL-safe `projection_authority_diff_command` from the receipt contract. It returns `CANNOT_ANSWER` for any identity, detached/clean-state, command, source-hash, authority-diff, projection, receipt, artifact, or digest failure; otherwise it returns `MEASURED`. Equal hashes set `parser_evidence=NO_PARSE_AFFECTING_CHANGE` and permit no parser projection only when that authority-diff artifact is empty (`projection_forced_reason=NONE`). A source-hash difference or any authority-diff path forces the canonical non-`NONE` reason and requires the exact one base projection, one candidate projection, and base-built comparator defined in the receipt contract. It records every command, environment, base/candidate worktree identity before and after the operation, clean state before and after, output capture, produced artifact, and artifact digest. `./scripts/gen-card-data.sh` and `cargo coverage` are never projection evidence. +### Step 5 — Verify the committed candidate -### Step 5 — Committed-candidate completion verification +Run the checks in a clean worktree at `CANDIDATE_SHA`, not in the implementation worktree — a check that passes against uncommitted edits has told you nothing about what you are shipping. Run every gate the changed surface calls for: formatting for any implementation change, the Rust/engine/parser block for Rust paths, the frontend block for frontend paths, the parser gate for parser paths. Markdown-only policy changes need scope and diff checks; do not run Cargo or Tilt for them. -Completion verification occurs only after the checkpoint, in the distinct clean detached `COMPLETION_WORKTREE` at `CANDIDATE_SHA`, and records detached identity, `HEAD`, and clean checks at start and end. Every completion or parser command must either use `git -C "$COMPLETION_WORKTREE"` or execute with `COMPLETION_WORKTREE` as its working directory. The candidate parser gate must enumerate its range NUL-safely with `git -C "$COMPLETION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- crates/engine/src/parser/`; any loop reading it uses `IFS= read -r -d ''`. The required set is both scope-/plan-derived checks and every surface-derived gate applicable to the changed paths in the executor's existing implementation/fix verification blocks: formatting for implementation changes, the Rust/engine/parser Tilt-first-or-isolated-direct block for Rust/engine/parser paths, the frontend Tilt-first-or-isolated-direct block for frontend paths, and the parser preparatory gate for parser paths. Instantiate those blocks only after substituting `COMPLETION_WORKTREE` for `IMPLEMENTATION_WORKTREE` and `CANDIDATE_SHA` for `START_SHA`; every recorded completion command must show those substitutions and must never name `IMPLEMENTATION_WORKTREE` or `START_SHA`. Use candidate-SHA-bound CI or Tilt evidence only when the evidence itself proves that binding; otherwise use the isolated direct fallback already specified by the applicable block. Never promote the implementation executor's preparatory result to completion evidence. Its receipt completion-check groups must enumerate every required check and, for each, record the exact command, exit result, detached `CANDIDATE_SHA` identity and clean status at start and end, including ordered `detached_before` and `detached_after` proofs that `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; any other result is an operational failure. A missing check, nonzero/unknown exit result, identity mismatch, detached-state failure, or dirty status fails completion. - -For Markdown-only policy updates, the mandatory completion-check set is limited to scope-path, SHA-identity, and Markdown/diff checks; do not run Cargo or Tilt. Receipt validation is an external reviewer/final-acceptance gate, never a self-referential completion-check record or receipt artifact. +The full suite is owed at the tree being shipped. An intermediate fix round may narrow to the touched surface — say so plainly when reporting it, since a narrowed run is not a suite pass — and re-run unfiltered before acceptance. ### Step 6 — Review the immutable candidate -Spawn a fresh `general-purpose` agent and instruct it to invoke `/review-impl` against `BASE_SHA..CANDIDATE_SHA`, the original task, reviewed plan, frozen scope paths, `START_SHA`, implementation-worktree start/end stable-HEAD attestations, the detached `COMPLETION_WORKTREE` start/end identity attestations, and the canonical receipt. The reviewer validates the receipt before applying the universal lenses: exact fixed-field order; percent encoding; no CR/NUL; every conditional/indexed section; candidate/head and worktree identity; the frozen NUL-sorted scope representation and hash; the exact scoped-diff and authority-diff commands and bytes; both mandatory source-hash records even on equal hashes; every artifact size/SHA256; and every projection/completion group's ordered `detached_before`/`detached_after` proof that `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr (any other result is an operational failure). It must accept an omitted projection only if the source hashes are equal, the authority-diff artifact is empty, and `projection_forced_reason=NONE`; otherwise, it must reject it. It must reject any forced-projection receipt only when it lacks the pinned `AtomicCards.json`, isolated exact tool builds, exactly one direct base and candidate projection, or one base-built comparator invocation with both `--base-sha` and `--head-sha`. After validating it, the reviewer emits the receipt SHA256 externally in its review result; that digest never appears in the receipt. It must validate that first-round `START_SHA == BASE_SHA`, each fix `START_SHA` is the prior reviewed candidate, and the implementation worktree was clean at start and stable at checkpoint. The reviewer MUST fail missing or unsuccessful mandatory completion checks, verify the originally reported bug or requirement is actually fixed via the existing discriminating runtime-test gate — in a chartered phase, scoped to *this phase's charter goal and discriminating test*, since the full requirement is deferred by construction for every interior phase — and audit the existing maintainer-simulation and coverage-honesty artifacts. In chartered runs the reviewer additionally receives the charter, phase index, and deferral allowlist and applies `/review-impl` phase mode. - -If review returns findings, spawn a **fresh** implementation/fix executor with the findings as constraints. The round is always: - -```text -edit + preparatory checks → orchestrator checkpoint → fresh measurement-only executor -→ committed-candidate completion verification → fresh BASE_SHA..CANDIDATE_SHA review -``` - -Every round keeps that run segment's base — the run-level `BASE_SHA`, or in a chartered phase its `PHASE_BASE_SHA` — and its frozen scope paths. Every checkpoint receives its own receipt. Never review a diff that includes a later unmeasured fix. +Spawn a fresh `general-purpose` agent to invoke `/review-impl` against `BASE_SHA..CANDIDATE_SHA`, with the original task, the reviewed plan, the in-scope paths, and any prior findings. It reviews the diff and the checks that were run; it re-runs whatever it needs to trust. Findings dispatch a fix round. ### Step 7 — Final acceptance -In a chartered run this step is **per-phase acceptance** — it applies to the phase exactly as written below, with `PHASE_BASE_SHA` substituted, and **emits no Final Report snapshot and no PR-handoff block**; those, plus chain integrity and the integration review, belong to run-level acceptance ("Run-level acceptance" above). Unphased runs use this step verbatim as the run's acceptance. +Accept when the plan-review loop is clean, the review returns no findings, the completion checks pass at the candidate, and `rev-parse HEAD == CANDIDATE_SHA`. In a chartered run this is per-phase acceptance, with `PHASE_BASE_SHA` substituted; it emits no Final Report snapshot and no PR handoff, which are run-level only. -Accept only when the plan-review loop is clean, the reviewer emitted a matching external receipt SHA256, final acceptance independently revalidates the receipt and every recorded artifact, parser evidence and completion checks pass, every surgical-fix mode round has the complete numbered entry Step 2 requires, and the fresh implementation review returns zero findings. The receipt validation/hash and those entries are acceptance evidence, never completion-check rows or receipt artifacts. Immediately run `git -C "$IMPLEMENTATION_WORKTREE" rev-parse HEAD` and compare its output to `CANDIDATE_SHA`; if it differs, the review is stale and the current head must repeat the checkpoint-to-review sequence. Do not treat review of an ancestor as review of current work. ### Post-acceptance PR handoff (non-gating) -Final acceptance emits an immutable Final Report snapshot: `Pipeline-reviewed head == Current branch head == accepted CANDIDATE_SHA`, `Pipeline status: current`, and `Current-head review: none`. Do not alter or replace that snapshot, the accepted candidate SHA, or any canonical receipt evidence after acceptance. +Final acceptance emits an immutable Final Report snapshot: `Pipeline-reviewed head == Current branch head == accepted CANDIDATE_SHA`, `Pipeline status: current`, and `Current-head review: none`. Do not alter or replace that snapshot, the accepted candidate SHA after acceptance. Copy the following mutable `PR handoff` block into the PR body beside the retained pipeline report: @@ -241,7 +209,7 @@ Current-head review: none | clean at | findings at Whenever the branch head changes after acceptance, including through a rebase, update `Current branch head`, set `Pipeline status` to `historical — `, and reset `Current-head review` to `none`. When current-head evidence is desired, run ordinary `/review-impl` against the complete current PR/head — not checkpoint mode and not an incremental-only diff — then record `clean at ` or `findings at ` for that exact SHA. Each future head change repeats the reset. `Pipeline status` remains historical unless the current head again equals the original accepted `CANDIDATE_SHA`, in which case set it to `current`. -If later work invalidates the approved plan or architecture, return to plan review. Otherwise, this is a concise reporting and navigation flow only: it is not a gate, a new receipt requirement, GitHub automation, an executor change, or a PR-handler change. +If later work invalidates the approved plan or architecture, return to plan review. Otherwise, this is a concise reporting and navigation flow only: it is not a gate, GitHub automation, an executor change, or a PR-handler change. ## Final Report @@ -251,7 +219,7 @@ Return after final acceptance: 2. What changed, grouped by subsystem and file. 3. Key architectural decisions. 4. `BASE_SHA`, accepted `CANDIDATE_SHA`, frozen scope paths, and run-artifact root. -5. `START_SHA`/`IMPLEMENTATION_WORKTREE` records for every round; the canonical receipt path, parser-evidence branch, direct-projection records when hashes differ, and completion-check identity. +5. The `START_SHA` each round began from, and what the parser measurement found when the change touched the parser. 6. Verification commands run and results, separated into preparatory and completion evidence. 7. Implementation-review rounds (count), reviewed SHA, and final clean result. 8. Checkpoint commit hash and staged file list. @@ -259,5 +227,5 @@ Return after final acceptance: 10. Deviations from the plan with reasons. 11. Self-flagged risks and judgment calls (yours + executor's). 12. Remaining items, if any, with reasons. -13. Phase-fit verdict and record path. Single-phase runs report these plus any abandoned receipted candidates whenever a T4 branch-(b) redo preceded the verdict — the Final Report is their only durable listing under the no-SHA process-record rule. -14. Chartered runs additionally: phase count; per-phase accepted `CANDIDATE_SHA`s and receipt paths; abandoned candidates from either source (restarts and branch-(b) returns); the phase-charter record path; the chain-integrity result; and the integration-review result. +13. Phase-fit verdict and record path. Single-phase runs report these plus any abandoned candidates whenever a T4 branch-(b) redo preceded the verdict — the Final Report is their only durable listing under the no-SHA process-record rule. +14. Chartered runs additionally: phase count; per-phase accepted `CANDIDATE_SHA`s; abandoned candidates from either source (restarts and branch-(b) returns); the phase-charter record path; the chain-integrity result; and the integration-review result. diff --git a/.claude/skills/engine-planner/SKILL.md b/.claude/skills/engine-planner/SKILL.md index 32e23b7be5..62b5b5690c 100644 --- a/.claude/skills/engine-planner/SKILL.md +++ b/.claude/skills/engine-planner/SKILL.md @@ -76,6 +76,67 @@ Find the existing feature most similar to what you're implementing. Trace it end Before proposing changes, read every file you plan to modify. Understand existing patterns, abstractions, and conventions in each. +### Step 3.5: MEASURE, don't trace — prove load-bearing assertions with runnable code probes + +**Hard gate. Reading is discovery; the probe is proof.** Steps 2 and 3 tell you what the code *looks +like*. Only a probe that compiles and runs tells you what it *does*. For a rules engine this intricate, +plans built by tracing repeatedly encode plausible-but-wrong assertions that survive plan review and +break at implementation — or worse, ship a predicate that is true for the wrong reason. + +**Every load-bearing assertion in the plan is either PROVEN by a probe or explicitly marked +`UNPROVEN`.** A load-bearing assertion is one the design would change if it were false: a predicate's +runtime verdict, a route/branch actually taken, an observed count or delta, "X never happens", "this +conjunct is what refuses". No exceptions for assertions that seem obvious from the source — the classic +failure is a predicate whose body reads correctly and whose runtime verdict is dominated by inputs the +source does not mention. + +- A probe is a throwaway `#[test]` or scratch driver, **compiled and run to a successful exit in the + worktree**, teed to a log on disk and grepped for the observed result. Record, per assertion: probe + name, exact command, the fixture it ran against, the exit status, the observed output verbatim, and + — where a cost figure is itself load-bearing — the measured cost. An assertion carrying no such + record is `UNPROVEN` however confident the prose around it reads. **A non-zero exit or a timeout is + `UNPROVEN` whatever the log contains** — a run that printed the expected verdict and then died still + leaves that verdict in the log, and `grep` cannot tell it apart from a clean run. Reachability and + exit status bracket the run at both ends: one says the instrument fired, the other says it survived + to the end. Before finalizing, delete the probe file and keep the scratch + log out of the tracked tree; the record in the plan is the artifact that survives, not the log. +- **Prefer probing against real committed fixtures/dumps over synthetic state.** A synthetic fixture + proves the predicate reads a field; a real board proves what the predicate answers in production. +- **A probe proves nothing until it shows it reached the code under test.** Print a positive + reachability marker alongside the verdict — a nonzero count, a hit recorded on the production branch, + the value observed at the seam. A run that died before the target, or that measured zero with no + positive control demonstrating the instrument fires at all, is `UNPROVEN` — not a negative result. + This is the Verification Matrix's paired-positive-reach-guard rule (Step 4) applied one step earlier, + to evidence rather than tests, and the two shapes it catches are measured failure modes rather than + style: a census that reports zero because the instrument never fired, and a discriminator whose + verdict is really decided by an upstream conjunct that dominates it. +- **Write the claim in the form that survives the next edit.** A probe yields a snapshot — a count, a + coordinate, a cardinality, a per-section tally. Transcribing it is correct provenance and fragile + text: the next edit falsifies it, review correctly flags it, and the repair mints a fresh snapshot + for the round after. Prefer the formulation that is invariant under further editing — a symbol name + over a line number, "every case in §Y" over "the four cases", "the red flags" over "two red flags"; + where only the fragile form carries the information, pin it to the command that regenerates the + figure rather than to the figure. **A falsified snapshot claim is repaired by reformulating it, not + by refreshing it** — refreshing costs a round every time the artifact moves. +- **Cost is not a reason to skip one.** An unmeasured cost estimate defers work as effectively as a + wrong result — if you gate a measurement decision on a cost figure, measure that figure or label it + unmeasured. Probing early is the token-*cheap* path: static-only plan loops converge asymptotically + on prose (measured: 36 rounds static vs 5 with probes on the same work), because reviewers with no + fresh evidence end up auditing the document. +- **Probes need the cargo target lock and a stable tree.** Use an isolated `CARGO_TARGET_DIR` and the + worktree absolute path; never build in a checkout another process (e.g. Tilt) owns. Serialize probe + activity behind any active implementation executor on a shared worktree — read-only discovery may run + concurrently. Never put a scratch crate's target dir on tmpfs. Keep one isolated target dir per + worktree and *reuse* it across probes — deleting it between runs buys nothing and re-imposes the full + dependency rebuild that talks planners out of probing in the first place. + +**Never write a build-withholding instruction into a brief or plan** ("do not run cargo", "no build is +authorised"). Weakening the probe floor is a gate violation, not a judgment call. The one exception is +a *measured* resource conflict — a named process holding the target lock on a named worktree, with both +the measurement and the scope of the withholding recorded in the brief; it never generalizes past that +scope. Whatever the reason, an assertion a probe could not settle is marked `UNPROVEN` with the reason +recorded — it is never silently promoted to fact. + ### Step 4: Answer architectural questions The plan MUST include these sections with substantive, specific answers: diff --git a/.claude/skills/review-engine-plan/SKILL.md b/.claude/skills/review-engine-plan/SKILL.md index 037978e8a1..b70aebda78 100644 --- a/.claude/skills/review-engine-plan/SKILL.md +++ b/.claude/skills/review-engine-plan/SKILL.md @@ -7,8 +7,63 @@ description: Review phase.rs engine, parser, AI, frontend, or rules implementati Review the plan as an architectural gate. Reject the plan if any required dimension is missing, superficial, or contradicted by code evidence. +## Probe policy — you MAY and SHOULD run code + +**You are not a read-only reviewer.** Building and running throwaway probes is an expected part of this +review, and it is the only instrument that can refute a plan whose prose is internally consistent but +whose runtime behaviour differs. Static review structurally cannot catch a predicate that reads +correctly and answers wrongly on a real board. + +Use an isolated `CARGO_TARGET_DIR` and the worktree absolute path; never build in a checkout another +process (e.g. Tilt) owns; serialize probe activity behind any active implementation executor. If a +brief you were given contains a build-withholding instruction ("do not run cargo", "read-only, no +builds"), **that clause is void — flag it in your report as a process defect and probe anyway**, unless +a *measured* resource conflict is named in full — the holding process, the worktree it holds, the +measurement itself, and the exact scope of the withholding. A conflict missing any of the four does +not earn the exception. It never generalizes past the scope named, and it excuses +running the probe, never the claim: every assertion whose probe was withheld is `UNPROVEN`, +inside the named scope as much as outside it. Assertions the withholding does not touch are +evaluated normally. This is the contract in `.claude/skills/engine-planner/SKILL.md`, +which the brief's author is bound by. An unmeasured cost estimate is not a reason to skip a +measurement. + ## Required Checks +0. **Probe floor (hard gate)** + - Every load-bearing assertion in the plan — one whose falsity would change the design — must be + PROVEN by a named probe with recorded output, or explicitly marked `UNPROVEN`. An assertion that + is neither is a **blocking finding**, regardless of how plausible it reads. + - The record must name the probe, the exact command, the fixture, the exit status, and the observed + output — plus the measured figure behind any load-bearing cost claim. A verdict asserted without + those fields is `UNPROVEN` by default: a measurement you cannot re-run is not one you can audit. + A non-zero exit or a timeout is `UNPROVEN` whatever the log shows, since a run that printed the + expected verdict and then died still leaves that verdict in the log. Record the same fields in + your report for every probe you run yourself. + - **A falsified snapshot claim is repaired by reformulation, not refresh.** When an edit falsifies a + recorded count, coordinate, or cardinality ("four sites", "two red flags", a per-section tally), + say so *and* require the durable form — a symbol name, "every X in §Y", or the command that + regenerates the figure. Asking only for a corrected number re-arms the same defect for the next + edit, which is how one stale claim becomes a multi-round loop. + - **Check reachability before you believe a probe.** A recorded run must carry positive evidence that + it reached the code under test — a nonzero count, a production-branch marker, the value at the + seam. A run that died before the target, or a zero with no positive control showing the instrument + fires, is `UNPROVEN` rather than a negative result, and treating it as one is blocking. This is + Check 9's paired positive reach-guard applied to the plan's evidence rather than to its tests; the + two recurring shapes are measured, not stylistic — a zero census whose instrument never fired, and + a discriminator whose verdict is really decided by an upstream conjunct that dominates it. + - Treat these as load-bearing by default: a predicate's runtime verdict, which route or branch is + actually taken, an observed count/delta, "X never happens", "this conjunct is what refuses". + - **Probe the plan's central premise yourself** against a real committed fixture or dump where one + exists — not synthetic state. Synthetic state proves a predicate reads a field; a real board + proves what it answers in production. A premise that only ever held on synthetic input is the + highest-value refutation available to you. + - **Verify every board-census premise against a hostile fixture.** The shape that survives static + review is a census predicate that ignores an applicability/filter field and so matches objects + with nothing to do with the phenomenon. Run the census on a real fixture that *contains irrelevant + objects* and confirm the predicate actually consults the applicability field. A census matching + "almost everything" is a defect signature, not a result: **reject the premise** until that field + check is shown. A plausible positive result on friendly input does not discharge this. + 1. **Class vs card** - Identify how many cards or patterns the plan covers. - Reject one-card plans unless the card is only the validating consumer of a reusable building block. diff --git a/.claude/skills/review-impl/SKILL.md b/.claude/skills/review-impl/SKILL.md index a58be3c1bf..3d82ca5aa5 100644 --- a/.claude/skills/review-impl/SKILL.md +++ b/.claude/skills/review-impl/SKILL.md @@ -13,7 +13,7 @@ Review for gaps: things that are missing or wrong. Do not spend findings on styl 2. Classify the surface area: engine logic, parser, frontend/UI, multiplayer/transport, AI heuristics, deck/format/feeds, build/CI/release, or docs. 3. Apply only the relevant lenses below. 4. If the scope is a PR, fetch whatever external review comments exist (CodeRabbit, human reviewers) and confirm-or-refute each against the current head with code evidence, folding confirmed findings into your own. **Assume none exist by default** — Gemini Code Assist has been sunset, so no bot is guaranteed to have pre-screened this PR. Your own lenses are the complete review, not a supplement to a bot's; do not under-invest expecting a backstop. Where an external finding *does* exist, silently omitting it — or returning a verdict less severe than an open, unrefuted finding from another reviewer — is itself a defect. Review comments, checks, and uploaded/sticky artifacts count only when their evidence identifies the current PR head SHA; otherwise report the evidence as missing rather than attributing it to the current diff. -5. If the scope is a PR touching engine/parser source, the parse-diff sticky comment (marker ``) is required evidence: fetch its full body and confront the card-level diff against the PR's claimed scope. Unexplained gained/lost/changed cards are findings (unintended parser blast radius). A *Baseline pending* body means the diff is unavailable — flag it so the handler brings the branch current to regenerate it; an absent comment despite changed engine source, or a comment/artifact not bound to the current PR head SHA, means CI evidence is missing for the current head. This PR-head requirement does not apply to Engine-Implementer Checkpoint Mode: before a push, validate its canonical receipt and, when forced, its direct projection/comparator artifacts against `BASE_SHA..CANDIDATE_SHA` instead. +5. If the scope is a PR touching engine/parser source, the parse-diff sticky comment (marker ``) is required evidence: fetch its full body and confront the card-level diff against the PR's claimed scope. Unexplained gained/lost/changed cards are findings (unintended parser blast radius). A *Baseline pending* body means the diff is unavailable — flag it so the handler brings the branch current to regenerate it; an absent comment despite changed engine source, or a comment/artifact not bound to the current PR head SHA, means CI evidence is missing for the current head. This PR-head requirement does not apply to Engine-Implementer Checkpoint Mode: review the committed `BASE_SHA..CANDIDATE_SHA` diff instead. 6. Report findings only. Silence means LGTM. When `pr-contribution-handler` explicitly requests the manual quality gate, add `Quality Gate: PASS|FAIL` before findings. PASS requires all three current-PR facts: (1) claimed parse-impact count equals the measured parse-diff count and the normalized card sets are identical, using the full artifact when the sticky comment truncates examples; (2) the change is at an existing authority/right seam and reuses its vocabulary; and (3) a production-pipeline test is demonstrated to fail when the production change is reverted. On PASS, return the applicable existing praise tokens (`right-seam`, `scope-discipline`, `discriminating-runtime-test`, `parameterized-not-proliferated`) for the ordinary review/enqueue event. Never infer quality from Tier or standing and never create a `quality_recommended` event. @@ -27,29 +27,19 @@ Skip checks CI already enforces: ## Engine-Implementer Checkpoint Mode -Default review output is findings-only. Exception: when `/engine-implementer` invokes this skill against a checkpointed candidate, it supplies `BASE_SHA`, `CANDIDATE_SHA`, the committed reproducible `BASE_SHA..CANDIDATE_SHA` diff, frozen scope paths, the named `START_SHA`/`IMPLEMENTATION_WORKTREE` clean-start/stable-HEAD attestations, the detached `COMPLETION_WORKTREE` start/end identity attestations, the canonical receipt, completion evidence, and the existing maintainer-simulation matrix. Validate the receipt before applying universal lenses. Confirm the first round has `START_SHA == BASE_SHA`, each fix starts from the prior reviewed candidate, the implementation worktree was clean at start and stable at checkpoint, the completion worktree was detached, clean, and exactly at `CANDIDATE_SHA` before and after completion, and the receipt's candidate/head values are exactly `CANDIDATE_SHA`. - -The receipt must be the un-hashed UTF-8/LF `engine-implementer-receipt-v1` document with one final LF, no CR/NUL, percent-encoded values (only `[A-Za-z0-9._~-]` raw), the fixed fields in their mandated order, the mandatory `source_hash_record.base.*` and `.candidate.*` groups, and complete ascending indexed groups. Verify the frozen scope representation is duplicate-free NUL-delimited bytes from `printf '%s\0' "${SCOPE_PATHS[@]}" | LC_ALL=C sort -z`, and verify its recorded path and SHA256. Verify `scoped_diff_command` is exactly `git -C "$IMPLEMENTATION_WORKTREE" -c color.ui=false -c diff.noprefix=false -c core.quotepath=true -c diff.orderFile=/dev/null -c diff.interHunkContext=0 -c diff.suppressBlankEmpty=false diff --no-color --no-ext-diff --no-textconv --no-renames --diff-algorithm=myers --no-indent-heuristic --full-index --binary --src-prefix=a/ --dst-prefix=b/ --unified=3 "$BASE_SHA" "$CANDIDATE_SHA" -- "${SCOPE_PATHS[@]}"`, then reproduce and hash those scoped-diff bytes. Verify `projection_authority_diff_command` is exactly `git -C "$IMPLEMENTATION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- Cargo.toml .cargo/config.toml rust-toolchain.toml scripts/engine-source-hash.sh`, reproduce and hash its NUL-delimited output, and require its recorded path/size/SHA256. Each source-hash group must contain, in order, `command`, `expected_sha`, `head_before`, `detached_before`, `clean_before`, `head_after`, `detached_after`, `clean_after`, `exit`, `stdout_path`, `stdout_sha256`, `stderr_path`, `stderr_sha256`, `artifact_path`, `artifact_size`, and `artifact_sha256`; it must bind the exact `scripts/engine-source-hash.sh` command and output artifact to its expected SHA. Verify every source-hash, projection, and completion group contains ordered `detached_before` and `detached_after` fields, each proving `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; a measurement-group failure is `CANNOT_ANSWER`, and a completion-group failure is a gate failure. Verify every recorded artifact path, size, and SHA256, but do not expect a hash for the receipt itself. Its fixed identity fields, scoped-diff command/path/size/digest, source hashes, authority-diff command/path/size/digest, canonical `projection_forced_reason`, mandatory source-hash records, parser evidence, completion records, artifacts, and declared absent inputs are mandatory; the projection section is omitted only when source hashes are equal, the authority-diff output is empty, and `projection_forced_reason=NONE`. A missing, duplicate, unordered, malformed, or digest-mismatched field is a blocking finding. +Default review output is findings-only. Exception: when `/engine-implementer` invokes this skill against a checkpointed candidate, it supplies `BASE_SHA`, `CANDIDATE_SHA`, the in-scope paths, the named `START_SHA`, what the completion checks were and how they came out, and the existing maintainer-simulation matrix. Confirm the first round has `START_SHA == BASE_SHA` and each fix round starts from the prior reviewed candidate. In this mode, emit these lines before findings: ```text Review Head: -Receipt SHA256: -Semantic-Impact Gate: PASS|FAIL Completion Gate: PASS|FAIL Maintainer-Simulation Gate: PASS|FAIL ``` -After the headers and findings, emit exactly one JSON object on its own line: `{"clean":,"findings":[,...]}`. Set `clean=true` only when all three gates pass and there are no findings; otherwise set it to `false` and include every failed gate and blocking finding in `findings`. - -`Review Head` must be the supplied `CANDIDATE_SHA`, the reviewed diff must reproduce from exactly `BASE_SHA..CANDIDATE_SHA`, and `Receipt SHA256` must be the SHA256 of the validated receipt bytes emitted externally in this review result; otherwise report a blocking finding. The receipt never contains or hashes itself. `Semantic-Impact Gate` passes only when the receipt validates and either (a) equal SHA-bound source hashes, an empty authority-diff output, and `projection_forced_reason=NONE` set `NO_PARSE_AFFECTING_CHANGE` with no projection section, or (b) a non-`NONE` canonical forced reason sets `PROJECTED_PARSE_DIFF` and contains all direct-projection records. - -`Semantic-Impact Gate` rejects missing/mismatched source-hash, authority-diff, or projection evidence, including either mandatory source-hash record, an extra projection, missing receipt evidence, or `CANNOT_ANSWER`. It passes only when equal source hashes retain complete `source_hash_record.base` / `.candidate` groups, the exact authority-diff artifact is empty, and `projection_forced_reason=NONE` has `NO_PARSE_AFFECTING_CHANGE` with no projection section, or when a source-hash difference and/or authority-diff path has the matching canonical non-`NONE` reason and exactly one base and one candidate direct projection with the complete evidence below. - -For any non-`NONE` forced-reason receipt, require the pinned `AtomicCards.json` artifact, exactly one detached clean base projection and one detached clean candidate projection, an isolated `cargo build --profile tool --features cli --bin oracle-gen --bin coverage-report --bin coverage-parse-diff` record for each side, and one base-built comparator record that contains both `--base-sha` and `--head-sha`. Every build/projection/comparison record must include command, environment, worktree, target, expected SHA, ordered `head_before`, `detached_before`, `head_after`, `detached_after`, `clean_before`, `clean_after`, exit, stdout/stderr paths and SHA256s, and produced artifacts; each detached field must prove `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr, and any other result is `CANNOT_ANSWER`. A source-hash difference or excluded-authority-input change is valid; any replica, quorum, manifest, seal, ledger, provenance-envelope, stale identity, omitted indexed field, noncanonical receipt encoding/order, missing/mismatched required source-hash, authority-diff, or projection record, extra projection, a projection when `projection_forced_reason=NONE`, or missing/mismatched artifact is a blocking finding. +`Review Head` must be the supplied `CANDIDATE_SHA`, and the reviewed diff must reproduce from exactly `BASE_SHA..CANDIDATE_SHA`; otherwise report a blocking finding. -`Completion Gate` passes only when the receipt's completion-check groups enumerate every scope-/plan-derived mandatory check and every applicable surface-derived check from the executor's existing implementation/fix verification blocks: formatting for implementation changes; the Rust/engine/parser Tilt-first-or-isolated-direct block for Rust/engine/parser paths; the frontend Tilt-first-or-isolated-direct block for frontend paths; and the parser preparatory gate for parser paths. Validate the set against the candidate diff; do not accept a generic plan-only list or preparatory executor success in place of candidate-SHA-bound records. Each entry must give its exact command, successful exit result, detached `CANDIDATE_SHA` identity and clean status at start, and detached `CANDIDATE_SHA` identity and clean status at end, including ordered `detached_before` and `detached_after` proof that `git symbolic-ref -q HEAD` exited exactly `1` with empty stdout and stderr; any other result is an operational failure. Every completion/parser command must either use `git -C "$COMPLETION_WORKTREE"` or execute in that worktree. For parser paths, validate the candidate range was enumerated NUL-safely with `git -C "$COMPLETION_WORKTREE" diff --name-only -z "$BASE_SHA" "$CANDIDATE_SHA" -- crates/engine/src/parser/` and `IFS= read -r -d ''`. Missing or unsuccessful mandatory checks fail the gate. For Markdown-only policy work, scope-path, SHA-identity, and Markdown/diff checks are the complete mandatory set, and Cargo/Tilt are not required. The reviewer validates the receipt before gate evaluation and final acceptance validates the receipt and its artifacts after review; neither action is a self-referential `completion_check` row or receipt artifact. `Maintainer-Simulation Gate` passes only if every changed seam has a concrete row covering production entry, first production branch reached, selected authority / bound value when applicable, binding time, live vs snapshotted semantics, storage, consuming function, invalidation behavior, hostile fixtures, and serialized-surface impact. Use `FAIL` when any row is missing, superficial, or contradicted by the diff, and report the specific gap as a normal finding. Outside this scoped mode, keep silence-as-LGTM behavior. +`Completion Gate` passes when every check the changed surface calls for was run against the committed candidate and passed: formatting for implementation changes, the Rust/engine/parser block for Rust paths, the frontend block for frontend paths, the parser gate for parser paths. Check the set against the candidate diff rather than against the plan, and re-run anything you doubt — you have the candidate SHA and a shell. A check run against uncommitted edits, or against a different tree, does not count. Markdown-only policy work needs scope and diff checks only. ## Phase Mode (chartered runs) @@ -58,7 +48,7 @@ Activated when the spawn inputs include a phase charter and **one phase's deferr - **Deferred ≠ gap; foreclosure = gap.** An item on the phase's deferral list is chartered to a named later phase: do not report it as missing, dropped, or incomplete. DO report any code that forecloses or contradicts a deferred item. - **Lens scoping:** the new-field-threading, sibling-coverage, claim-to-test, and serialized-surface lenses evaluate consumers against the charter's phase attribution — a consumer attributed to a later phase is deferred, not silently dropped. The class-vs-single-case lens is satisfied from the charter's class attribution — an infrastructure phase's class coverage lives in the charter and lands with its consumer phase. - **Maintainer-Simulation Gate carve-out:** a matrix row whose consuming function or hostile fixture is deferral-listed to a named later phase is recorded `DEFERRED(phase n)` and does not FAIL the gate; a row the phase's own code forecloses still FAILs. "FAIL when any row is missing" is otherwise unchanged. -- **Checkpoint mode's receipt-validation and artifact rules are untouched** — phase mode scopes what counts as a finding, never what counts as valid provenance. The supplied `BASE_SHA` is the phase base, and the reviewed span is that phase's `BASE_SHA..CANDIDATE_SHA`. +- **Checkpoint mode's gates are untouched** — phase mode scopes what counts as a finding, never what counts as a valid check. The supplied `BASE_SHA` is the phase base, and the reviewed span is that phase's `BASE_SHA..CANDIDATE_SHA`. ## Integration Mode (chartered runs, final review) @@ -67,7 +57,7 @@ Activated when the spawn inputs include a phase charter and the **run-span diff 1. **Cross-phase seams** — files touched by two or more phases, deferral handoff points, and the surfaces the charter's seam notes name. 2. **Charter completeness** — every deferral-list item either landed in its attributed phase or is reported unresolved. -The universal-lens full sweep is NOT re-run over the cumulative diff — each phase's review already ran it phase-scoped, and re-running it over the whole span would recreate the oversized-review non-convergence this machinery exists to eliminate. Chain integrity is NOT this review's job — the orchestrator verifies it mechanically at run-level acceptance. This mode is findings-only: no run-span receipt exists and none is required, because each per-phase receipt was validated at that phase's acceptance. +The universal-lens full sweep is NOT re-run over the cumulative diff — each phase's review already ran it phase-scoped, and re-running it over the whole span would recreate the oversized-review non-convergence this machinery exists to eliminate. Chain integrity is NOT this review's job — the orchestrator verifies it mechanically at run-level acceptance. This mode is findings-only. ## Universal Lenses @@ -147,4 +137,4 @@ Severity calibration: a latent bug — one not reachable today because a guard o Findings first. No praise, no diff recap. -Exception: in Engine-Implementer Checkpoint Mode, the `Review Head`, `Semantic-Impact Gate`, `Completion Gate`, and `Maintainer-Simulation Gate` lines precede findings. +Exception: in Engine-Implementer Checkpoint Mode, the `Review Head`, `Completion Gate`, and `Maintainer-Simulation Gate` lines precede findings.