fix(acp): move tool explanation from thought stream to tool call content (#26554)

This commit is contained in:
Sri Pasumarthi
2026-05-06 08:42:01 -07:00
committed by GitHub
parent 80e091a8e1
commit 97a2bd7507
2 changed files with 135 additions and 7 deletions
+121
View File
@@ -586,4 +586,125 @@ describe('Session', () => {
},
});
});
it('should add explanation to tool call content instead of thought chunk', async () => {
mockTool.build.mockReturnValue({
getDescription: () => 'Test Tool',
getExplanation: () => 'Test Explanation',
toolLocations: () => [],
shouldConfirmExecute: vi
.fn()
.mockResolvedValue({ type: 'info', onConfirm: vi.fn() }),
execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }),
});
mockConnection.requestPermission.mockResolvedValue({
outcome: {
outcome: 'selected',
optionId: 'proceed_once',
},
});
const stream1 = createMockStream([
{
type: GeminiEventType.ToolCallRequest,
value: {
callId: 'call-1',
name: 'test_tool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-1',
},
},
]);
const stream2 = createMockStream([
{
type: GeminiEventType.Content,
value: '',
},
]);
mockSendMessageStream
.mockReturnValueOnce(stream1)
.mockReturnValueOnce(stream2);
await session.prompt({
sessionId: 'session-1',
prompt: [{ type: 'text', text: 'Call tool' }],
});
expect(mockConnection.sessionUpdate).not.toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'agent_thought_chunk',
content: { type: 'text', text: 'Test Explanation' },
}),
}),
);
expect(mockConnection.requestPermission).toHaveBeenCalledWith(
expect.objectContaining({
toolCall: expect.objectContaining({
content: expect.arrayContaining([
{
type: 'content',
content: { type: 'text', text: 'Test Explanation' },
},
]),
}),
}),
);
});
it('should add explanation to tool_call update content instead of thought chunk when no permission required', async () => {
mockTool.build.mockReturnValue({
getDescription: () => 'Test Tool',
getExplanation: () => 'Test Explanation',
toolLocations: () => [],
shouldConfirmExecute: vi.fn().mockResolvedValue(null),
execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }),
});
const stream1 = createMockStream([
{
type: GeminiEventType.ToolCallRequest,
value: {
callId: 'call-1',
name: 'test_tool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-1',
},
},
]);
const stream2 = createMockStream([
{
type: GeminiEventType.Content,
value: '',
},
]);
mockSendMessageStream
.mockReturnValueOnce(stream1)
.mockReturnValueOnce(stream2);
await session.prompt({
sessionId: 'session-1',
prompt: [{ type: 'text', text: 'Call tool' }],
});
expect(mockConnection.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call',
content: expect.arrayContaining([
{
type: 'content',
content: { type: 'text', text: 'Test Explanation' },
},
]),
}),
}),
);
});
});
+14 -7
View File
@@ -619,13 +619,6 @@ export class Session {
? invocation.getExplanation()
: '';
if (explanation) {
await this.sendUpdate({
sessionUpdate: 'agent_thought_chunk',
content: { type: 'text', text: explanation },
});
}
const confirmationDetails =
await invocation.shouldConfirmExecute(abortSignal);
@@ -648,6 +641,13 @@ export class Session {
});
}
if (content.length === 0 && explanation) {
content.push({
type: 'content',
content: { type: 'text', text: explanation },
});
}
const params: acp.RequestPermissionRequest = {
sessionId: this.id,
options: toPermissionOptions(
@@ -708,6 +708,13 @@ export class Session {
} else {
const content: acp.ToolCallContent[] = [];
if (explanation) {
content.push({
type: 'content',
content: { type: 'text', text: explanation },
});
}
await this.sendUpdate({
sessionUpdate: 'tool_call',
toolCallId: callId,