# Plan — fix sub-agent / wake streaming race on claude-cli

## Empirical findings (the data, not the theory)

**Repro:** parent sends `agent_spawn(async_inform: true)` to a sub-agent, parent's turn 1 ends quickly with "spawned", sub-agent runs in background, sub-agent's reply triggers a 2nd parent turn via wake.

**Bug rate:** 2/10 runs (20%) on a clean 5050 instance.

**Bad-run signature** (parent phase 2 — events AFTER turn 1's `chat.response`):
```
{"session.stream/done": 1, "chat.user_message": 1}
```

**Good-run signature:**
```
{"session.stream/done": 2, "chat.user_message": 1, "chat.message": 1-2,
 "session.stream/inference.chunk": 1-2, "session.stream/thinking.chunk": 0-2,
 "chat.response": 1}
```

**Adding `console.log` to `runClaudeSession`'s finally makes the bug disappear (10/10 good).** Heisenbug — timing race.

## Diagnosis

The bug-signature decomposition reveals the mechanic:

- `done` count = number of completed `runClaudeChat` invocations during phase 2.
- Good runs: `done:2` → TWO runChat calls completed (turn 1's tail + wake's runChat).
- Bad runs: `done:1` → ONE only (turn 1's tail). **Wake's runChat never ran the for-await loop.**
- But bad runs DO emit `chat.user_message:1` → something fired this. Two emitters exist:
  1. `pushToExistingSession` ([engines/claude-engine.js:76](engines/claude-engine.js#L76))
  2. `runClaudeSession` initial push ([engines/claude-engine.js:354](engines/claude-engine.js#L354))

Since wake's runChat never ran the for-await (no done #2), the `chat.user_message` must have come from `pushToExistingSession`. **That means `_activeRuntimes` still had a stale entry** when wake fired.

### The race

1. Parent's turn 1 runChat is in for-await, processing chat.response.
2. for-await `break` fires → JS calls `iterator.return()` on `runClaudeSession`.
3. `iterator.return()` returns a promise. JS awaits it.
4. Meanwhile, sub-agent's reply lands → `wakeIdleTargetIfNeeded` checks `runningSessions.has(parentSid)`. It has been unregistered at this point (or not — race).
5. Wake fires `runChat` for parent → reaches `pushToExistingSession`.
6. `pushToExistingSession` reads `_activeRuntimes.get(sid)`. **The runClaudeSession generator's finally hasn't run yet** (still being awaited from step 3). The map entry IS still there.
7. `pushToExistingSession` finds the entry, emits `chat.user_message`, pushes message into the (about-to-die) prompt queue, returns true.
8. wake's runClaudeChat early-returns.
9. The original runClaudeSession generator's finally runs → removes the runtime — but the queued message is in a dead queue. The SDK will never see it.
10. **The injected message is lost. No further events fire.** The user only sees the "popped-in" `chat.user_message` and nothing else.

### Why the order matters

`runningSessions.unregister` and `removeRuntime` BOTH live in `runClaudeSession`'s finally ([engines/claude-engine.js:733-734](engines/claude-engine.js#L733)). If wake checks `runningSessions.has` after unregister but `_activeRuntimes.get` before removeRuntime, OR if any caller checks `_activeRuntimes` directly, the inconsistency window allows the race.

Wake.js checks `runningSessions.has` (not `_activeRuntimes`), so its OWN gate is robust. But `pushToExistingSession` checks `_activeRuntimes.get` — these two indices can be out of sync during teardown if their unregister/remove calls aren't atomic.

Looking at the finally:
```js
removeRuntime(sessionId);          // line 733
runningSessions.unregister(sessionId);  // line 734
```

They're sequential. Between them, `_activeRuntimes` is empty BUT `runningSessions.has` is true. After: both empty.

But the race is on the OTHER side. When `iterator.return()` is called (from for-await break), the generator's finally is scheduled. Multiple microtasks run before the finally completes. During that window, `_activeRuntimes` still has the entry, `runningSessions` still has the entry.

Wake's check `runningSessions.has(parentSid)` returns TRUE — so wake **doesn't even try** to fire. But somehow wake DID fire (we see the chat.user_message). That means the wake fired AFTER unregister but BEFORE removeRuntime — but they're sequential within the same synchronous block.

Wait — actually, looking again: lines 733-734 are both synchronous in a sync finally. Between them there is no await. So no other code can interleave.

So the race must be elsewhere. Re-examining: wake doesn't directly call pushToExistingSession. Wake calls runChat, which calls runClaudeChat, which calls pushToExistingSession.

What if multiple wakes are scheduled? Wake.js's `_pendingWakes` and `_wakingNow` sets dedup, but the dedup is local-per-process. The first wake might call `runningSessions.has(parentSid)` → true → return without doing anything (no pushToExistingSession call).

Hmm but if no wake actually fires runChat, where does the chat.user_message come from?

**A third possibility:** `agent_message` from a DIFFERENT path. Perhaps the sub-agent's `postCorrelatedResponse` itself eventually triggers some `agent_message` style enqueue+drain that goes through pushToExistingSession.

Or maybe wake.js's drainNonFollowup eventually fires from a setImmediate scheduled before the runtime was torn down.

**For the plan, the right fix is defensive:** make `pushToExistingSession` guard against the dying-runtime case so a queued message doesn't get silently dropped. Plus ensure runtime removal happens atomically with respect to all consistency checks.

## The fix (proposed)

### Option A — make `pushToExistingSession` reject during teardown

When the runClaudeSession generator's `for-await` over the SDK runtime has exited (i.e., the SDK is being torn down), `pushToExistingSession` should return `false` instead of pushing into a dead queue. The caller (`runClaudeChat`) will then fall through to fresh `runClaudeSession` — which DOES emit streaming events.

**How to detect "tearing down":** add a `_torndown` boolean to the runtime entry. Set it when the for-await of the SDK runtime completes (just before the finally), so subsequent `pushToExistingSession` calls return false. Better yet: wrap removal in a function that atomically does `_torndown = true` AND removes from `_activeRuntimes`.

### Option B — remove from `_activeRuntimes` IMMEDIATELY when for-await exits

Move `removeRuntime(sessionId)` to fire as soon as the for-await loop exits (before the rest of the finally). The earliest place this can run is inside the for-await loop body itself, on the result-event path. Or use a try/finally pattern that wraps just the for-await.

### Recommendation: Option A (atomic teardown gate)

Implementation sketch:

1. **In `engines/claude-engine.js`:**
   - At the start of the `result`-handling branch where `mode !== 'chat'` causes `break`, add a no-op for chat (existing). For chat, keep going. The actual teardown happens via for-await closing.
   - Wrap the for-await loop in a try/finally that sets `runtimeEntry._torndown = true` and calls `removeRuntime(sessionId)` synchronously, BEFORE the outer finally's correlation-flush code.

2. **In `pushToExistingSession`:**
   - If `active._torndown` is true, return false. Don't emit chat.user_message. Don't push.

3. **In `runClaudeChat`'s pushToExistingSession-gate:**
   - The check `if (message && pushToExistingSession(sid, message))` is unchanged — when push returns false (torndown case), runClaudeChat falls through to fresh `runClaudeSession`, which will properly stream.

### Side effects to watch

- Could inject EARLIER pushes (before teardown begins) be dropped? No — once `_torndown` is set, no more pushes can succeed. But ALREADY-PUSHED messages remain in the queue. The SDK might consume them before its runtime ends. Or it might not. Today's behavior: dropped (since runtime ends without consuming them, their bus events are lost). Same outcome — except now the FALLBACK fresh runtime will pick them up (because they were persisted to DB at line 75 before the push).

  WAIT — actually I need to re-examine. `pushToExistingSession` at line 75 calls `db.addMessage(...)` BEFORE pushing. So the user message is persisted. If the push later fails / runtime dies, the next runClaudeChat would re-run with the same user message... but wait, runClaudeChat is called with `message: ...`, and inside runClaudeSession it ALSO does `db.addMessage` at line 352. So we'd duplicate the user message.

  Actually, runClaudeSession at line 352 only fires when the `userMessage` is non-empty AND we go through the fresh-session path. The wake's runChat passes `message: seedMessage` which becomes `userMessage` in runClaudeSession. So if pushToExistingSession persisted the message AND runClaudeSession persists it again, duplicate.

  **Mitigation:** in `pushToExistingSession`, only persist + push if NOT torndown. If torndown: return false WITHOUT persisting. Caller's fresh runClaudeChat will persist via runClaudeSession's normal path. No duplicate.

This is the cleanest approach.

## Tasks

### Task A — Add `_torndown` flag to runtime entry

In `engines/claude-engine.js` `storeRuntime` function (~line 25), add `_torndown: false` to the entry default state.

### Task B — Set `_torndown = true` before runtime can be observed dying

Wrap the for-await loop in `runClaudeSession` with an inner try/finally that sets `runtimeEntry._torndown = true` AS THE FIRST STATEMENT after the for-await exits. This must run BEFORE any awaitable code in the outer finally, so concurrent `pushToExistingSession` calls see the flag.

### Task C — Guard `pushToExistingSession`

In `engines/claude-engine.js` `pushToExistingSession`:
```js
function pushToExistingSession(sessionId, content) {
  const active = _activeRuntimes.get(sessionId);
  if (!active || !active.promptQueue || active._torndown) return false;
  // ... existing code ...
}
```

Place the check BEFORE the `db.addMessage` and `emitBusEvent` calls so a dying runtime doesn't double-persist.

## Verification — real tests

1. **Bug reproducer:** the existing `/tmp/repro_wake.js` script (parent sonnet, async sub-agent haiku, then count phase-2 events).
2. **Acceptance criteria:** 30 consecutive runs, ZERO bad-signature outputs (no run with `done:1` and missing `chat.message`/`chat.response`). Goal: 30/30 good.
3. **Regression checks:**
   - Direct chat (no sub-agent): streams normally.
   - agent_spawn sync (no async_inform): sub-agent streams to bus.
   - agent_message sync to existing session: streams.
4. **Make a script** that does all 4 scenarios in a loop.

## Criticizer prompt
After plan written, criticize:
- Is the `_torndown` placement correct? Will any code path set the flag too late?
- Does setting `_torndown` block the LEGITIMATE in-runtime injection (when the parent's tool calls fire INSIDE the parent's own active turn)? — Important: tool execution happens DURING the parent's active turn, so `_inFlight` is true and pushToExistingSession buffers via `_pendingInjections` rather than direct push. This is a different path from the wake's pushToExistingSession injection at session-end.
- Race between `_torndown = true` and a concurrent wake: is the synchronous `_torndown = true` ordering relative to `_activeRuntimes.delete` correct?
- Is there ANY code that reads `_activeRuntimes.get(...)` without going through `pushToExistingSession`? They'd also need the guard or bypass.