mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
fix(acp): normalize tool_call_update content for agents without rawOutput (#521)
This commit is contained in:
Junmo Kim
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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'
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user