the scoped-review descendant walk fails closed forever after a squash, and its guarantee is unavailable rather than violated

[0xLeif]

The second half of #689. The readiness half is fixed separately; this one needs a decision rather than a patch.

Symptom

After a squash-merge, ship refuses:

error: independent scoped review is stale; open or update the PR so `SpecSync scoped review` can run

Re-running change review does not help on a change that has advanced to accepted — record_scoped_review requires ChangeState::Verifying (src/change.rs:5850). So a squash-merged change that reached accepted has no forward verb at all. That is the wedge in #690.

Why this one cannot be fixed the way readiness was

#689's readiness fix replaced commit ancestry with content agreement, because verification.commit ancestry was freshness wearing a trust costume — never resolved to a tree, never compared to a digest.

This check is not that. review_commit_is_current_checked (src/change.rs:5686-5760) walks every commit between review.implementation_commit and HEAD and requires every changed path to sit under:

let allowed_prefix = git_repo_relative_path(root, &format!("{CHANGES_PATH}/{}/", record.id))?;

So it proves: between the review and HEAD, nothing changed except this change's own lifecycle records.

Compare what scoped_review_is_current already checks by content — contract_digest, execution_digest, and project_input_digest == review.workspace_digest. Those cover product and spec files. But project_input_is_volatile (src/change.rs:10493) excludes:

.specsync/changes/
.specsync/archive/

precisely the region the walk polices. So the walk's unique contribution is detecting that another change's records, or the archive, moved after the review was recorded. Content digests cannot see that, by design.

That is a real guarantee. Deleting the walk would lose it silently.

The actual defect

A squash destroys review.implementation_commit, so the walk cannot run at all. The guarantee is not violated — it is unavailable. The code treats those identically and fails closed forever.

Measured on this repository: 2 of 105 archived reviews are reachable in a fresh clone. The walk is inert for 98% of history already; it simply fails loudly instead of admitting it cannot answer.

Options, none chosen

1. Fall back to content when the commit is unreachable, and say so. Cheapest. scoped_review_is_current's other four checks still run. The output must state that the descendant guarantee was unavailable — an unavailable guarantee reported as a satisfied one is worse than the current failure.
2. Re-anchor the review at finalize, as accept_change_with_gate already re-anchors verification.commit when allow_verified_tree_adoption is set. Keeps a walk, but over a window that no longer spans the reviewed work — worth checking whether that window proves anything.
3. Move the region into a digest. .specsync/changes/ and .specsync/archive/ are volatile by design because they churn during the lifecycle; a scoped digest over other changes' records might be tractable and would survive any merge strategy.

What I am not doing

Guessing. Three explanations of this repository's change coupling were wrong today, each killed by someone running a command. This one changes what a review proves, so it should be decided deliberately and stated in the spec rather than inferred from whichever patch lands.

Related: #689 (readiness, fixed), #690 (the wedge this produces), #688 (the two-sided archive block).

[0xLeif]

Correcting this issue against measurement

A four-way design pass ran on this. It refuted most of what I wrote above, including the parts I was most confident about. Corrections first, since the framing was wrong.

The measurement was wrong three ways

test	count
archived review.json records	106 (I wrote 105)
commit object resolves locally	103/106
ancestor of origin/main	2/106 — but one distinct commit, 9a3b1f03, shared by CHG-0079 and CHG-0080 in a single PR
walks that would actually pass	0/106

That last row is the one that matters and I never computed it. Reachability is necessary, not sufficient — the walk also has to find no disallowed path. Simulating it on the single reachable commit: 94 intervening commits to HEAD, and the overwhelming majority touch a path outside the allowlist. The first offender is the change's own archive move into .specsync/archive/.

That is structural, not incidental. Archiving relocates the workspace out from under allowed_prefix = .specsync/changes/<id>/, so every change that ships fails its own walk forever after. I wrote "inert for 98% of history." It is 100% inert, by construction.

"That is a real guarantee" — no, it is #689 again

I claimed this check was different in kind from verification.commit ancestry. It is not. Its complete git vocabulary:

