F: guard start and resume follow-ups

[mchwang]

Closes #107.

Decisions

1. Start and resume require approvals for every item in the current plan revision. Before execution, approvals must match the current snapshot; after a completed item, revision approvals survive runner-created snapshots, while a completed-prefix head guard detects unrelated movement. Later attribution choices require later approvals. Schema v12 gives legacy choices a finite ordering boundary and clears legacy unversioned approvals so the review UI can offer fresh approval.
2. Start and resume require both expectedStateVersion and expectedReviewVersion. The review version is part of the idempotent request and is owned for the whole multi-item run, with approval revalidation before every later admission. Durable actions saved before this field existed retain a replay-only legacy path.
3. When Resume claims recovery but settles owed work without admission, it atomically clears requeue_pending in the same user action. Start never consumes a Resume-owned recovery claim.
4. Owed safety work is durably retried before pending-cancel, active-merge, and F2: continue a task after a scope pause (reconcile the prefix, validate the rest) #88 refusals. A current run whose initial evidence write failed retries it through the settlement shutdown capability before escalation can clear memory. Existing human gates are preserved; schema v13 persists the deferred escalation across restart until the gate is resolved. Storage failures roll back instead of recording a false settled outcome. Scope-only debt is offered only when its pause can commit.
5. Resume verifies that the completed current-revision prefix still ends at the current task head before skipping it.

The lifecycle contract is updated in docs/implementation/runner-lifecycle.md.

Validation

Validated head: 092b117

• npm run typecheck — passed
• npm test -- --run test/runner-start.test.ts --testTimeout=30000 — 38 passed on the exact validated head
• npm test -- --run test/runner-lifecycle-store.test.ts test/runner-execution.test.ts test/runner-start.test.ts test/publish.test.ts test/planning-drafts.test.ts --testTimeout=30000 — 437 passed on the pre-reconciliation implementation head
• After the final safety-settlement fix: npm test -- --run test/runner-lifecycle-store.test.ts test/runner-execution.test.ts test/runner-start.test.ts test/runner-publishing.test.ts test/runner-shutdown.test.ts test/publish.test.ts test/planning-drafts.test.ts test/merge.test.ts test/merge-task-pr.test.ts --testTimeout=30000 — 704 passed on the exact validated head
• npm test -- --run test/runner-branch-push.test.ts --testTimeout=20000 — 24 passed
• npm test -- --run test/history.test.ts test/review.test.ts --testTimeout=30000 — 61 passed
• npm test -- --run test/review.test.ts --testTimeout=30000 — 17 passed on the exact validated head
• Independent high-effort lifecycle review of the exact validated diff — no findings
• git diff --check — passed

npm test was attempted again at the preceding review checkpoint. Under concurrent Docker load, five branch-push tests plus one history and one review test exceeded their 5-second per-test ceilings; all 85 passed in the isolated commands above. The full run was stopped after reproducing the known load-dependent failure pattern. Follow-up: #129.

Review rounds

• Independent review round 1: fixed legacy start/resume action replay across the new request shape.
• Independent review round 2: no findings.
• Copilot round 1: fixed all three findings — legacy choice ordering, scope-only action availability, and mid-run review/approval revalidation.
• Independent review round 3: no findings on the complete updated diff.
• Copilot round 2: fixed the A→B→A approval-snapshot inheritance gap while preserving runner-created snapshot inheritance.
• Independent review round 4: no findings on the approval-snapshot fix, including the conservative choice ordering.
• Copilot round 3: fixed the review UI dead end by marking fingerprint-approved but execution-invalid approvals stale and allowing them to be reapproved normally.
• Independent review round 5: no findings on the execution-stale reapproval fix.
• Copilot round 4: no formal findings; reproduced and fixed its summary-only fixture concern so the A→B→A regression now proves approvals were saved against simulated head B before returning to A.
• Copilot round 5: fixed its human-gate finding and summary-only recovery-claim concern; Start no longer consumes Resume recovery, and deferred safety escalation preserves the gate.
• Independent lifecycle rounds 6–8: fixed durable restart ownership, marker clearing through payOwed, and the distinction between a durably deferred escalation and a failed Store write.
• Independent review round 9: no findings on the complete lifecycle, schema, restart, rollback, and claim-ownership diff.
• Base reconciliation: merged current origin/main (91be40c), preserved its publish-escalation bookkeeping in the sole conflict, and reran the expanded exact-head gate.
• Copilot round 6: fixed the current-run settlement gap by durably retrying a failed safety-evidence write before escalation clears memory; added reopen, shutdown-capability, and storage-failure regressions.
• Independent lifecycle round 10: found and fixed a missing shutdown-capability assertion, then returned no findings on the complete exact-head fix.
• Copilot round 7: no findings on the exact validated head; the prior settlement finding is resolved.
• Declined: none.

Review lesson audit

