feat: durable session-stack recovery + attempt ids + atomic artifacts (P0.1/P2.2)#64
Merged
Conversation
… (P0.1/P2.2) Closes the double loop's crash gap: a coordinator restart used to reopen the latest TOP-LEVEL session, silently orphaning a running child (request file already consumed, child stuck "running" forever, parent free to dispatch duplicate work). Session stack (P0.1): - The parent's LoopState records active_child_session_id while a child runs; the pointer commits atomically with the same parent mutation that records the child and unlinks the request. - Startup (--resume) walks the pointer chain to the deepest live session: running child -> becomes the active session (parent stays suspended); terminal child -> finalized (children.json completed, pointer cleared) and parent resumed; dangling pointer (dispatch crashed before the child state landed) -> cleared; fully-created child whose parent commit never landed -> ADOPTED via its running children.json record. - children.json records the originating request_file, making the dispatch scan idempotent across the record-then-unlink crash window: a leftover request never dispatches twice. Runtime child completion clears the pointer and removes any leftover request file. - Invalid child requests are terminally rejected (renamed *.json.rejected + warning) instead of being re-read on every scan forever. Attempt ids: - Every CurrentTask gets a unique attempt_id, carried in TaskResponse and echoed in FinishedRequest (and thus the pending handoff file). A late /finished from a superseded attempt of the SAME coordinates is stale — previously it could be recorded as the current iteration's result. Atomic artifacts: - write_text_atomic/write_json_atomic in sessions.py; used for result.json, result_text.txt, prompt.txt, harness_run_id.txt, pending_finished_request.json, children.json, and salvage.json — recovery decisions are made from these files, so none may exist truncated. _advance() (P2.2): - The three duplicated dispatch blocks (register phase-B, finished no-task, finished matched) collapsed into one _advance(state, caller, now) step, so the copies can no longer drift. Tests: 9 new in test_session_stack_recovery.py — pointer recorded at dispatch; runtime completion clears it; restart mid-child resumes the CHILD; restart after child-terminal finalizes + resumes parent; dangling pointer cleared; unpointed child adopted; leftover request never dispatches twice; invalid request rejected terminally; attempt id carried + stale attempt not processed. 152 total; ruff/format/pyright clean. Docs: http-contract (attempt ids, resume walk; both copies), session-layout (children.json request_file + the stack pointer; both copies), CHANGELOG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt
The review (REQUEST CHANGES: 1 critical, 7 major, 1 minor — several reproduced) is addressed in full: C1 (critical, reproduced): overlapping /finished retries could put a parent task and a child task live simultaneously. _advance() now enforces the suspended-parent invariant — a parent with a live child never acquires its own current_task; a duplicate retry gets the child's live task (idempotent with the first response), and a terminal child is finalized as the legitimate resume. A coordinator-level RLock serializes cross-store transitions (reentrant: a child's terminal /finished resumes the parent via register_worker on the same thread); phase-A drain stays outside it. M1: request-file tombstones apply only to RUNNING records — a completed child's filename (e.g. a stable child.json protocol) is reusable for genuinely new work instead of being silently swallowed forever. M2: the children.json record now lands BEFORE the child state, so a crash between the two leaves a discoverable "running record, missing state" projection instead of an unindexed orphan child; reconciliation marks it failed_dispatch and the request redispatches exactly once. M3: startup reconciliation inspects EVERY running-projected record: a terminal child with no parent pointer (real pre-pointer upgrade state) is finalized and its leftover request removed — previously ignored forever, wedging the PM dispatcher. Parent linkage is validated; multiple running records log an explicit ambiguity warning. M4: the first task of a child session now carries an attempt id (it bypassed _advance's stamping). M5: attempt provenance is strict whenever the live task has an attempt: a missing attempt no longer wildcards direct /finished, pending recovery, or phase-A/B revalidation (_same_task), and result.json now records the attempt (worker-stamped) so a stale artifact cannot complete a NEW attempt right after its stale pending file was rejected. Legacy tolerance remains only for persisted pre-attempt tasks. M6: child-request processing is a total transition: unknown workflow sets, broken configs, and sets with no eligible workflow are terminally rejected (collision-safe *.rejected records) instead of raising out of the mutator as HTTP 500 on every completion forever. M7: packaged prompts (outer, eval_runner, planner) instruct atomic publication of control.json/goal_check.json (temp + rename); the crash model (process crash; no fsync; workflow-signal caveat) is documented in session-layout. m1: children.json finalization keeps the FIRST observed completed_at across crash-replayed finalizations. Tests: 8 new review-driven regressions (duplicate-finished invariant, filename reuse, record-without-state reconciliation, unpointed terminal child, first-child attempt, stale-artifact rejection, semantic request rejection, idempotent completed_at); legacy suites updated to echo attempt ids like real workers. 160 total; ruff/format/pyright clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt
…ith strict attempts) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes the double loop's headline crash gap (P0.1, decision D6's prerequisite): a coordinator restart used to reopen the latest top-level session, silently orphaning a running child — request file consumed, child stuck "running" forever, parent free to dispatch duplicate work.
LoopStaterecordsactive_child_session_id; startup (--resume) walks the pointer chain to the deepest live session. Running child → resumes (previously orphaned); terminal child → finalized + parent resumed; every interrupted-dispatch projection reconciles deterministically (dangling pointers cleared, fully-created-but-unpointed children adopted,failed_dispatchfor record-without-state, leftover requests never dispatch twice).result.json): a late/finished— or a stale on-disk artifact — from a superseded attempt of the same coordinates can never be recorded as the current result.result.json,pending_finished_request.json,children.json,salvage.json, …)._advance()(P2.2): the three duplicated dispatch blocks are one helper — now also enforcing the suspended-parent invariant.Codex review (gpt-5.6-sol @ xhigh) — REQUEST CHANGES, all findings fixed
The review reproduced several failures against the first commit; the second commit addresses everything:
/finishedretries dispatched parent AND child work simultaneously. Now: a suspended parent can never acquire its own task (duplicate retries get the child's live task, idempotent), plus a reentrant coordinator transition lock around cross-store handoffs.child.json) silently swallowed genuinely new requests. Now running-records-only.failed_dispatchwith exactly-once redispatch.result.jsoncomplete a NEW attempt right after its stale pending file was rejected. Now strict provenance whenever the live task carries an attempt (including worker-stampedresult.json).control.json/goal_check.jsonhad no atomicity story — packaged prompts now instruct temp+rename publication; the crash model (process crash, no fsync) is documented.completed_atis now first-observation-stable across crash-replayed finalizations.Verification
160 tests (17 new: 9 crash-scenario + 8 review-driven regressions), ruff/format/pyright clean. Crash windows covered per the review's Q1 matrix: every row now reconciles or is explicitly documented (leaked pre-record child directories are inert garbage).
Notes
TaskResponse/FinishedRequestgainattempt_id(additive).children.jsonrecords gainrequest_file+ afailed_dispatchstatus; parents gainactive_child_session_id(documented in session-layout, both copies).🤖 Generated with Claude Code
https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt