fix(review): recover workspaces at pull request heads

[justinhelmer]

Review workspaces are now moved to the pull request’s expected head before a review starts, and the checked-out commit is verified directly. If recovery fails, the refusal reports distinct initial and retry observations instead of trusting resident metadata.

Why: Review runs were refused when resident workspaces lagged the requested head, even though Switchboard could recover them automatically. Attach and move metadata could also claim the expected commit while the actual checkout differed. Fixes #2235.

Where to look

1. Head guard adopts a raced current head, retries once, and preserves initial-versus-retry provenance. ⚠ A mistake can start a review on the wrong commit or refuse every review.
2. Checkout observation reads HEAD through the executor before and after recovery rather than trusting attach or move metadata.
3. Initial metadata regression proves a matching resident binding cannot hide a different checkout.
4. Retry metadata regression proves a matching moveTo() result cannot hide a still-mismatched checkout.
5. Published binding regression pins the selected binding to observed checkout truth after recovery.
6. Behavioral contract defines direct checkout observation, bounded recovery, refusal, and no-provider-call behavior.

Feedback wanted: Is one bounded reprovision attempt the right recovery policy, and are the executor observation and failure-provenance boundaries correct across backends?

Risk: This is on every PR review’s pre-model attach path. A regression can refuse reviews or verify the wrong checkout; rollback is the single commit.

Verified: 593 targeted tests passed; typecheck, lint, formatting, specs, coverage/test guard, hygiene, and title checks passed. CI is running.

Decisions (3)

• Observe the checkout through the executor. Resident binding and moveTo() SHAs remain useful metadata, but only git rev-parse HEAD proves what the model will read.
• Recover once, then fail closed. One move/rebuild handles stale workspaces without allowing an unbounded retry loop; a second mismatch or unreadable retry refuses before a model call.
• Keep observations separate. A failed or invalid retry never reuses the initial SHA as though it were observed after reprovisioning.

Validation (8 criteria)

Criterion	Proof
Matching attach metadata cannot hide a different checked-out HEAD	src/core/dispatch/authorize.test.ts::authorizeAttachedHead — every review workspace is at the PR head before the first model turn::a resident binding SHA cannot verify a checkout whose observed HEAD differs — passed
Matching moveTo() metadata cannot hide a mismatched retry checkout	src/core/dispatch/authorize.test.ts::authorizeAttachedHead — every review workspace is at the PR head before the first model turn::resident moveTo metadata cannot verify a retry whose observed HEAD is still mismatched — passed
A successful retry publishes the observed checkout HEAD, not false moveTo() metadata	src/core/dispatch/authorize.test.ts::authorizeAttachedHead — every review workspace is at the PR head before the first model turn::a successful retry publishes the observed checkout HEAD instead of moveTo metadata — passed
A stale review workspace is reprovisioned at the reviewed head and the review runs	src/core/dispatcher.test.ts::repo/ref resolution + resident prompt selection::a genuine push during workspace preparation is reprovisioned at the reviewed head and the review runs — passed
Thrown reprovision and invalid retry SHA preserve initial-versus-retry provenance	src/core/reviewRound.test.ts::guardAttachedHead (before any model call)::* — passed
Regression tests prove metadata was previously trusted	Red steps: three authorization regressions failed under prior behavior; the newest received false metadata SHA c… instead of observed checkout SHA a…
Types and changed-file quality gates pass	tsc --noEmit -p tsconfig.json; ESLint and Prettier on changed files; npm run hygiene:check; PR-title check — passed
Living specs remain bound and changed source paths are covered	npm run specs:check; npm run specs:coverage -- --changed origin/main...HEAD --test-guard --require — passed

[justinhelmer]

Re-review requested at 3660628 — rebased onto current main; corrected initial-versus-retry HEAD provenance, added failed reprovision coverage, updated the tracing spec, and fixed the lint failure. Independent OpenCode review is in progress.

[justinhelmer]

Re-review requested at b28c0d3 — updated agent-review item 12(d) to document the one-reprovision policy and workspace_head_mismatch refusal for thrown, unreadable, or mismatched retries. Independent OpenCode review is in progress.

[coreplane-switchboard]

Changes requested: Resident review attachment still verifies metadata rather than the checked-out workspace HEAD.

Warning

Changes requested · head b28c0d3 · 1 finding: 1 minor

Severity	Finding	Where
minor	F1 Spec contradiction — docs/reference/specs/agent-review.md item 10: resident reviews trust attach metadata instead of checked-out HEAD	src/core/dispatch/authorize.ts:488

Full review

F1 — The resident path uses binding.sha as proof before the guard and moveTo()’s returned SHA after reprovision, without executing git rev-parse HEAD. If resident metadata is stale or disagrees with the actual worktree, the model can review the wrong commit while the gate reports it verified. Read the checked-out HEAD through the executor before both comparisons, as item 10 requires.

[justinhelmer]

Re-review requested at c7c4173 — rebased over merged #2276; resident attachment now verifies the actual checked-out HEAD through the executor seam, with regressions for unverified binding metadata and mismatched moveTo metadata. OpenCode and Switchboard re-reviews are in progress.

[coreplane-switchboard]

LGTM: The workspace-head verification fix is correct; only one stale comment remains.

Note

Approved · head c7c4173 · 1 finding: 1 nit

Severity	Finding	Where
nit	F2 Update the stale resident-binding proof comment	src/core/dispatch/authorize.ts:448

Full review

F2: The function comment still says a resident’s attach binding is proof, while the corrected implementation verifies git rev-parse HEAD through the executor. Update the comment to match the new invariant.

The two test-guard title changes are refactors with verification intact: actual workspace observation and successful and failed reprovision paths remain covered.

[github-actions]

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

[justinhelmer]

Re-review requested at c426679 — retry bindings now publish the executor-observed checkout HEAD instead of moveTo metadata; stale metadata-authority comments are corrected; regression proves wrong moveTo metadata with a correct checkout. OpenCode and Switchboard re-reviews are in progress.

[coreplane-switchboard]

LGTM: The retry now verifies and publishes the executor-observed checkout head, and the stale authority comments are corrected.

Note

Approved · head c426679 · no findings

Full review

No findings remain. The test-guard title changes are refactors with verification intact across workspace observation and successful and failed recovery paths.

[github-actions]

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

[justinhelmer]

Production inclusion receipt — 1.260.6. v1.260.6 is full commit 903ae2855833637fbcc0c0a22d54e7268be75965; GitHub compare reports this PR's merge commit as the merge base (release ahead 1, behind 0). The release workflow succeeded, bot/memory/resident /healthz report the full tag SHA, and the authenticated sandbox gate passed on the tag. This proves deployment inclusion, not that a stale/mismatched review workspace was reproduced live after deployment.