Skip to content

fix(harness): preserve coding work through model outages - #2136

Merged
justinhelmer merged 1 commit into
mainfrom
plan/fix-issue-2120-a-model-c-83f44f/u1
Sep 21, 2026
Merged

justinhelmer merged 1 commit into
mainfrom
plan/fix-issue-2120-a-model-c-83f44f/u1

Conversation

@coreplane-switchboard

@coreplane-switchboard coreplane-switchboard Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Transport failures now hold and retry model turns inside the run lease. If a coding child still ends, its branch gets a WIP checkpoint so interrupted work resumes instead of being reviewed or discarded.

Why: Issue #2120 showed a gateway failure could erase half an hour of coding. This closes the provider-hold record at the execution boundary so outages spend lease time rather than work.

Where to look

  1. Proxy transport classification Classifies resets, retryable statuses, and gateway HTML as transport failures before model output reaches the harness. ⚠ A false positive could retry a legitimate provider response.
  2. Lease-backed held turn Parks the failed turn and repeats the backoff ladder within the loop lease, with provider recovery able to release it early. ⚠ A stuck hold could consume the run's remaining lease.
  3. Abnormal-ending checkpoint Routes failure, stop, restart, and lease-end paths through one unit-branch preservation step before teardown. ⚠ Teardown must remain after this checkpoint.
  4. Clean-tree WIP marker Stages dirty work or creates an empty WIP marker on a clean abnormal ending, then pushes and reports the unit head. ⚠ A failed push leaves only the explicit checkpoint failure note.
  5. Durable push provenance Folds the last head per branch together with whether an agent push or mechanical salvage produced it.
  6. Runner abort on salvage Treats a salvage head as unfinished work, aborts before review, and names the branch head a re-issue resumes. ⚠ Misclassification could send incomplete work to review.
  7. Transport hold contract Specifies the retryable transport classes, lease-backed hold, stable exhausted ending, and exact proof bindings.

Feedback wanted: Check the held-turn wake/backoff against the lease and that every abnormal ending leaves salvage—not ordinary-push—provenance.

Risk: This is over 400 changed lines: 22 files, +662/-254. Bugs could stall turns or checkpoint the wrong branch; rollback is this commit. I considered separate PRs, but retry exhaustion is the checkpoint boundary joining both fixes.

Verified: 594 focused tests and local docs, TypeScript, format, hygiene, specs, and title gates pass. Two replacement docs CI checks passed; fresh CI on 2024440 remains GitHub-gated.

Decisions (4)
  • The lease is the retry budget. Rejected another fixed attempt count: the issue fixture exhausted three quick attempts after substantial useful work. The final backoff rung repeats until the loop boundary, while provider recovery can release the same hold early.
  • A clean ending still gets a marker. Rejected treating a clean, already-pushed tree as finished: an ordinary intermediate push proves only that a head reached the remote. An empty WIP commit makes the abnormal ending the durable final source.
  • Salvage provenance survives the fold. Rejected inferring unfinished work from failed status because a child may make an ordinary completed push before failing. Persisted salvage provenance lets the runner abort only the mechanical checkpoint path.
  • Checkpoint non-ignored untracked files. Rejected git add -u: a new source or regression test can be the most valuable interrupted work. git add -A preserves it while ignore rules still exclude caches, credentials, and staged attachments.
Validation (7 criteria)
Criterion Proof
Transport failures recover within the lease and exhausted retries preserve coding work. npx vitest run on the 7 changed test files after the final rebase — 7 files passed, 594 tests passed.
The exact failed docs CI job commands succeed. npm run verify -w docs && npm run check:site && npm run verify -w deploy/cloudflare-docs — passed locally; two replacement ci / docs checks also completed successfully.
Touched TypeScript compiles. NODE_OPTIONS=--max-old-space-size=6144 npx tsc --noEmit -p tsconfig.json — passed after the final rebase.
Changed files match repository formatting. npx prettier --check on all 22 changed files — passed after the final rebase.
Public-tree hygiene remains within the recorded baseline. npm run hygiene:check — passed; 9 files, 37 hits, 138 allowed lines.
Every spec proof resolves. npm run specs:check — passed; 50 specs and 5600 proof references checked.
The pull-request title is a valid changelog line. npm run check:pr-title -- "fix(harness): preserve coding work through model outages" — passed.
For agents

Rebased onto origin/main at 1bec076 before the final fast gates. The branch is one coherent commit at 2024440; its tree matches 82e39f0 and 05c75e4, amended only to retrigger Depot legs that failed without logs or annotations while passing at the identical tree on adjacent runs. The agent-ship conflict retained main's empty-check refire behavior and this PR's salvage-provenance rule. No generated files were edited.
Requested by @justinhelmer in slack:C0BRRHKFLCB

🤖 Generated with Claude Code

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: The ending salvage can still let an unfinished, previously pushed coding child proceed to review, and the durable pushed-head spec is stale.

Warning

Changes requested · head 887e774 · 2 findings: 1 major, 1 minor

Severity Finding Where
major F1 A clean already-pushed failure bypasses the WIP checkpoint src/core/codingPrPostStep.ts:329
minor F2 Spec contradiction — run-history.md item 2: pushed records now retain source and ending salvage docs/reference/specs/run-history.md:14
Full review

F1 — Major, high confidence. If a coding child pushes an intermediate commit, then fails while its tree is clean, salvageBudgetPush returns “nothing to preserve” without recording a by: "salvage" checkpoint. The coordinator sees only the earlier ordinary push and proceeds through PR recovery into review, treating unfinished work as review-ready. Create and push a WIP marker commit on every abnormal ending, including when the prior ordinary push left a clean tree, so the coordinator reliably aborts with the resumable checkpoint.

F2 — Minor, high confidence. Item 2 still defines pushed as { ref, sha }[] and says only budget and compaction salvage produce by: "salvage". The implementation now persists by and adds ending salvage. Update the behavior statement to match the new durable record contract.

The unlicensed codingPrPostStep.test.ts title change is a refactor with verification intact; its replacement strengthens coverage by adding an untracked file.

@coreplane-switchboard
coreplane-switchboard Bot force-pushed the plan/fix-issue-2120-a-model-c-83f44f/u1 branch from 887e774 to 58a24d6 Compare September 21, 2026 17:24

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: Both prior findings are resolved: clean abnormal endings now leave a salvage-marked WIP head, and the durable record spec documents its source and compatibility.

Note

Approved · head 58a24d6 · no findings

Full review

Both prior findings are resolved. The coding post-step test title changes are refactors with verification intact and strengthened abnormal-ending coverage.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@coreplane-switchboard
coreplane-switchboard Bot force-pushed the plan/fix-issue-2120-a-model-c-83f44f/u1 branch 2 times, most recently from 82e39f0 to 05c75e4 Compare September 21, 2026 17:42
Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: The replacement docs checks pass and no unresolved code or spec findings remain.

Note

Approved · head 2024440 · no findings

Full review

The prior docs CI finding is resolved; the test-title changes preserve or strengthen their verification.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant