From 1238dcfe91c8b4ae90f759a2ac87d773c69d507d Mon Sep 17 00:00:00 2001 From: Sri Pasumarthi <111310667+sripasg@users.noreply.github.com> Date: Fri, 8 May 2026 14:21:54 -0700 Subject: [PATCH] feat(acp/core): prefix tool call IDs with tool names to support tool rendering in ACP compliant IDEs. (#26676) --- packages/core/src/core/geminiChat.test.ts | 124 ++++++++++++++++++++++ packages/core/src/core/geminiChat.ts | 46 +++++++- packages/core/src/core/turn.test.ts | 16 +-- packages/core/src/core/turn.ts | 11 +- 4 files changed, 186 insertions(+), 11 deletions(-) diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 8bf002a3c8..4f4cce3b54 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -18,6 +18,7 @@ import { StreamEventType, SYNTHETIC_THOUGHT_SIGNATURE, type StreamEvent, + stripToolCallIdPrefixes, } from './geminiChat.js'; import { type CompletedToolCall, @@ -2969,4 +2970,127 @@ describe('GeminiChat', () => { 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'); + }); + }); }); diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 174c766337..3f3cbe6d65 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -746,10 +746,12 @@ export class GeminiChat { lastConfig = config; lastContentsToUse = contentsToUse; + const finalContents = stripToolCallIdPrefixes(contentsToUse); + return this.context.config.getContentGenerator().generateContentStream( { model: modelToUse, - contents: contentsToUse, + contents: finalContents, config, }, prompt_id, @@ -1016,10 +1018,20 @@ export class GeminiChat { } 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); } runningFunctionCallCounter += chunk.functionCalls.length; } 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); } } @@ -1288,3 +1300,35 @@ export function isSchemaDepthError(errorMessage: string): boolean { export function isInvalidArgumentError(errorMessage: string): boolean { 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; + }), + })); +} diff --git a/packages/core/src/core/turn.test.ts b/packages/core/src/core/turn.test.ts index 9a9fce2f75..be94945476 100644 --- a/packages/core/src/core/turn.test.ts +++ b/packages/core/src/core/turn.test.ts @@ -172,7 +172,7 @@ describe('Turn', () => { expect(event1.type).toBe(GeminiEventType.ToolCallRequest); expect(event1.value).toEqual( expect.objectContaining({ - callId: 'fc1', + callId: 'tool1__fc1', name: 'tool1', args: { arg1: 'val1' }, isClientInitiated: false, @@ -190,7 +190,7 @@ describe('Turn', () => { }), ); 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.getDebugResponses().length).toBe(1); @@ -326,22 +326,22 @@ describe('Turn', () => { // Assertions for each specific tool call event const event1 = events[0] as ServerGeminiToolCallRequestEvent; expect(event1.value).toMatchObject({ - callId: 'fc1', - name: 'undefined_tool_name', + callId: 'generic_tool__fc1', + name: 'generic_tool', args: { arg1: 'val1' }, }); const event2 = events[1] as ServerGeminiToolCallRequestEvent; expect(event2.value).toMatchObject({ - callId: 'fc2', + callId: 'tool2__fc2', name: 'tool2', args: {}, }); const event3 = events[2] as ServerGeminiToolCallRequestEvent; expect(event3.value).toMatchObject({ - callId: 'fc3', - name: 'undefined_tool_name', + callId: 'generic_tool__fc3', + name: 'generic_tool', args: {}, }); }); @@ -858,7 +858,7 @@ describe('Turn', () => { expect(toolCallEvent).toMatchObject({ type: GeminiEventType.ToolCallRequest, value: expect.objectContaining({ - callId: 'fc1', + callId: 'ReadFile__fc1', name: 'ReadFile', args: { path: 'file.txt' }, }), diff --git a/packages/core/src/core/turn.ts b/packages/core/src/core/turn.ts index 13ea724569..cc5335981a 100644 --- a/packages/core/src/core/turn.ts +++ b/packages/core/src/core/turn.ts @@ -408,15 +408,22 @@ export class Turn { fnCall: FunctionCall, traceId?: string, ): 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 const args = (fnCall.args as Record) || {}; - const callId = + const rawCallId = fnCall.id ?? (this.chat.context.config.isContextManagementEnabled() ? `synth_${this.prompt_id}_${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); let display; if (tool) {