• Legacy choice ordering and reapproval: covered by Async jobs and polling (all owned version counters must remain unchanged) and Async review UI (preserve historical results but mark stale input visibly).
• Scope-only action availability: covered by Async jobs and polling (a response may move state only along a transition allowed from the guarded source state).
• Mid-run approval/review revalidation: covered by Async jobs and polling (an in-flight action owns every version counter and must revalidate all of them).
• Approval inheritance across A→B→A snapshots: covered by Async jobs and polling (older responses and retry context must be proven current against snapshot and referenced code).
• Execution-invalid approvals shown as approved: covered by Async review UI (completed historical results remain visible but stale, and controls must reflect the same state as their content).
• A→B→A fixture weakness: covered by Review readiness (a summary-only concern requires a fixture that asserts the disputed intermediate state).
• Human-gate preservation and Start consuming Resume recovery: covered by Async jobs and polling (derive transitions from the guarded source state and validate retry context).
• Durable owed-safety ownership, restart rehydration, payOwed clearing, and storage-error classification: covered by Async jobs and polling (persisted state and in-memory jobs are separate holders; preserve actionable reasons through every layer) and Review readiness (trace claims through error and early-return paths).
• Current-run settlement retry and shutdown-capability coverage: covered by Async jobs and polling (settle owned work during shutdown without losing durable state) and Required race regressions (shutdown and delayed lifecycle interleavings require failing-before/passing-after tests).
• No new AGENTS.md rule is needed; all findings were concrete applications of existing rules. No finding was one-off or declined.

[Copilot]

Copilot review overview

🟡 Changes recommended

Execution approval checks and scope-only action availability still have correctness gaps.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Legacy choices can block reapproval indefinitely · New
• Prevent scope-only actions in incompatible task states · New
• Revalidate approvals before admitting later items · New

What changed in this PR

Strengthens runner start/resume safeguards for #107 while leaving scope-pause continuation to #88.

Changes:

• Requires plan-item approvals and both state and review versions.
• Adds owed-work persistence, recovery-claim clearing, and completed-prefix head checks.
• Updates lifecycle documentation and regression coverage.

File	Description
web/​server.ts	Adds action guards and legacy request replay.
test/​runner-start.test.ts	Tests approvals, version guards, recovery, and replay.
test/​runner-shutdown.test.ts	Checks required review-version validation.
test/​runner-publishing.test.ts	Updates approval setup and action requests.
test/​runner-execution.test.ts	Checks completed-prefix head reporting.
runner/​store.ts	Adds approval checks, persistence, and recovery helpers.
runner/​execution.ts	Retries owed findings and tracks prefix heads.
docs/​implementation/​runner-lifecycle.md	Documents the updated execution contract.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The approval-snapshot gap needs correction, and the coupled recovery transitions warrant end-to-end human verification.

Review effort: Balanced
Findings: 1

Open (1)

• Prevent inherited approvals from mismatching execution snapshots · New

Resolved since last review (3)

• Legacy choices can block reapproval indefinitely
• Revalidate approvals before admitting later items
• Prevent scope-only actions in incompatible task states

[Copilot]

Copilot review overview

🔵 Needs a closer look

Durable migration and runner lifecycle changes need human review, and approval-state handling remains unresolved.

Review effort: Balanced
Findings: 1

Open (1)

• Execution-invalid approvals remain valid after attribution changes · New

Resolved since last review (1)

• Prevent inherited approvals from mismatching execution snapshots

[Copilot]

Copilot review overview

🔵 Needs a closer look

Durable approval migration and cross-run recovery behavior require final human review.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Execution-invalid approvals remain valid after attribution changes

Previously missed (1)

In code that hasn't changed since last review

approvePlan() overwrites simulated B snapshot with real HEAD A

test/​runner-start.test.ts:430

approvePlan() calls service.load(), which observes the fixture's real HEAD. Since P1 completed unchanged, HEAD is still A, so this replaces the simulated B snapshot with A before saving approvals. The test therefore never approves unrelated head B. Save approvals directly against B and assert both its head and the saved approval snapshots before returning to A.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Cross-layer recovery and migration changes have unresolved lifecycle concerns and require final human review.

Review effort: Balanced
Findings: 1

Open (1)

• Preserve approval and fix gates when persisting owed findings · New

Previously missed (1)

In code that hasn't changed since last review

Prevent Start from consuming Resume-only recovery claims

web/​server.ts:123

When recovery has set requeuePending and safety or scope work is owed, these shortcuts offer Start and pass claimRequeue: true. Start can therefore consume the recovery claim reserved for Resume. Refuse Start on recovery-pending tasks before either shortcut. Merely setting claimRequeue to false would leave the flag stranded after Start settles the debt and moves the task to a human-gated state.

[Copilot]

Copilot review overview

🟡 Changes recommended

Safety evidence can still be cleared without durable persistence, and the recovery migrations also warrant human review.

Review effort: Balanced
Findings: 1

Open (1)

• Persist safety findings before clearing them during settlement · New

Resolved since last review (1)

• Preserve approval and fix gates when persisting owed findings

[Copilot]

Copilot review overview

🔵 Needs a closer look

Approval inheritance, schema migrations, and safety settlement across recovery and shutdown warrant final human review.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Persist safety findings before clearing them during settlement