Skip to content

fix(harness): a Superseded exit must actually exit — drop cmd_tx so the engine terminates - #597

Merged
nikhilunni merged 1 commit into
mainfrom
fix/harness-superseded-must-exit
Jul 7, 2026
Merged

nikhilunni merged 1 commit into
mainfrom
fix/harness-superseded-must-exit

Conversation

@nikhilunni

Copy link
Copy Markdown
Contributor

Found by PR #583's prod verification (leg B: evict_local → resume → prompt).

The wedge

A checkpoint captured with a LIVE harness (periodic checkpoints; anything except the idle-evict drain) restores a harness whose binding epoch is stale. On the post-restore SIGUSR1 nudge it re-dials, gets the typed Superseded rejection, logs "exiting cleanly" — and never exits: the connection loop breaks, but main holds a cmd_tx clone across engine.await, and the engine only terminates when all senders drop. The process lingers alive with its connection loop and SIGUSR1 handler dead.

agentd's SpawnHarness reattach arm (correctly) reattaches-not-respawns for a live pid — so every subsequent start_agent "succeeds", nudges the phantom, and no fresh harness ever spawns. The outbox loops NotFound → "harness reattach issued (start_agent fallback)" forever. Prod session 7ed23d9f: 22 delivery attempts over 15 minutes, zero recoveries. This is also the ADR 0028 host-loss recovery path — a host crash today would wedge every recovered session's prompt delivery.

Fix

drop(cmd_tx) before engine.await — the engine's designed ChannelClosed teardown fires, the process exits, agentd reaps and respawns fresh with the current epoch. The ChannelClosed arm now also SIGINTs claude (it skips the reap block; an orphaned twin would contend with the successor's --resume on the same transcript).

Deploys via the harness bundle republish (detector #591 covers harness→bundle). Verification: re-run the evict_local→resume→prompt cycle on prod post-deploy — expect recovery within one SpawnHarness (~seconds), not a permanent wedge.

🤖 Generated with Claude Code

https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx

…he engine terminates

The ADR 0073 Superseded rejection arm logged "exiting cleanly" and broke
the connection loop — but main holds a cmd_tx clone for the whole
process, so `engine.await` never completed: the harness lingered ALIVE
with its connection loop (and SIGUSR1 handler) dead. agentd's
SpawnHarness reattach arm then saw a live pid on every subsequent
start_agent, nudged it forever, and never respawned a fresh harness —
so every resume from a live-harness checkpoint (evict_local, and the
ADR 0028 host-loss recovery path) wedged prompt delivery permanently:
the outbox looped NotFound -> "harness reattach issued" indefinitely
(prod session 7ed23d9f, 2026-07-06; the #583 verification caught it).

Fix: drop cmd_tx before `engine.await` — the engine's designed
ChannelClosed teardown ("all command senders gone = the connection loop
exited = process teardown") fires, the process exits, agentd reaps it,
and the next SpawnHarness spawns a FRESH harness with the current
binding epoch. The ChannelClosed arm now also SIGINTs claude (it skips
the reap block, and an orphaned twin would contend with the successor's
--resume on the same transcript; claude flushes per-message, so SIGINT
is resume-safe).

The idle-evict path never hit this (its drain produces claude-free
snapshots); only checkpoints capturing a LIVE harness restore into the
stale-epoch -> Superseded -> phantom-exit shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
@nikhilunni
nikhilunni merged commit 6738343 into main Jul 7, 2026
18 checks passed
@nikhilunni
nikhilunni deleted the fix/harness-superseded-must-exit branch July 7, 2026 04:51
nikhilunni added a commit that referenced this pull request Jul 7, 2026
GCS-free resume moves 4+5, rebased onto main past #583/#584/#597. Close
the put_chunk exists()-arm write-through hole + add put_chunk_unchecked
to drop the per-dirty-chunk GCS HEAD on flush paths. ADR renumbered
0075 -> 0078 at land (main's 0075 = substrate-single-writer, #583).

Review fix folded in: the flush path's explicit cache.put after
put_chunk_unchecked was a duplicate 16 MiB local write per flushed
chunk (the store's internal write-through already warms the ONE cache —
prod wires it in host-agent main). Cut to one path; the
flush_write_throughs_chunks_into_local_cache test now mirrors the prod
wiring (store carries the cache).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
GCS-free resume moves 4+5, rebased onto main past #583/#584/#597. Close
the put_chunk exists()-arm write-through hole + add put_chunk_unchecked
to drop the per-dirty-chunk GCS HEAD on flush paths. ADR renumbered
0075 -> 0078 at land (main's 0075 = substrate-single-writer, #583).

The flush path keeps its explicit cache.put alongside the store-internal
write-through: cache and store travel separately into the disk backend,
so the store carrying a cache is prod wiring, not a structural
guarantee — collapsing to one cache identity is ADR 0076 (substrated)
territory. (A review pass tried cutting it; reverted — the CI-proven
shape stands. Note: eviction_finalize_redrive fails on the DEV-VM for
main and this branch alike — environmental, tracked in the dev-vm
pitfalls; CI is the arbiter for that suite.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
GCS-free resume moves 4+5, rebased onto main past #583/#584/#597. Close
the put_chunk exists()-arm write-through hole + add put_chunk_unchecked
to drop the per-dirty-chunk GCS HEAD on flush paths. ADR renumbered
0075 -> 0078 at land (main's 0075 = substrate-single-writer, #583).

The flush path keeps its explicit cache.put alongside the store-internal
write-through: cache and store travel separately into the disk backend,
so the store carrying a cache is prod wiring, not a structural
guarantee — collapsing to one cache identity is ADR 0076 (substrated)
territory. (A review pass tried cutting it; reverted — the CI-proven
shape stands. Note: eviction_finalize_redrive fails on the DEV-VM for
main and this branch alike — environmental, tracked in the dev-vm
pitfalls; CI is the arbiter for that suite.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
GCS-free resume moves 4+5, rebased onto main past #583/#584/#597. Close
the put_chunk exists()-arm write-through hole + add put_chunk_unchecked
to drop the per-dirty-chunk GCS HEAD on flush paths. ADR renumbered
0075 -> 0078 at land (main's 0075 = substrate-single-writer, #583).

The flush path keeps its explicit cache.put alongside the store-internal
write-through: cache and store travel separately into the disk backend,
so the store carrying a cache is prod wiring, not a structural
guarantee — collapsing to one cache identity is ADR 0076 (substrated)
territory. (A review pass tried cutting it; reverted — the CI-proven
shape stands. Note: eviction_finalize_redrive fails on the DEV-VM for
main and this branch alike — environmental, tracked in the dev-vm
pitfalls; CI is the arbiter for that suite.)


Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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