Skip to content

fix: stop losing turn-lifecycle events (dangling sub-agents, stuck 'thinking') - #273

Merged
saucam merged 2 commits into
mainfrom
fix/subagent-lifecycle-leak
Aug 2, 2026
Merged

fix: stop losing turn-lifecycle events (dangling sub-agents, stuck 'thinking')#273
saucam merged 2 commits into
mainfrom
fix/subagent-lifecycle-leak

Conversation

@saucam

@saucam saucam commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Sub-agent counts climbed turn after turn and never came down, and sessions sat spinning at "thinking" after the model had visibly answered. Both trace to the same place: the provider's event channel loses turn-lifecycle events, silently.

This PR fixes the causes and keeps reconciliation as a backstop.


Cause 1 — mid-turn accounting counted pushes that produce no turn_done

#pendingMidTurnCount exists so #consumeEvents can absorb the intermediate turn_done the SDK emits when a mid-turn push starts a new query. It was incremented for every mid-turn push — including "later", which merges into the running turn and starts no query at all (ClaudeProvider: shouldQuery = priority !== "later").

session.send exposes priority on the wire, so any client sending "later" while the session was working left the counter one too high. The consumer then hit its terminal turn_done, treated it as an intermediate boundary, decremented, re-asserted "thinking" (session.ts:3379) and continued — waiting for a turn_done that was never going to be emitted.

That is the stuck spinner. It also stranded that turn's sub-agents and tools, because the loop never exited, so neither the boundary flush nor the consumer's finally ever ran. Recovery came only from the 5-minute stall watchdog or the next send — which is why it looked intermittent.

Now only querying pushes are counted.

Cause 2 — #emit dropped events silently

// before
#emit(event) { try { this.#currentTurnQueue?.push(event); } catch {} }

Three distinct losses in one expression — null queue, closed queue, full queue — indistinguishable and none logged. A lost subagent_stop left its sub-agent dangling with a live delegated ZeroID token; a lost tool_complete stranded status at tool_running. Nothing recorded either, which is why this was invisible for so long.

Now #emit reports the disposition, and id-keyed lifecycle events (subagent_stop, tool_complete) are buffered and replayed into the next turn rather than vanishing — their handlers are idempotent on unknown ids, so a late duplicate costs nothing. turn_done and streamed text are deliberately not carried: a stale turn_done would end the next turn the instant it began. Buffer is bounded (100); the log is deduped per loop generation so a persistent fault reports once, not once per event.

The fourth, invisible loss

