feat(acp/core): prefix tool call IDs with tool names to support tool rendering in ACP compliant IDEs. (#26676)

This commit is contained in:
Sri Pasumarthi
2026-05-08 14:21:54 -07:00
committed by GitHub
parent 90e7155971
commit 1238dcfe91
4 changed files with 186 additions and 11 deletions
+124
View File
@@ -18,6 +18,7 @@ import {
StreamEventType, StreamEventType,
SYNTHETIC_THOUGHT_SIGNATURE, SYNTHETIC_THOUGHT_SIGNATURE,
type StreamEvent, type StreamEvent,
stripToolCallIdPrefixes,
} from './geminiChat.js'; } from './geminiChat.js';
import { import {
type CompletedToolCall, type CompletedToolCall,
@@ -2969,4 +2970,127 @@ describe('GeminiChat', () => {
expect(curatedHistory[1].parts![0].fileData).toBeDefined(); expect(curatedHistory[1].parts![0].fileData).toBeDefined();
}); });
}); });
describe('stripToolCallIdPrefixes', () => {
it('should strip tool name prefix matching the tool name', () => {
const contents: Content[] = [
{
role: 'model',
parts: [
{
functionCall: {
id: 'my_tool__call_123',
name: 'my_tool',
args: {},
},
},
],
},
{
role: 'user',
parts: [
{
functionResponse: {
id: 'my_tool__call_123',
name: 'my_tool',
response: { result: 'success' },
},
},
],
},
];
const stripped = stripToolCallIdPrefixes(contents);
expect(stripped[0].parts![0].functionCall!.id).toBe('call_123');
expect(stripped[1].parts![0].functionResponse!.id).toBe('call_123');
});
it('should correctly handle tool names that contain double underscores', () => {
const contents: Content[] = [
{
role: 'model',
parts: [
{
functionCall: {
id: 'my__custom__tool__call_abc',
name: 'my__custom__tool',
args: {},
},
},
],
},
{
role: 'user',
parts: [
{
functionResponse: {
id: 'my__custom__tool__call_abc',
name: 'my__custom__tool',
response: { result: 'success' },
},
},
],
},
];
const stripped = stripToolCallIdPrefixes(contents);
expect(stripped[0].parts![0].functionCall!.id).toBe('call_abc');
expect(stripped[1].parts![0].functionResponse!.id).toBe('call_abc');
});
it('should not strip if prefix does not match the tool name', () => {
const contents: Content[] = [
{
role: 'model',
parts: [
{
functionCall: {
id: 'other_tool__call_123',
name: 'my_tool',
args: {},
},
},
],
},
];
const stripped = stripToolCallIdPrefixes(contents);
expect(stripped[0].parts![0].functionCall!.id).toBe(
'other_tool__call_123',
);
});
it('should correctly handle fallback to generic_tool when name is missing or has whitespace', () => {
const contents: Content[] = [
{
role: 'model',
parts: [
{
functionCall: {
id: 'generic_tool__call_123',
name: ' ',
args: {},
},
},
],
},
{
role: 'user',
parts: [
{
functionResponse: {
id: 'generic_tool__call_123',
name: undefined as unknown as string,
response: { result: 'success' },
},
},
],
},
];
const stripped = stripToolCallIdPrefixes(contents);
expect(stripped[0].parts![0].functionCall!.id).toBe('call_123');
expect(stripped[1].parts![0].functionResponse!.id).toBe('call_123');
});
});
}); });
+45 -1
View File
@@ -746,10 +746,12 @@ export class GeminiChat {
lastConfig = config; lastConfig = config;
lastContentsToUse = contentsToUse; lastContentsToUse = contentsToUse;
const finalContents = stripToolCallIdPrefixes(contentsToUse);
return this.context.config.getContentGenerator().generateContentStream( return this.context.config.getContentGenerator().generateContentStream(
{ {
model: modelToUse, model: modelToUse,
contents: contentsToUse, contents: finalContents,
config, config,
}, },
prompt_id, prompt_id,
@@ -1016,10 +1018,20 @@ export class GeminiChat {
} }
fnCall.id = id; fnCall.id = id;
} }
const name = fnCall.name?.trim() || 'generic_tool';
if (fnCall.id && !fnCall.id.startsWith(`${name}__`)) {
fnCall.id = `${name}__${fnCall.id}`;
}
finalFunctionCallsMap.set(fnCall.id, fnCall); finalFunctionCallsMap.set(fnCall.id, fnCall);
} }
runningFunctionCallCounter += chunk.functionCalls.length; runningFunctionCallCounter += chunk.functionCalls.length;
} else { } else {
for (const fnCall of chunk.functionCalls) {
const name = fnCall.name?.trim() || 'generic_tool';
if (fnCall.id && !fnCall.id.startsWith(`${name}__`)) {
fnCall.id = `${name}__${fnCall.id}`;
}
}
legacyFunctionCalls.push(...chunk.functionCalls); legacyFunctionCalls.push(...chunk.functionCalls);
} }
} }
@@ -1288,3 +1300,35 @@ export function isSchemaDepthError(errorMessage: string): boolean {
export function isInvalidArgumentError(errorMessage: string): boolean { export function isInvalidArgumentError(errorMessage: string): boolean {
return errorMessage.includes('Request contains an invalid argument'); return errorMessage.includes('Request contains an invalid argument');
} }
export function stripToolCallIdPrefixes(contents: Content[]): Content[] {
return contents.map((content) => ({
...content,
parts: (content.parts || []).map((part) => {
const newPart = { ...part };
if (newPart.functionCall) {
const fc = newPart.functionCall;
const name = fc.name?.trim() || 'generic_tool';
if (fc.id && fc.id.startsWith(`${name}__`)) {
newPart.functionCall = {
name: fc.name,
args: fc.args,
id: fc.id.substring(name.length + 2),
};
}
}
if (newPart.functionResponse) {
const fr = newPart.functionResponse;
const name = fr.name?.trim() || 'generic_tool';
if (fr.id && fr.id.startsWith(`${name}__`)) {
newPart.functionResponse = {
name: fr.name,
response: fr.response,
id: fr.id.substring(name.length + 2),
};
}
}
return newPart;
}),
}));
}
+8 -8
View File
@@ -172,7 +172,7 @@ describe('Turn', () => {
expect(event1.type).toBe(GeminiEventType.ToolCallRequest); expect(event1.type).toBe(GeminiEventType.ToolCallRequest);
expect(event1.value).toEqual( expect(event1.value).toEqual(
expect.objectContaining({ expect.objectContaining({
callId: 'fc1', callId: 'tool1__fc1',
name: 'tool1', name: 'tool1',
args: { arg1: 'val1' }, args: { arg1: 'val1' },
isClientInitiated: false, isClientInitiated: false,
@@ -190,7 +190,7 @@ describe('Turn', () => {
}), }),
); );
expect(event2.value.callId).toEqual( expect(event2.value.callId).toEqual(
expect.stringMatching(/^tool2_\d{13}_\d+$/), expect.stringMatching(/^tool2__tool2_\d{13}_\d+$/),
); );
expect(turn.pendingToolCalls[1]).toEqual(event2.value); expect(turn.pendingToolCalls[1]).toEqual(event2.value);
expect(turn.getDebugResponses().length).toBe(1); expect(turn.getDebugResponses().length).toBe(1);
@@ -326,22 +326,22 @@ describe('Turn', () => {
// Assertions for each specific tool call event // Assertions for each specific tool call event
const event1 = events[0] as ServerGeminiToolCallRequestEvent; const event1 = events[0] as ServerGeminiToolCallRequestEvent;
expect(event1.value).toMatchObject({ expect(event1.value).toMatchObject({
callId: 'fc1', callId: 'generic_tool__fc1',
name: 'undefined_tool_name', name: 'generic_tool',
args: { arg1: 'val1' }, args: { arg1: 'val1' },
}); });
const event2 = events[1] as ServerGeminiToolCallRequestEvent; const event2 = events[1] as ServerGeminiToolCallRequestEvent;
expect(event2.value).toMatchObject({ expect(event2.value).toMatchObject({
callId: 'fc2', callId: 'tool2__fc2',
name: 'tool2', name: 'tool2',
args: {}, args: {},
}); });
const event3 = events[2] as ServerGeminiToolCallRequestEvent; const event3 = events[2] as ServerGeminiToolCallRequestEvent;
expect(event3.value).toMatchObject({ expect(event3.value).toMatchObject({
callId: 'fc3', callId: 'generic_tool__fc3',
name: 'undefined_tool_name', name: 'generic_tool',
args: {}, args: {},
}); });
}); });
@@ -858,7 +858,7 @@ describe('Turn', () => {
expect(toolCallEvent).toMatchObject({ expect(toolCallEvent).toMatchObject({
type: GeminiEventType.ToolCallRequest, type: GeminiEventType.ToolCallRequest,
value: expect.objectContaining({ value: expect.objectContaining({
callId: 'fc1', callId: 'ReadFile__fc1',
name: 'ReadFile', name: 'ReadFile',
args: { path: 'file.txt' }, args: { path: 'file.txt' },
}), }),
+9 -2
View File
@@ -408,15 +408,22 @@ export class Turn {
fnCall: FunctionCall, fnCall: FunctionCall,
traceId?: string, traceId?: string,
): ServerGeminiStreamEvent | null { ): ServerGeminiStreamEvent | null {
const name = fnCall.name || 'undefined_tool_name'; const name = fnCall.name?.trim() || 'generic_tool';
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const args = (fnCall.args as Record<string, unknown>) || {}; const args = (fnCall.args as Record<string, unknown>) || {};
const callId = const rawCallId =
fnCall.id ?? fnCall.id ??
(this.chat.context.config.isContextManagementEnabled() (this.chat.context.config.isContextManagementEnabled()
? `synth_${this.prompt_id}_${Date.now()}_${this.callCounter++}` ? `synth_${this.prompt_id}_${Date.now()}_${this.callCounter++}`
: `${name}_${Date.now()}_${this.callCounter++}`); : `${name}_${Date.now()}_${this.callCounter++}`);
const callId = rawCallId.startsWith(`${name}__`)
? rawCallId
: `${name}__${rawCallId}`;
// Mutate the function call object ID so that history consolidation inherits it
fnCall.id = callId;
const tool = this.chat.loopContext.toolRegistry.getTool(name); const tool = this.chat.loopContext.toolRegistry.getTool(name);
let display; let display;
if (tool) { if (tool) {