Repository navigation
fix(coding-agent): keep snapshot catch-up failures isolated #11
Description
Activity
Coordination hold after PR #12
PR #12 is merged into
pylonasf728316dabbaa85aa561e6d6b08550ed337574be. The documented dependency order now advances to this issue.An existing implementation worktree is present and must be treated as protected:
- branch:
fix/snapshot-recovery-integrity - worktree:
/Users/rynfar/.prime/worktrees/prime-snapshot-recovery-integrity - committed head:
2e189d2ac951a92631847ffb21151f19c6b38d71, based on the formerpylon@8551520f... - current state: one committed implementation plus 16 modified files and one untracked test
This comment is a collision-avoidance hold, not a takeover claim. Do not edit, rebase, merge, reset, clean, or otherwise mutate that worktree until its owner checkpoints the current work and records the required issue claim.
The owner claim must name the branch/worktree, complete owned file inventory, contract and compatibility boundary, dependency on
pylon@f728316d, test plan, reviewers, and merge order. Only after the dirty state is checkpointed should the owner integrate the neworigin/pylon; the committed overlap with PR #12 is limited topackages/coding-agent/src/core/agent-session.tsand a read-only merge-tree review found it automatically mergeable. Combined snapshot-integrity and correlated-lifecycle coverage is required afterward.The old #8 upstream candidate remains stale and must not be used as review evidence. It will be regenerated only after #11 merges.
- branch:
Correction to the worktree count above: current
git status --porcelain=v2shows 15 modified tracked files plus one untracked test, not 16 modified files. The branch, head, worktree, and collision-avoidance hold are unchanged.Recovery ownership claim
No reachable active agent owns or knows the owner of the protected worktree; both active sibling sessions confirmed they have not touched it. I am taking over this issue only in a new isolated worktree. The original dirty worktree remains untouched and protected.
- branch:
fix/snapshot-recovery-integrity-recovery-f728 - worktree:
/Users/rynfar/.prime/worktrees/prime-snapshot-recovery-integrity-recovery - exact base:
origin/pylon@f728316dabbaa85aa561e6d6b08550ed337574be - protected source: committed
2e189d2ac951a92631847ffb21151f19c6b38d71plus its read-only captured dirty/untracked state - dependency: PR fix(coding-agent): preserve correlated prompt ownership #12 is merged; old Prime upstream sync candidate ready #8 candidate
a37efe92...is stale and must not be merged; fix(coding-agent): bound public daemon client JSONL ingress #13 remains ordered afterward
Owned inventory
.pylon/features.yaml.pylon/upstream-review.mdpackages/coding-agent/.changes/11-snapshot-recovery-integrity.mdpackages/coding-agent/src/core/agent-session.tspackages/coding-agent/src/modes/agent-connection/daemon-agent-connection.tspackages/coding-agent/src/modes/daemon/active-session-state.tspackages/coding-agent/src/modes/daemon/daemon-mode.tspackages/coding-agent/src/modes/daemon/daemon-protocol.tspackages/coding-agent/src/modes/daemon/daemon-session-list.tspackages/coding-agent/src/modes/daemon/daemon-supervisor.tspackages/coding-agent/src/modes/daemon/daemon-worker-client.tspackages/coding-agent/src/modes/daemon/snapshot-transcript-cache.tspackages/coding-agent/src/modes/session-worker/private-framing.tspackages/coding-agent/test/agent-connection-daemon.test.tspackages/coding-agent/test/agent-session-recursion.test.tspackages/coding-agent/test/daemon-mode.test.tspackages/coding-agent/test/daemon-protocol.test.tspackages/coding-agent/test/daemon-session-list.test.tspackages/coding-agent/test/daemon-supervisor-lazy-subagents.test.tspackages/coding-agent/test/daemon-supervisor-monitor.test.tspackages/coding-agent/test/daemon-supervisor-process.test.tspackages/coding-agent/test/daemon-version-compatibility.test.tspackages/coding-agent/test/session-worker-private-framing.test.tspackages/coding-agent/test/snapshot-transcript-cache.test.tspackages/coding-agent/test/suite/regressions/4601-worker-snapshot-cache.test.tspackages/coding-agent/test/suite/regressions/4602-snapshot-transfer-idempotency.test.tspackages/coding-agent/test/suite/regressions/4677-snapshot-catchup-replacement.test.ts
Contract
- preserve protocol 7 and backward compatibility;
- expose
immutable_snapshot_transfer_v1only through explicit daemon/worker capability negotiation; - keep ordering cursors separate from opaque transfer identities that name immutable bytes;
- prepare bounded immutable memory/file-backed chunks before deferred streaming, with cancellation and private cleanup on all exits;
- retire and retry only the bad generation without closing an otherwise healthy worker;
- bound retries and prove worker/recovery ownership before session reuse;
- reduce child snapshot fan-in without dropping terminal child state;
- keep socket paths, session paths, raw snapshot payloads, worker tokens, and diagnostics host-private;
- provide the public worker/descriptor integrity evidence needed by Comet chore(pylon): merge Prime upstream through d60fab8a #5 or document the exact remaining gap.
Validation and review
The recovery will first reproduce the protected tree without mutating it, then integrate
f728316d, resolve/review theagent-session.tsoverlap, and run focused snapshot/protocol/connection/supervisor/recursion tests, real-process isolation/recovery tests, pinned stock-v0.8.1 compatibility when the artifact is available, full checks, and independent Comet/Pylon-consumer rereviews. Any inherited test claim will be rerun rather than trusted.Merge order: #12 (done) → this issue → regenerated/reviewed #8 → #13 → Comet #5. No production claim will be made until the recovered implementation and its public integrity proof pass review.
- branch:
Additional merge blocker: authoritative owned-session cleanup proof
A focused read-only review of the recovered candidate found that the current
client_owned_sessionsbehavior is not strong enough for the cross-process host contract. This is separate from the four snapshot/framing/authority P1 repairs now committed atba86fb349ba43bf4179b58bde19b34eacce4d8f2.Blocking findings:
complete_owned_sessionawaits exact worker stop, butdeleteWorkerDescriptor()swallows descriptor-removal failures after removing the in-memory registration. The command can therefore report success while a stale durable descriptor remains.- Owner disconnect cleanup is best-effort. An initial stop-tombstone persistence failure is only logged and is not rearmed, so a crashed owner can leave a client-owned worker live indefinitely until supervisor restart. Descriptor-removal failure can likewise leave a stale descriptor without a current-supervisor retry.
- Ownership reconnect only works for the same
DaemonClientobject's private protocol ID. A replacement host process cannot impersonate the original owner safely. - A different client has no public absence proof. Owner-filtered
list, global busy count, and asynchronousshutdownadmission cannot prove that both the exact worker generation and descriptor are gone.
Required before merge (exact names remain reviewable):
- Add a new server capability such as
authoritative_owned_session_cleanup_v1; do not reinterpret stock 0.8.1'sclient_owned_sessionsoffer. - Make durable cleanup fail closed: remove ancillary journals first and the registration descriptor last; retain the tombstoned in-memory worker and retry on tombstone/descriptor failures; never return successful completion until exact process-generation and descriptor absence are proved.
- Add a non-owning, privacy-safe query such as
get_owned_session_cleanup { activeSessionId } -> active | stopping | settled.settledmust mean no in-memory/durable registration and no exact worker process generation, without exposing PID, path, owner ID, or raw descriptor. - Add deterministic descriptor-unlink and tombstone-write failure regressions, direct completion proof, real socket-crash polling from a different client, and supervisor-replacement coverage. Stock 0.8.1 must lack the new capability and the client must reject the query locally.
The compatibility test now waits for natural supervisor and worker exit in both stock/current directions and asserts zero worker descriptors; forced process cleanup is limited to
afterEachfallback. This improves evidence but does not replace the public runtime proof above.The candidate remains blocked. No PR should be opened or merged until this contract is implemented and independently rereviewed.
Ownership inventory amendment for the authoritative-cleanup blocker
The accepted cleanup design requires a new public capability-gated query and exported result type. Before editing, I am extending the existing recovery claim to these additional paths:
packages/coding-agent/src/modes/index.tspackages/coding-agent/src/index.tspackages/coding-agent/test/daemon-client.test.tspackages/coding-agent/docs/agent-connection.md
The already-claimed protocol, supervisor, process/compatibility tests,
.pylonreview files, and change note remain owned by the same isolated recovery worktree. The new surface will keep protocol 7, add an additive schema revision and supervisor-onlyauthoritative_owned_session_cleanup_v1offer, reject stock 0.8.1 locally, expose onlyactive | stopping | settled, and keep all worker/process/path/owner details private. No other checkout may edit these paths until this claim is released.Exact candidate awaiting final independent review
The recovery branch is now pushed at exact clean head
3ab9110d11643c8bc1da9472843a1b1c17b45d2c.New commits after the previously recorded four-blocker head:
5d1c9e224b04c351bb1cdb69b216bea0fdd82d3b— fences attach admission, closes committed-but-interrupted private frames, and makes worker authority current-channel/current-roster scoped.3ab9110d11643c8bc1da9472843a1b1c17b45d2c— adds supervisor-onlyauthoritative_owned_session_cleanup_v1, schema 27, privacy-safeget_owned_session_cleanup, descriptor-last verified removal, durable retry finalization, direct/crash/replacement proof, stock-v0.8.1 local rejection, documentation, and review ledger updates.
Validation at this exact source state:
- 688 focused tests across 13 protocol/client/session/snapshot/supervisor files;
- 40 correlated lifecycle/queue/continuation tests;
- 12 real-process supervisor tests, 8 fixture-gated skips;
- both stock/current v0.8.1 adoption directions;
npm run check, root build, installer/browser smoke, andgit diff --check.
The real owner-socket-loss regression forced a visible
stoppinginterval withSIGSTOP, then a different client provedsettled, exact worker exit, and zero descriptors. The build-generated model catalog was restored.Two independent read-only exact-head reviews are still running. The candidate remains blocked from PR creation until both conclude and any P0/P1 findings are repaired and revalidated.
Five final-review P1 races repaired
Exact clean head
f163bddc8a76d5ecd02b848cdbcc80f7b9fa0753addsf163bddc8on top of the prior candidate and is pushed to the recovery branch.Repairs:
- Stop intent advances a revision, becomes durable, and then joins any admitted recovery/deferred-recovery task before process or descriptor absence can be proved. Every recovery/adoption path revalidates the current map object, stop revision, tombstone state, pid/start generation, and authenticated channel after asynchronous gates and before spawn or persistence.
- Shutdown no longer disables finalizers. It retains registry ownership, the catalog, and server while tombstone/process/archive/descriptor cleanup drains; non-timeout cleanup failures cannot abort the only retry path.
- Same-generation removal is single-flighted through one shared stop operation, so concurrent
complete_owned_sessioncalls cannot race descriptor removal. - Finalizers are keyed by pid, process start ID, and stop revision. An old finalizer cannot suppress or clear a newer stop generation.
- Client-owned create handling rechecks owner liveness after the worker is registered, and a disconnected pre-ready command cannot cancel its cleanup timer.
New deterministic proof includes a pre-spawn recovery fence, recovery-join ordering, concurrent completion, generation handoff, shutdown drain, and a real socket test that commits a create, proves no descriptor exists before disconnect, then observes the late registration progress to authoritative
settled, process absence, and zero descriptors.Validation at this source state:
- 693 focused tests across 13 files;
- 40 correlated lifecycle/queue/continuation tests;
- 13 real-process supervisor tests passed with 8 fixture-gated skips;
- both stock/current v0.8.1 adoption directions (the combined compatibility run passed on repeat after one transient temp-directory cleanup race);
- full
npm run check, root build, installer/browser smoke, andgit diff --check.
The two original reviewers are re-reviewing exact head
f163bddc8a76d5ecd02b848cdbcc80f7b9fa0753. The PR gate remains closed pending both approvals.Published-recovery and reentrant-shutdown races repaired
Exact clean pushed head
90e0f092c0b79d5ef2c4538b1fa2be69d61da6e9adds90e0f092cafter the final re-review findings.- A canceled existing-worker replacement rollback still stops and joins its direct child, but it now retains the shared in-memory registration when an outer durable tombstone or supervisor shutdown owns cleanup. The outer authoritative stop can therefore verify and remove the descriptor last instead of losing its finalizer route.
- Supervisor shutdown is now a single task. Reentrant public shutdown commands or signals join the active cleanup drain and cannot call
process.exitwhile tombstone/archive/descriptor retries are pending. - Deterministic monitor regressions exercise a replacement child after publication with concurrent exact cleanup, and a second shutdown call while descriptor cleanup is blocked.
Revalidated at this source state: 694 focused tests, 40 correlated-lifecycle tests, 13 real-process tests with 8 fixture skips, both stock/current v0.8.1 adoption directions, full check/build, installer/browser smoke, and
git diff --check. The one earlier 36 MiB event-loop threshold miss passed both its immediate isolated rerun and the complete 694-test rerun and was treated as environmental load, not a source failure.All three reviewers are rechecking exact head
90e0f092c0b79d5ef2c4538b1fa2be69d61da6e9. The PR gate remains closed pending approvals.Ordinary recovery rollback now retains cleanup authority
Exact clean pushed head
49c592ffc863263123020c7a8aaa97158bbbc7ecadds49c592ffc.The published-replacement preservation rule no longer depends on a direct child handle. Any non-descriptor recovery cleanup retains the same mapped registration when an outer tombstone or supervisor shutdown owns cleanup. The deterministic regression now executes both the cancellation/direct-child branch and the ordinary error/no-direct-child branch before allowing the outer descriptor-last stop to settle.
Exact validation: 695 focused tests, 40 correlated-lifecycle tests, 13 real-process tests with 8 fixture skips, both stock/current v0.8.1 directions, full check/build, installer/browser smoke, and
git diff --check.All three reviewers are rechecking
49c592ffc863263123020c7a8aaa97158bbbc7ec. The PR remains gated.Pull request opened after exact-head approval
PR #14 is open against
pylonat exact head49c592ffc863263123020c7a8aaa97158bbbc7ec.Three independent read-only reviews approved this exact commit with no P0/P1 findings: security/correctness, concurrency/durability, and a fresh full-regression review. Local validation and compatibility receipts are recorded in the PR body and comments above.
Hosted checks are now queued. Merge remains gated on all required checks, resolved conversations, and recorded maintainer approval.
PR #14 hosted-CI repair receipt
The initial hosted run at
49c592ffc863263123020c7a8aaa97158bbbc7ecpassed build/check, process smoke, kernel, compatibility coverage, and most test jobs, but exposed six failures in coding-agent shards 1/3 and 3/3.Independent diagnosis found no production defect:
- heartbeat and resume tests used stale partial doubles that violated the new exact worker-identity and post-validation attach-admission contracts;
- update-restart assertions captured serialized
Bufferoutput as if it were always a string; - the two-pass 5 MiB spill/re-encode regression retained a 15-second per-test timeout that was too tight under hosted shard load.
Test-only repairs:
c37f4bddb360826b5878a9989f3779bea5ee2e33c5a34dffcd44eaaa4d66ea43786d19260a10e7f2(widens only that expensive regression's bounded timeout to 60 seconds)
Validation:
- the 55 affected tests pass at the final head;
- local replay of hosted shard 1: 1,473 passed / 24 skipped;
- local replay of hosted shard 3: 1,243 passed / 22 skipped;
- exact final-head
npm run checkand pre-commit checks pass; - production files remain byte-for-byte unchanged from the three-reviewer-approved
49c592ffc863263123020c7a8aaa97158bbbc7ec.
Updated upstream synthetic receipt:
- merge
8afe6a8d910e0f9fc3d896aa0bae871c2c59aeba/ treef6cff7f22608329b3e9fd4e99c4bd8c66ad14ccd; - exact parents
c5a34dffcd44eaaa4d66ea43786d19260a10e7f2anda903d4b6768f484bd6d459b7b0aa7dee38e461e2; - same independently approved conflict resolutions as the prior synthetic merge;
- the new synthetic tree differs from the prior approved synthetic tree only in the four test-only CI repairs;
- exact synthetic
npm run checkand 55/55 affected tests pass; exact-commit synthetic re-review is queued.
The final hosted run for
c5a34dffcd44eaaa4d66ea43786d19260a10e7f2is now in progress. PR #14 remains blocked until every required check, conversation, and maintainer-approval gate is satisfied.Exact final-head upstream synthetic re-review: approved with no P0/P1.
- synthetic merge:
8afe6a8d910e0f9fc3d896aa0bae871c2c59aeba - tree:
f6cff7f22608329b3e9fd4e99c4bd8c66ad14ccd - parents:
c5a34dffcd44eaaa4d66ea43786d19260a10e7f2+a903d4b6768f484bd6d459b7b0aa7dee38e461e2 - compared with prior approved synthetic
3c12d54895edb69ff7db64abb928605f15ba6ca3: exactly the four test-only hosted-CI repair files differ; no production path differs - all four resolved-conflict blobs are byte-for-byte identical to the prior approved synthetic
git diff --check, exact-treenpm run check, and 55/55 repaired tests pass
The final hosted check run remains the only current validation gate before conversation audit and exact-head maintainer approval.
- synthetic merge:
- added 2 commits that reference this issue
on Aug 30, 2026
Problem
The daemon can reuse a positional snapshot ID (
session-generation-event cursor) after snapshot bytes change. Catch-up then detects different bytes under the same ID, closes the worker control channel, and aborts every resident session.This has recurred in the Pylon-installed fork under large transcripts and high RLM child-update fan-in. Prime upstream issue PrimeIntellect-ai#1229 described the same invariant but closed without a fix.
Required outcome
Compatibility
Keep the existing daemon protocol backward-compatible unless a capability-gated change is required. Preserve stock Prime behavior outside the Pylon integration boundary.