# Issue 2d — claude-cli mid-run injection corrupts session history

**Severity:** CRITICAL — silently corrupts parent agent state.
**Engine scope:** claude-cli ONLY (openai engine handles injection cleanly via DB-queue + drainNonFollowup).
**Discovery:** user-reported. When a queued `agent_messages` row is injected mid-tool-call into an active claude-cli session, the SDK treats it as an interrupt: tool result becomes an SDK-generated "interruption" sentinel (agent never sees real tool output), and any in-flight assistant text is **dropped from context entirely**.

User words: "async responses from sub-agents break parent agent work — this is a CRITICAL issue."

---

## Investigation findings

### SDK API surface (`@anthropic-ai/claude-agent-sdk` 0.2.114)

The SDK's `SDKUserMessage` type ([sdk.d.ts:3228-3246](node_modules/@anthropic-ai/claude-agent-sdk/sdk.d.ts#L3228)) exposes two public fields that are exactly what we need:

```ts
priority?: 'now' | 'next' | 'later';
shouldQuery?: boolean;     // false = append to transcript without triggering an assistant turn;
                           //         merges into the next user message that does query.
```

The SDK input channel is `streamInput(stream: AsyncIterable<SDKUserMessage>)` ([sdk.d.ts:2045](node_modules/@anthropic-ai/claude-agent-sdk/sdk.d.ts#L2045)). Our [engines/claude-prompt-queue.js](engines/claude-prompt-queue.js) implements this channel. The SDK ALSO exposes `interrupt()` ([sdk.d.ts:1880](node_modules/@anthropic-ai/claude-agent-sdk/sdk.d.ts#L1880)) — explicitly documented as "stop processing and return control to the caller."

The SDK distinguishes **interrupt** (explicit method) from **input** (queue with priority). The `priority` field exists exactly so callers can say "deliver this AFTER the current turn ends" (`'next'` / `'later'`) vs "interrupt now" (`'now'`).

When `priority` is **omitted**, the SDK falls back to its default ingestion behavior — which, when called mid-tool/mid-stream, has no choice but to truncate the in-flight content block. This matches the user's bug exactly.

### Confirming with our existing code

`pushSilent` ([engines/claude-prompt-queue.js:62](engines/claude-prompt-queue.js#L62)) already uses `shouldQuery: false` and is in production via `drainNonFollowup` — that path works because `drainNonFollowup` runs at iteration boundaries where the SDK is naturally between user/assistant turns. `pushMessage` doesn't set either field, so when called mid-turn, it interrupts.

---

## Root cause

[engines/claude-engine.js:45-56](engines/claude-engine.js#L45) `pushToExistingSession` calls `promptQueue.pushMessage(null, content)` unconditionally. [engines/claude-prompt-queue.js:42-49](engines/claude-prompt-queue.js#L42) `pushMessage` emits an `SDKUserMessage` **without `priority`** and **without `shouldQuery: false`**.

When the SDK pulls this message off `streamInput` while a tool call or assistant text block is mid-flight, it cancels the current content block and writes the SDK's "interruption" sentinel into the transcript instead of the real `tool_result` / partial assistant text.

Call chain for `agent_message` async_inform: [core/async-inform.js](core/async-inform.js) → `runChat` → `runClaudeChat` → `pushToExistingSession`. Same defect.

The openai engine doesn't have this problem because [core/router.js:148-266 `injectIntoActiveOpenaiSession`](core/router.js#L148) is purely DB-backed: it `enqueueAgentMessage`-then-polls, and the running openai `runLoop` only drains via `drainNonFollowup` at iteration boundaries — naturally turn-aligned.

---

## Recommended approach (REVISED post-criticizer)

### Why we abandoned the SDK `priority: 'next'` approach

The original brainstorm proposed using the SDK's `SDKUserMessage.priority` field as the primary mechanism. The criticizer flagged this as risky (no docstring, no documented runtime behavior). Verification by `grep` on the compiled SDK revealed:

- **`shouldQuery`**: 0 occurrences in any compiled `*.mjs` file. The field exists in `.d.ts` but appears to be type-only.
- **`priority`**: 5 occurrences in `bridge.mjs`, but all are unrelated (request/fetch `priority`, not `SDKUserMessage.priority`). 1 occurrence in `sdk.mjs` is a Zod schema for MCP annotations (numeric 0–1), not the `'now' | 'next' | 'later'` enum.
- **`'next'` literal**: appears in compiled JS but not tied to message-priority handling.

**Conclusion: `priority: 'next'` would be silently dropped at runtime — a no-op fix.** Same for `shouldQuery` (existing `pushSilent` uses it; if it works today, the SDK is consuming it via JSON IPC to the underlying `claude` binary, but we can't verify that. Safe to keep `shouldQuery: false` in `pushSilent` since it's existing working behavior; but we will NOT introduce a NEW reliance on `priority` since that's clearly not consumed.)

### Adopted approach: buffering-only (Option C)

We control the timing entirely via per-runtime `_inFlight` flag and `_pendingInjections` buffer. No reliance on undocumented SDK fields.

- The for-await loop in `runClaudeSession` already iterates SDK messages one at a time — it IS the boundary watcher.
- `_inFlight = true` while we're processing an `assistant` / `stream_event` / `tool_progress` message.
- `_inFlight = false` immediately before each `drainNonFollowup` call (the natural iteration boundary, where the SDK has just landed a `tool_result` user-message and is about to read the next user-message in `streamInput`).
- `pushToExistingSession`: when `_inFlight === true`, append to `_pendingInjections` array (do NOT call `pushMessage`). When `_inFlight === false` (session waiting for next user input — runtime is parked on the `streamInput` async iterator's next-promise), call `pushMessage` normally as it currently does.
- `flushPendingInjections(runtime)`: drain the buffer in FIFO order, calling `pushMessage` for each. Invoked at the same `assistant`/`drainNonFollowup` boundary, AND in the `result` ingestion branch, AND in the `finally` cleanup.

This guarantees mid-tool/mid-stream injections are deferred until the SDK has finished the current content block — the documented bug (interruption sentinel + dropped assistant text) is structurally impossible.

**Why not B (boundary-only injection without `_inFlight`):** would require introspecting SDK-message types from inside `pushToExistingSession` to decide whether we're at a boundary — too coupled. The flag is cleaner.

**Why not D (engine fallback):** agent's engine is pre-configured; mid-run switch loses Claude SDK state.

---

## File-by-file changes

### `engines/claude-prompt-queue.js`

**No new exported function.** `pushSilent` and `pushMessage` keep their existing semantics. We do NOT add a `pushDeferred` (since the SDK doesn't honor `priority`, that name was misleading anyway).

### `engines/claude-engine.js`

Three additions to the existing per-runtime state and ingestion loop:

1. **Track in-flight state.** Add `_inFlight: false` per-runtime (alongside the existing `_pendingCorrelationIds` etc. fields). Set `true` at the START of processing `assistant`, `stream_event`, or `tool_progress` SDK messages (top of each respective `if` branch in the for-await loop). Set `false` immediately before the next `drainNonFollowup` call. The for-await loop is single-threaded async, so the toggle is race-free.

2. **Buffer mid-turn injections.** Modify `pushToExistingSession(sessionId, content)` ([claude-engine.js:45-56](engines/claude-engine.js#L45)):
   - Look up the runtime in `_activeRuntimes`.
   - If runtime not found → existing fallback (false / no-op).
   - If `runtime._inFlight === true` → append `content` to `runtime._pendingInjections` (initialize the array if absent). Emit a debug trace `[claude-engine] buffered mid-turn injection for sessionId=...`.
   - If `runtime._inFlight === false` (runtime parked on streamInput's next-promise, waiting for input) → call `promptQueue.pushMessage(sessionId, content)` directly (existing path).

3. **Flush at boundaries.** Add a `flushPendingInjections(runtime, sessionId)` helper that iterates `runtime._pendingInjections` (FIFO), calls `promptQueue.pushMessage` for each, then resets the array to empty. Invoke at:
   - The `drainNonFollowup` site around [line 487](engines/claude-engine.js#L487), AFTER setting `_inFlight = false`. This is THE primary boundary.
   - The `result` ingestion branch around [line 571](engines/claude-engine.js#L571) (final turn flush).
   - The `finally` block around [line 618](engines/claude-engine.js#L618) — but treat differently: if the runtime is being torn down (cancel/error), don't push to the dying SDK runtime; instead persist each buffered injection AS a user-row directly via `db.addMessage` (so the user sees what was queued for the next turn) and `postCorrelatedResponse(cid, '(target session ended before injection delivered)', null, …)` for any associated correlation IDs.

### `core/router.js`

No structural change. `pushToExistingSession` remains the public entry; buffering happens inside the engine. The early-return shape stays — correlation-ID polling continues to work because messages still land via the DB path.

### `core/async-inform.js`

No code change needed. The dispatcher does the right thing — only the engine's downstream injection needed fixing.

### Documentation updates

- [docs/guide/09-multi-agent.md](docs/guide/09-multi-agent.md) line ~177: clarify that on **claude-cli**, mid-processing injection is now delivered at the next iteration boundary (after the current `tool_result` lands, before the next assistant turn) via the SDK's `priority: 'next'` semantics. On **openai**, same effective behavior via DB queue + `drainNonFollowup`. End-user behavior identical across engines.
- [docs/guide/06-tools.md](docs/guide/06-tools.md) "Engine parity — claude-cli" section (line 715+): add a paragraph documenting the new injection rule: msg#2+ on claude-cli delivers as a single user message after the current turn's `tool_result`, not as a streaming continuation. Parity callout: mid-tool injections no longer corrupt tool history.

---

## Behavior delta (user perspective)

**Before fix:** mid-tool async_inform on claude-cli could silently corrupt the parent's history (tool_result replaced with "interruption" sentinel; partial assistant text dropped from context).

**After fix:**
- Tool results always land intact — no interruption sentinel from injection.
- Partial assistant text preserved — SDK finishes its current content block before consuming the queued user message.
- Injected messages appear as a clean user-role row in `messages` AFTER the current turn's tool_result / final assistant text, identically to the openai engine.
- **Latency change:** msg#2+ on claude-cli no longer streams a reply mid-tool; reply lands after the current tool completes (matching openai). For long-tool sessions this can mean +30–120s of perceived delay before the injected message gets a response — but the alternative is corrupted history, which is worse.
- No semantic difference vs openai for end users. Document as "msg#2+ delivery is iteration-aligned on both engines."

---

## Test design

All against `localhost:5050`, real `cc-sonnet`. New file: `test/e2e/scenario-claude-mid-run-injection.js`. Register in `phase-3-suite.js`.

### Test 1 — mid-tool injection preserves tool_result

1. POST `/agents/cc-sonnet/chat` with prompt: *"Read `/home/ixi/khacloud/drive/Plugins/VeilCli/REPORT.md` then write a 1500-word essay summarizing it. Take your time."* Forces a Read tool call + long generation (~60s+).
2. Subscribe to SSE. Wait for `tool.start { toolName: 'Read' }` event, then add a 2s safety delay to ensure deeply mid-tool.
3. From a second client, POST `/agents/<other agent>/chat` whose system prompt instructs it to call `agent_message` against the cc-sonnet session — sync (`async_inform: false`), message="Quick question — what's 2+2?".
4. Wait for the original session to settle (`chat.response` event).
5. **Assertions** (`GET /sessions/<sid>/messages`):
   - Find the tool message for the Read call. `content` contains REPORT.md text — NOT "interruption".
   - Injected user message appears as a separate row AFTER the tool_result.
   - Assistant's reply to the injected question lands cleanly.

### Test 2 — mid-stream injection preserves assistant text

1. Same spawn, prompt: *"Write me a 2000-word essay on databases. No tool calls. Just write."* Pure text generation, multi-second stream.
2. Wait for first stream chunk via SSE.
3. ~5s into the stream, fire `agent_message async_inform: true` from second client.
4. Wait for completion.
5. **Assertions:**
   - Last assistant row before the injection has `content` length > 1000 chars (real text, not partial-then-empty).
   - Injected message lands as a fresh user row.
   - Subsequent assistant turn responds to the injection.

### Test 3 — regression: sync `agent_message` mid-tool still works

1. Same setup as Test 1, second client sends `async_inform: false`.
2. **Assertions:** same intactness checks as Test 1 + the sync caller receives a real reply (not a timeout, not an injection_timeout error).

---

## Risks / edge cases

- **Mid-`thinking_delta`.** SDK treats `priority: 'next'` after-thinking-block boundary same as after-assistant-text. Safe — verified by the d.ts comment "merged into the next user message that does query."
- **Multiple injections queue up.** Drain in FIFO order at the next iteration boundary. The engine's `_pendingInjections` array preserves order; flush iterates and pushes each with `priority: 'next'`. Each becomes its own user-row turn — the LLM may decide to bundle-respond.
- **Cancel during buffered period.** `finally` block ([engines/claude-engine.js:618-636](engines/claude-engine.js#L618)) already runs `postCorrelatedResponse` for pending correlation IDs with cancel-aware reason. Extend with `flushPendingInjections`-or-discard: persist them as user-rows in `messages` (so user sees what was queued) and call `postCorrelatedResponse(cid, '(target session cancelled before injection delivered)', null, …)`.
- **SDK errors out before buffer drains.** Same `finally` path; treat identically to cancel.
- **`priority` field unrecognized on older SDKs.** d.ts marks both fields as optional; older SDKs ignore unknown fields silently (standard JSON-typed input). Worst case = pre-fix behavior on stale install. Pin minimum `@anthropic-ai/claude-agent-sdk` to `>=0.2.114` in `package.json` to make this explicit.
- **Race between drain and inject.** `_inFlight` toggle set/cleared on the same single-threaded async loop in `runClaudeSession`. Node is single-threaded; the for-await loop guarantees serialization. No additional lock needed.
- **`pushToExistingSession` return value.** Caller in [router.js:525](core/router.js#L525) only uses it to check "did the session exist?". Buffered = still successful from caller's POV. No API change visible.

---

## Interaction with Issue 2a

Issue 2a (idle parent wake) starts a FRESH `runChat` when a parent is idle. Fresh runChat creates a fresh `runClaudeSession` runtime — the SDK injection bug doesn't apply (we only inject when in-flight, which a fresh runtime isn't yet). Independent of 2d.

Issue 2d's fix also REDUCES reliance on mid-run injection: by the time the buffer flushes (at next iteration boundary), the in-flight tool/text has settled. So the parent agent sees a clean transcript. Combined with 2a, the full delivery picture becomes:
- Active parent loop, mid-iteration: inject buffered, flush at next iteration boundary (2d).
- Idle parent: wake fires fresh `runClaudeSession` that drains via `drainNonFollowup` on iteration 1 (2a). No mid-run injection involved.

---

## Implementation workflow

1. Modify `engines/claude-prompt-queue.js` — add `priority` opt + `pushDeferred`.
2. Modify `engines/claude-engine.js` — add `_inFlight`, `_pendingInjections`, `flushPendingInjections`, hook in ingestion loop and finally.
3. Update docs.
4. Write `test/e2e/scenario-claude-mid-run-injection.js`. Register in suite.
5. Run tests until green.
6. Pin SDK min version in `package.json`.
7. Run criticizer subagent on diff + test results.
8. Apply criticizer-driven fixes if any.
9. Notify user.

## Files touched

| File | Type |
|------|------|
| `engines/claude-engine.js` | MODIFIED (add `_inFlight`, `_pendingInjections`, buffering in `pushToExistingSession`, `flushPendingInjections` at 3 boundary points) |
| `docs/guide/09-multi-agent.md` | MODIFIED (clarify async_inform timing on claude-cli) |
| `docs/guide/06-tools.md` | MODIFIED (engine-parity injection note) |
| `test/e2e/scenario-claude-mid-run-injection.js` | NEW |
| `test/phase-3-suite.js` | MODIFIED (register new scenario) |

`engines/claude-prompt-queue.js` and `package.json` are NO LONGER touched (per the criticizer-driven revision).

---

## Criticizer-driven addenda (post-review v2)

### SDK API verification (the big finding)

The original brainstorm proposed using `SDKUserMessage.priority: 'next'` and `shouldQuery: false`. The criticizer flagged the lack of runtime documentation; verification by `grep` on the compiled SDK confirmed:

- `priority` (in the `'now' | 'next' | 'later'` enum sense) — **NOT consumed at runtime**. The 5 `priority` occurrences in `bridge.mjs` are all unrelated (request-fetch priority). The 1 in `sdk.mjs` is a numeric Zod schema for MCP annotations.
- `shouldQuery` — **0 occurrences in any compiled file**.

The .d.ts type definitions exist but no runtime code consumes them. **Switching the strategy entirely** to local buffering keyed off `_inFlight`. This is what Option C in the original brainstorm proposed; it's now the only mechanism (no longer "defense-in-depth").

### `_inFlight` toggle clarified

The criticizer noted that the for-await loop has sequential `if` branches that `continue` — a single SDK message is one type per iteration, never two. So the toggle wording "set true on assistant/stream_event/tool_progress" was ambiguous.

**Clarified:** at the TOP of each of those branches, set `_inFlight = true`. The toggle to `_inFlight = false` only happens in ONE place: immediately before the `drainNonFollowup` call at line 487 — that's the genuine iteration boundary (SDK has just landed a `tool_result` user-message and is parked on `streamInput`'s next-promise, waiting for our input). At that exact moment, no SDK content block is mid-flight, and pushing to `streamInput` is safe.

The `_inFlight` flag is per-runtime and toggled in a single-threaded async generator — no race possible.

### Test orchestrator clarified

Test 1 step 3 mentioned vaguely "POST `/agents/<other agent>/chat`". The orchestrator can be any agent that is configured to call `agent_message` — easiest is the openai-engine `assistant` agent, which has `agent_message` available via its tool config. Using a separate engine for the orchestrator avoids accidental SDK-state coupling between the test runner and the cc-sonnet target.

**Updated Test 1, step 3:** "From the test driver, fire `POST /agents/assistant/chat` with body `{ message: '<instructions to call agent_message against the cc-sonnet session with sync mode>', sessionId: <fresh orchestrator session> }`."

### Token-budget guardrail for the test suite

Tests use real cc-sonnet with long content. Add a per-scenario token cap: `budget_override: { max_tokens: 50000 }` on each chat call. If the cap is hit, the test fails fast rather than burning $$ on a runaway. Document this in the test file header.

### Documentation reconciliation

The existing `docs/guide/09-multi-agent.md:182` reads:

> Any SUBSEQUENT `async_inform` to the same target while the seed's runChat is still running (msg#2+, possibly from a different caller) gets the **first mid-run text** the LLM produces after that message is drained at an iteration boundary

This is consistent with the buffering approach: msg#2+ still gets "first mid-run text," it's just delayed by up to one tool-call duration on claude-cli (the buffer waits until the next iteration boundary). The `<system-reminder>` "sub-agent is still running" remains accurate.

**Doc update:** add an engine-parity note. On openai, `drainNonFollowup` already runs at every iteration boundary (so msg#2+ is naturally aligned). On claude-cli, the new buffer ensures the same alignment — no behavior change visible to the LLM, just no more transcript corruption.

### `_pendingInjections` unbounded growth concern

If a parent issues many async_informs during a very long claude-cli tool call (e.g. a 5-minute web_fetch), `_pendingInjections` could grow. Two safeguards:

1. **Soft cap.** If the array exceeds 50 entries, log a warning and persist surplus entries directly as user-rows via `db.addMessage` (skipping the SDK push). The agent will see them on the NEXT runChat, not this one.
2. **Cleanup on cancel.** Already covered by the `finally` block in change #3.

### Correlation-ID timeout interaction

Buffered injections may carry correlation IDs (when caller is sync-poll-waiting for a reply). If the buffer drains AFTER the caller's sync timeout has expired, the caller already gave up. The flush still posts the row's response via existing `postCorrelatedResponse` plumbing — late-arriving response into a closed agent_messages row is the existing well-tested "sync timeout late delivery" scenario (verified in scenario-5b earlier). No new failure mode introduced.

### Sync-vs-async injection coverage

`pushToExistingSession` is called by both sync `agent_message` and async `agent_message` paths. The buffering treats them identically — both routes are protected. The only difference is sync calls poll on a correlation_id; async calls don't. Both paths have well-defined late-arrival behavior.

### Removed: `package.json` SDK pin change

The original plan proposed `>=0.2.114` (less restrictive than the existing `^0.2.114`). The criticizer correctly noted this is LESS safe. Since we no longer rely on any specific version of `priority`/`shouldQuery`, the existing `^0.2.114` pin stays. **No `package.json` change needed.**