fix: bind code-generation review receipts to the workspace source state (#629)

[iuryeng]

Summary

Implements #629: code-generation reviewer receipts now carry a fingerprint of the workspace source state the reviewer inspected, and every completion route refuses while the current source differs from the state the most recent recorded review saw. Source edited after the last review can no longer be shipped by that review's receipt. It does not establish per-unit attribution; see Known limitation below. Closes #629.

The mechanism follows the issue's framing ("does not bind that receipt to the source state"): a binding, not new audit events — workspace writes stay deliberately invisible to the audit trail, exactly as aidlc-audit-logger.ts intends.

Updated after three review rounds. The most recent one found five P1s in the fingerprintChainIsAdditionsOnly chain rule that an earlier round had introduced; all five reproduce, the rule is removed, the sibling-repo exclusion is fixed, and the limitation that remains is stated rather than claimed away. See the review-response comments below for the point-by-point.

Changes

1. Fingerprint (aidlc-lib.ts — workspaceSourceFingerprint / gitTreeFingerprint): a git-native content fingerprint per repo — git write-tree over a temporary index (GIT_INDEX_FILE) seeded from HEAD with git add -A applied, so it covers tracked edits, untracked adds, and deletions without ever touching the real index or worktree. Content-addressed: reverting an edit restores the original fingerprint. Recurses into every initialized git submodule (detected off the temp index's own gitlink entries, read with ls-files -z so a non-ASCII path is not lost to git's quoted-path escaping) and folds its own fingerprint into the parent's, so an uncommitted submodule edit is visible even though the gitlink itself doesn't move. Multi-repo: the intent's recorded repo set via repoDir(), combined as a sha256 over sorted name=sha lines; no recorded repos = the legacy single-repo default (projectDir).

The aidlc/.aidlc workspace shell is excluded only at the top level of the directory that carries the shell — the workspace roof, or a Bolt worktree with its own record mirror. A directory of those names inside a fingerprinted repo or submodule is application source and is part of the fingerprint. The pathspecs are directory-anchored, so a root file named aidlc is source too. .aidlc-sensors is excluded at any depth (a monorepo's per-package tsconfig can anchor the type-check sensor's cache anywhere). Git-ignored paths and uninitialized submodules are outside the binding.

2. Stamping (aidlc-log.ts review): a terminal --verdict for a workspace_requires stage (code-generation, the only one) records the fingerprint as a Source Fingerprint field on the REVIEW_COMPLETED row. A null fingerprint (not a git checkout) records no field.

3. Stage-level guard (aidlc-state.ts — verifyReviewerPrecondition): during receipt collection, only the chronologically newest fingerprinted receipt is tracked; after collection, ONE reconciliation against the recomputed current-tree fingerprint — a match keeps every collected receipt (a normal sequential multi-unit run, unit A reviewed then unit B coded and reviewed, is not falsely invalidated), a mismatch discards all of them (a workspace-global hash can't attribute which unit changed). A settled autonomous swarm (isSettledSwarmForArtifactGuard) is exempt from this reconciliation entirely — the main checkout's mismatch before finalize merges the AIDLC data back is expected, not a staleness signal. Runs on all four completion routes (approve/advance/finalize/complete-workflow). Fail-open where binding is impossible: legacy receipts without the field and unbindable workspaces keep today's behavior. Off-switch: AIDLC_SKIP_SOURCE_FRESHNESS=1.

4. Swarm finalize guard (aidlc-swarm.ts — reviewerReceiptError): for a converged, claimed unit, the terminal receipt's Source Fingerprint is compared against workspaceSourceFingerprint recomputed scoped to that unit's OWN Bolt worktree (not the main checkout) — a mismatch refuses the merge. This is the worktree-scoped half the settled-swarm exemption above depends on: the stage-level guard steps aside at settle because finalize does this check instead. It is also the one route with genuine per-unit attribution, since a Bolt worktree is a per-unit checkout.

5. Docs: audit-format.md's REVIEW_COMPLETED row lists the new field; 12-state-machine.md's REVIEW_COMPLETED and SWARM_UNIT_CONVERGED rows describe the semantics, the limitation and the fail-open cases; the off-switch is documented in prose alongside its two siblings; stage-definition.md, which defines workspace_requires, names the binding as a consequence of the flag.

6. Tests (tests/unit/t244-source-freshness-receipts.test.ts): fingerprint semantics in-process (deterministic, content-addressed, tracked-edit + untracked-add sensitive, null off-git, real index untouched, submodule-recursive including a non-ASCII path, and the exclusion scope — a directory named aidlc inside a registered sibling repo or a submodule, and a root file of that name, must all move the fingerprint, while the roof's own shell must not and a nested .aidlc-sensors must not, including inside a registered repo); stamping + stage-level guard via the real aidlc-log/aidlc-state CLI (passes while source matches, refuses after a post-review edit, off-switch restores the legacy pass, stripped-field legacy rows stay fail-open); the multi-unit policy (ordinary sequential run, the §12a rework loop, a shared-file M transition, and a recorded-repo workspace all pass; the two limitation cases assert the documented behaviour); swarm finalize via real Bolt worktrees.

Known limitation (explicit policy)

The fingerprint is one workspace-global hash while receipts are per unit, so a match proves only that the current source tree is identical to the state the newest recorded review inspected — not that each unit was reviewed against its own code. Source written or changed before that newest review, including an earlier unit's file and including code no reviewer was shown, is inside the matching tree and passes; one fresh receipt re-validates every earlier receipt.

An earlier revision of this PR tried to recover attribution from the shape of the diff between consecutive receipts (fingerprintChainIsAdditionsOnly: every transition had to be a pure addition). It was removed after being reproduced failing in both directions at once: it refused the rework loop stage-protocol.md §12a mandates (record NOT-READY, fix the artifact in place, re-review — an M transition) and any second unit touching a pre-existing shared file; it could never be cleared by re-reviewing, since every fingerprinted receipt is appended unconditionally; it could never be satisfied by the default recorded-repo layout, whose fingerprint is a sha256 composite rather than a diffable git object; and it still admitted a pure addition nobody reviewed.

Per-unit attribution needs a machine-readable manifest of the paths each unit wrote. code-summary carries that in prose only, and no such manifest exists in core/tools/ or core/hooks/ today, which is why it is follow-up work rather than part of this PR. #629's acceptance criterion is conditional — "ambiguous attribution fails closed or follows an explicitly documented policy" — and this takes the documented-policy branch. The tradeoff is real: this ships a weaker guarantee than the earlier prose claimed, and the laundering window stays open on the stage-level route until attribution lands.

Version note

This branch is 2.5.12, which is unreleased here (v2's tip is 2.5.11); the entry was rewritten in place rather than opening a second heading for one feature. The test file holds slot t244, free on v2 today. Both are contested by other open PRs — if either is taken first, I'll rebase, re-bump and rename per the AGENTS.md conflict-trap before merge.

User experience

Before: produce a unit's source, record the architecture review, then edit src/whatever.ts — approve/advance/finalize still complete, shipping source the reviewer never saw. After: the completion route refuses with Refusing to complete "code-generation": the workspace source no longer matches the state of the most recent recorded review (source-fingerprint mismatch) … and names both recoveries — record a fresh verdict, or revert the edit (content-addressed, so an undone change restores the original value). A normal multi-unit sequential run, the §12a rework loop, shared-file integration and a settled autonomous swarm are all unaffected; a swarm unit whose worktree source changed after its own review is refused at finalize with a worktree-scoped mismatch message.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• I have reviewed the contributing guidelines
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented

Test Plan

• bun scripts/package.ts --check → green for all 5 harnesses.
• bun run typecheck → clean across all 3 tsconfigs; biome check --error-on-warnings → clean.
• Coverage registry regenerated (bun tests/gen-coverage-registry.ts) — --check reports fresh, guards green, ratchet held.
• bun tests/run-tests.ts --unit --filter "t244|t115|t205|t206|t208|t186|t68|t134|t152|t166|t201|t221|t242|t174|t246|gen-coverage" → the reviewer-guard, multi-repo and version-sync families.
• Reproduced all five findings from the latest review round, independently, against this branch's own tools, before changing anything.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

[apackeer]

Reviewed at a1e4d67. The mechanism is well built: the temp-index git write-tree genuinely never touches the real index (t245 pins it), content-addressing correctly un-strands reverted edits, fail-open covers legacy receipts and non-git workspaces cleanly, and the refusal messages name the cause and the exact recovery command. The single-unit direct-invocation flow (the issue's literal repro) is correctly fixed.

The problem is the binding's scope: one workspace-global fingerprint, anchored at time-of-stamp, bound to receipts that are per-unit and (on swarm) per-worktree. That combination refuses the normal multi-unit flow and deadlocks autonomous swarm settle. Findings below, most severe first; everything was verified against this branch's head, and the two flow-breaking items were reproduced with the PR's own tools.

P1: the engine-driven multi-unit flow is falsely refused (reproduced)

code-generation is for_each: unit-of-work with a single stage-level gate presented once, after ALL units are built (core/aidlc-common/stages/construction/code-generation.md frontmatter; stage-protocol's engine-driven per-unit iteration). The default sequence is therefore: code unit A, review A (receipt stamped over a tree that does not yet contain B's code), code unit B, review B, then one approve. At that approve the recomputed fingerprint matches only the LAST unit's receipt; every earlier receipt is discarded by the mismatch filter (core/tools/aidlc-state.ts:1408-1414), missing becomes non-empty, and completion refuses.

Reproduced end to end with this branch's tools: two units seeded via the Bolt DAG, sequential code+review with nothing edited un-reviewed, single approve:

Refusing to complete "code-generation": ... 1 of 2 applicable units have no fresh
recorded review (alpha). At least one recorded review was discarded because the
workspace source changed after its verdict (source-fingerprint mismatch) ...

Re-recording unit A's review against the final tree unblocks (a second approve passes), but that bakes in N-1 redundant re-reviews per batch on every multi-unit run, and it contradicts the issue's acceptance criterion: "Per-unit completion does not incorrectly invalidate unrelated units where attribution is available." Note the declared-artifact invalidation directly above the new filter already scopes per unit via the construction/<unit>/ path segment; the fingerprint filter has no analogous scoping.

t245 cannot catch this: its guard scenarios run with no units doc, so the per-unit branch resolves none and falls back to the stage-level path that needs only one fresh receipt, which is exactly the receipt that always matches (the last one recorded). A two-unit guard test would have surfaced this immediately.

P1: autonomous swarm cannot complete code-generation (settle deadlock, single-repo layout)

The swarm path records receipts inside each Bolt worktree (SKILL.md: aidlc-log.ts review ... --project-dir "<worktree>"), so the stamp is a fingerprint of the WORKTREE tree. finalize merges those receipts verbatim into the main audit, but the referee merges only the aidlc data (state, audit, runtime fragment), not the code. At the settle directive's approve, verifyReviewerPrecondition recomputes over the MAIN checkout, where the swarm's code is not merged, so every worktree-stamped receipt mismatches and is discarded. This guard has no swarm carve-out (the settled-swarm exemption covers only verifyStageArtifacts, aidlc-state.ts:1240-1244), while SKILL.md's settle instruction says "do not dispatch the reviewer again; the per-unit receipts already cover the stage". The guard demands fresh receipts and the protocol forbids producing them; the only exit is AIDLC_SKIP_SOURCE_FRESHNESS=1. Also reproduced during this review: an unchanged, reviewed unit finalized successfully, then the settle approve refused with the mismatch message.

(A multi-repo intent escapes only by accident: repoDir() targets are absent inside the worktree, workspaceSourceFingerprint returns null, no stamp lands, and those receipts stay fail-open. The default single-repo layout deadlocks.)

P1: swarm finalize itself never checks the fingerprint

reviewerReceiptError (core/tools/aidlc-swarm.ts, the receipt scan) accepts any terminal READY/NOT-READY verdict and never reads the Source Fingerprint field, so on the swarm path review, then edit source, then finalize still merges unreviewed code. The PR body declares this half as a follow-up scope cut, which is a fair call on its own, but combined with the previous item the swarm route currently gets the breakage (settle refusal) without the protection (merge-back verification). The issue's acceptance criterion "Autonomous swarm finalize verifies the reviewed source state before merge-back" stays unmet.

All three P1s share one root cause and one fix space. Two shapes that would resolve them together:

• scope the fingerprint per unit (the attribution design the issue's design-considerations section sketches), or
• compare only the newest surviving receipt's fingerprint for stage-level freshness, and validate swarm receipts inside their worktrees at finalize plus a settled-swarm exemption at approve (mirroring isSettledSwarmForArtifactGuard).

P2: fingerprint repo-boundary semantics

• Submodules are invisible. git add -A records an initialized submodule as a gitlink (its HEAD sha), so editing tracked source inside a submodule leaves the fingerprint unchanged even while git status reports the submodule modified. Reproduced in a scratch parent+submodule repo using the PR's exact recipe. A reviewed-then-edited submodule ships unreviewed.
• Nested aidlc dirs are spuriously included. The git rm -r --cached aidlc .aidlc .aidlc-worktrees exclusion is top-level only; a nested */aidlc/ change still alters the fingerprint (verified the same way). The engine writes such a tree in at least one case: the type-check sensor anchors .aidlc-sensors/.tsbuildinfo at the tsconfig dir (core/tools/aidlc-sensor-type-check.ts:270 via sensorsDir/docsRoot), so a monorepo with a nested tsconfig gets engine-written churn that invalidates receipts with no source change.

P2: documentation

No docs changes in the diff, while the repo's Documentation Policy and the issue's criterion ("Reviewer/audit documentation describes how source freshness is established") both require them in the same commit:

• core/knowledge/aidlc-shared/audit-format.md:85 lists REVIEW_COMPLETED's fields without Source Fingerprint; this knowledge file ships in every harness dist, so the shipped reference is now wrong about an event it defines.
• docs/reference/12-state-machine.md:275 documents exactly the receipt semantics this PR changes, and its guard off-switch list does not mention AIDLC_SKIP_SOURCE_FRESHNESS.

P2: mechanical

• The branch is CONFLICTING with v2, and the 2.5.9 slot is taken (v2's tip is 2.5.10): rebase, re-bump to the next free patch, and rename the CHANGELOG heading per the AGENTS.md conflict-trap convention.
• t245 collides with tests/unit/t245-kiro-ide-hook-registrations.test.ts on v2, and t246 is also taken: renumber the new file to t247 and regenerate the coverage registry.

Verified green at this head

t245 7/7 plus the reviewer-guard regression family (t111, t115, t205, t206, t68: 179 assertions, 0 failures), bun run typecheck clean, bun scripts/package.ts --check clean across all five harnesses.

This is worth landing: the fingerprint idea is the right instrument for the gap the issue describes, and the implementation discipline (temp index, fail-open, off-switch symmetry) is exactly right. It needs the binding scoped to match how receipts are actually recorded (per unit, per worktree) before it can hold on the flows that matter most, which are the multi-unit and swarm runs.

[iuryeng]

Thank you for the extremely thorough review — reproducing both P1s yourself before writing them up made this much easier to fix correctly. I reproduced all three P1s and both P2 findings independently using this branch's own tools, then implemented the second shape you sketched (per-receipt fingerprint scoped to the newest-stamp reconciliation + worktree-scoped swarm finalize validation + settled-swarm exemption), plus both P2 fixes and the docs.

P1 #1 — multi-unit false refusal → fixed (last-receipt-wins)

verifyReviewerPrecondition (aidlc-state.ts) no longer compares each receipt's fingerprint against the current tree individually. During the event walk it now tracks only the chronologically newest fingerprinted receipt; after the walk, ONE reconciliation: a match keeps every collected receipt (so unit A's earlier receipt survives unit B's later one), a mismatch clears all of them (mirroring the existing ambiguous-artifact clear-all precedent, since a workspace-global hash can't attribute which unit changed). Reproduced your exact scenario first (two units, sequential code+review, single approve → falsely refused with "1 of 2 applicable units"), confirmed the fix resolves it, and added a dedicated regression test.

P1 #2 — swarm settle deadlock → fixed (settled-swarm exemption)

verifyReviewerPrecondition now checks isSettledSwarmForArtifactGuard (the same exemption already proven for the produces-existence guard) before ever recording a fingerprinted receipt as the "newest" one to reconcile. Once every DAG unit has a current-run SWARM_UNIT_CONVERGED row, the fingerprint reconciliation is skipped entirely at approve — the main checkout's mismatch (expected, since the code isn't merged yet) no longer blocks settle. Reproduced the deadlock (unchanged, reviewed unit finalized, then settle approve refused with the mismatch message), confirmed the fix, added a regression test.

P1 #3 — swarm finalize never checked the fingerprint → fixed

reviewerReceiptError (aidlc-swarm.ts) now reads the terminal receipt's Source Fingerprint and recomputes workspaceSourceFingerprint scoped to that unit's OWN worktree (not the main checkout), refusing the merge on a mismatch with a descriptive error. This closes the gap you flagged: settle refusal without merge-back protection. One perf note from getting there: git submodule status measured 1s+ per invocation on Windows, so the submodule-detection code (next section) reads gitlinks directly off the already-populated temp index (ls-files -s, mode 160000) instead — a fingerprint recomputed on every completion route can't absorb a 1s+ tax per call. Added regression tests for both the refusal and the pass-through cases.

P2 — repo-boundary semantics → both fixed

• Submodules: gitTreeFingerprint now recurses into every initialized submodule (detected via the temp index's own gitlink entries) and folds its own fingerprint into the parent's, so an uncommitted edit inside a submodule changes the combined hash even though the gitlink itself doesn't move. Reproduced your repro shape (edit inside submodule, gitlink sha unchanged, fingerprint now changes), added a regression test.
• Nested aidlc dirs: the exclusion now uses :(glob)**/<name>/** pathspec magic instead of a plain top-level pathspec, so aidlc/.aidlc/.aidlc-worktrees are excluded at any depth — and I added .aidlc-sensors to the excluded family too, since that's exactly the concrete case you cited (the type-check sensor's .tsbuildinfo, anchored at the tsconfig dir per sensorsDir, wasn't excluded at ANY depth before, root included). Verified against a nested-monorepo fixture that real nested source still changes the fingerprint (the exclusion targets the 4 exact names, not the whole subtree).

P2 — docs → both fixed

• audit-format.md's REVIEW_COMPLETED row now lists the optional Source Fingerprint field.
• 12-state-machine.md's REVIEW_COMPLETED and SWARM_UNIT_CONVERGED rows now describe the fingerprint semantics (last-receipt-wins, settled-swarm exemption, worktree-scoped finalize check) and the AIDLC_SKIP_SOURCE_FRESHNESS off-switch.

P2 — mechanical → both fixed

• Rebased onto v2 (now at 2.5.10); this branch is 2.5.11.
• t245 → t247 (the collision you flagged), coverage registry regenerated.

Verified at this head

• bun tests/run-tests.ts --unit --filter "t247|t205|t206|t111.|t115|t68|t166|t201|t226|t227|t230|t152" → PASS (12 files, 291 assertions) — t247 now carries 13 tests (the original 5 + your multi-unit reproduction + 6 new: submodule, nested-dirs, settled-swarm, swarm-finalize refusal, swarm-finalize pass-through).
• bun tests/run-tests.ts --unit --verbose (full unit tier) → PASS.
• bun run typecheck clean across all 3 tsconfigs; biome check --error-on-warnings clean; bun scripts/package.ts --check clean across all 5 harnesses; coverage registry regenerated and fresh.
• Reproduced all three P1s and both P2s against this branch's own tools before fixing any of them, per your write-up.

Really appreciate the depth here — the per-unit/per-worktree scoping mismatch was exactly right as the root cause, and pointing at the two concrete resolution shapes saved a lot of design time.

[leandrodamascena]

The source-freshness mechanism still has two paths that silently allow unreviewed source to satisfy the completion gate:

• A later receipt for one unit refreshes the workspace-global fingerprint while older receipts for modified units remain valid.
• Recursive **/aidlc/** exclusions remove legitimate application source from the fingerprint.

Submodule paths subject to Git quoting are also skipped, and the swarm documentation incorrectly states that finalization merges application source.

I reproduced all three fingerprint bypasses against head 965a161e. The added tests pass because they do not cover these adversarial sequences and paths.

The PR is also currently conflicting with v2 in CHANGELOG.md and tests/unit/gen-coverage-registry.test.ts, with no CI checks reported.

[leandrodamascena]

[P1] Do not let the newest receipt refresh every unit’s source binding

This only proves that the workspace has not changed since the last review, not that each unit’s current source was reviewed.

Reproduction: review alpha, review beta, edit beta, then review alpha again. The new global fingerprint matches the workspace, while the old beta receipt remains in reviewedUnits, so approval succeeds with unreviewed beta changes. A receipt for an unknown unit can produce the same result.

Please use unit-scoped source bindings or require an explicit final workspace-wide review rather than allowing one receipt to validate all older per-unit receipts.

[iuryeng]

Reproduced your exact scenario (and its inverse — re-reviewing an earlier unit after tampering a later one) and fixed at fe0810c: every consecutive fingerprint transition in a multi-receipt chain must now be a pure addition, not just the newest-matches-current check. See the summary comment above for the full explanation; both orderings are now covered by regression tests.

[iuryeng]

Update, and it is a retraction of what I wrote here earlier: I'm removing fingerprintChainIsAdditionsOnly. Your finding stands — the newest-matches-current comparison alone does not prove each unit's current source was reviewed, and with the rule gone your exact sequence (review alpha, review beta, edit beta, re-review alpha) passes again. I'm not claiming otherwise, and I'd rather say that plainly than leave this thread reading as fixed.

The reason is that the rule I built for it is wrong in both directions, and I reproduced both at 6300ad3b before deciding.

It refuses work the protocol itself mandates. stage-protocol.md §12a requires recording a NOT-READY receipt, re-invoking the lead to fix the artifact in place, then re-reviewing. Fixing in place is an M transition, so the chain fails closed even though the reviewer inspected exactly the current tree:

newest == current: true
transition       : "M\talpha.ts"
approve          : rc 1  source-fingerprint mismatch

And it still does not prove review. A file written after alpha's review and read by nobody is a pure addition, so the chain check passes and approve returns rc 0. So the rule buys a false refusal of the normal rework loop and of any second unit that touches a shared file, without closing the hole it was built for. Two further failures are in the summary comment above: re-review can never clear the chain once a transition is in it, and the recorded-repo layout — which is the default, since sibling auto-discovery populates repos at intent birth — produces a composite hash that is not a diffable git object at all.

Where that leaves your finding:

• It is documented, not buried. The CHANGELOG bullet and docs/reference/12-state-machine.md claimed the stronger property; both now state that a match proves only that the current tree is identical to the one the newest recorded review inspected, not that each unit was reviewed against its own code. The same paragraph goes in verifyReviewerPrecondition's comment block.
• Your repro survives as a permanent test, inverted rather than deleted — same scenario, asserting the documented behaviour, citing this round. When attribution lands it goes red on purpose.
• It is genuinely fixed on one route already: aidlc-swarm.ts finalize compares each claimed unit's terminal receipt against its own Bolt worktree's fingerprint, which is the unit-scoped binding you asked for. It is the stage-level route where the hash is workspace-global and the receipts are per unit.
• Your other finding here — the :(glob)**/aidlc/** pathspec swallowing real source — stays closed, and this round tightened it further: the exclusion was still being applied inside every recorded sibling repo and inside submodules, where a directory of that name is application source. Fixed, with repros.
• Follow-up for the real fix is [RFC]: per-unit attribution for code-generation review receipts #662. It needs a machine-readable manifest of the source paths each unit wrote; code-summary has that in prose only and there is no such manifest in core/tools/ or core/hooks/ today, which is why I don't think it fits in this PR.

This leans on #629's criterion being conditional — "ambiguous attribution fails closed or follows an explicitly documented policy" — and I've taken the documented-policy branch. That is a judgement call on a finding you raised, so if you'd rather I keep a fail-closed rule and fix the flows it breaks instead, or hold the PR until attribution exists, say so and I'll do that.

[leandrodamascena]

[P1] Restrict exclusions to framework-owned paths

:(glob)**/aidlc/** excludes every directory named aidlc, including legitimate application source such as src/aidlc/engine.ts or services/compiler/aidlc/index.ts.

Changes under those paths leave the fingerprint unchanged and can pass approval without another review. Please anchor framework exclusions to their actual workspace locations instead of excluding these names recursively.

[iuryeng]

Fixed at b4195e9 — restricted the any-depth exclusion to .aidlc-sensors only (the one name that genuinely isn't root-anchored); aidlc/.aidlc/.aidlc-worktrees are back to root-level-only. Verified with your exact examples: an src/aidlc/engine.ts-shaped path and the 2-level-nested services/compiler/aidlc/index.ts both correctly change the fingerprint now.

[leandrodamascena]

[P2] Parse submodule paths without Git quoting

With the default core.quotePath behavior, git ls-files -s quotes and escapes non-ASCII paths. For a submodule such as vendor/cafe-é, subPath contains Git’s quoted representation, isGitRepoDir returns false, and edits inside the submodule are omitted from the fingerprint.

This is reproducible: an uncommitted edit inside that submodule leaves the workspace fingerprint unchanged. Please use git ls-files -s -z and parse NUL-delimited entries.

[iuryeng]

Fixed at b4195e9 — gitlinks are now read via ls-files -s -z (NUL-delimited, quoting disabled) instead of -s. Verified with an accented submodule path (vendor/café-módulo): an uncommitted edit inside it now correctly changes the fingerprint.

[leandrodamascena]

[P2] Do not claim that swarm finalization merges application source

aidlc-swarm finalize invokes aidlc-bolt complete --merge, which merges state, audit, and runtime metadata only. It never invokes aidlc-worktree merge; aidlc-orchestrate.ts also explicitly states that generated source changes are not copied.

The new comments, documentation, and test names rely on a merge-back that does not happen. Please correct this description and avoid using it as lifecycle justification. The underlying missing source merge appears pre-existing and may require a separate fix.

[iuryeng]

Fixed at fe0810c — this specific copy of the claim was missed in the first pass (my doc sweep only grepped docs/CHANGELOG/aidlc-swarm.ts, not this comment block); caught via this inline comment, thanks. Corrected to match the other three spots: finalize's own merge-back (aidlc-bolt complete --merge) carries only the AIDLC data, never application source. Agreed the missing source merge itself is a separate, pre-existing concern out of scope here.

[iuryeng]

Thank you for the deep dive — reproducing all three bypasses before writing them up made these very actionable. I reproduced all three, plus the docs inaccuracy, against this branch's own tools before fixing any of them, then fixed all four at b4195e9e.

Bypass 1 — a later receipt refreshing the global fingerprint while an earlier unit's edit went unreviewed → fixed

Confirmed exactly as described: unit A reviewed, A's file edited with no new review, unit B coded and reviewed — B's receipt stamps a fingerprint over the tree that already contains A's unreviewed edit, so the newest-matches-current check alone passed even though nobody reviewed A's edit.

verifyReviewerPrecondition now requires more than a newest-match for a chain of 2+ receipts: every consecutive fingerprint transition must also be a pure addition — the only diff shape a legitimate sequential multi-unit flow actually produces (each unit's own new files, nothing pre-existing touched). Since a receipt's fingerprint is (for the common single-repo, no-submodule case) itself a raw git tree object, this is checked directly with git diff --name-status --no-renames <old> <new> between two tree shas — no commits needed. Any modified/deleted pre-existing path fails the whole chain closed; so does an unverifiable transition (a multi-repo or submodule-folded composite hash isn't a diffable git object, and neither is a pruned tree object) — both are exactly the "ambiguous attribution" case #629's acceptance criteria call out to refuse rather than trust. New regression test reproduces your scenario verbatim and asserts refusal.

Bypass 2 — recursive **/aidlc/** removing legitimate application source → fixed

Also confirmed: the any-depth exclusion I added for apackeer's .aidlc-sensors finding got applied to all four workspace names, which silently dropped real application source living under a directory coincidentally named aidlc/.aidlc/.aidlc-worktrees (e.g. a feature module literally named after the methodology).

Checked where each name is actually anchored: .aidlc-sensors genuinely isn't root-anchored (the type-check sensor can put it at any tsconfig dir in a monorepo), but aidlc/.aidlc/.aidlc-worktrees are — aidlc/ and .aidlc/ (worktreePath) only ever live at the fingerprinted repo's own top level, and for a multi-repo intent aidlc/ is a sibling of the repo dirs, never nested inside one. So the fix scopes the any-depth glob to .aidlc-sensors only and puts the other three back to root-level-only exclusion (their one legitimate location). New regression test proves a root-level occurrence of all three still excludes correctly, while a nested, coincidentally-named directory with real source now changes the fingerprint.

Bypass 3 — submodule paths subject to Git quoting skipped → fixed

Confirmed with a submodule at vendor/café-módulo: without -z, git ls-files -s quotes non-ASCII paths ("vendor/caf\303\251-m\303\263dulo"), and that literal quoted-and-escaped string never resolves to the real on-disk directory, so isGitRepoDir silently reports "not a submodule" and the fingerprint drops it — a reviewed-then-edited submodule at such a path shipped unreviewed. Fixed by reading gitlinks via ls-files -s -z (NUL-terminated, quoting disabled) instead. New regression test edits a file inside an accented-path submodule and asserts the fingerprint changes.

Docs inaccuracy — fixed

You're right, finalize never merges application source. aidlc-bolt complete --merge (what finalize actually calls) carries only the AIDLC data — state, audit, runtime-graph fragment. Corrected the three spots that implied otherwise (12-state-machine.md, CHANGELOG.md, and an aidlc-swarm.ts comment) to say what's actually true: the settled-swarm exemption exists because the swarm's application source lives only in the unit's own worktree, not because finalize is about to merge it — finalize's own merge-back never touches application source at all.

Mechanical

The CHANGES_REQUESTED review predates a rebase I'd already pushed to resolve a v2 conflict (this PR is now 2.5.12, not 2.5.11) — that part should be moot now; branch is MERGEABLE.

Verified at b4195e9e

• bun tests/run-tests.ts --unit --filter "t244|t205|t206|t111.|t115|t68|t166|t201|t226|t227|t230|t152|gen-coverage" → PASS (13 files, 328 assertions) — t244 now carries 3 more regression tests (16 total) reproducing each bypass.
• bun tests/run-tests.ts --integration --filter "t121|t135|t166|t185" and --e2e --filter "t134" → PASS (both exercise the swarm/multi-repo paths these fixes touch most directly).
• bun run typecheck clean, biome check --error-on-warnings clean, bun scripts/package.ts --check clean across all 5 harnesses, coverage registry regenerated and fresh.

Really appreciate the adversarial rigor here — the newest-match-only reconciliation and the blanket any-depth exclusion were both my own overcorrections from the first review round, and pointing at concrete repro scenarios for each made them trivial to close correctly instead of guessing.

[iuryeng]

Follow-up: I'd only read the review summary when I replied above — just went through the 4 inline comments individually and verified each against fe0810cb:

• aidlc-state.ts:1430 — reproduced your exact scenario (review alpha, review beta, tamper beta, re-review alpha) against the already-fixed code: refuses correctly (source-fingerprint mismatch). This is the inverse ordering of the case I'd tested myself (later unit masking an earlier one) — good catch that it needed checking both ways. Added it as a permanent regression test.
• aidlc-lib.ts:2201 — reproduced your exact examples (src/aidlc/engine.ts-shaped path, and the 2-level-nested services/compiler/aidlc/index.ts) against the fix: both now correctly change the fingerprint.
• aidlc-lib.ts:2229 — already fixed and tested with an accented submodule path.
• aidlc-state.ts:1399 (the "finalize merges the code back" claim) — this one I'd missed. My original doc sweep only grepped docs/, CHANGELOG.md, and aidlc-swarm.ts, and didn't catch that verifyReviewerPrecondition's own comment block in aidlc-state.ts had the same claim. Fixed now at fe0810cb to match the other three spots. Thanks for catching that.

All fixed and verified at fe0810cb.

[apackeer]

Reviewed at 6300ad3b. All four of @leandrodamascena's inline findings are genuinely fixed and I verified each against my own repros from the previous round: src/aidlc/engine.ts-shaped paths change the fingerprint again, the accented submodule (vendor/café) is covered by ls-files -s -z, and the "finalize merges the code back" claim is corrected in all four places. The reported laundering sequences now refuse. Thank you for reproducing each one before fixing it.

The fingerprint mechanism itself is good work and I want to be clear that I think it should ship. The temp-index git write-tree is the right primitive: gitignore-respecting, content-addressed so a revert un-strands a receipt, and it never touches the real index. Stamping it on REVIEW_COMPLETED and recomputing at all four completion routes is exactly what #629 asks for, and it catches source changed by generators, shell commands, and commits that no audit-event scheme can see.

My concern is confined to the new fingerprintChainIsAdditionsOnly chain rule. Its premise, that a pure addition is the only diff shape a legitimate sequential multi-unit flow produces, does not hold, and it is also not sufficient to prove review. All findings below are reproduced at this head; scripts are noted per finding.

P1: the chain rule refuses the reviewer rework loop the protocol itself mandates

stage-protocol.md:993-999 requires recording the NOT-READY receipt, then re-invoking the lead to fix the artifact in place, then re-reviewing. Fixing a file in place is a M transition, so the chain check fails closed.

Instrumented, single unit, nothing tampered:

chain fps:   b7de93c3a8c442a2fabb5e69275d4983585a5716 -> a270d9e9ae909b2e0cf00b3e07d8c211a50aa184
current fp:  a270d9e9ae909b2e0cf00b3e07d8c211a50aa184   (newest == current: true)
transition:  rc=0  diff="M\talpha.ts"
approve:     rc 1  source-fingerprint mismatch

Note the newest fingerprint equals the current tree. Nothing is unreviewed; the reviewer looked at exactly this source. Only the diff shape between two receipts triggers the refusal.

The same shape refuses a second unit whose work modifies a pre-existing shared file, which is ordinary integration wiring:

transition:  rc=0  diff="A\tbeta.ts\nM\tindex.ts"
approve:     rc 1  source-fingerprint mismatch   (newest == current: true)

P1: re-reviewing cannot recover, and the refusal names re-review as the fix

aidlc-state.ts:1441-1443 pushes every fingerprinted receipt unconditionally, so a fresh receipt only appends to the chain; the offending transition stays in it permanently. Three successive re-review-then-approve cycles all return rc 1 with nothing edited in between.

The only escapes are AIDLC_SKIP_SOURCE_FRESHNESS=1, or a full gate reject --feedback ... revision cycle, which bumps the attempt floor and clears the chain. The reject escape is not in the message, and the recovery the message does suggest never works. Round-1's false refusal at least had re-review as a way out.

P1: the standard recorded-repo layout can never satisfy the chain check

Two independent reasons, either fatal on its own:

1. workspaceSourceFingerprint returns a sha256 composite, not a git object, for any intent with recorded repos (aidlc-lib.ts:2284) or any repo with an initialized submodule (:2269).
2. fingerprintChainIsAdditionsOnly runs git -C projectDir (aidlc-lib.ts:2311), but in the workspace model projectDir is the roof, which is not a git repo at all (aidlc-lib.ts:2118-2126). Even two genuine tree shas fail there:

workspace roof is a git repo?  NO  -> fatal: not a git repository
diff of two REAL tree shas from the roof:  rc 1  "Could not access '1f402601...'"
same diff from inside repo-a:              rc 0  "A\tb.ts"

This is not an edge case: sibling auto-discovery auto-populates repos at intent birth (aidlc-utility.ts:3500-3507, resolveBirthRepoSet), so a single recorded repo is the normal layout. A textbook clean two-unit run, pure additions, nothing edited after the last review, is refused, and re-reviewing does not recover.

P1: aidlc/ is excluded inside sibling repos, hiding real application source

aidlc-lib.ts:2214-2221 applies the root-only exclusions relative to each fingerprinted repo, so for a recorded repo it strips repo-a/aidlc/**, repo-a/.aidlc/**, and repo-a/.aidlc-worktrees/**. But the record tree is the sibling <workspace>/aidlc/, and bolt worktrees are <workspace>/.aidlc/worktrees/ (aidlc-lib.ts:2114-2116, used at aidlc-worktree.ts:211). None of the three ever legitimately lives inside a repo.

before                        : daa781e7c30078511acbbc1fda991fca9dfa036a
after repo-a/aidlc/application.ts : daa781e7c30078511acbbc1fda991fca9dfa036a  UNCHANGED
after repo-a/control.ts           : d3e02cbe3f400a080c9220cb4cca75d936a1c0c5  (changed)

This is the mirror image of the nested-glob fix from the last round: that one over-excluded by depth, this one over-excludes by repo. The comment at aidlc-lib.ts:2164-2169 asserts these names live only at "the fingerprinted repoDir's own top level": true of the roof, false of a repoDir.

P1: additions-only does not prove review

An added file is exactly as unreviewed as a modified one. A rogue new file written after alpha's review and reviewed by nobody is a pure addition, so the chain check passes and approve returns rc 0. This is reachable in practice rather than theoretical, because the per-unit reviewer is read-scoped to its own unit (stage-protocol.md:965-975 plus the reviewer-scope PreToolUse hook), so beta's reviewer never sees it.

Suggested direction

The two failure classes have different causes and I'd treat them differently.

The fingerprint's scope is wrong in two spots, and both are small fixes: the per-repo exclusion above, and the roof-as-git-cwd. Worth fixing regardless of what happens to the chain rule.

The chain rule's premise is the deeper issue. Receipts are per-unit, but the fingerprint is one workspace-global hash, so any per-unit conclusion drawn from it is inference, and inference from diff shape is wrong in both directions at once, as the findings above show. I don't think this is a matter of tightening the rule; allowing modifies reopens laundering, and handling composites leaves additions-only unsound.

What I'd suggest, and I recognise this is a scope call that's ultimately the maintainers':

• In this PR: keep the fingerprint, the stamping, the newest-matches-current comparison, and the swarm-finalize check. That set fixes [Bug]: code-generation review receipts remain valid after workspace source changes #629's literal repro and the finalize gap, and breaks no ordinary flow. Drop fingerprintChainIsAdditionsOnly (aidlc-lib.ts:2307, called at aidlc-state.ts:1467); the git -C projectDir problem disappears with it, since that was its only consumer. Fix the sibling-repo exclusion. Then state the limitation plainly in the CHANGELOG bullet and 12-state-machine.md:275: the guard proves nothing changed since the last review, not that each unit was reviewed against its own code. Both currently claim the stronger property.
• Follow-up: per-unit attribution, which needs a machine-readable manifest of the source paths each unit wrote. code-summary already summarises "files, tests, key decisions" in prose, so the information exists but isn't machine-readable today. I checked and there is no per-unit source-path manifest anywhere in core/tools/ or core/hooks/, which is why I think this can't be solved inside this PR.

That reading leans on #629's own criterion being conditional: "does not incorrectly invalidate unrelated units where attribution is available; ambiguous attribution fails closed or follows an explicitly documented policy." Attribution isn't available yet, so the documented-policy branch looks like the intended path here. The tradeoff is real and worth naming: it ships a weaker guarantee than the current CHANGELOG claims, and the laundering window stays open until the follow-up lands.

Test coverage

t244's four multi-unit tests are all adversarial tamper cases. Whatever direction you take, these are the gaps: the rework loop, a modify-shaped legitimate transition, a recorded-repo (composite fingerprint) workspace, and an unreviewed addition, with that last one asserting whatever the documented policy ends up being.

Gates at 6300ad3

• bun run typecheck clean; bun scripts/package.ts --check green for all 5 harnesses.
• smoke+unit: 178 files, 4406 assertions, 0 failures.
• CI reports no checks on the branch; MERGEABLE but BLOCKED on review-required.
• Version 2.5.12 is contested with feat(kiro-ide): read the 1.x stdin hook context channel #615 and feat(tools): add upgrade subcommand for in-place AI-DLC install upgrades #535; test slot t244 is contested with docs: add stage-by-scope matrix to the scopes guide (guarded by t244) #596, feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.67) #617, and feat: move the ARS arithmetic into a deterministic ars subcommand (2.5.25) #644, though the slot is still free on v2.

[iuryeng]

Thank you — and specifically for reproducing all five before writing them up. I reproduced each one independently at 6300ad3b with this branch's own tools before changing anything, and took the direction you sketched: keep the fingerprint, the stamping, the newest-matches-current comparison and the swarm-finalize check; drop fingerprintChainIsAdditionsOnly; fix the sibling-repo exclusion; and state the limitation plainly rather than claiming the stronger property.

One thing up front, because it is the substance of this change and not a footnote: the chain rule exists because of @leandrodamascena's P1 on aidlc-state.ts, and removing it reopens exactly the laundering window he identified. His finding is not resolved by this change — it is deferred, with the limitation written into the CHANGELOG, 12-state-machine.md, the guard's own comment block and two regression tests. I've replied in his thread rather than only here, and his review is the one that has to clear this PR.

P1 — the rework loop the protocol mandates → confirmed, rule dropped

Reproduced stage-protocol.md §12a end to end (record NOT-READY, re-invoke the lead, fix the artifact in place, re-review). Fixing in place is an M transition:

newest == current: true
chain transition : rc=0  "M\talpha.ts"
approve          : rc 1  source-fingerprint mismatch

The second shape refuses too — a second unit that touches a pre-existing shared file, which is ordinary integration wiring:

chain transition : rc=0  "A\tbeta.ts\nM\tindex.ts"
approve          : rc 1  source-fingerprint mismatch   (newest == current: true)

P1 — re-review cannot recover, and the message names re-review → confirmed

aidlc-state.ts pushes every fingerprinted receipt unconditionally, so a fresh receipt only appends; the offending transition is permanent. Three re-review-then-approve cycles with nothing edited in between: rc 1, rc 1, rc 1. With the rule gone the refusal message is accurate again — a fresh verdict stamps over the current tree, newest == current, and the route passes.

P1 — the recorded-repo layout can never satisfy the chain check → confirmed, and worse than reported

Both of your reasons reproduce. Reproducing the second one turned up something slightly stronger than "the roof is not a repo": even when the roof is a git repo, its object store is a different one, so two genuine tree shas from sibling repos still fail:

diff of two REAL sibling-repo tree shas, run from a git-repo roof:
  rc 128  fatal: bad object 1f402601...
same diff from inside repo-a:
  rc 0    "A\tb.ts"

Confirmed too that this is the default and not an edge case: resolveBirthRepoSet → discoverSiblingRepos auto-populates repos at intent birth.

P1 — aidlc/ excluded inside sibling repos → confirmed, fixed

Reproduced exactly as you have it: repo-a/aidlc/application.ts leaves the fingerprint byte-identical while repo-a/control.ts moves it. Four changes:

• gitTreeFingerprint now takes a required carriesWorkspaceShell: boolean rather than deriving it. Required so a future call site has to decide; deriving it from dir === projectDir would have to survive case-insensitivity, separator normalisation, trailing separators, 8.3 short names and realpathSync divergence on Windows, and it needs projectDir threaded through the recursion anyway — the same plumbing cost for a comparison that can go wrong.
• workspaceSourceFingerprint passes true only on the repos.length === 0 branch (the dir the caller handed in) and false for every dir the function derives itself: each repoDir(projectDir, name), and the recursive submodule call. That last one fixes a second instance of the same defect I hit while writing the repro: vendor/lib/aidlc/application.ts inside an initialized submodule was invisible before and counts now.
• The pathspecs are now directory-anchored (trailing slash). A plain pathspec also matches a file of that name, so a root file named aidlc — a plausible CLI wrapper — was being dropped from the walk. Verified that the trailing slash drops the directory, keeps the file, and still does not touch src/aidlc/engine.ts.
• I dropped .aidlc-worktrees from the exclusion list. No code path in this repo creates that directory — worktreePath is <projectDir>/.aidlc/worktrees/bolt-<slug>, already covered by the .aidlc entry — and repo-wide the string occurs only in the constant, its own comment, its own test and the CHANGELOG bullet this PR added. As shipped it was pure false-negative surface: a project with a real top-level .aidlc-worktrees/ would silently lose that source for no benefit.

The Bolt worktree is the case a naive fix breaks: intentRepos(worktreePath(...)) returns [] in the layouts the engine produces, so the worktree takes the repos.length === 0 branch and keeps the exclusion.

P1 — additions-only does not prove review → confirmed, and it stays true

Reproduced: a file written after alpha's review and reviewed by nobody is a pure addition, so approve returns rc 0 both before and after this change. Worth adding that the same is true in the single-receipt case the rule never applied to — so the asymmetry it bought was a partial block on M-shaped tampers while A-shaped ones were always free. This one is not fixed by dropping the rule and I am not claiming it is; it is the case the documented policy now covers explicitly, and it is pinned by a test asserting rc 0 so the policy is visible in code and not only in prose.

Documentation — the stronger claim is gone

You were right that the CHANGELOG bullet and 12-state-machine.md:275 both asserted the stronger property. Both now say what the guard actually establishes: the current tree is identical to the tree the newest recorded review inspected — not that each unit was reviewed against its own code. The same paragraph replaces the removed rationale block in verifyReviewerPrecondition, so the comment does not go silent about a known hole.

Three prose fixes fell out of writing that:

• The CHANGELOG's exclusion sentence ("never legitimately nested") was false as shipped for the reason in the finding above — the exclusion was applied per repo dir, not at the workspace root. Rewritten in the same commit as the fix.
• The refusal message said "the current source tree was never reviewed". That is reachable-false: with a receipt on tree X, a later receipt on tree Y, and a revert to X, the guard refuses on newest != current while X was in fact reviewed. It now says the source no longer matches the most recent recorded review, and names revert as well as re-review.
• The per-unit refusal note said "at least one recorded review was discarded" and told the user to record one per unit. The clear is unconditional — every receipt is discarded — and a single fresh receipt restores them all, so the note now says that.

I also documented the off-switch in prose where its two siblings already are, including that it is check-time only: stamping with it set still records the field, so there is no durable poisoned receipt.

Tests

Your four gaps, plus the exclusion fix. Nothing was deleted quietly:

• The two multi-unit tamper tests that asserted the chain rule are inverted in place, not removed — same scenarios, now asserting rc 0 and naming the documented limitation, each citing the review round it came from. When per-unit attribution lands they turn red, which is the point.
• The third (3-unit mid-chain variant) is removed. Its only distinct claim was that the chain walk was not an adjacent-pair special case; with no walk, inverting it would assert exactly what the 2-unit test already asserts.
• New: the §12a rework loop; a legitimate M-shaped transition (second unit touching a pre-existing shared file); a recorded-repo workspace through the CLI (clean two-unit run passes, post-review edit refuses); an unreviewed addition asserting rc 0 per the documented policy; and the exclusion repros — repo-a/aidlc/**, repo-a/.aidlc/**, a submodule's own aidlc/** and a root file named aidlc must all move the fingerprint, while the roof's own shell must not and a nested .aidlc-sensors must not, including inside a registered repo.

Follow-up

Opened #662 for per-unit attribution, with your framing: it needs a machine-readable manifest of the source paths each unit wrote, code-summary carries that in prose only, and there is no such manifest in core/tools/ or core/hooks/ today. The tradeoff you named is real and I've said so in both the CHANGELOG and the PR body: this ships a weaker guarantee than the current prose claimed, and the laundering window stays open on the stage-level route until that lands.

One thing I found while probing and did not fix here, because it is pre-existing and separable: if any recorded repo or initialized submodule yields no fingerprint, workspaceSourceFingerprint returns null for the whole workspace, which is fail-open with no diagnostic. Since sibling auto-discovery registers repos at intent birth, later renaming or not cloning one sibling turns the binding off invisibly. Filed as #663 rather than folded in.

Gates at 3cbf1917

• bun run typecheck clean across all 3 tsconfigs; biome check --error-on-warnings clean; bun scripts/package.ts --check green for all 5 harnesses.

• Coverage registry regenerated; --check reports fresh, guards green, ratchet held (fingerprintChainIsAdditionsOnly was UNCOVERED, so no class drops).

• One caveat on the test numbers, since yours were clean and mine are not: I develop on Windows, where this suite has a standing set of environment failures (path-separator assertions, the packaging and build-binary tiers). Rather than quote a number you can't compare against, I ran the full unit tier twice — once on 6300ad3b in a detached worktree, once on this head — and diffed:

  6300ad3b   166 files, 10 failed files, 3288 assertions, 17 failed
  this head  166 files, 10 failed files, 3296 assertions, 16 failed
  

  Identical failing-file list, +8 assertions (the new tests), one fewer failing assertion. Two of the ten are t244 (the swarm-finalize pair, whose --check-cmd "test -f …" needs a POSIX shell) and t246 (docs\page.md vs docs/page.md); neither touches a file this PR changes, and both fail the same way on 6300ad3b. CI on Linux is the authority here, not my box.

• The reviewer-guard family specifically — t244|t115|t205|t206|t208|t186|t68|t134|t152|t166|t201|t221|t242|t174|t246|gen-coverage — is 14/16 files green, the two exceptions being the same environment failures above. t68 (version ↔ CHANGELOG ↔ badge) passes.

• Version and slot contention re-checked against origin/v2 immediately before this push: 2.5.12 (unreleased on this branch, v2 is 2.5.11), t244 free. I'll re-check both again before any final push and re-bump or renumber per the AGENTS.md conflict-trap if another PR takes them first.

[apackeer]

Approving, reviewed at 3cbf1917. All five round-3 findings are genuinely fixed, and I verified each by re-running my own round-3 reproductions against this head (same scripts, same worktree, PR dist confirmed via the carriesWorkspaceShell symbol before trusting any verdict):

• §12a rework loop (NOT-READY, fix in place, re-review): approve now rc 0, no mismatch. Same for the second M-shaped case, a second unit wiring into a pre-existing shared file.
• Recorded-repo / composite deadlock: the clean two-unit run through a recorded repo now passes (rc 0), and the multi-repo and submodule-bearing variants pass their clean flows too. With fingerprintChainIsAdditionsOnly gone (zero references left anywhere in the tree, dist/ included), the roof-not-a-git-repo failure mode is gone with it.
• No-recovery: with the chain rule removed there is no permanent refusal state left to recover from; the mismatch path's own instruction (fresh verdict OR revert) is now accurate, and the revert wording in reviewerPreconditionError matches what the content-addressed fingerprint actually does - I re-verified tamper-then-revert restores rc 0.
• Sibling-repo aidlc/ exclusion: repo-a/aidlc/**, repo-a/.aidlc/**, and a submodule's own aidlc/ all move the fingerprint now; the roof's shell still doesn't. The carriesWorkspaceShell parameter being required rather than defaulted is the right call - a new call site has to make the decision consciously. Dropping .aidlc-worktrees entirely checks out too: I grepped the tree and no code path ever creates that name (worktreePath is .aidlc/worktrees/bolt-<slug>), so it was pure fingerprint blindness.
• Additions-only proves nothing: correctly not claimed as fixed. The inverted limitation tests are the honest way to carry this - the policy is pinned in code, cited to the review round, and turns red the day attribution lands. #662's framing (needs a machine-readable per-unit path manifest; code-summary is prose-only today) matches what I found in round 3.

Negative controls, since a fix that also deletes the guard would pass every deadlock repro: a post-review edit still refuses with source-fingerprint mismatch and reverting restores approval; Source Fingerprint is still stamped on REVIEW_COMPLETED (verified against this head's dist, not just the unit tests). The recorded-repo test's second act (edit inside repo-a after the last review, must refuse) covers the same concern in-suite.

Docs match the shipped behavior now. The CHANGELOG limitation bullet, the 12-state-machine.md §Source-freshness binding paragraph plus the rewritten REVIEW_COMPLETED row, and verifyReviewerPrecondition's comment block all say the same thing: source-state equality against the newest recorded review, not per-unit attribution. The two refusal-message corrections (reachable-false "was never reviewed", and the "at least one receipt" wording on the per-unit path) are both accurate against the code as it stands. Good catch on #663 - workspaceSourceFingerprint returning null for the whole workspace when one recorded repo is unusable is indeed pre-existing and worth its own issue rather than scope creep here.

Gates on my side (Linux, this head): bun run check (all 3 tsconfigs + biome) rc 0; bun scripts/package.ts --check green for all 5 harnesses; smoke+unit 178 files / 4414 assertions / 0 failed (t244 26/26, t68 green); integration t166 17/17. One caveat this approval rests on: the repo's CI has not actually executed on this head - the fork's workflow runs sit at action_required awaiting a maintainer's "Approve and run", so the green above is my local Linux run, not the v2 gate itself. Version 2.5.12 and the t244 filename are both still free on origin/v2 as of this review (v2 is at 2.5.11; tests/unit/t244-scope-matrix-doc-sync.test.ts from #596 merged since your push shares the number but not the filename - the suite keys on filenames, so no collision, but worth knowing it's there).

One optional nit, not blocking: docs/reference/12-state-machine.md:178 says "See the REVIEW_COMPLETED row above" but the row is at line 277, below the paragraph. "Below" (or dropping the direction) would be accurate.

This resolves everything I raised. As you said yourself, the laundering window @leandrodamascena flagged is deferred to #662 rather than closed, so his CHANGES_REQUESTED remains the review that has to clear - but from my side the mechanism, the documented policy, and the test evidence are all in a state I'd be happy to see merge.

[iuryeng]

@leandrodamascena — a status note rather than a nudge, in case you come back to this thread expecting the version you reviewed. The shape of the change moved in the direction you pushed for, and it would be unfair to leave you to reconstruct that from the diff.

The rule you flagged as a bypass is gone — not defended, not patched. fingerprintChainIsAdditionsOnly was my answer to your laundering case, and it turned out to be the weaker half of the change: your alpha → beta walkthrough was the first crack, and @apackeer's round 3 reproduced five more ways it either refused legitimate work (the §12a rework loop the stage protocol itself prescribes) or never recovered once tripped. The honest conclusion was that the property it advertised is not one this mechanism can provide, so it was removed rather than hardened.

What ships instead is the narrower claim: source-state equality against the newest recorded review, with the limitation stated outright in the CHANGELOG and docs/reference/12-state-machine.md instead of being implied away. The laundering window you identified is not marked solved — it is carried as an open design question in #662, because closing it properly needs per-unit path attribution that does not exist in the tools today.

Your other three inline findings are fixed, each with a test: the :(glob)**/aidlc/** pathspec that was swallowing a legitimate src/aidlc/engine.ts, the accented-submodule path needing ls-files -z, and the false "finalize merges code back" claim in aidlc-state.ts.

I retracted the newest-wins argument in your thread when I pushed the fix, so this is a summary and not a second attempt at persuasion.

@apackeer approved at 3cbf1917 after re-running his own round-3 reproductions against that head. Your CHANGES_REQUESTED is the remaining gate and that is the right place for it to sit — you found the hole that reframed the whole change. Whenever you have time for it. And if documented-and-deferred is not how you would want that limitation handled, I would rather rework it than have it merge on a technicality.

[iuryeng]

All five are fixed, plus the ident minor. New head: 292db10f, rebased onto v2 at 2.5.36 and re-bumped to 2.5.38 (2.5.37 is taken by #686). The nine conflicts you simulated are resolved.

Thank you for going back through this one. Items 4 and 5 are both my own code, from the two rounds where I was answering earlier review — and 5 is specifically the perf gate I introduced to avoid paying for check-attr. That gate traded a measurable cost for a silent hole, which is the wrong trade in a guard whose job is refusing.

#	you asked for	delivered	verdict
1	partial-approval recovery must not skip freshness	Scope validated before any write; freshness un-gated from the checkbox on advance (incl. replay), finalize, complete-workflow	exact
2	bind convergence through the eventual source merge	SWARM_UNIT_CONVERGED carries the worktree fingerprint; the source merge re-verifies against it	exact
3	new receipts must not be born fail-open	unbindable receipts record the reason; guard refuses on unbindable-now-bindable and on un-recomputable	exact
4	buffer/stream + fail closed on ls-files failure	maxBuffer sized; status !== 0 returns null	exact
5	cover the missed attribute sources	default global path, repo-local core.attributesFile via -C, linked-worktree/submodule info attributes	exact
5b	ident is raw-bound too	gate matches ident; check-attr now asks for filter ident	exact

1 — the checkbox stopped being a standing permission

Two changes, because one is not enough. approve now validates Scope before it writes anything, which closes the window at its source. But that cannot cover a process killed between the write and the route it delegates to, so verifyReviewerPrecondition now runs on every route into the transition — including the replay short-circuit, which returned before any guard. verifyStageArtifacts (#366's expensive half) stays gated exactly as it was; a stale receipt is not what it defends against.

Both halves are proven separately against the fixed engine:

repro	before	after
abort after [x] (Scope corrupted)	advance rc=0, no mismatch	rc=1, and the checkbox never reaches [x] — it stays [?]
SIGKILL inside the write→route window	advance rc=0, no mismatch	rc=1, source-fingerprint mismatch

The second is the one the first fix cannot reach, and it is the one that proves the route-level check is load-bearing.

3 — and why the receipt now says why

Omitting the field made a new receipt indistinguishable from a pre-#629 one, so it inherited that row's documented fail-open. New receipts record unbindable instead. The guard now refuses three cases where it previously refused one: a receipt that claims unbindable while the workspace can be fingerprinted now, a real fingerprint that cannot be recomputed, and the mismatch it always caught. A receipt carrying no field at all still passes — that is the migration for genuinely old receipts, which is the line you drew.

The swarm's own convergence check had the same currentFp !== null fail-open and now uses the same three-case rule.

5 and 5b — reproduced with controls

Your $XDG_CONFIG_HOME/git/attributes case, run against both engines:

	88fb9ef3	292db10f
core.attributesFile	unset	unset
check-attr	app.ts: filter: tidy	app.ts: filter: tidy
clean vs dirty bytes	both d51318ad… (bare tree sha)	distinct composites

ident behaves identically: $Id: aaaaaaa $ and $Id: bbbbbbb $ collapsed before, diverge now. Fixing the gate alone was not enough there — check-attr was only ever asked about filter, so the scan had to learn ident as well.

Performance, since the gate exists for it: alternating baseline and fix in the same window, five rounds each, 60-file repo — baseline median 243 ms, fix median 235 ms, −3.5%, within noise. The two added spawns are cached per repo.

4 — what I could and could not prove

The mechanism reproduces exactly as you describe: 8,000 indexed paths produce a 1.68 MB listing (you measured ~1.69) and a plain spawnSync of that command returns status=null, ENOBUFS. The fix is what you asked for — sufficient buffer, and return null on failure rather than falling through to the bare tree sha.

What I could not do is reproduce the end-to-end bypass on this machine. The spawn inside the fingerprint does not overrun where an identical standalone spawn does, and I did not determine why. So the t259 row I added for it is a behaviour pin, not a regression test — it passes with and without the fix, and its own comment says so. I would rather tell you that than let a green row imply cover it does not give. You have the reproduction on Linux; if it stays red for you after this head, that is the signal that matters.

Something I got wrong and removed

I also added a line treating an untracked .gitattributes as an unread source, with a test. The test passed on the old code, so I removed both: the fingerprint's temp index runs add -A, which means untracked files are already in the listing that feeds the gate. My premise was wrong, and a fix for a hole that does not exist is just diff.

Gates

• t259 29 → 32. The two real regression tests (repo-local core.attributesFile, ident) are red on 88fb9ef3 and green here; the large-index row is the pin described above.
• Non-regression, measured against the baseline rather than assumed: t115 + t111 + t205 + t206 + t201 + t202 = 156 pass / 26 fail on 88fb9ef3 and 156 / 26 with every fix applied — I stashed, repackaged and re-ran to get that number. The 26 are the pre-existing Windows-environment failures.
• bun run check clean (557 files); bun scripts/package.ts --check byte parity across all five dists; t68 + gen-coverage-registry green at 2.5.38 (registry regenerated with the tool; its EXPECTED_NONE_TO_CLI needed the union with v2's new rows, not my side of the conflict).
• Consolidated run: 110 pass, 1332 assertions, same 26 environment failures.
• Still missing, and I would rather name it: the three lifecycle P1s are proven by mechanical repro, not by committed tests — t259 covers 4, 5 and 5b only. Say the word and I will add the route-level rows to t115; I left them out because you asked for regression coverage on the two fingerprint bypasses specifically, and I did not want to widen the diff without you asking.
• The full unit tier still does not complete on this box, so all of the above is a targeted green.

[apackeer]

Re-reviewed at 292db10f. The partial-approval ordering fix is sound, the route-level check is present on all four completion commands, and the new ls-files/attribute-source cases improve the mechanism. This still is not merge-ready: the source merge can validate one tree and merge another, and two of the claimed fail-closed fixes remain reproducibly fail-open.

P1: swarm source merge validates worktree bytes but merges the branch ref

workspaceSourceFingerprint includes staged, unstaged, and untracked worktree bytes (core/tools/aidlc-lib.ts:2611-2613), and the new merge guard recomputes that fingerprint over the Bolt worktree (core/tools/aidlc-worktree.ts:273-295). The actual squash/merge paths consume bolt-<slug>, not those worktree bytes (:337-338, :399-429).

Reproduced end to end with this head's tools:

1. Commit unreviewed source X on bolt-foo.
2. Revert the worktree to source B without committing.
3. Record the review and converge/finalize against B.
4. Run aidlc-worktree merge --strategy squash.

The fingerprint guard passed, but main received X:

finalize_rc: 0
merge_reported_fingerprint_mismatch: false
landed: export const value = 1; // UNREVIEWED_BRANCH

The merge command eventually returned 1 only because cleanup noticed the dirty worktree, after the unreviewed commit had already landed (aidlc-worktree.ts:482-495, tagged [merge-succeeded:...]). The positive t259 swarm test creates uncommitted source and stops after metadata finalization (tests/unit/t259-source-freshness-receipts.test.ts:1106-1124), so it never checks which bytes the later source merge takes.

The convergence binding needs to name the exact Git object that will be merged, or source merge must require/snapshot the reviewed worktree state and merge that object. Comparing a dirty worktree and then merging a different branch ref cannot establish freshness.

P1: newly unbindable receipts still fail open while Git remains unavailable

aidlc-log now records Source Fingerprint: unbindable (core/tools/aidlc-log.ts:323-325), but the guard marks that receipt stale only when the current workspace becomes bindable (core/tools/aidlc-state.ts:1462-1469). If fingerprinting is still unavailable, the new receipt is accepted without proving anything.

I reproduced the original path with Git unavailable for both review and approval: the receipt contained unbindable, source was edited after review, and approve returned 0 with no mismatch. This is still a newly emitted receipt failing open. Only fieldless pre-#629 receipts need migration behavior; a new unbindable receipt should refuse until a binding can be established. The same conditional appears in swarm finalize and source merge (aidlc-swarm.ts:360-363, aidlc-worktree.ts:291-294).

P1: large filtered repositories still lose the raw-byte binding

The ls-files call now has a larger buffer, but the downstream check-attr call still uses the default buffer and turns every failure into an empty raw-binding set (core/tools/aidlc-lib.ts:2527-2532). The caller then returns the bare filtered tree SHA when that set is empty (:2691-2695).

Reproduced with 12,002 real files:

ls-files bytes: 2196122
check-attr status: null
check-attr error: ENOBUFS
fingerprints equal after byte-distinct clean-filtered edit: true

The new large-index test cannot catch this: its 8,000 synthetic entries exist only in the committed index, so the fingerprint's git add -A removes their absent worktree paths before the inner listing (tests/unit/t259-source-freshness-receipts.test.ts:475-501); the test also explicitly says it passes without the fix (:463-467). Every subprocess in the raw-binding phase needs sufficient output handling, and failures must propagate as null rather than [].

Related false negatives remain in the same gate: a process-only filter is ignored because driver detection checks only filter.<name>.clean (aidlc-lib.ts:2553-2559), and an ignored worktree .gitattributes file is read by Git but absent from the temporary index path list scanned at :2421-2425. Both shapes reproduce byte-distinct worktrees with identical fingerprints.

P1: the convergence fingerprint is recomputed instead of carrying the reviewed value

reviewerReceiptError verifies a receipt against currentFp but discards that value (core/tools/aidlc-swarm.ts:353-374). After the metadata merges, emitUnitConverged recomputes the fingerprint independently (:789-818, :425-451). An edit between those two reads is therefore stamped as the converged state even though it never matched the reviewer receipt. Carry the fingerprint that actually passed receipt validation into SWARM_UNIT_CONVERGED; do not take a second, authorizing snapshot.

The eventual source check has two further holes:

• aidlc-worktree merge --intent A reads convergence rows from the active intent because assertConvergedSourceUnchanged calls readAllAuditShards(pd) without the command's --intent/--space selectors (core/tools/aidlc-worktree.ts:273-288). If B is active, A's row is invisible and the guard passes.
• The check runs at :346, before audit emission and all merge/rebase operations. The worktree/branch remains writable through that gap, and the rebase path performs fetch/rebase before the final ff merge (:379-456).

P2: true advance replays are no longer idempotent

Freshness runs before isReplay is evaluated (core/tools/aidlc-state.ts:1658-1673). After code-generation had genuinely advanced, I changed source for later work and replayed advance code-generation; instead of returning {"replay":true}, it refused with a source-fingerprint mismatch.

The crash-window state this fix targets is distinguishable: after approve writes [x] but before delegation, Current Stage has not moved to nextSlug, so it does not satisfy isReplay. A true replay can return before freshness validation without reopening the bypass.

P2: schema docs and regression coverage are incomplete

SWARM_UNIT_CONVERGED now emits Source Fingerprint, but the shipped audit registry still omits it (core/knowledge/aidlc-shared/audit-format.md:202), and the reference still says an uncomputable workspace keeps the previous behavior (docs/reference/12-state-machine.md:277,399). No committed test drives convergence through aidlc-worktree merge, persistent unbindable, partial-approval recovery, or true replay after later source work. Those are the paths carrying the new guarantees.

Verification

STAMP:    tests/logs/2026-08-03T20-06-28Z
TRACES:   /home/ubuntu/src/aidlc-workflows/.claude/worktrees/pr-646-rereview/tests/logs/2026-08-03T20-06-28Z/*.log  (10 files)
SUMMARY:  tests/logs/2026-08-03T20-06-28Z/summary.txt + failures.txt - Result: PASS; Failed files: 0
RESULT:   unit . 10 pass/0 fail . reds: none . live vars set: none . invariant grep hits: 0 (path-excluded)

Focused unit slice: 255 assertions, 0 failures. bun run check, bun scripts/package.ts --check, git diff --check, base ancestry, and mergeability are clean. GitHub reports no checks on this head.

[iuryeng]

Thank you for the careful re-review and for spelling out the remaining failure modes so concretely. I addressed the requested scope in 0c38645, without taking on the separate per-unit attribution work from #662.

What changed:

• The validated worktree fingerprint is now carried from receipt verification into SWARM_UNIT_CONVERGED; it is no longer recomputed at emission time.
• finalize materializes the reviewed application bytes as an immutable Source Commit without moving bolt-<slug>. Raw bytes for clean/process-filtered paths are written into that object, and the later source merge targets that exact commit rather than the movable branch.
• New Source Fingerprint: unbindable receipts fail closed in state, swarm finalization, and worktree merge even when Git remains unavailable. Only genuinely legacy fieldless receipts retain migration behavior.
• Effective attributes are queried directly for every indexed path. check-attr and raw hashing now use the large buffer, unexpected subprocess failures return an unbindable result, process-only drivers are recognized, and ignored worktree .gitattributes files are covered.
• aidlc-worktree merge --intent/--space reads the selected audit shards. The source check is the final guard before source mutation; source-bound rebases are refused until re-review/finalize, and squash/merge consume the immutable reviewed object.
• A fully applied advance replay short-circuits before freshness recomputation, while partial-approval recovery still runs the guard.
• The SWARM_UNIT_CONVERGED audit registry and state-machine docs now describe Source Fingerprint + Source Commit and the corrected unbindable/replay behavior.

Regression coverage now includes the real large check-attr output path, process-only filters, ignored .gitattributes, persistent unbindable receipts, true replay, partial-approval recovery, validated-fingerprint carry-through, explicit intent selection, raw-byte preservation, and an end-to-end source merge.

Validation completed locally:

• bun test tests/unit/t259-source-freshness-receipts.test.ts — 37 passed, 0 failed
• bun run typecheck
• bun run lint
• bun scripts/package.ts --check — all five harness trees in sync

[apackeer]

Thanks, Iury. I re-reviewed current head 0c386455. This revision genuinely closes the previously reported persistent-unbindable, large-output, process-filter, ignored-attribute, validated-fingerprint carry-through, intent-selection, replay, and movable-branch paths. I ran the official t259 slice from this head: 37/37 passed. Typecheck, lint, and package parity across all five harnesses are also clean.

I found two remaining merge-integrity blockers in the new immutable-source path:

1. The fingerprint includes dirty initialized-submodule content, but bindReviewedSource writes only the superproject index, so its Source Commit retains the old gitlink. I reproduced this end to end: review and finalize passed, the source merge passed and landed a normal foo.ts change, but the reviewed uncommitted submodule edit was omitted. Please either materialize representable reviewed submodule commits and update the parent gitlinks, or fail closed when the snapshot cannot carry the reviewed submodule state.
2. Finalizing with AIDLC_SKIP_SOURCE_FRESHNESS=1 emits a fieldless convergence row. After the switch is unset, source merge treats that new row as legacy and falls back to the movable Bolt branch. I reproduced an unreviewed branch commit landing through that sequence. Fieldless evidence needs to stay reserved for genuine migration; an explicit bypass marker should require the switch at merge time or require a bound re-finalization once the switch is unset.

Two smaller correctness cases should come with that update. A tracked symlink matched by a broad clean/process filter is included in raw-path scanning but rejected because snapshot materialization accepts only 100644/100755, so finalization cannot complete. Also, commit-tree leaves Source Commit unreferenced; git gc --prune=now removes it and a delayed source merge cannot proceed. A private per-Bolt ref retained until merge/discard would make the object durable.

The existing tests cover submodule fingerprinting separately and an end-to-end ordinary filtered-file merge, so please add lifecycle coverage for the submodule snapshot, switch-on-finalize/switch-off-merge sequence, filtered symlink, and object retention. I am not reopening the per-unit attribution decision tracked in #662, and this branch does not need another rebase; it is already current with v2. Once these paths are covered and current-head CI runs, it should be ready for Leandro's fresh pass.

[iuryeng]

Thanks again for the precise reproductions and for confirming the paths already closed. I addressed the four remaining lifecycle cases in f811ddfd without reopening #662 or rebasing the branch:

1. Dirty initialized submodules fail closed. Finalize now verifies initialized gitlinks before creating the source snapshot and refuses when reviewed bytes exist only as uncommitted submodule state, rather than emitting a parent commit with the old gitlink.
2. Bypass evidence is explicit. A finalize run under AIDLC_SKIP_SOURCE_FRESHNESS=1 now emits Source Freshness Bypass: true. With the switch absent at merge time, that row is rejected and cannot fall through to the fieldless migration path; genuine pre-binding rows remain compatible.
3. Filtered symlinks remain symlinks. Raw filter supplementation is restricted to regular modes (100644/100755) in both fingerprinting and snapshot materialization. Mode 120000 stays represented by Git's link-text blob, and the fingerprint no longer follows an external target.
4. Reviewed commits are durable. Each immutable source commit is retained under refs/aidlc/reviewed-source/<bolt>/<commit> until successful merge cleanup or discard. Cleanup uses compare-and-delete, and retry commits cannot move the only ref away from earlier audit evidence.

Lifecycle coverage now exercises the dirty-submodule refusal, switch-on-finalize/switch-off-merge sequence, filtered symlink finalize+merge, external symlink-target isolation, retention through reflog expire + git gc --prune=now, successful delayed merge cleanup, and discard cleanup.

Local validation on the pushed head:

• bun test tests/unit/t259-source-freshness-receipts.test.ts --timeout 120000 — 42/42 passed
• bun run typecheck — passed
• bun run lint — passed
• bun scripts/package.ts --check — all five harness trees in sync
• Relevant worktree merge/lifecycle and Windows portability slices also passed

The new CI run and documentation run are currently action_required, awaiting maintainer approval to start, so I am not treating remote CI as green yet. Once those runs are enabled, I will follow through on any current-head result before asking for Leandro's fresh pass.

[apackeer]

Thanks, Iury. I re-reviewed current head f811ddfd.

The four lifecycle cases from my last comment are genuinely closed. I verified each with independent fixtures that go past the branch's own tests:

• Dirty initialized submodules. Finalize refuses on a modified tracked file inside the submodule (your test covers the untracked shape) and on dirt inside a nested submodule two levels down; no convergence row and no retention ref are left behind on refusal. The control holds too: a clean submodule advanced to a new commit still finalizes, and the snapshot tree carries the new gitlink.
• Bypass marker. A bypassed finalize stamps Source Freshness Bypass: true; merge without the switch refuses and leaves HEAD unmoved, merge with the switch lands, and a hand-seeded fieldless pre-binding row still takes the migration path.
• Filtered symlinks. Finalize and merge complete under a repo-wide clean filter with the link intact (mode 120000, original link text). The fingerprint ignores edits to an external target's bytes, changes when the link text changes, and a post-review retarget of a tracked symlink from safe.txt to evil.txt is refused as a source-fingerprint mismatch.
• Retention. The ref appears at finalize, the Source Commit survives reflog expire --expire=now --all plus git gc --prune=now, a delayed merge succeeds, and the ref is removed after merge and after discard. A finalize retry preserves the earlier commit's ref, and the compare-and-delete cleanup refuses to delete a ref another process moved.

t259 passes 42/42 here as well, and typecheck, lint, and package parity across all five harnesses are clean.

I found two blockers at this head, one of them a regression this branch introduces on a mainstream path:

1. Hoisting verifyReviewerPrecondition out of the !alreadyMarkedCompleted block breaks recovery on an already-completed stage (aidlc-state.ts:1679, 1801, 1919, from the "close the freshness bypasses" commit). The guard does not only compare fingerprints; it also requires a REVIEW_COMPLETED receipt to exist at all. Re-reporting an [x] stage whose receipts are absent (fresh clone, moved record) now errors with "declares a reviewer ... but no fresh REVIEW_COMPLETED is recorded". Two committed tests pin this exact path and fail at this head while passing at the merge base 046a9a6c under the identical runner env: t186 "10: the coverage guard does not block reporting an already-completed per-unit stage" and t236 "already-completed current stage recovers forward without evidence". Neither file is touched by this branch, and v2's current copies carry the same expectations, so a rebase will not absorb it. The handleAdvance replay carve-out does not cover this shape either: Current Stage has already moved past the completed stage, so the currentStageField === nextSlug predicate fails and the guard is reached. I think the fix is scoping rather than reverting: on an already-completed stage, run only the staleness comparison (a recorded fingerprint that no longer matches the current source must still refuse), and skip the receipt-existence requirement, which the crash-window rationale does not need.

2. The committed coverage registry is stale. The enumerated universe grew from 489 to 492 with the new exports, so bun tests/gen-coverage-registry.ts --check fails, which is a deterministic unit-tier red in CI. Regenerate tests/.coverage-registry.json and .coverage-ratchet.json.

One smaller correctness issue in the messaging: the recovery that the bypass refusal and the CHANGELOG advertise ("re-run review and finalize with source freshness enabled") is not actually reachable. A Source Freshness Bypass row only exists for a unit whose AIDLC data already merged back during the bypassed finalize, so the bound finalize retry fails at the state merge with "already merged: not in Bolt Refs" and never mints a bound convergence row, and a re-prepare of the same unit fails because the worktree still exists. I reproduced the full sequence: the newest row stays the bypassed one, and merge without the switch refuses indefinitely. The behavior fails closed, so this is not a bypass, but the operator is sent to a dead end. Either make the advertised path real (allow a finalize retry that re-binds an already-state-merged unit) or point the refusal at the two exits that do work: merge with the switch present, or discard and redo the unit.

Housekeeping: v2 has moved to 2.5.42, and this branch now conflicts in ten non-dist files (version, CHANGELOG, README, aidlc-lib/aidlc-state/aidlc-swarm, audit-format, the state-machine chapter, and both coverage files), so the earlier "no rebase needed" no longer holds; it needs a rebase and a re-bump past 2.5.42, which is also the natural moment to fold in the registry regeneration. Also worth knowing: open PR #687 introduces tests/unit/t259-turn-markers.test.ts, so the t259 number will be shared after both land (different filenames, no suite collision, just naming awareness). Remote CI on this head is still action_required.

Full smoke+unit at f811ddfd: 188 files, 185 pass, and exactly the three reds named above (gen-coverage-registry, t186, t236). Once the regression is scoped, the registry regenerated, the recovery message corrected, and the branch rebased, this should be ready for Leandro's fresh pass.

[iuryeng]

Thank you for the precise reproduction and for narrowing the completed-stage fix to the correct seam. I reproduced the reported recovery failures and the bypass-message dead end before changing the implementation, then rebased and validated the result at 6399c02b.

What changed:

• Already-completed recovery is now scoped. advance, finalize, and complete-workflow skip only receipt existence/cardinality when the stage is already [x]; a recorded modern source binding is still compared and a mismatch still refuses. A fully applied advance replay keeps its existing idempotent short-circuit.
• Source and artifact freshness remain independent. During final review I found and reproduced a combined-change edge case: changing both the declared artifacts and application source could make the receipt disappear before its source binding was compared. A syntactically valid Artifact Fingerprint now preserves the associated Source Fingerprint for completed-stage staleness checking, while current artifact equality still independently controls stageVerdict/unitVerdicts. Missing or malformed artifact bindings do not make the source field trusted.
• Coverage metadata is regenerated. The current rebased universe is 511 units, including 271 functions; both registry and ratchet are fresh. I also removed three duplicate expected test-file entries exposed by the registry's own unit test after the rebase.
• The bypass refusal names only reachable exits. It now directs the operator either to merge with AIDLC_SKIP_SOURCE_FRESHNESS=1 present, or to discard the Bolt and redo the unit from prepare through review/finalize. No rebind path was added. The discard/redrive path is exercised end to end through a new bound convergence and a merge without the switch.
• Housekeeping is complete. The branch is rebased onto current v2 at e135e8e5 (2.5.45), re-bumped to the next unclaimed patch 2.5.54, and all five generated harness trees were regenerated.

Local validation on the published head:

• t259-source-freshness-receipts.test.ts: 52/52
• Exact reported regressions: t186 test 10 and t236 already-completed recovery: pass
• Combined source + artifact mutation matrix across advance / finalize / complete-workflow: 3/3 refuse with source-fingerprint mismatch
• Missing/malformed Artifact Fingerprint controls: 2/2 pass as absent/untrusted evidence
• gen-coverage-registry.test.ts: 34/34; registry --check: fresh, guards green, ratchet held
• Bolt lifecycle t78: 16/16
• Version sync t68: 7/7; documentation/audit parity: 16/16
• bun run typecheck: pass
• bun run lint: pass, no warnings (three informational notices in the existing t267 file)
• bun scripts/package.ts --check: all five harnesses in sync

For transparency, the full smoke run on this Windows host is 1133 pass / 21 fail, all 21 in the existing t05-run-tests-parallel.test.ts shell-runner suite with process status 127. I reproduced the same representative t05 failure against a clean origin/v2 worktree, so I am not attributing it to this PR or claiming the global Windows smoke tier is green.

GitHub now recognizes 6399c02b as MERGEABLE. The check rollup is currently empty, so I am not claiming remote CI green; it still needs the repository-side runs/authorization and a fresh review.

[apackeer]

Addressed the latest review findings at 26b64e13.

• Verified the completed-stage recovery fix: already-[x] routes skip only receipt existence/cardinality, while valid recorded source bindings still reject stale source. The t186 regression now requires the exact done directive.
• Made the advertised bypass recovery path real. A convergence marked Source Freshness Bypass: true can be merged only when AIDLC_SKIP_SOURCE_FRESHNESS=1 is repeated.
• The bypassed source merge now consumes a captured Bolt commit OID, rechecks branch movement before cleanup, and compare-deletes the branch. Uncommitted, untracked, or ignored application paths refuse before audit/source mutation and remain preserved. Cleanup removes only framework-owned residue; legacy fieldless convergence behavior is unchanged.
• Updated the 2.5.54 changelog/reference text and regenerated all five harness distributions.

Verification on this head:

• t259-source-freshness-receipts: 54/54
• t78-bolt-worktree-lifecycle: 16/16
• t186 + coverage registry + documentation parity + version sync: 66 assertions, 0 failures
• bun run check: pass (package parity, all TypeScript projects, Biome; only the three existing informational t267 notices)
• git diff --check: pass

GitHub has not reported remote checks on this head yet, so I am not claiming remote CI green.

[apackeer]

The current head (26b64e13) looks good to me, and I have approved it. @leandrodamascena, could you please take a final review pass and sign off?