fix(sdk,acp,agent,gc): twelve SDK lifecycle, ACP session-survival, and disk-retention defects - #4021
fix(sdk,acp,agent,gc): twelve SDK lifecycle, ACP session-survival, and disk-retention defects#4021probepark wants to merge 19 commits into
Conversation
One ACP session burned 121 rejected requests over 29 minutes and then went silent: every repair reset the provider retry budget it was meant to consume, and the per-call repair flags let a layer above re-enter the same latest/all pair ~60 times against a request the provider would never accept. The repair budget now lives in providerSessionState, so it survives stream re-invocation for the same session, and a "blocks cannot be modified" 400 is answered by dropping native thinking replay entirely instead of mutating the very blocks the provider demands verbatim. Lore-id: 7c3f9a12 Constraint: repairs must not renew PROVIDER_MAX_RETRIES -- they are bounded independently Constraint: a turn that stops must emit a terminal error frame carrying the provider message Rejected: raise PROVIDER_MAX_RETRIES | the request shape is unacceptable, more retries only widen the loss Rejected: keep escalating latest -> all on the mutation 400 | the provider states those blocks must be replayed verbatim, so mutation can never converge Confidence: high Scope-risk: narrow Reversibility: easy Directive: do not reset providerRetryAttempt from any degradation branch Tested: mutation 400 degrading into masked api_error stops at 2 requests with a terminal error Tested: repair budget survives stream re-invocation sharing providerSessionState Not-tested: live CLIProxyAPI replay against api.anthropic.com
A `!cmd` typed while the agent was Working ran, then vanished: completion detached the block from the pending container without ever re-parenting it, so it lived on in a plain array with no parent and was never drawn. Ordinary mid-turn events made it worse — `updatePendingMessagesDisplay()` and the transcript rebuild both called `Container.clear()`, which disposes children, tearing down a still-streaming block and dropping its buffered output. Completion now moves the block into the chat transcript, and both rebuild paths detach-and-reattach parked execution components instead of disposing them. `Container.clear()` keeps its disposing contract; the retention lives in a private helper on the coding-agent side. Lore-id: 5b81e40c Constraint: Container.clear() must keep disposing children -- other callers depend on it Constraint: a running block must stay parented while it streams, not be flushed early into chat Rejected: flush parked components on the streaming submit path | an in-flight block would jump into the transcript before it finished Rejected: change clear() to detach instead of dispose | silently leaks every other container's children Confidence: high Scope-risk: narrow Reversibility: easy Tested: `!` submitted mid-turn is parented while streaming and lands in the transcript on completion Tested: pending-queue refresh and transcript rebuild no longer dispose a running execution block Not-tested: interactive TUI smoke on a real terminal
…uggable A machine accumulated 54 `.broker.lock.stale-*` tombstones, a 126 MB `lifecycle-ledger.jsonl.corrupt` next to a 6 MB live ledger, and a dead-owner lock that wedged broker startup. The reclaim path renamed dead locks to tombstones but nothing ever removed them, and the quarantine sidecar was appended to forever. The broker now reaps abandoned lock artifacts on startup after it owns the lock, the corrupt sidecar rotates at a bounded size, and the spawn no longer throws away its own diagnostics: `stdio: "ignore"` is replaced by a truncated per-spawn log whose stderr tail is attached to the discovery failure. Lore-id: 9d24af61 Constraint: reaping is fail-closed -- owner-alive, unreadable, or ambiguous artifacts are kept with a reason Constraint: only the lock holder reaps, so concurrent brokers cannot race the removal Constraint: corruption evidence must survive rotation, not be silently dropped Rejected: reap before acquiring the lock | racing brokers would delete each other's artifacts Rejected: delete the corrupt sidecar outright | destroys the only evidence of what was quarantined Confidence: high Scope-risk: narrow Reversibility: easy Tested: aged tombstones removed, live-owner and unreadable artifacts retained with reasons Tested: corrupt quarantine rotates at the cap instead of growing unbounded Tested: a broker that exits before discovery now reports why Not-tested: multi-machine concurrent broker contention
… budget The chat daemon holds its attached-session clients until an explicit detach or endpoint roll, under the same host liveness reaper that Yeachan-Heo#4012 fixed for ACP, yet attach() still dialed with the transport's one-shot defaults -- 3 attempts at 25ms base, 175ms total. The host drops any session not ponged within HEARTBEAT_TTL_MS (20s), so every stall long enough to be reaped left the attachment permanently lost. ACP_SESSION_RECONNECT is hoisted out of the ACP adapter into src/sdk/session-reconnect.ts so the bus layer can reach it without importing the ACP layer, and attach() now connects on that shared budget. Lore-id: 5b3e1c74 Constraint: exactly one definition of the budget -- ACP call sites keep importing it Constraint: the bus layer must not import sdk/acp -- the constant moves, not the dependency Rejected: put it in bus/daemon-paths.ts | that module is deliberately paths-only and would gain an SdkClientOptions dependency Rejected: raise the bridge-client transport defaults | correct for the one-shot request clients that already rely on them Confidence: high Scope-risk: narrow Reversibility: easy Directive: leave the per-request broker client on the one-shot defaults -- it closes in its own finally Tested: the real ChatDaemonRuntime attach path driven to reconnect exhaustion on a fake transport and clock, asserting the concrete 250/500/1000/2000 schedule and a cumulative budget of 41750ms Not-tested: recovery against a host that comes back mid-schedule
Every broker-launched session ran the local memory backend's phase-1 stage-1 LLM jobs synchronously inside the 10s readiness window. Whenever the queue had pending rollouts, those calls ate the whole budget, the child was killed at the cutoff, and the broker answered "No ready SDK endpoint remains available." Six such failures landed in ~15h; the same shape recurred while running this batch. Lifecycle sessions now defer memory startup as an invariant and resume it once readiness is published, unawaited, with failure logged rather than fatal. The ACP carve-out in main.ts goes away with it, and postmortem stops collapsing a thrown record to "[object Object]" so the next cutoff keeps its phase/reason. Lore-id: 3f6ad920 Constraint: session readiness must never depend on LLM calls -- startup work is unbounded Constraint: memory failure after readiness is a degraded-memory condition, not a startup failure Rejected: widen READY_TIMEOUT_MS | startup work scales with rollout backlog and provider latency, so any constant is just a wider flake window Rejected: keep deferral a caller option for lifecycle sessions | the broker deadline makes it an invariant, not a choice Confidence: high Scope-risk: narrow Reversibility: easy Directive: never await memory startup on a path that publishes readiness Tested: readiness is published before a memory start that outlives the budget resolves Tested: a post-readiness memory rejection neither tears down the session nor becomes an unhandled rejection Tested: a non-Error throwable keeps its payload in the crash record Not-tested: live broker under a real multi-day rollout backlog
… load A session whose transport dropped could never be recovered: `session/prompt` answered `Unknown session, not found`, and the documented recovery through `session/load` then died on `ACP cannot replay a transcript entry without its production body`. One malformed row revoked `loadSession` for the entire session, and the agent ended `closed` with its work unreachable — reproduced across a broker restart and a fresh binary. Replay now decides per entry: an entry without its production body is skipped and reported through the same `_meta` boundary channel that already reports unavailable historical images, so a session with zero replayable rows still loads. Lore-id: c4e7b285 Constraint: never fabricate an empty body -- that replays a message that never existed Constraint: gjc advertises loadSession: true, so load must not be revocable by one bad row Rejected: keep throwing and let the client retry | the failure is deterministic, every retry dies identically Rejected: substitute a placeholder body | silently invents transcript content the user never wrote Confidence: high Scope-risk: narrow Reversibility: easy Tested: one bad entry among good ones -- good ones replay, the skip is reported with count and reason Tested: every entry unreplayable -- load still succeeds with zero replayed messages Tested: healthy transcripts replay with an unchanged update sequence Not-tested: recovery against a live host that dropped mid-turn
Detached session hosts had no exit path while the broker stayed alive. The one automatic reaper, watchSessionHostBrokerLiveness, is conditioned on broker discovery going ABSENT, and the default warm broker never disappears — so the grace window never opened. The other leg, session.close, is never issued by ordinary SDK usage. The reporter measured 119 leaked hosts holding 4.1 GB under one healthy 6-day broker; this machine reached 29 hosts for 9 agents with 20 of them holding zero connections, within hours. Hosts now self-reap after a bounded idle interval with no attached client, and the broker drops registrations whose host is provably gone. The broker-absence watcher is untouched; this is a second, independent bound. Lore-id: e81c4a37 Constraint: a host that has never seen its first attachment must not be reaped -- the client may still be dialing Constraint: attachment is decided from the host's own subscription state, never by scraping ps or lsof Constraint: detached + unref stays -- the spawn shape is deliberate, the missing bound was the bug Rejected: rely on session.close alone | ordinary SDK usage never issues it, which is exactly how the leak starts Rejected: cap concurrent hosts | evicts live sessions under legitimate parallel load Confidence: high Scope-risk: narrow Reversibility: easy Tested: a host whose client detaches exits after the idle bound Tested: a host with an attached client is never reaped, driven well past the bound Tested: a freshly spawned host survives the first-attach grace Tested: broker-absence behavior unchanged Not-tested: multi-day accumulation against a live warm broker
The harness already resolved these calls with certainty and then refused to run them: `Tool mcp__<server>__<instance>_search not found. It is active as `search``. Every occurrence burned a whole model turn, and it recurs constantly because bridges mint a fresh instance segment per session, so a name replayed from earlier context differs from the live registry only in that segment. When the namespace-stripped base name resolves to exactly one active tool, the call is now dispatched to it and the rename is recorded. Zero or multiple candidates keep today's error, unchanged and still naming every candidate -- guessing between two tools would route the model at one it did not ask for. Lore-id: 2a9f6d31 Constraint: resolution stays the existing exact namespace-stripped base match -- no fuzzy matching Constraint: validation, hooks, permissions and telemetry run against the resolved tool Rejected: keep only improving the error text | Yeachan-Heo#3917 did exactly that and the turn is still lost Rejected: pick the first of several candidates | silently runs a tool the model did not name Confidence: high Scope-risk: narrow Reversibility: easy Tested: a stale bridge namespace with one active match executes, with arguments intact Tested: two candidates still error and list both names Tested: no candidate still reports the unchanged base not-found message Not-tested: live bridge reconnect churn across a long session
An ACP prompt settled only on a terminal frame. There was a rejection path for a lost owner connection and one for a malformed terminal, but none for "the producer stopped and no terminal will ever arrive", so `activePrompt` stayed unsettled forever and `session/prompt` never returned. Captured twice in one hour on this machine: host alive at 0% CPU, only the loopback socket left, last log line then silence, while the client reported the agent running for 21 more minutes. One host had already produced its complete final answer. A per-prompt inactivity watchdog now settles the prompt with a diagnosable terminal error naming the silence duration and the last frame seen. It settles the ACP prompt only; it never cancels the agent's underlying work. Lore-id: 8e5c1f43 Constraint: the bound is derived from MAX_TOOL_RUNTIME_MS + 10 x HEARTBEAT_TTL_MS, never a magic number Constraint: any frame for the prompt refreshes the watchdog, so slow-but-healthy work is untouchable Constraint: the session must still accept a subsequent prompt after a watchdog rejection Rejected: cancel the agent run on expiry | the work may be fine, only its terminal frame is missing Rejected: a short fixed timeout | a single long bash or ssh call would trip it Confidence: high Scope-risk: narrow Reversibility: easy Tested: silence past the bound rejects the prompt instead of hanging Tested: frames arriving just under the bound, repeatedly, never trip it Tested: a normal agent_end settles exactly once and the watchdog does not double-settle Not-tested: a live host going silent against a real model gateway
`session/new` failed intermittently with "Lifecycle terminal evidence could not be verified after persistence" — ~29% in a large tree, 0% in small ones, and non-deterministic. One failed probe permanently poisons an ACP host: the provider goes `status: "error"` for the daemon's whole lifetime, so gjc cannot be selected until the host restarts. Observed on this machine today. The race: a recovery pass stamps `terminal_uncertain` on every row it finds mid-flight, so a slow operation can have that marker interleaved before its own terminal row lands. `readTerminal` treated only `terminal_ok`/`terminal_error` as terminal, so it reported a row that WAS on disk as unpersisted and replaced its real reason with the generic uncertainty error. Reading back the durable record fixes the report at its source. Lore-id: b6d02e59 Constraint: genuinely unverifiable evidence must still report terminal_uncertain -- the signal is not suppressed Constraint: a proven terminal row stays immutable and admits no successor Rejected: retry the verification | the slow path is normal on a large tree, so it must be handled, not out-waited Rejected: widen the startup scan deadline | being slow on a big tree is legitimate, not the defect Confidence: high Scope-risk: narrow Reversibility: easy Tested: the owner's terminal record is read after a concurrent recovery stamps its in-flight row Tested: a durable terminal_uncertain is read back instead of reported unpersisted Tested: proof is still withheld when a persisted row cannot be reproduced Not-tested: live repeated session/new against a multi-gigabyte home directory
Giving the chat daemon's attached-session client the long-lived reconnect budget changed `chat-daemon-runtime.ts:attach`, so its declaration digest moved for the discord and slack surfaces. Regenerated with --write-manifest rather than hand-edited. Lore-id: 4d17b9e2 Confidence: high Scope-risk: narrow Reversibility: easy Tested: check:sdk-closure
The exhaustion test dialed a dead endpoint with the production budget, which Yeachan-Heo#4012 deliberately grew to outlive the host heartbeat TTL (41.75s of real backoff). It has been timing out at 5s on origin/dev ever since, so check:sdk-closure fails on a clean checkout. Injecting a one-shot client keeps the assertion about the typed rejection and drops the run from 41.9s to 96ms. Lore-id: 7a2c8e04 Constraint: the budget itself stays asserted from its constants in acp-session-reconnect.test.ts Rejected: raise the test timeout past 42s | pays 42s of wall clock on every run to assert an error code Rejected: shrink the production budget | it is sized against HEARTBEAT_TTL_MS on purpose Confidence: high Scope-risk: narrow Reversibility: easy Tested: the file now runs 2 pass / 0 fail in 96ms
d94aa86 to
9f7fc39
Compare
`attach()` is a protected chat-daemon lifecycle declaration, so changing it to dial on the long-lived session reconnect budget requires a strictly higher generation: an owner still running the old code gives up reconnecting after 175ms and permanently loses its attachment, and the generation is what stops it from serving requests captured against the new contract. Lore-id: 6f21b8ac Confidence: high Scope-risk: narrow Reversibility: easy Tested: telegram-daemon-generation-guard against origin/dev
|
CI is currently blocked by — |
`gjc gc` was a liveness reaper only: it removes a record when its owning PID is provably dead, and nothing in GJC ever reclaimed bytes. One daily-driver workstation reached 13 GB under ~/.gjc in three weeks — 6.9 GB of session transcripts, 371 MB of blobs, 552 MB across 14 cached native versions, 717 MB of update backups — none of it reachable through any command. The only remedy was `rm -rf` with hand-written find predicates against a live session store. `gjc gc --disk` reports reclaimable bytes per surface and mutates nothing; `--disk --prune` performs it. Session retirement is reference-aware and releases the retired transcripts' blobs through a mark-and-sweep of the canonical store. Lore-id: 1c93e7f5 Constraint: the existing PID-liveness contract and exit-code policy are untouched when --disk is absent Constraint: dry-run by default -- deleting resumable user work requires an explicit --prune Constraint: fail-closed -- live, referenced, permission-denied or ambiguous entries are kept, with a reason Rejected: auto-GC on startup | an implicit deletion pass over resumable work must never be a startup side effect Rejected: age-only pruning | orphans blobs and races a live session index, which is what users improvise today Confidence: high Scope-risk: moderate Reversibility: migration-needed Directive: never delete a session referenced by an active lease or recently resumable Tested: --disk without --prune mutates nothing and reports per-surface bytes Tested: a blob referenced by a surviving session outlives the retirement of another that shared it Tested: natives retention keeps the running version plus the configured predecessors Not-tested: reclaiming a multi-gigabyte real store end to end
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Blocking reconnect regression:
ChatDaemonRuntime only installs client.onFrame() when it attaches a session. SdkClient does not automatically open a replacement socket after an active socket closes; it merely retires the active incarnation and reconnects when a later request/connect call occurs. The new longer budget therefore covers only an initial dial (as the added test exercises) or a later command. It does not restore the passive event subscription after an established chat attachment disconnects, and any later command-triggered reconnect does not replay from the prior cursor. Chat notifications silently stop after a transient connection loss, contrary to this change’s session-survival premise.
Register a reconnect path for attached clients that replays events from the last acknowledged generation/sequence (or reattaches through the normal endpoint-generation fence), and add an integration test that drops an already-active socket, restores it, and verifies delivery/replay before merging.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
probepark
changed the title
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Still blocking: current head does not resolve the active-socket reconnect blocker.
Verification of head 764b01983ab4dd7e09cb3a28b8fb3c3965da51a1:
- The successor commit is GC/disk-retention only.
git diff ea49cd93..764b0198overpackages/coding-agent/src/sdk/,packages/coding-agent/src/modes/acp/,chat-daemon-session-reconnect.test.ts,acp-session-reconnect.test.ts, andsdk-acp-provider-reconnect.test.tsis empty — none of the reconnect/replay sources or tests changed after the previous review. chat-daemon-runtime.ts:461-491still installsclient.onFrame()only insideattach(), and the only client creation path isSdkClient.connect(...)with the longer budget (session-reconnect.ts). Nothing watches for an established socket closing, re-opens a replacement socket, re-installs the passive frame subscription, or replays from the last acknowledged generation/sequence.- The added
chat-daemon-session-reconnect.test.tshas a single test asserting the connect-dial budget (reconnect_exhausted,FakeWebSocket.instances === ACP_SESSION_RECONNECT.reconnectAttempts + 1). It never drops an already-attached, already-active socket and verifies delivery/replay resumes.
The longer budget therefore still covers only an initial dial or a later command-triggered connect; an established chat attachment that loses its socket still loses its passive event subscription silently, and a later reconnect does not replay from the prior cursor. Requested before merge: a reconnect path for attached clients that reattaches through the endpoint-generation fence and replays from the last acknowledged sequence, plus an integration test that drops an already-active socket, restores it, and verifies delivery/replay.
— re-review at current head, exact-sha verification; prior blocker unchanged.
Giving the attached-session client the long-lived reconnect budget covered the initial dial only. `SdkClient` never opens a replacement socket on its own — it retires the closed incarnation and re-dials on the next `connect`/`request` — and a chat attachment is purely passive, so a transient drop silently ended delivery for good and any later command-triggered reconnect resumed from nowhere. Chat notifications just stopped, which is the opposite of the session-survival premise the budget change was made for. The attachment now keeps its socket dialed through `onReconnect` and replays from its own cursor on the way back, fenced exactly as `attach()` already fences: this session, this endpoint generation. A superseded attachment is disposed rather than resurrected. Lore-id: 3ba7c209 Constraint: replay is fenced by session id and endpoint generation -- a stale incarnation must never replay onto a newer one Constraint: the cursor advances on every delivered frame, including filtered ones, or a reconnect re-delivers them interleaved Constraint: revival lives outside #pending -- the reconnect budget outlives the heartbeat TTL and stop() must not wait for it Rejected: auto-reopen inside SdkClient | changes reconnect semantics for every consumer, including one-shot request clients Rejected: reattach from scratch on every drop | loses the cursor and re-delivers the whole backlog Confidence: high Scope-risk: narrow Reversibility: easy Tested: an established attachment loses its open socket, regains it, and resumes from the last acknowledged event Tested: a superseded endpoint generation disposes the old attachment instead of resuming it Tested: the connect-dial budget assertion is intact and unweakened Not-tested: a live chat daemon across a real network partition
Resuming an established attachment through onReconnect changed `chat-daemon-runtime.ts:attach` again, moving its declaration digest for the discord and slack surfaces. Regenerated with --write-manifest, not hand-edited; the generation bump from the earlier attach change still covers this. Lore-id: 8c4f2d71 Confidence: high Scope-risk: narrow Reversibility: easy Tested: telegram-daemon-generation-guard against origin/dev -- v43 required generation bump verified
|
Blocker addressed in You were right, and the finding reproduced exactly as described: What landed:
Requested integration test is Verified by me, not taken from the agent's report:
|
The rejection listed the allowed keys but never said which key was wrong, so a
caller could not tell what to remove and a retry reproduced the failure
verbatim. Two agent sessions burned turns on it today, both by passing `note` —
`op: "note"` is valid and its body field is `text`, an easy and repeatable
confusion.
`hasUnknownKeys` returned a boolean and threw the offending keys away, so
nothing downstream could name them. It now returns the keys, the rejection
carries them as structured detail, and the thrown message appends them to the
existing guidance. A correction is suggested only where the mapping is exact.
Lore-id: 5e8b3c47
Constraint: the allowed key set is unchanged -- the schema was right, only the diagnostic was wrong
Constraint: codes that carry no detail (the ask-* family) keep their exact current message
Rejected: accept `note` as an entry key | would silently drop the note body, since the field is `text`
Rejected: fuzzy/edit-distance suggestions | a confident wrong suggestion is worse than naming the key and stopping
Confidence: high
Scope-risk: narrow
Reversibility: easy
Tested: a `note` key is named and pointed at `{ op: "note", text }`
Tested: unknown keys with no exact mapping are named without a suggestion
Tested: multiple unknown keys on one entry are all named
Tested: root and init-list-entry rejections name their offending key
Tested: `content` still works as the `task` alias, and ask-* messages are unchanged
The watchdog was sized at MAX_TOOL_RUNTIME_MS + 10 x HEARTBEAT_TTL_MS = 63.3 minutes, derived from the bash/ssh ceiling a caller may *request*. That is the rarest possible case, so the safety net could not react to an ordinary hang: every stuck prompt this session had to be killed by hand long before it fired. The bound now follows evidence. A frame-free gap with nothing executing is held to the default tool runtime plus one reconnect budget (5.67 min); the wide worst-case bound applies only while the host has an unfinished tool call, and lapses as soon as that evidence does. A long bash is protected by the fact that it is running, not by a constant sized for a tool that might run. Lore-id: 4f7d1e83 Constraint: both bounds stay derived from named constants -- no magic literals Constraint: the wide bound must terminate -- no tool may outlive MAX_TOOL_RUNTIME_MS without publishing tool_execution_end Rejected: divide the old constant by a factor | picks a number with no defensible relationship to any healthy interval Rejected: keep one bound sized for the slowest possible tool | blind to every ordinary hang, which is the case that actually happens Confidence: high Scope-risk: narrow Reversibility: easy Tested: a tool running past the narrow bound does not trip the watchdog while it runs, and becomes eligible once it ends Tested: silence past the applicable bound still rejects with the diagnosable terminal error Tested: the concrete bound values are asserted so a future input change cannot silently inflate them Not-tested: a live host dying mid-tool against a real gateway
A turn that completed successfully stayed reported as running: the agent delivered its full final answer, the host went quiet, and the client kept the turn live until the inactivity watchdog rescued it an hour later. Killing the agent by hand was the only exit, which masked the defect all session. None of the terminal guards were at fault — every one was satisfied. The bug is ordering. `#handleSdkFrame` awaited `#emitEndOfTurnUpdates` before settling, and those advisory `context.get` / `session.metadata` queries are exactly what a host that stops the moment it publishes its terminal never answers. Settlement now happens on the terminal frame and the metadata follows it. Lore-id: 9e4c7a15 Constraint: settlement must not depend on any host response -- the terminal frame is the authority Constraint: a late advisory answer must not report the session idle once the next turn has begun Rejected: shorten the watchdog to cover this | the watchdog is a dead-producer safety net, not a substitute for terminating a healthy turn Rejected: drop the end-of-turn metadata | it is useful, it just must not gate settlement Confidence: high Scope-risk: narrow Reversibility: easy Tested: a completed turn settles on its terminal frame even when end-of-turn metadata never answers Tested: settlement happens exactly once under duplicate and late terminal frames Tested: the dead-producer watchdog path still fires Not-tested: a live host against a real gateway
|
Three more defects found and fixed while driving this PR, all reproduced live on this machine during the batch itself.
Verified by me, not taken from the agents' reports, each with a with-fix / without-fix comparison:
The |
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES
Exact-head red-team review of f770bb97590eb2151dff36b5c595e68253bde964 against the PR base object cf8a50ae04079fd3afed811aff081eae6d54985c found a remaining reconnect correctness bug.
Blocker: replacement-socket live frames race replay, causing reordering and duplicates
The new reconnect callback starts #resumeAttachment() asynchronously after the replacement hello, while the normal onFrame() subscription remains live. There is no replay barrier or (generation, seq) deduplication:
onReconnectschedules replay atchat-daemon-runtime.ts#L517-L519.- Replay events are enqueued only after
event_replayreturns at#L581-L599. - Live events arriving meanwhile are independently enqueued by the persistent frame observer at
#L511-L513. handleFrameadvances the cursor but does not reject an event whoseseq <= cursor, so replay can deliver an already-delivered live frame again at#L696-L710.
I extended the PR's real FakeSessionHost integration harness locally (temporary test removed afterward) with this sequence:
- deliver seq 1;
- drop the established socket;
- record seq 2 during the outage;
- accept the replacement socket;
- deliver live seq 3 immediately after replacement hello, before replay answers.
The required exactly-once order was [seq1, seq2, seq3]. Exact-head behavior was:
[seq1, seq3, seq2, seq3]
Bun result: the PR's 3 reconnect tests passed, while this red-team case failed (3 pass, 1 fail, 47 assertions). This proves passive delivery resumes, but not correctly: a normal frame arriving in the hello→replay-result window is published out of order and duplicated. The existing test at chat-daemon-session-reconnect.test.ts#L237-L271 emits no live event during that window, so it cannot detect this race.
Before merge, fence replacement-socket ingress behind replay completion (or merge live/replayed events through a generation/sequence-aware ordered dedupe barrier), and add the above race as an integration test. Also cover stop/disposal and endpoint supersession while that barrier is pending.
Other exact-head review/validation
I inspected the full 53-file PR diff (6,490 additions / 360 deletions), including Anthropic repair budgeting, deferred shell transcript retention, SDK lock cleanup/diagnostics, memory readiness deferral, transcript replay degradation, host idle reap, ACP watchdog and terminal settlement, tool alias dispatch, todo_write diagnostics, disk retention/GC, schema changes, and the generated daemon manifest. I found no second independently reproduced blocker in those surfaces.
Validation performed on the exact head:
- reconnect + ACP lifecycle focused suite:
42 pass, 0 fail; - GC / broker artifacts / SDK lifecycle / host reap / postmortem / todo suite after building the matching native addon:
95 pass, 0 fail; - agent-loop / Anthropic repair / coercion suite:
83 pass, 0 fail; - Telegram daemon generation guard against the immutable PR base:
v43 required generation bump verified; git diff --check: passed;- PR CI is green, but does not cover the reproduced hello→replay live-frame race.
The PR must remain open until ordered exactly-once reconnect delivery is proven.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
Blocker fixed in Your reproduction was exact, and the cursor guard was the hole: it only advanced on Ingress is now fenced for the duration of the resume — live frames arriving in the hello→replay window are held and drained in sequence order behind the replayed events — and any frame at or below the cursor is dropped instead of republished. The fence cannot deadlock: an unanswered replay leaves the cursor intact so the next reconnect re-issues it, which is the pre-existing failure path. Your four required cases are covered, and each one fails without the src change (verified by stashing only
The full published array is asserted, not a set or a length. The three pre-existing reconnect tests pass unchanged in both states — confirming, as you said, that they could not detect this race. Local validation on the rebased head:
|
2 participants
Twelve fixes for SDK lifecycle, ACP session survival, provider loops, and disk retention. Every one was reproduced on a real machine during a single overnight batch of parallel agents, and the batch itself is what surfaced most of them — agents kept dying in ways the reported issues predicted.
What this fixes
18e30308320f21fa00!cmdoutput typed while the agent is Working vanished from the transcript52555a2eecode=0broker exit5d4b70dd00a3c4b739No ready SDK endpoint remains available.060e90ff2session/loadfail permanently for the whole session8fd783604cc4c09c43c9d5c4535df2f8cf24session/newintermittently reportedterminal_uncertain, permanently poisoning an ACP host's provider764b01983~/.gjcin three weeks, unreachable from any commandd94aa8673sdk-acp-provider-reconnectexhaustion test burned the real 41.75 s budget and timed out at 5 s onorigin/devPlus two generated/contract updates required by the above:
989ced538regenerates the telegram generation manifest for theattachdigest (via--write-manifest, not hand-edited), andea49cd930bumpsCHAT_DAEMON_GENERATIONSbecauseattach()is a protected lifecycle declaration — an owner on the old generation gives up reconnecting after 175 ms and permanently loses its attachment.Two defects that were live during this batch
#4013 is why parallel agents intermittently refused to start. Memory startup issues one LLM request per queued rollout, so its duration scales with the backlog; running it inside the 10 s readiness window killed the child at the cutoff. Deferring it past readiness fixed agent creation immediately. The secondary half —
handleFatalErrorcollapsing a thrown record to[object Object]— is why six earlier occurrences left no diagnostic at all.#4018 / #4019 together are why a dropped transport was unrecoverable: the prompt never settled (client hangs), and the documented recovery through
session/loaddied on the first entry without a production body (session unloadable). Either alone is bad; together they madepaseo stopthe only exit.Verification
Each slice was verified independently rather than trusted from its agent's self-report, including a with-fix / without-fix comparison per regression test:
Suite level on this branch:
packages/agent— 702 pass / 0 failpackages/coding-agent/test/acp+ watchdog — 164 pass / 0 failcheck:schemasclean,lint:tsclean,checkclean forai/agent/utils/tui/coding-agenttelegram-daemon-generation-guard—v43 required generation bump verifiedPre-existing failures, confirmed identical on clean
origin/devVerified by running the same command on a detached
origin/devcheckout, not assumed:check:sdk-closurecanonicalization: 25 violations, introduced by feat(sdk): expose model profiles as synthetic gajae-code/<profile> models #3988 (6e45efead) viaacp-agent.ts → model-profile-model.ts → model-registry.ts → settings.ts. Violation target set is byte-identical betweenorigin/devand this branch.credential-rotation-session.e2e— 1 pass / 1 fail on both.session-storage— 76 pass / 4 fail on both.anthropic-*— 7 failures on both (this branch adds 4 passing tests: 223→227 pass, 7 fail unchanged)./copy command— 2 failures on both.lint:rsclippy errors incrates/pi-natives; this PR touches no Rust.The reconnect-exhaustion timeout was also pre-existing, but it is fixed here (
d94aa8673) because #4012 caused it and #4014 extends the same budget: 41.9 s → 96 ms.Not included
#3798 stays open pending device evidence — three of its four contributing paths are already fixed by #3806, and no further source change is justified until someone reproduces it on the iOS client.
Closes #3639, #3853, #3903, #3963, #4010, #4011, #4013, #4014, #4018, #4019, #4020