diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts index 095dca85..af28ace0 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts @@ -779,4 +779,247 @@ describe('AcpMessageHandler', () => { expect(messages).toHaveLength(0); }); + + describe('tool_call_update content normalization (Gemini/OpenCode path)', () => { + it('unwraps text content block to string output', () => { + // Gemini sends content: [{type:'content', content:{type:'text', text:'...'}}] + // when the tool has stdout. HAPI must normalize this to a plain string. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'gem-1', + title: 'shell', + rawInput: { cmd: 'echo hello' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'gem-1', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: 'hello\n' } }] + }); + + const result = getToolResult(messages, 'gem-1'); + expect(result.status).toBe('completed'); + expect(result.output).toBe('hello\n'); + }); + + it('normalizes empty content array to empty string output', () => { + // Gemini sends content: [] when returnDisplay is falsy (no visible output). + // Raw [] must not be forwarded to the web renderer. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'gem-2', + title: 'shell', + rawInput: { cmd: 'touch file' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'gem-2', + status: 'completed', + content: [] + }); + + const result = getToolResult(messages, 'gem-2'); + expect(result.status).toBe('completed'); + expect(result.output).toBe(''); + }); + + it('preserves diff content block fields in output', () => { + // Gemini sends content: [{type:'diff', path, oldText, newText, _meta:{kind}}] + // for file-edit tools. HAPI must surface these fields intact. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'gem-3', + title: 'write_file', + rawInput: { path: 'src/foo.ts' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'gem-3', + status: 'completed', + content: [{ + type: 'diff', + path: 'src/foo.ts', + oldText: 'old content', + newText: 'new content', + _meta: { kind: 'modify' } + }] + }); + + const result = getToolResult(messages, 'gem-3'); + expect(result.status).toBe('completed'); + expect(result.output).toEqual({ + path: 'src/foo.ts', + oldText: 'old content', + newText: 'new content', + kind: 'modify' + }); + }); + + it('prefers rawOutput over content when both are present (regression guard)', () => { + // Claude/Codex always send rawOutput. If both fields arrive, rawOutput wins + // and the ACP content array is ignored to preserve existing behavior. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'reg-1', + title: 'Bash', + rawInput: { cmd: 'ls' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'reg-1', + status: 'completed', + rawOutput: { stdout: 'file.txt\n' }, + content: [{ type: 'content', content: { type: 'text', text: 'should be ignored' } }] + }); + + const result = getToolResult(messages, 'reg-1'); + expect(result.status).toBe('completed'); + expect(result.output).toEqual({ stdout: 'file.txt\n' }); + }); + + it('passes through non-array content value unchanged when rawOutput is absent', () => { + // If an ACP agent sends content as a non-array value (e.g. a plain string or + // object), normalizeAcpToolContent returns null and we fall back to the + // original content to avoid silent data loss. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'reg-2', + title: 'Bash', + rawInput: { cmd: 'ls' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'reg-2', + status: 'completed', + content: { stdout: 'file.txt\n' } + }); + + const result = getToolResult(messages, 'reg-2'); + expect(result.status).toBe('completed'); + expect(result.output).toEqual({ stdout: 'file.txt\n' }); + }); + + it('falls back to raw content for mixed text+diff array (null from normalizer)', () => { + // A mixed array [{type:'content',...}, {type:'diff',...}] cannot be safely + // collapsed into either a string or a single diff object without losing data. + // normalizeAcpToolContent must return null so the caller falls back to the + // original content array, preserving all information. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + const mixedContent = [ + { type: 'content', content: { type: 'text', text: 'some stdout' } }, + { type: 'diff', path: 'src/foo.ts', oldText: 'old', newText: 'new', _meta: { kind: 'modify' } } + ]; + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'mixed-1', + title: 'run_and_edit', + rawInput: { cmd: 'patch' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'mixed-1', + status: 'completed', + content: mixedContent + }); + + const result = getToolResult(messages, 'mixed-1'); + expect(result.status).toBe('completed'); + // Must fall back to original content array — no information loss + expect(result.output).toEqual(mixedContent); + }); + + it('falls back to raw content for multi-diff array (null from normalizer)', () => { + // Multiple diff blocks cannot be collapsed into a single diff object. + // normalizeAcpToolContent must return null so we keep the full array. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + const multiDiffContent = [ + { type: 'diff', path: 'a.ts', oldText: 'a-old', newText: 'a-new', _meta: { kind: 'modify' } }, + { type: 'diff', path: 'b.ts', oldText: 'b-old', newText: 'b-new', _meta: { kind: 'modify' } } + ]; + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'multidiff-1', + title: 'edit_files', + rawInput: { files: ['a.ts', 'b.ts'] }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'multidiff-1', + status: 'completed', + content: multiDiffContent + }); + + const result = getToolResult(messages, 'multidiff-1'); + expect(result.status).toBe('completed'); + // Must fall back to original content array — no information loss + expect(result.output).toEqual(multiDiffContent); + }); + + it('falls back to raw content for unknown block type (null from normalizer)', () => { + // An unrecognized block type (e.g. {type:'image',...}) cannot be safely + // normalized. We must return null and let the caller fall back to the original + // content to avoid silent data loss. + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + + const unknownContent = [ + { type: 'image', url: 'https://example.com/screenshot.png' } + ]; + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'unknown-1', + title: 'screenshot', + rawInput: { url: 'https://example.com' }, + status: 'in_progress' + }); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCallUpdate, + toolCallId: 'unknown-1', + status: 'completed', + content: unknownContent + }); + + const result = getToolResult(messages, 'unknown-1'); + expect(result.status).toBe('completed'); + // Must fall back to original content array — no information loss + expect(result.output).toEqual(unknownContent); + }); + }); }); diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.ts b/cli/src/agent/backends/acp/AcpMessageHandler.ts index c3f934c3..4feec31b 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.ts @@ -70,6 +70,77 @@ function extractAudienceField(value: unknown): string[] { return value.filter((entry): entry is string => typeof entry === 'string'); } +/** + * Normalizes the ACP `tool_call_update` content array sent by agents (e.g. + * Gemini, OpenCode) that do not populate `rawOutput`. + * + * ACP ToolCallContent union (as emitted by Gemini CLI): + * - `{type:'content', content:{type:'text', text:…}}` — tool stdout/stderr + * - `{type:'diff', path, oldText, newText, _meta:{kind}}` — file edits + * + * Only normalizes unambiguous cases. Returns null for anything that cannot be + * safely collapsed without losing information, so the caller can fall back to + * the original content value. + * + * Returns: + * - string — concatenated text from a pure-text-block array + * - object — structured diff from a single-diff-block array + * - "" — empty string when content array is empty (no visible output) + * - null — non-array, mixed types, multiple diffs, or unknown block type; + * caller should pass through the original value unchanged + */ +function normalizeAcpToolContent(content: unknown): string | object | null { + if (!Array.isArray(content)) { + return null; + } + // Empty array: no display output from the agent (e.g. touch, silent command) + if (content.length === 0) { + return ''; + } + // Classify every block. If any block has an unrecognized type or the array + // contains a mix of text and diff blocks we cannot collapse losslessly, so + // return null and let the caller fall back to the original content array. + let diffCount = 0; + let textCount = 0; + const parts: string[] = []; + let diffBlock: object | null = null; + + for (const block of content) { + if (!isObject(block)) { + return null; // Non-object element — unrecognized + } + if (block.type === 'diff') { + diffCount++; + if (diffCount > 1) { + return null; // Multiple diffs cannot be merged into one object + } + diffBlock = { + path: typeof block.path === 'string' ? block.path : undefined, + oldText: typeof block.oldText === 'string' ? block.oldText : undefined, + newText: typeof block.newText === 'string' ? block.newText : undefined, + kind: isObject(block._meta) && typeof block._meta.kind === 'string' ? block._meta.kind : undefined + }; + } else if (block.type === 'content' && isObject(block.content)) { + const inner = block.content; + if (inner.type === 'text' && typeof inner.text === 'string') { + textCount++; + parts.push(inner.text); + } else { + return null; // Unknown inner content type (e.g. image, resource) + } + } else { + return null; // Unknown top-level block type + } + } + + // Mixed text + diff: cannot represent as a single value without losing data + if (diffCount > 0 && textCount > 0) { + return null; + } + + return diffBlock ?? parts.join(''); +} + function normalizePlanEntries(entries: unknown): PlanItem[] { if (!Array.isArray(entries)) return []; @@ -297,11 +368,21 @@ export class AcpMessageHandler { } if (status === 'completed' || status === 'failed') { - const result = update.rawOutput ?? update.content; + // Prefer rawOutput (Claude/Codex path). When absent, normalize the + // ACP content array sent by agents such as Gemini and OpenCode. + // If content is not an array (normalizeAcpToolContent returns null), + // fall back to the original content value to avoid silent data loss. + let output: unknown; + if (update.rawOutput !== undefined) { + output = update.rawOutput; + } else { + const normalized = normalizeAcpToolContent(update.content); + output = normalized !== null ? normalized : update.content; + } this.onMessage({ type: 'tool_result', id: toolCallId, - output: result, + output, status: status === 'failed' ? 'failed' : 'completed' }); }