diff --git a/src/servers/sidecar/claude/stream-parser.test.ts b/src/servers/sidecar/claude/stream-parser.test.ts index 641e02cd..ae23fe7c 100644 --- a/src/servers/sidecar/claude/stream-parser.test.ts +++ b/src/servers/sidecar/claude/stream-parser.test.ts @@ -407,3 +407,56 @@ describe('parseStream', () => { expect(state.gotResult).toBe(true); }); }); + +describe('an answered question is not a failed tool call', () => { + test("the denial that carries the user's answer does not reach the transcript as an error", () => { + const events: ChatEvent[] = []; + const state = createParseState(); + const onEvent = (e: ChatEvent) => events.push(e); + + processLine( + JSON.stringify({ + type: 'assistant', + message: { content: [{ type: 'tool_use', id: 'toolu_q1', name: 'AskUserQuestion', input: {} }] }, + }), + state, + { onEvent, onSessionId: () => {} }, + ); + processLine( + JSON.stringify({ + type: 'user', + message: { content: [{ type: 'tool_result', tool_use_id: 'toolu_q1', content: 'Teal', is_error: true }] }, + }), + state, + { onEvent, onSessionId: () => {} }, + ); + + const result = events.find((e) => e.type === 'tool:result'); + expect(result).toMatchObject({ toolCallId: 'toolu_q1', output: 'Teal', isError: false }); + }); + + test('while an ordinary tool that really failed keeps its flag', () => { + const events: ChatEvent[] = []; + const state = createParseState(); + const onEvent = (e: ChatEvent) => events.push(e); + + processLine( + JSON.stringify({ + type: 'assistant', + message: { content: [{ type: 'tool_use', id: 'toolu_b1', name: 'Bash', input: {} }] }, + }), + state, + { onEvent, onSessionId: () => {} }, + ); + processLine( + JSON.stringify({ + type: 'user', + message: { content: [{ type: 'tool_result', tool_use_id: 'toolu_b1', content: 'no such file', is_error: true }] }, + }), + state, + { onEvent, onSessionId: () => {} }, + ); + + expect(events.find((e) => e.type === 'tool:result')).toMatchObject({ isError: true }); + }); +}); diff --git a/src/servers/sidecar/claude/stream-parser.ts b/src/servers/sidecar/claude/stream-parser.ts index d3ebdc99..6eeb4a66 100644 --- a/src/servers/sidecar/claude/stream-parser.ts +++ b/src/servers/sidecar/claude/stream-parser.ts @@ -24,8 +24,20 @@ type ParseState = { */ textBuffers: Map; gotResult: boolean; + /** + * Tool call ids belonging to questions the user is being asked. + * + * Kept so their results can be un-flagged: the sidecar answers `AskUserQuestion` by DENYING it with the + * user's words (the only hook decision that can carry a result), and the harness stamps every denied tool + * `is_error`. So a perfectly good answer arrives looking like a failed tool call — red in the transcript, + * and read by the agent as something that went wrong rather than as the reply to its own question. + */ + questionCallIds: Set; }; +/** Both harnesses ask the human the same way and name the tool differently. */ +const QUESTION_TOOLS = new Set(['question', 'AskUserQuestion']); + /** '' when a message is the main agent's own. Keys the buffer map and stamps every event we emit. */ function parentOf(msg: Record): string { const id = msg.parent_tool_use_id; @@ -68,6 +80,7 @@ function handleAssistant(msg: Record, state: ParseState, onEven state.textBuffers.delete(parent); } else if (block.type === 'tool_use') { flushTextBuffer(state, parent, onEvent); + if (QUESTION_TOOLS.has((block.name as string) ?? '')) state.questionCallIds.add((block.id as string) ?? ''); onEvent({ type: 'tool:start', toolCallId: (block.id as string) ?? '', @@ -79,7 +92,7 @@ function handleAssistant(msg: Record, state: ParseState, onEven } } -function handleUser(msg: Record, onEvent: (event: ChatEvent) => void): void { +function handleUser(msg: Record, state: ParseState, onEvent: (event: ChatEvent) => void): void { const message = msg.message as Record | undefined; const content = message?.content as Array> | undefined; if (!Array.isArray(content)) return; @@ -97,13 +110,14 @@ function handleUser(msg: Record, onEvent: (event: ChatEvent) => .map((c) => c.text as string) .join('\n'); } - onEvent({ - type: 'tool:result', - toolCallId: (block.tool_use_id as string) ?? '', - output, - isError: (block.is_error as boolean) ?? false, - ...parented(parent), - }); + const toolCallId = (block.tool_use_id as string) ?? ''; + // An answered question is not a failed tool call. See `questionCallIds`: the answer channel is a + // denial, so the flag says only that the tool was prevented from resolving itself — which is the + // mechanism working, not a fault. A question that timed out or lost its session lands here too, and + // is equally not an error: its output is a sentence explaining that, which the agent should read as + // information rather than as something to retry. + const isError = state.questionCallIds.has(toolCallId) ? false : ((block.is_error as boolean) ?? false); + onEvent({ type: 'tool:result', toolCallId, output, isError, ...parented(parent) }); } } } @@ -208,7 +222,7 @@ export function processMessage( handleAssistant(msg, state, callbacks.onEvent); break; case 'user': - handleUser(msg, callbacks.onEvent); + handleUser(msg, state, callbacks.onEvent); break; case 'system': handleSystem(msg, callbacks); @@ -244,7 +258,7 @@ export function processLine(line: string, state: ParseState, callbacks: StreamPa * Create a fresh parse state for a new stream. */ export function createParseState(): ParseState { - return { textBuffers: new Map(), gotResult: false }; + return { textBuffers: new Map(), gotResult: false, questionCallIds: new Set() }; } /**