After the consumer broke on turn_done, the turn queue stayed open and unread until the next turn replaced it. Late events were accepted into a queue nobody would ever drain — push succeeded, so there was no error and no log to catch. TurnRun.endTurn() (optional, called once from the consumer's finally) closes it, converting that silent case into the observable carryover path.

Cause 3 — the sweep was in the wrong place

#completeActiveTools() has always run in the consumer's finally precisely because provider events can be lost. Sub-agents were simply never added to that same backstop. The sweep now sits beside it, covering every exit path — clean turn_done, error, stall recovery, ownership loss — plus the mid-turn continuation branch, which continues without dispatching to #handleProviderEvent and so was missed entirely by the earlier turn_done-case sweep.

Abort-path sweeps (interrupt(), #teardownProvider()) stay as defence in depth, since those abort the SDK query and the hooks provably never fire.

What was leaking

All bounded by session destroy (deactivateSessionAgent cascades), but real while a session lives:

Session.#subagents one entry per orphan — this drove the wrong count
Session.#subagentRegistrations a retained Promise<void> per orphan
AgentIdentityManager.#agents identityId, wimseUri, token, apiKey — a live credential for a dead sub-agent

The third is the one that matters: revocation is meant to ride the sub-agent's own stop, and instead waited for session teardown.

Verification

src/tests/session-subagent-lifecycle.test.ts — 13 cases. Verified by reverting the source and re-running:

Fail against original main (the sweep work): turn-end sweep · non-accumulation across turns · ZeroID revocation of an orphan · sweep on interrupt · sweep on provider teardown · info_update broadcast.

Fail against the first commit (the root-cause work): the "later" mid-turn push no longer swallowing its terminal turn_donewhich fails by timing out, the bug exactly · sub-agent reconciliation at an absorbed mid-turn boundary · endTurn() closing the turn stream.

Also pinned: "now" pushes still correctly absorb their intermediate boundary (so the fix didn't just disable the mechanism), the normal subagent_stop path still works, orphans are revoked exactly once, and the carryover policy excludes turn_done/text.

  • bun run typecheck clean (root + protocol + core)
  • bun run lint clean, 347 files
  • bun test2203 pass, 19 skip, 0 fail across 152 files

Known gaps

  • The provider's carryover/replay path has no automated integration test — exercising it needs a live SDK query loop. Only the policy (which event types carry) is unit-tested. The Session-side half (endTurn) is covered.
  • A dropped turn_done still hangs the consumer until the stall watchdog. Making the queue close a non-lossy terminal signal would need the provider to distinguish terminal from intermediate turn_done, duplicating a fragile state machine that currently lives only in Session — deliberately not attempted here. With Cause 1 fixed, the reachable path to that hang is closed; what remains is now logged rather than silent.
  • active: boolean on each sub-agent entry is still vestigial (set true, never false). Untouched because subagents is client-visible in the protocol (types.ts:220).

🤖 Generated with Claude Code

The provider's `subagent_stop` event was the ONLY path that removed an
entry from Session's #subagents map. That event originates in the Claude
SDK's SubagentStop hook, which cannot fire once the query is aborted --
and interrupt, setModel, rotate and switchProvider all abort it via
#abortController.abort(). Every such abort therefore orphaned each
in-flight sub-agent permanently, with nothing anywhere reconciling the
map afterwards.

Three consequences:

- subagentSnapshot (feeding /who and toInfo().subagents) only ever grew.
  The displayed sub-agent count climbed turn after turn and never came
  back down, which is how this was noticed.
- #subagents and #subagentRegistrations grew unbounded for the lifetime
  of a long-lived session.
- Worst: each orphan kept a LIVE delegated ZeroID token. Revocation is
  meant to ride the sub-agent's own stop; instead it fell through to
  deactivateSessionAgent's cascade at session destroy, so a dead
  sub-agent's credential stayed valid as long as the session lived. That
  quietly weakens the per-agent revocation guarantee.

Adds #sweepStaleSubagents, which revokes and drops whatever remains. A
Task sub-agent cannot outlive the turn that spawned it, so it runs at
turn_done as the principled backstop, plus on the two abort paths that
leave the session alive: interrupt() and #teardownProvider(). It is
idempotent and returns immediately on an empty map (the common case), and
double-revocation is free because deactivateSubagent no-ops on an id it
already dropped -- so a trailing subagent_stop racing a sweep costs
nothing.

Not swept at destroy(): deactivateSessionAgent already cascades over the
session's sub-agent keys there, and the Session object is discarded.

Tests: 6 of the 8 new cases fail against the pre-fix code, covering the
turn-end sweep, non-accumulation across turns, both abort paths, the
identity revocation, and the info_update broadcast so clients see the
count drop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

The sweep added earlier reconciled dangling sub-agents but never asked why
they dangled. Root-causing that turned up two defects, one of which is the
"model answered but the spinner keeps spinning" report.

1. Mid-turn accounting counted pushes that produce no turn_done.

#pendingMidTurnCount exists so #consumeEvents can absorb the intermediate
turn_done the SDK emits when a mid-turn push starts a NEW query. It was
incremented for every mid-turn push, including "later" -- which merges
into the running turn and starts no query at all (ClaudeProvider:
shouldQuery = priority !== "later"). Any client sending with an explicit
"later" priority while the session was working therefore left the counter
one too high, and the consumer swallowed the turn's REAL terminal
turn_done as if it were an intermediate boundary: it re-asserted
"thinking" and continue'd, waiting for a turn_done that would never be
emitted.

That is the stuck spinner. It also stranded the turn's sub-agents and
tools, because the loop never exited and so neither the boundary flush nor
the consumer's finally ever ran. Recovery came only from the 5-minute
stall watchdog or the next send. Now only querying pushes are counted.

2. The provider's event channel dropped events silently.

ClaudeProvider.#emit was `try { queue?.push(e) } catch {}` -- three silent
losses in one expression: a null queue, a closed queue, and a full queue,
all indistinguishable and none logged. A lost subagent_stop left its
sub-agent dangling with a live delegated ZeroID token; a lost
tool_complete stranded the status at tool_running. Nothing said so.

#emit now reports the disposition, and id-keyed lifecycle events
(subagent_stop, tool_complete) are buffered and replayed into the next
turn instead of vanishing -- their handlers are idempotent on unknown ids,
so a late duplicate costs nothing. turn_done and streamed text are
deliberately not carried: a stale turn_done would end the next turn as it
began. The buffer is bounded and the log is deduped per loop generation.

There was also a fourth, invisible loss: after the consumer broke on
turn_done, the turn queue stayed OPEN and unread until the next turn
replaced it, so late events were accepted into a queue nobody would drain
-- push succeeded, so no error, no log. TurnRun.endTurn() (optional, called
once from the consumer's finally) closes it, converting that into the
observable carryover path.

3. Sweep moved to where reconciliation already lives.

#completeActiveTools has always run in the consumer's finally precisely
because provider events can be lost; sub-agents were simply never added to
the same backstop. The sweep now sits beside it, covering every exit path,
plus the mid-turn continuation branch -- which continue's without
dispatching to #handleProviderEvent and so was missed entirely by the
previous turn_done-case sweep. The abort-path sweeps stay as defence in
depth for interrupt/teardown, where hooks provably never fire.

Tests: 5 new cases on top of the existing 8. The three covering these
changes fail against the previous commit -- the stuck-spinner case by
timing out, which is the bug exactly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@saucam saucam changed the title fix: sweep orphaned sub-agents at turn boundaries and on abort paths fix: stop losing turn-lifecycle events (dangling sub-agents, stuck 'thinking') Aug 1, 2026
@saucam
saucam merged commit 925d427 into main Aug 2, 2026
4 checks passed
saucam added a commit that referenced this pull request Aug 5, 2026
…T NULL FK

The follow-up findings from the background-task investigation, folded in.

## Finding 1 was not an interrupt bug

The "interrupt audited with session_id: null" (row 13824) was never written
null. audit_log carried

    FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE SET NULL

so destroying a session retroactively anonymized its ENTIRE audit trail —
observed in the reporting database as 40 rows across 13 actions, each
session.destroy row nulled by its own delete. An audit log exists precisely
to survive its subjects; this FK made it forget them on schedule. It also
rejected legitimately history-first inserts, which is what the fold-into-
detail fallback in audit() was compensating for.

Fresh databases get the FK-free DDL. Existing ones are rebuilt once (SQLite
cannot drop a constraint via ALTER): rows and their ids are preserved
verbatim, because an audit citation like "row 13824" must keep meaning row
13824 across the rebuild. Rows already nulled are unrecoverable — the id was
destroyed at delete time, not hidden.

Mutation notes: gutting the rebuild fails the legacy-DB test; reinstating the
FK in the fresh DDL is an EQUIVALENT mutant — the rebuild migration strips it
back out on open, which is the defense-in-depth working, not a gap.

## Finding 2 resolved: one leaked identity, and why forensics was ambiguous

Pairing registered/deactivated audit rows by SUBJECT (the wimse URI — they
were attributable all along) identifies exactly one leak:
codeoid-subagent-a42e6a2a72e9, registered 2026-05-19 04:44:27, the last
registration before a daemon stop. That is the restart-orphan class —
in-memory registrations die with the process — and it predates #273's sweep,
which only covers in-process orphans. The restart-orphan cascade itself
(persisting registrations, or a server-side deactivate-by-parent) is a
design piece and stays a follow-up, now with precise evidence.

What WAS fixable here is why the investigation stayed ambiguous:

- a FAILED deactivation was console-logged but never audited, so "daemon
  died before deactivating" and "deactivation was attempted and failed" were
  indistinguishable from the audit trail. Failures now write
  subagent.identity.deactivation_failed with the error.
- the destroy-time cascade never audited its SUCCESSES either, so identities
  revoked at session destroy looked identical to leaks when pairing rows.
  It now writes the same subagent.identity.deactivated row the direct path
  does.

Daemon suite 2233 to 2237.
saucam added a commit that referenced this pull request Aug 5, 2026
… every backend (#284)

* feat: see background tasks and wake the session when they settle — on every backend

The incident (real audit data, session 882d0a15): a model spawned background
agents through its harness, promised "I'll report as soon as the three agents
land", and ended its turn. The landings arrived when no turn was in flight.
codeoid tracked nothing, showed nothing, and delivered nothing — the session
sat idle until the owner interrupted it, 5 seconds after the last spawn.

Two structural gaps, not one bug:

1. The daemon had NO model of harness-side background work. Zero sub-agent
   identities across 5 Agent-tool calls in the incident session (31 calls, 0
   identities across four sessions) — the SDK's background tasks bypass the
   SubagentStart/Stop hooks entirely.
2. Provider events are TURN-scoped by construction. TurnRun's own contract
   documents that an event arriving between turns is "accepted and then
   discarded unread" — and settling between turns is precisely what a
   deferred task does.

## The generic seam

`SessionScopedEvent` + `SessionProvider.onSessionEvent`: a session-lifetime
channel, deliberately separate from the turn queue because these events fire
when the queue has no reader. Two events, mirroring the level+edge design the
Claude SDK itself settled on:

- `background_tasks` — a LEVEL: the full live set, REPLACE semantics, so a
  missed event can never wedge a stale indicator.
- `background_task_settled` — the EDGE carrying the outcome digest.

Nothing provider-specific leaks into core: `kind` is the harness's own
vocabulary and display-only, the daemon never branches on it, and the claude
provider's SDK subtype names appear in exactly one translation function
(threaded as a pure `emitSession` parameter, so the mapping is unit-testable
without a process). pi/gemini/codex emit nothing until their harnesses grow
background work; a future harness only implements this shape.

## What the Session does with it

- **Visibility**: live tasks on `SessionInfo.backgroundTasks` (additive
  field), broadcast on change, cleared on provider teardown — the level is
  per-harness-process, so a rebuilt provider starts empty. The web sidebar
  shows a pulsing "N bg" chip; an idle-looking session with live background
  work is exactly the state that used to read as a hang.
- **The wake**: settles queue and deliver as ONE batched injection at idle
  (burst-collapse, the <fleet_events> rule), from two triggers so both
  arrival orders work — a settle landing while idle, and an idle transition
  with settles queued mid-turn. Exactly once per task; a failed wake requeues
  its digests. The injection is a normal send under `system:background`: it
  rides the ordinary turn machinery, so tools still ask for approval in
  guarded mode and the autonomous budget still applies. Nothing here grants
  authority; it only supplies the information the model was waiting on.

## Verification

Daemon suite 2200 → 2233, web 387 → 392. Five session-level mutations each
caught by exactly the right tests (no idle-transition wake, no immediate
wake, dedup removed, level appending instead of replacing, teardown keeping
dead tasks), and the provider mapping asserts ROUTING — session events reach
emitSession and never the turn queue — plus forward-compat (an unknown settle
status is still a settle) and malformed-payload tolerance.

Verified live with a real model: a claude session told to background
`sleep 10 && echo BG_RESULT_42` and end its turn. The task appeared on the
wire while the session idled, the daemon woke it (`session.background_wake`,
subject `system:background`), and the model reported the exact output line.
The sequence that hung the incident session now closes.

One honest gap: one full-suite run showed 4 unreproducible failures before
this change was complete; five hammer rounds of the timing-sensitive session
suites plus two clean full runs followed. Noted rather than hidden — if it
recurs in CI, treat it as pre-existing flake, not this change's regression,
but do not auto-retry past it without reading the failures.

* fix: an audit log that forgets its subjects — remove the ON DELETE SET NULL FK

The follow-up findings from the background-task investigation, folded in.

## Finding 1 was not an interrupt bug

The "interrupt audited with session_id: null" (row 13824) was never written
null. audit_log carried

    FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE SET NULL

so destroying a session retroactively anonymized its ENTIRE audit trail —
observed in the reporting database as 40 rows across 13 actions, each
session.destroy row nulled by its own delete. An audit log exists precisely
to survive its subjects; this FK made it forget them on schedule. It also
rejected legitimately history-first inserts, which is what the fold-into-
detail fallback in audit() was compensating for.

Fresh databases get the FK-free DDL. Existing ones are rebuilt once (SQLite
cannot drop a constraint via ALTER): rows and their ids are preserved
verbatim, because an audit citation like "row 13824" must keep meaning row
13824 across the rebuild. Rows already nulled are unrecoverable — the id was
destroyed at delete time, not hidden.

Mutation notes: gutting the rebuild fails the legacy-DB test; reinstating the
FK in the fresh DDL is an EQUIVALENT mutant — the rebuild migration strips it
back out on open, which is the defense-in-depth working, not a gap.

## Finding 2 resolved: one leaked identity, and why forensics was ambiguous

Pairing registered/deactivated audit rows by SUBJECT (the wimse URI — they
were attributable all along) identifies exactly one leak:
codeoid-subagent-a42e6a2a72e9, registered 2026-05-19 04:44:27, the last
registration before a daemon stop. That is the restart-orphan class —
in-memory registrations die with the process — and it predates #273's sweep,
which only covers in-process orphans. The restart-orphan cascade itself
(persisting registrations, or a server-side deactivate-by-parent) is a
design piece and stays a follow-up, now with precise evidence.

What WAS fixable here is why the investigation stayed ambiguous:

- a FAILED deactivation was console-logged but never audited, so "daemon
  died before deactivating" and "deactivation was attempted and failed" were
  indistinguishable from the audit trail. Failures now write
  subagent.identity.deactivation_failed with the error.
- the destroy-time cascade never audited its SUCCESSES either, so identities
  revoked at session destroy looked identical to leaks when pairing rows.
  It now writes the same subagent.identity.deactivated row the direct path
  does.

Daemon suite 2233 to 2237.
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.

2 participants