rev-parse · merge-base --is-ancestor · rev-list · rev-list --parents · diff-tree --name-only

No blob is ever read. No digest is ever compared. scoped_review_file_at_commit exists at src/change.rs:5661 and the walk never calls it. It proves a history-shape property, sensitive to rewriting and insensitive to content — the exact signature I used to convict verification.commit in #689. I applied the test to one check and not to its sibling.

Worse: the allowlist permits arbitrary mutation of review.json, review-attempts.json, state.json, verification.json and verification-attempts.json with no content check. The walk licenses editing the very records it validates.

"No forward verb at all" — false

record_scoped_review does require Verifying (src/change.rs:5850, correct). But the error I pasted is raised at src/change.rs:5966 inside accept_change_with_gate, which itself requires Verifying at :5946. That error can only fire in the one state where change review is legal.

And re-running it works: record_scoped_review_with_verdict reuses the old anchor only when review_commit_is_current_checked(...).is_ok(), otherwise .unwrap_or(current_commit) re-anchors to HEAD (:5888-5899), after which the walk short-circuits at :5695. From accepted, the forward verb is archive — pinned by accepted_evidence_survives_integrated_squash_merge_and_archives (src/change_tests.rs:5849), which squash-merges, asserts the verification commit is orphaned, and archives successfully.

The wedge in #690 needs re-examination on this basis.

Options 2 and 3 are dead

Option 2 is theater. Re-anchoring makes review.implementation_commit == HEAD, so the walk returns Ok at :5695-5696 without enumerating one commit. The window is HEAD..HEAD — a check that cannot fail. It also does not survive a squash (the finalize-time SHA is a branch commit the squash discards) and the next commit, the archive tip, violates the allowlist anyway.

Option 3 is forbidden by the spec in as many words. Invariant 22 of change.spec.md: "never infer safety from a net diff or broad volatile-path exclusion." A scoped digest is both. requirements.md:657 additionally requires that change-then-revert stale a review, which no digest can see. Measured cost, separately disqualifying: the region is 2,813 files / 7.18 MB against 627 files digested today, and including it would make any concurrent agent's state.json write abort every command computing project_input_digest.

Option 1 needs a condition I did not state. :5708 conflates two different git exits: 128 (object absent — fresh clone, gc, shallow) and 1 (object present, not an ancestor). Only 128 may soften. Exit 1 is the precise signal the walk exists to catch, and softening it is the same fail-open hole the first #689 design had. But 103/106 objects resolve here, so an availability-keyed fallback fixes almost nothing locally. My "unavailable, not violated" framing was wrong: post-squash the commit is usually available and simply not an ancestor.

What the walk uniquely protects, and the option nobody listed

validate_archived_integrity (src/change.rs:14623) already covers "the archive moved" at content level, byte-comparing accepted-state.json against the authenticated transition — strictly better than the walk and merge-strategy-agnostic. So the archive half is not unique to the walk.

The genuine residue is narrower and sharper than anything in the original issue: delta bodies are hashed by nothing. validate_delta_files checks filenames only, project_input_digest excludes the path, and definition_digest hashes record fields. Editing .specsync/changes/<id>/deltas/<module>.md after review — the file that rewrites the living spec at acceptance — is caught only by a walk that passes 0/106 times.

Option 4: bind the delta bodies into the review digest. That converts the one property worth keeping from a topology assertion into a content assertion, survives every merge strategy, and satisfies invariant 22 rather than contradicting it. It is also the only option here that closes a real hole instead of relabelling an inert check.

Recommendation: Option 4, with Option 1's exit-128 narrowing and honest three-state reporting as the disposition for the walk itself. Invariant 22 and REQ-change-046 have to be rewritten in the same change — a living requirement describing a check that passes 0/106 times is exactly the drift this tool exists to catch.

Separately filed-worthy: REQ-change-016 states verification.commit "is retained as an informational correlation key and is never a gate," but verification_commit_is_accepted_current gates at src/change.rs:13782, :13787, and :14516.