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

src/daemon/providers/claude/index.ts

@@ -119,6 +119,25 @@ export interface ClaudeProviderInit {
   onRecoveryNeeded?: (content: string) => void;
 }
 
+/**
+ * Events safe to replay into a LATER turn when they can't be delivered now.
+ *
+ * Both are addressed by an id the Session tracks (`agentId`, `sdkToolUseId`)
+ * and both handlers no-op on an unknown id, so arriving late is harmless —
+ * whereas losing them leaves a dangling sub-agent (with a live delegated token)
+ * or a tool stuck "running". Deliberately excludes `turn_done` (a stale one
+ * would end the next turn the moment it starts) and streamed text (it would
+ * corrupt the next turn's transcript).
+ */
+export const CARRYOVER_EVENT_TYPES: ReadonlySet<ProviderEvent["type"]> = new Set([
+  "subagent_stop",
+  "tool_complete",
+]);
+
+/** Ceiling on buffered undeliverable events, so a pathological loop can't turn
+ *  a delivery failure into unbounded memory growth. */
+export const MAX_CARRYOVER_EVENTS = 100;
+
 // ── ClaudeProvider ────────────────────────────────────────────────────────────
 
 export class ClaudeProvider implements SessionProvider {
@@ -164,6 +183,12 @@ export class ClaudeProvider implements SessionProvider {
   // Long-running event queue — closed only when the SDK loop ends.
   // turn_done events are emitted as regular items; Session decides when to stop.
   #currentTurnQueue: AsyncQueue<ProviderEvent> | null = null;
+  /** Id-keyed lifecycle events that arrived with no live queue, replayed into
+   *  the next turn. See #handleUndeliverable. */
+  #carryover: ProviderEvent[] = [];
+  /** Dedupe keys for the undeliverable-event log, so a persistent loss reports
+   *  once per loop generation instead of once per event. */
+  #undeliverableLogged = new Set<string>();
 
   /**
    * Mutable callback — Session updates this before each runTurn() call to
@@ -226,6 +251,28 @@ export class ClaudeProvider implements SessionProvider {
     const turnQueue = new AsyncQueue<ProviderEvent>();
     this.#currentTurnQueue = turnQueue;
 
+    // Replay lifecycle events that arrived while no queue was live — a
+    // SubagentStop hook resolving after the previous turn ended, a trailing
+    // tool_result. Their handlers are id-keyed and idempotent, so a duplicate
+    // costs nothing; losing them is what left sub-agents dangling and tools
+    // stuck "running" for the rest of the session.
+    if (this.#carryover.length > 0) {
+      const replay = this.#carryover;
+      this.#carryover = [];
+      console.error(
+        `[claude-provider ${this.#claudeCodeSessionId.slice(0, 8)}] replaying ${replay.length} carried-over event(s) into the new turn`,
+      );
+      for (const ev of replay) {
+        try {
+          turnQueue.push(ev);
+        } catch {
+          // A brand-new queue should never reject; if it somehow does, the
+          // boundary sweep is still the backstop.
+          break;
+        }
+      }
+    }
+
     let userMessage = opts.userMessage;
     if (this.#pendingHistorySeed && userMessage) {
       // Post-switch seeding: this fresh Claude Code session has never seen
@@ -256,8 +303,24 @@ export class ClaudeProvider implements SessionProvider {
         // A "later" mid-turn injection is meant to MERGE into the running turn
         // (no new query); "now"/"next" should query. Only the primary prompt
         // (above, via the default) always queries.
+        //
+        // NOTE for callers: only the querying variants produce an additional
+        // turn_done. Session mirrors this when deciding whether to expect an
+        // intermediate turn boundary — counting a "later" push there makes it
+        // swallow the turn's terminal turn_done and hang at "thinking".
         this.#pushSDKMessage(content, priority, priority !== "later");
       },
+      endTurn: () => {
+        // The consumer for THIS turn has stopped reading. Close its queue so a
+        // late event surfaces through #handleUndeliverable (logged, and buffered
+        // if it's id-keyed) instead of being pushed into a queue nobody drains.
+        // Generation/identity guarded: a rebuilt loop or a newer turn may
+        // already own #currentTurnQueue, and closing that would kill a live turn.
+        if (this.#currentTurnQueue === turnQueue) {
+          this.#currentTurnQueue.close();
+          this.#currentTurnQueue = null;
+        }
+      },
     };
   }
 
@@ -652,11 +715,54 @@ export class ClaudeProvider implements SessionProvider {
 
   /** Push a ProviderEvent to the active per-turn queue. */
   #emit(event: ProviderEvent): void {
-    try {
-      this.#currentTurnQueue?.push(event);
-    } catch {
-      // Queue may be closed if the turn ended early — ignore.
+    const queue = this.#currentTurnQueue;
+    if (queue) {
+      try {
+        queue.push(event);
+        return;
+      } catch (err) {
+        this.#handleUndeliverable(event, err instanceof Error ? err.name : "unknown");
+        return;
+      }
+    }
+    this.#handleUndeliverable(event, "no-queue");
+  }
+
+  /**
+   * A provider event had nowhere to go — the turn queue was closed (the
+   * consumer finished and called endTurn), full, or already nulled after the
+   * SDK loop ended.
+   *
+   * This used to be a bare `catch {}`, which is how the whole class of bugs
+   * stayed invisible: a lost `subagent_stop` left its sub-agent dangling for the
+   * life of the session (and its delegated ZeroID token live), and a lost
+   * `tool_complete` stranded the status at `tool_running`, with nothing logged
+   * to explain either.
+   *
+   * Id-keyed lifecycle events are buffered and replayed into the next turn's
+   * queue, where their handlers are idempotent no-ops if the boundary sweep
+   * already reconciled them. Everything else is logged only — replaying a stale
+   * `turn_done` would end the next turn the instant it began, and stale text
+   * would corrupt its transcript.
+   */
+  #handleUndeliverable(event: ProviderEvent, reason: string): void {
+    const carryable = CARRYOVER_EVENT_TYPES.has(event.type);
+    if (carryable && this.#carryover.length < MAX_CARRYOVER_EVENTS) {
+      this.#carryover.push(event);
     }
+    // One line per (loop generation, event type, reason): enough to diagnose a
+    // recurring loss, not enough to flood a long session.
+    const key = `${this.#loopGeneration}:${event.type}:${reason}`;
+    if (this.#undeliverableLogged.has(key)) return;
+    this.#undeliverableLogged.add(key);
+    const disposition = carryable
+      ? this.#carryover.length < MAX_CARRYOVER_EVENTS
+        ? "buffered for the next turn"
+        : "DROPPED (carryover full)"
+      : "dropped";
+    console.error(
+      `[claude-provider ${this.#claudeCodeSessionId.slice(0, 8)}] "${event.type}" undeliverable (${reason}) — ${disposition}`,
+    );
   }
 
   /**

src/daemon/providers/interface.ts

@@ -248,6 +248,22 @@ export interface TurnRun {
   interrupt(): Promise<void>;
   /** Push a message mid-turn (ClaudeProvider only). */
   pushMidTurn?(content: string, priority: "now" | "next" | "later"): void;
+  /**
+   * Signal that the consumer has stopped reading `events` — called from
+   * Session's turn-exit path, exactly once per run.
+   *
+   * Keep-warm providers hold one queue per turn but only close it when the NEXT
+   * turn replaces it. Between those points the queue is open with nobody
+   * draining it, so a late event (a `SubagentStop` hook resolving after the
+   * result message, a trailing tool_result) is accepted and then discarded
+   * unread — the silent loss that leaves sub-agents dangling and tools stuck
+   * "running". Closing here turns that into an observable, recoverable case:
+   * the provider sees the closed queue and can buffer or log instead.
+   *
+   * Optional and best-effort — providers with no per-turn queue omit it, and it
+   * must never throw into the consumer's finally.
+   */
+  endTurn?(): void;
 }
 
 // ── ModelInfo ─────────────────────────────────────────────────────────────────

src/daemon/providers/mock/session-provider.ts

@@ -107,6 +107,27 @@ export class MockSessionProvider implements SessionProvider {
   /** Every pushMidTurn injection observed — inspect in tests. */
   readonly midTurnPushes: Array<{ content: string; priority: string }> = [];
 
+  /** Times Session called `TurnRun.endTurn()` — the turn-exit signal. */
+  endTurnCount = 0;
+
+  /**
+   * Push an event into the LIVE turn queue from a test: the deterministic
+   * stand-in for "the SDK emitted this later in the turn". Needed to model a
+   * terminal turn_done arriving after a mid-turn push (the pendingMidTurnCount
+   * hang), which a static script can't express. Returns false when the turn's
+   * queue is already gone or closed.
+   */
+  emitLive(event: ProviderEvent): boolean {
+    const q = this.#currentQueue;
+    if (!q) return false;
+    try {
+      q.push(event);
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
   constructor(
     id = "mock-session",
     script: ProviderEvent[][] = [],
@@ -199,6 +220,13 @@ export class MockSessionProvider implements SessionProvider {
       interrupt: async () => {
         queue.close(); // idempotent — safe to call even if already closed
       },
+      endTurn: () => {
+        // Mirror ClaudeProvider: the consumer has stopped reading, so close the
+        // queue rather than leaving it open and undrained.
+        this.endTurnCount++;
+        queue.close();
+        if (this.#currentQueue === queue) this.#currentQueue = null;
+      },
     };
     if (this.#midTurn) {
       run.pushMidTurn = (content: string, priority: string) => {

src/daemon/session.ts

@@ -1747,8 +1747,21 @@ export class Session {
       this.#persistAndBuffer(midTurnMsg);
       this.#broadcastRaw(midTurnMsg);
       this.#accumulator.pushUserTurn(effectivePrompt);
-      this.#pendingMidTurnCount++;
-      this.#activeRun.pushMidTurn(effectivePrompt, effectivePriority ?? "now");
+      // Count ONLY pushes that start a new query, because only those produce an
+      // extra turn_done for #consumeEvents to absorb. A "later" push merges into
+      // the running turn (ClaudeProvider: `shouldQuery = priority !== "later"`),
+      // so counting it made the consumer treat the turn's REAL terminal
+      // turn_done as an intermediate boundary: it decremented, re-asserted
+      // "thinking", and `continue`d — waiting forever for a turn_done that was
+      // never going to be emitted.
+      //
+      // That is the stuck spinner. The model has answered, but the session sits
+      // at "thinking" until the 5-minute stall watchdog fires or the next send
+      // closes the queue. It also strands the turn's sub-agents and tools:
+      // neither the mid-turn flush nor the consumer's finally reconciles them,
+      // because the loop never exits.
+      if (effectivePriority !== "later") this.#pendingMidTurnCount++;
+      this.#activeRun.pushMidTurn(effectivePrompt, effectivePriority);
       // Keep waiting_approval visible — the approval is still pending and
       // every frontend keys its approval bar off it; the queued text is
       // consumed after the user answers.
@@ -1840,6 +1853,52 @@ export class Session {
     this.#broadcastInfoUpdate();
   }
 
+  /**
+   * Drop every sub-agent still registered and revoke its ZeroID identity.
+   *
+   * A Task sub-agent cannot outlive the turn that spawned it, so once that turn
+   * ends — or the provider it was running under goes away — any surviving entry
+   * is stale by definition.
+   *
+   * Before this existed the ONLY cleanup path was the provider's
+   * `subagent_stop` event, which originates in the SDK's SubagentStop hook.
+   * That hook cannot fire when the query is aborted mid-turn (interrupt,
+   * setModel, rotate, provider switch — all of which call
+   * `#abortController.abort()`), so each abort permanently orphaned every
+   * in-flight sub-agent. Three consequences, all of which this fixes:
+   *
+   *   1. `subagentSnapshot` (→ `/who`, `toInfo().subagents`) only ever grew —
+   *      the reported count climbed across turns and never came back down.
+   *   2. `#subagents` + `#subagentRegistrations` grew unbounded for the
+   *      lifetime of a long-lived session.
+   *   3. Worst: each orphan kept a LIVE delegated ZeroID token. Revocation is
+   *      supposed to ride the sub-agent's own stop; instead it waited for
+   *      `deactivateSessionAgent`'s cascade at session destroy, so a dead
+   *      sub-agent's credential stayed valid for as long as the session lived.
+   *
+   * Idempotent and cheap — a no-op when the map is empty, which is the common
+   * case. Double-revocation is safe: `deactivateSubagent` no-ops on an id it
+   * has already dropped, so a `subagent_stop` arriving after a sweep (or a
+   * sweep racing a trailing stop) costs nothing.
+   */
+  #sweepStaleSubagents(reason: string): void {
+    if (this.#subagents.size === 0) return;
+    const orphaned = [...this.#subagents.keys()];
+    for (const agentId of orphaned) {
+      // Fire-and-forget, matching the subagent_stop path: revocation must never
+      // block a turn boundary, and deactivateSubagent logs its own failures.
+      void this.#identityManager?.deactivateSubagent(this.id, agentId);
+      this.#subagentRegistrations.delete(agentId);
+      this.#subagents.delete(agentId);
+    }
+    // Worth a line: a non-empty sweep means a SubagentStop never arrived, which
+    // is expected on abort but would otherwise be invisible.
+    console.log(
+      `[codeoid/session ${this.id}] swept ${orphaned.length} stale sub-agent(s) at ${reason}`,
+    );
+    this.#broadcastInfoUpdate();
+  }
+
   async #teardownProvider(): Promise<void> {
     // Capture before nulling: provider.teardown() may trigger onRecoveryNeeded,
     // which installs a new #eventConsumerTask. Awaiting the snapshot drains
@@ -1851,6 +1910,10 @@ export class Session {
     this.#eventConsumerTask = null;
     await this.#provider.teardown();
     try { await taskToAwait; } catch { /* consumer handles its own errors */ }
+    // teardown() aborts the SDK query, so any sub-agent still in flight will
+    // never get its SubagentStop hook. This is the setModel / rotate /
+    // switchProvider path — the session survives, so the orphans would too.
+    this.#sweepStaleSubagents("provider teardown");
     // The drained consumer's `finally` skips its own idle reset here: we nulled
     // #activeRun above, so its run-ownership guard (`#activeRun === run`) is
     // false. Without this, tearing a provider down mid-turn (setModel / rotate)
@@ -1917,12 +1980,16 @@ export class Session {
     if (run) {
       try {
         await run.interrupt();
+        // Interrupting kills the turn, so every sub-agent it spawned is done
+        // whether or not the SDK got to run their stop hooks.
+        this.#sweepStaleSubagents("interrupt");
         if (this.#status !== "error") this.#setStatus("idle");
         return;
       } catch {
         // fall through to hard abort
       }
     }
+    this.#sweepStaleSubagents("interrupt");
     if (this.#status !== "error") this.#setStatus("idle");
   }
 
@@ -3309,6 +3376,11 @@ export class Session {
           this.#recordTurnFromResult(event.result);
           // Flush per-turn accumulators so the continuation turn starts clean.
           this.#completeActiveTools();
+          // Same boundary, same reasoning: the sub-agents of the partial turn
+          // are done with it. This branch `continue`s without dispatching to
+          // #handleProviderEvent, so it is the only place that can reconcile
+          // them for an absorbed mid-turn boundary.
+          this.#sweepStaleSubagents("mid-turn boundary");
           this.#flushActiveAssistant();
           this.#finalizeActiveThinking();
           this.#chunker?.onTurnEnd();
@@ -3340,6 +3412,17 @@ export class Session {
     } finally {
       this.#pendingMidTurnCount = 0; // safety: reset on any exit path
       this.#completeActiveTools();
+      // Sub-agent reconciliation belongs beside the tool reconciliation: this
+      // finally is the one path every turn exit goes through — clean turn_done,
+      // error, stall recovery, ownership loss. #completeActiveTools has always
+      // existed here because provider events can be lost; sub-agents were simply
+      // never added to the same backstop.
+      this.#sweepStaleSubagents("turn exit");
+      // Tell the provider this turn's stream has no reader anymore. Without it
+      // the queue stays open and unconsumed until the NEXT turn replaces it, so
+      // a late event is silently buffered into a queue nobody will ever drain.
+      // Closing converts that invisible loss into the provider's carryover path.
+      try { run.endTurn?.(); } catch { /* best-effort */ }
       this.#flushActiveAssistant();
       this.#finalizeActiveThinking();
       this.#chunker?.onTurnEnd();
@@ -3762,6 +3845,9 @@ export class Session {
       case "turn_done": {
         this.#accumulator.handleEvent(event);
         this.#recordTurnFromResult(event.result);
+        // No sweep here: a terminal turn_done breaks #consumeEvents, whose
+        // finally reconciles sub-agents alongside tools. Sweeping here too would
+        // just be a redundant pass a few statements earlier.
         // Hook seam: observe-only (git-checkpoint per turn, usage export).
         this.#hookBus?.emit("after_turn", this.#hookContext(), {
           result: event.result,