clear stale emitted tool calls, trim comments

Author: charlesvien

packages/agent/src/adapters/claude/claude-agent.slash-command.test.ts

@@ -185,9 +185,7 @@ describe("ClaudeAcpAgent.prompt — early idle handling", () => {
         expect(text).toContain(tc.commandInMessage);
       }
     } else {
-      // No unsupported chunk. The idle is absorbed; the stream then ends
-      // without the turn ever starting, so the consumer rejects the queued
-      // prompt with the session-ended error.
+      // Idle absorbed; the stream then ends before the turn ever starts.
       await expect(promptPromise).rejects.toThrow(/session has ended/);
       expect(
         findUnsupportedChunkText(client.sessionUpdate.mock.calls),
@@ -233,8 +231,7 @@ describe("ClaudeAcpAgent.prompt — force-cancel backstop", () => {
     });
 
     await new Promise((resolve) => setImmediate(resolve));
-    // Activate the turn via its echo so cancel() arms the backstop for it
-    // (a still-queued turn is settled directly, with no SDK work to force).
+    // cancel() only arms the backstop for an activated (echoed) turn.
     echoQueuedTurn(agent, query);
     await new Promise((resolve) => setImmediate(resolve));
 

packages/agent/src/adapters/claude/claude-agent.ts

@@ -66,11 +66,8 @@ interface AnthropicMessageWithContent {
 type ChunkHandlerContext = {
   sessionId: string;
   toolUseCache: ToolUseCache;
-  /** Tool_use ids already surfaced as a `tool_call` (a permission request
-   *  emits one eagerly, before the tool_use block streams). When the streamed
-   *  block arrives second it refines with a `tool_call_update` instead of
-   *  emitting a duplicate `tool_call`. Mutated in place; pruned at
-   *  `tool_result` time alongside `toolUseCache`. */
+  /** Tool_use ids already surfaced as a `tool_call` (permission requests emit
+   *  eagerly); the second emitter refines instead of duplicating. */
   emittedToolCalls?: Set<string>;
   fileContentCache: { [key: string]: string };
   enrichedReadCache?: EnrichedReadCache;
@@ -87,17 +84,11 @@ type ChunkHandlerContext = {
 };
 
 /**
- * The text/thinking blocks that actually streamed live as `stream_event`
- * deltas for the message the next consolidated `assistant` will repeat, in
- * stream order, each accumulated to its full streamed text. The consolidated
- * handler diffs each assembled block against these and forwards only the
- * un-streamed remainder — nothing if it streamed in full (the common case),
- * the whole block if it never streamed (a non-streaming gateway), or just the
- * tail if the stream was cut short mid-block. Matching on content rather than
- * the Anthropic message id makes dedupe robust to gateways that don't carry a
- * stable/matching id across the stream and the consolidated message. Cleared
- * at each top-level `message_start` and again once a consolidated message
- * consumes it, so the record stays bounded to the in-flight message.
+ * Text/thinking actually streamed live for the in-flight message, in order.
+ * The consolidated assistant message prefix-diffs its blocks against this and
+ * forwards only the un-streamed remainder. Content matching (not message ids)
+ * keeps dedupe robust to gateways whose ids don't line up; cleared per
+ * message so it stays bounded.
  */
 export interface StreamedAssistantBlocks {
   blocks: { index: number; type: "text" | "thinking"; text: string }[];
@@ -211,9 +202,6 @@ function handleToolUseChunk(
   ctx: ChunkHandlerContext,
 ): SessionUpdate | null {
   const alreadyCached = chunk.id in ctx.toolUseCache;
-  // A permission request may have already surfaced this tool call to the
-  // client (see `emittedToolCalls`); if so this streamed encounter refines it
-  // rather than emitting a duplicate `tool_call`.
   const alreadyEmitted =
     alreadyCached || ctx.emittedToolCalls?.has(chunk.id) === true;
   ctx.toolUseCache[chunk.id] = chunk;
@@ -846,11 +834,8 @@ export async function handleSystemMessage(
       break;
     }
     case "informational": {
-      // Free-form notice from the SDK (e.g. why a UserPromptSubmit/Stop hook
-      // blocked continuation). Surface the text so the user sees it instead
-      // of a silent stop. ACP's agent_message_chunk has no severity field, so
-      // fold the level into the text for the more prominent levels ('info' is
-      // transcript-only noise — leave plain).
+      // Surface hook-blocked stops; the level is folded into the text since
+      // agent_message_chunk has no severity field.
       const informationalText =
         message.level === "info"
           ? message.content
@@ -1022,26 +1007,16 @@ export async function handleStreamEvent(
 
   const streamed = context.streamedAssistantBlocks;
   if (streamed) {
-    // A new top-level message starts: clear any streamed-content residue from
-    // a prior message that never reached its consolidated reset (a cancelled
-    // turn breaks out before it). Block indices restart at 0 each message, so
-    // leftover entries would otherwise collide with this message's blocks and
-    // re-emit (or truncate) already-streamed text. Gated on
-    // `parent_tool_use_id === null` so a subagent stream can't clear the
-    // top-level record.
+    // Clear residue from a message that never reached its consolidated reset
+    // (e.g. a cancelled turn); indices restart per message and would collide.
     if (
       message.event.type === "message_start" &&
       message.parent_tool_use_id === null
     ) {
       streamed.blocks.length = 0;
     }
-    // Accumulate the text/thinking actually streamed live, so the assistant
-    // handler can diff its assembled blocks against what already reached the
-    // client as chunks and forward only the remainder. Only top-level streams
-    // are recorded — subagent text is never streamed and must stay filtered,
-    // as it is internal to the tool call. Contiguous deltas of the same block
-    // (same index and type) extend the current entry; anything else opens a
-    // new one.
+    // Record only top-level streams; subagent text is never streamed and
+    // must stay filtered.
     if (
       message.parent_tool_use_id === null &&
       message.event.type === "content_block_delta"
@@ -1053,11 +1028,7 @@ export async function handleStreamEvent(
           : delta.type === "thinking_delta"
             ? { type: "thinking" as const, text: delta.thinking }
             : undefined;
-      // Skip empty deltas (some gateways emit empty thinking chunks):
-      // appending "" is a no-op, but pushing a "" entry would create a block
-      // the consolidated handler's `text.length > 0` guard can never consume,
-      // stalling the diff cursor and re-emitting the next block as a
-      // duplicate.
+      // An empty entry would stall the diff cursor in the assistant handler.
       if (chunk && chunk.text.length > 0) {
         const index = message.event.index;
         const last = streamed.blocks[streamed.blocks.length - 1];
@@ -1221,16 +1192,9 @@ function logSpecialMessages(
   }
 }
 
-/**
- * Diffs each assistant `text`/`thinking` block against what already streamed
- * live as chunks (`StreamedAssistantBlocks`, in document order) and forwards
- * only the un-streamed remainder — nothing if it streamed in full (the common
- * case), the whole block if it never streamed (a non-streaming gateway), or
- * just the tail if the stream was cut short mid-block. Subagent assistant
- * content (`parent_tool_use_id !== null`) is never streamed and stays
- * internal to its tool call, so it is always dropped, as is everything when
- * no tracker is available (replay).
- */
+// Forwards only the un-streamed remainder of each assistant text/thinking
+// block: nothing, the whole block (non-streaming gateway) or a cut-short
+// tail. Subagent content and tracker-less replay stay dropped.
 function filterAssistantContent(
   message: SDKAssistantMessage,
   streamed: StreamedAssistantBlocks | undefined,
@@ -1253,10 +1217,8 @@ function filterAssistantContent(
     );
   }
 
-  // `streamPos` walks the streamed blocks in step with the assembled
-  // text/thinking blocks; tool_use and other blocks pass through untouched
-  // (their own `toolUseCache` collapses the streamed/assembled pair) without
-  // advancing it.
+  // streamPos walks the streamed record in step with the assembled
+  // text/thinking blocks; other block types pass through without advancing.
   const kept: typeof content = [];
   let streamPos = 0;
   for (const block of content) {
@@ -1265,15 +1227,11 @@ function filterAssistantContent(
       continue;
     }
     const full = block.type === "text" ? block.text : block.thinking;
-    // Empty assembled blocks carry nothing (some gateways emit an empty
-    // `thinking` block before the real text) — drop them.
     if (full.length === 0) {
       continue;
     }
-    // A streamed block of the same type whose accumulated text is a prefix of
-    // this one was already (at least partly) delivered as chunks; consume it
-    // and forward only what's left. A non-empty streamed text is required so
-    // an empty/aborted streamed block doesn't swallow the assembled copy.
+    // A same-type streamed prefix means the block (or its head) was already
+    // delivered as chunks; consume it and forward only what's left.
     const streamedBlock = streamed.blocks[streamPos];
     if (
       streamedBlock &&
@@ -1286,9 +1244,7 @@ function filterAssistantContent(
       if (remainder.length === 0) {
         continue;
       }
-      // Overwrite in place with just the un-streamed tail (the assembled
-      // message isn't read again after this) so the block keeps its exact SDK
-      // type.
+      // Overwrite in place so the block keeps its exact SDK type.
       if (block.type === "text") {
         block.text = remainder;
       } else {
@@ -1297,12 +1253,8 @@ function filterAssistantContent(
       kept.push(block);
       continue;
     }
-    // Not matched: never streamed (or the stream diverged from the assembled
-    // text) — forward the block in full.
     kept.push(block);
   }
-  // Consumed: reset so the next message's blocks accumulate fresh and the
-  // record stays bounded to the in-flight message.
   streamed.blocks.length = 0;
   return kept;
 }

packages/agent/src/adapters/claude/conversion/sdk-to-acp.ts

@@ -570,8 +570,7 @@ export function toolUpdateFromToolResult(
     toolResult.is_error &&
     toolResult.content &&
     (toolResult.content as unknown[]).length > 0 &&
-    // Bash errors keep flowing through the terminal-output channel below so
-    // they render as terminal output rather than plain error text.
+    // Bash errors keep rendering through the terminal-output channel below.
     !(toolUse?.name === "Bash" && options?.supportsTerminalOutput)
   ) {
     return toAcpContentUpdate(toolResult.content, true);
@@ -676,11 +675,8 @@ export function toolUpdateFromToolResult(
             .map((c: { text?: string }) => c.text ?? "")
             .join("\n");
         } else {
-          // Image (or mixed non-text) content. Binary payloads can't be
-          // streamed through the terminal-output _meta channel, so bypass it
-          // and surface the blocks as ACP content. This handles the local
-          // Bash tool's image output, which previously failed the text-only
-          // guard and was silently dropped.
+          // Binary payloads can't ride the terminal-output _meta channel;
+          // surface image/mixed content as ACP content blocks instead.
           return toAcpContentUpdate(result, isError === true);
         }
       }

packages/agent/src/adapters/claude/conversion/tool-use-to-acp.ts

@@ -10,9 +10,7 @@ function createClient(response: Record<string, unknown>) {
   return {
     sessionUpdate: vi.fn().mockResolvedValue(undefined),
     requestPermission,
-    // Permission requests go through the generic request() so the tool
-    // call's abort signal rides along as a cancellationSignal; delegate to
-    // the requestPermission spy so assertions keep one target.
+    // Delegate so assertions keep one target.
     request: vi.fn((_method: string, params: unknown) =>
       requestPermission(params),
     ),

packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts

@@ -65,16 +65,12 @@ interface ToolHandlerContext {
   updateConfigOption: (configId: string, value: string) => Promise<void>;
   applySessionMode: (modeId: string) => Promise<void>;
   allowedDomains?: string[];
-  /** Tool_use ids already surfaced to the client as a `tool_call`. Shared
-   *  with the streamed tool_use path so whichever side emits first wins and
-   *  the other refines with a `tool_call_update`. */
+  /** Shared with the streamed tool_use path; first emitter wins. */
   emittedToolCalls?: Set<string>;
   supportsTerminalOutput?: boolean;
 }
 
-/** Built-in tools whose tool_use never surfaces as a standalone `tool_call`
- *  (Task* are rendered as plan snapshots at tool_result time; TodoWrite as a
- *  plan), so a permission prompt for them must not emit a stray one. */
+// Task*/TodoWrite render as plans, never as standalone tool_calls.
 function shouldEmitToolCall(toolName: string): boolean {
   return (
     toolName !== "TodoWrite" &&
@@ -85,12 +81,8 @@ function shouldEmitToolCall(toolName: string): boolean {
   );
 }
 
-/** Emit the `tool_call` a permission request references if it hasn't been
- *  sent yet, so the client has the tool call before being asked to approve
- *  it. The SDK may invoke `canUseTool` before the assistant message's
- *  tool_use block streams to us; the streamed chunk later refines this with a
- *  `tool_call_update` rather than emitting a duplicate (see
- *  `emittedToolCalls` in the conversion layer). */
+// The SDK can invoke canUseTool before the tool_use block streams; make
+// sure the tool_call exists before the client is asked to approve it.
 async function ensureToolCallEmitted(
   context: ToolHandlerContext,
 ): Promise<void> {
@@ -129,13 +121,8 @@ async function ensureToolCallEmitted(
   });
 }
 
-/** Forward a permission request to the client, emitting the referenced
- *  `tool_call` first and wiring the tool call's abort `signal` through as a
- *  `cancellationSignal`. When the turn is cancelled while the client's prompt
- *  is still open the signal aborts, the SDK sends `$/cancel_request`, and the
- *  client settles the request (a `cancelled` outcome or a rejection). Either
- *  way callers see the same "Tool use aborted" they already expect, so a
- *  cancelled dialog no longer leaves the `await` hanging. */
+// The cancellationSignal lets a turn cancel dismiss the client's open
+// dialog ($/cancel_request) instead of leaving this await hanging.
 async function requestPermissionFromClient(
   context: ToolHandlerContext,
   params: RequestPermissionRequest,

packages/agent/src/adapters/claude/permissions/permission-handlers.ts

@@ -76,10 +76,7 @@ export function supportsFastMode(modelId: string): boolean {
   return MODELS_WITH_FAST_MODE.has(modelId);
 }
 
-/** Map the SDK's tri-state `fast_mode_state` onto the boolean toggle.
- *  `cooldown` (fast mode temporarily suspended after a rate limit) keeps the
- *  toggle on so it reflects the user's intent — only an explicit `off` clears
- *  it. */
+// cooldown keeps the toggle on (user intent); only an explicit off clears it.
 export function fastModeStateEnabled(state: string | undefined): boolean {
   return state !== undefined && state !== "off";
 }

packages/agent/src/adapters/claude/session/models.ts

@@ -39,29 +39,13 @@ export type BackgroundTerminal =
       pendingOutput: TerminalOutputResponse;
     };
 
-/** One in-flight `prompt()` call. A persistent per-session consumer (see
- *  `runConsumer` in claude-agent.ts) drains the SDK query stream for the whole
- *  session and settles each Turn's deferred when that turn's outcome is known,
- *  so `prompt()` itself holds no loop. Turns are processed FIFO: the SDK
- *  echoes queued user messages back in submission order, so the first
- *  unsettled queue entry is the turn currently running. */
+/** One in-flight `prompt()` call, settled by the session's consumer. */
 export type Turn = {
-  /** uuid stamped on the pushed `SDKUserMessage`; the SDK echoes it back so
-   *  the consumer can match the replayed user message to this turn. */
   promptUuid: string;
-  /** Local-only slash commands (e.g. `/context`) return a result without an
-   *  echo, so the consumer can't promote them via the replay; it falls back
-   *  to promoting the queue head when the result arrives. */
   isLocalOnlyCommand: boolean;
-  /** Leading slash command of the prompt (e.g. "/foo"), if any. Drives the
-   *  unsupported-slash-command gate when idle arrives without an echo. */
   commandName?: string;
-  /** Mirrors the prompt's chunks to the feed/history. Invoked once, when the
-   *  turn activates, preserving the pre-consumer timing where a queued
-   *  prompt's broadcast fired when its turn took over the loop. */
+  /** Invoked once at activation, matching the pre-consumer broadcast timing. */
   broadcast: () => Promise<void>;
-  /** Set once the deferred has been resolved/rejected, so the consumer never
-   *  settles a turn twice (idle + handoff + stream-end can all race). */
   settled: boolean;
   resolve: (response: PromptResponse) => void;
   reject: (error: unknown) => void;
@@ -90,12 +74,9 @@ export type Session = BaseSession & {
   lastPlanFilePath?: string;
   lastPlanContent?: string;
   effort?: EffortLevel;
-  /** The user's Fast mode intent. Persists across model switches; the "fast"
-   *  config option is only surfaced while the selected model supports it. */
+  /** User intent; retained while a non-fast model hides the "fast" option. */
   fastModeEnabled: boolean;
-  /** Last session title pushed to the client via `session_info_update`. The
-   *  SDK auto-generates the title in a background task and persists it to the
-   *  session file; it is polled at turn-end and only pushed on change. */
+  /** Last title pushed via `session_info_update`, to dedupe turn-end polls. */
   lastTitle?: string;
   configOptions: SessionConfigOption[];
   accumulatedUsage: AccumulatedUsage;
@@ -110,30 +91,15 @@ export type Session = BaseSession & {
   contextSize?: number;
   /** Persists across prompt() calls so SDK-reported values survive turn boundaries */
   lastContextWindowSize?: number;
-  /** FIFO of in-flight prompts. The head is the turn the SDK is currently
-   *  processing; later entries are queued and will be echoed in order. */
+  /** FIFO of in-flight prompts; the SDK echoes them back in order. */
   turnQueue: Turn[];
-  /** The turn whose messages the consumer is currently attributing output to
-   *  (the head of `turnQueue` once its user message has been echoed). */
   activeTurn: Turn | null;
-  /** Count of result messages the consumer should treat as orphans and skip.
-   *  When cancel() settles+removes a queued turn, that turn's user message was
-   *  already pushed to the SDK, so the SDK still runs it and emits a result
-   *  with no uuid to match. The SDK processes input FIFO, so those orphan
-   *  results arrive before the next live turn's; skipping exactly this many
-   *  leaves the genuine head untouched. Reset to 0 on every activation. */
+  /** Echo-less results still owed by turns cancelled while queued. */
   pendingOrphanResults: number;
-  /** The long-lived consumer task. Lazily started on the first `prompt()` and
-   *  kept alive for the session so between-turn/background messages are still
-   *  drained and forwarded. */
   consumer?: Promise<void>;
-  /** Bumped by refreshSession before it swaps `query`/`input`, so the old
-   *  consumer (which captured the previous generation) exits quietly instead
-   *  of tearing down the refreshed session. */
+  /** Bumped by refreshSession so the retired consumer exits quietly. */
   queryGeneration: number;
-  /** Set once the SDK query stream has terminated (ran to `done` or threw).
-   *  The query iterator is not reusable afterward, so a later `prompt()`
-   *  rejects up front instead of enqueueing onto a dead stream and hanging. */
+  /** The query iterator ended and can't be revived; new prompts reject. */
   queryClosed?: boolean;
   cancelController?: AbortController;
   forceCancelTimer?: ReturnType<typeof setTimeout>;