From 86e862d4a24215f14b6e128cbf591d7d8c09dac9 Mon Sep 17 00:00:00 2001 From: Luis Felipe Quevedo Date: Fri, 2 Oct 2026 13:28:36 +0000 Subject: [PATCH 1/5] fix(core): enforce terminal user turn invariant and normalize request contents Ensure outgoing conversation contents dispatched to generateContentStream satisfy the Gemini API contract requiring requests to end with a valid user turn containing non-empty parts. - In GeminiChat, normalize trailing turns after prefix stripping by synthesizing paired responses for dangling tool calls and appending a continuation turn when history terminates on a model turn or empty turn. - In GeminiChat, verify role equality during turn deduplication to ensure identical user prompt text following a model turn is retained. - In stripToolCallIdPrefixes, preserve nested functionResponse parts and filter turns whose parts array becomes empty after stripping. - In useGeminiStream, avoid updating historyLengthAfterUserPromptRef during stream continuation turns to maintain correct rollback slicing. - In RewindViewer, filter synthetic tool response messages from the rewind interaction selector. - Add comprehensive regression tests in geminiChat.test.ts. Closes #29530 --- .../cli/src/ui/components/RewindViewer.tsx | 18 +- packages/cli/src/ui/hooks/useGeminiStream.ts | 2 +- packages/core/src/core/geminiChat.test.ts | 326 ++++++++++++++++++ packages/core/src/core/geminiChat.ts | 170 ++++++--- 4 files changed, 464 insertions(+), 52 deletions(-) diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index e77b17db32f..fb1ad699624 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -69,7 +69,23 @@ export const RewindViewer: React.FC = ({ ); const interactions = useMemo( - () => conversation.messages.filter((msg) => msg.type === 'user'), + () => + conversation.messages.filter((msg) => { + if (msg.type !== 'user') return false; + const content = msg.content; + const parts = Array.isArray(content) + ? content + : content !== undefined && content !== null + ? [content] + : []; + const isToolResponse = + parts.length > 0 && + parts.every( + (p) => + typeof p === 'object' && p !== null && 'functionResponse' in p, + ); + return !isToolResponse; + }), [conversation.messages], ); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index ee7c7fa2967..a561011c6f4 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -1739,7 +1739,7 @@ export const useGeminiStream = ( return; } - if (geminiClient) { + if (geminiClient && !options?.isContinuation) { historyLengthAfterUserPromptRef.current = geminiClient.getHistory().length; } diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 5bda3dab871..9a5d98d47b3 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -23,6 +23,7 @@ import { stripToolCallIdPrefixes, type HistoryTurn, coalesceConsecutiveRoles, + INTERRUPTED_RESPONSE_PLACEHOLDER, stripThoughts, THINKING_ONLY_NUDGE_MESSAGE, NO_RESPONSE_TEXT_NUDGE_MESSAGE, @@ -4760,6 +4761,47 @@ describe('GeminiChat', () => { expect(stripped[0].parts![0].functionCall!.id).toBe('call_123'); expect(stripped[1].parts![0].functionResponse!.id).toBe('call_123'); }); + + it('should preserve functionResponse parts when stripping prefix', () => { + const contents: Content[] = [ + { + role: 'user', + parts: [ + { + functionResponse: { + id: 'my_tool__call_123', + name: 'my_tool', + response: { result: 'success' }, + parts: [{ inlineData: { mimeType: 'image/png', data: 'abc' } }], + }, + }, + ], + }, + ]; + + const stripped = stripToolCallIdPrefixes(contents); + expect(stripped[0].parts![0].functionResponse!.id).toBe('call_123'); + expect(stripped[0].parts![0].functionResponse!.parts).toEqual([ + { inlineData: { mimeType: 'image/png', data: 'abc' } }, + ]); + }); + + it('should remove turns whose parts become empty after removing empty text parts', () => { + const contents: Content[] = [ + { + role: 'user', + parts: [{ text: 'valid message' }], + }, + { + role: 'user', + parts: [{ text: '' }], + }, + ]; + + const stripped = stripToolCallIdPrefixes(contents); + expect(stripped).toHaveLength(1); + expect(stripped[0].parts).toEqual([{ text: 'valid message' }]); + }); }); describe('coalesceConsecutiveRoles', () => { @@ -5076,4 +5118,288 @@ describe('GeminiChat', () => { expect(result).toEqual(contents); }); }); + + describe('Terminal user turn invariant enforcement and request contents integrity', () => { + it('should ensure request contents end with a valid user turn when history ends with a model turn after rewind and trailing turn thoughts are stripped', async () => { + chat.setHistory([ + { role: 'user', parts: [{ text: 'Read package.json' }] }, + { + role: 'model', + parts: [{ text: 'Here is the summary of package.json.' }], + }, + ]); + + let capturedContents: Content[] | undefined; + vi.mocked(mockContentGenerator.generateContentStream).mockImplementation( + async (params) => { + capturedContents = params.contents as Content[]; + return (async function* (): AsyncGenerator { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'Response' }], + }, + finishReason: 'STOP' as unknown as undefined, + }, + ], + } as unknown as GenerateContentResponse; + })(); + }, + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.5-pro' }, + [{ text: 'internal reasoning only', thought: true } as Part], + 'prompt-after-rewind', + new AbortController().signal, + LlmRole.MAIN, + ); + + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toBeDefined(); + const lastContent = capturedContents![capturedContents!.length - 1]; + expect(lastContent.role).toBe('user'); + expect(lastContent.parts?.length).toBeGreaterThan(0); + }); + + it('should ensure request contents end with a valid user turn after interrupted tool turn closure', async () => { + chat.setHistory([ + { role: 'user', parts: [{ text: 'Search for files' }] }, + { + role: 'model', + parts: [ + { + functionCall: { + id: 'call_1', + name: 'grep_search', + args: { query: 'files' }, + }, + thoughtSignature: 'skip_thought_signature_validator', + }, + ], + }, + { + role: 'user', + parts: [ + { + functionResponse: { + id: 'call_1', + name: 'grep_search', + response: { output: 'file.txt' }, + }, + }, + ], + }, + ]); + + let capturedContents: Content[] | undefined; + vi.mocked(mockContentGenerator.generateContentStream).mockImplementation( + async (params) => { + capturedContents = params.contents as Content[]; + return (async function* (): AsyncGenerator { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'Response' }], + }, + finishReason: 'STOP' as unknown as undefined, + }, + ], + } as unknown as GenerateContentResponse; + })(); + }, + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.5-pro' }, + [{ text: '' }], + 'prompt-after-interrupt', + new AbortController().signal, + LlmRole.MAIN, + ); + + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toBeDefined(); + const lastContent = capturedContents![capturedContents!.length - 1]; + expect(lastContent.role).toBe('user'); + expect(lastContent.parts?.length).toBeGreaterThan(0); + expect( + capturedContents!.some((c) => + c.parts?.some((p) => p.text === INTERRUPTED_RESPONSE_PLACEHOLDER), + ), + ).toBe(true); + }); + + it('should not skip recording user turn when context management is enabled and preceding turn is a model turn with matching text', async () => { + vi.mocked(mockConfig.isContextManagementEnabled).mockReturnValue(true); + + const turns: HistoryTurn[] = [ + { id: 'u1', content: { role: 'user', parts: [{ text: 'Say yes' }] } }, + { id: 'm1', content: { role: 'model', parts: [{ text: 'yes' }] } }, + ]; + chat.setHistory(turns); + + let capturedContents: Content[] | undefined; + vi.mocked(mockContentGenerator.generateContentStream).mockImplementation( + async (params) => { + capturedContents = params.contents as Content[]; + return (async function* (): AsyncGenerator { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'Response' }], + }, + finishReason: 'STOP' as unknown as undefined, + }, + ], + } as unknown as GenerateContentResponse; + })(); + }, + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.5-pro' }, + 'yes', + 'prompt-cm-dedup', + new AbortController().signal, + LlmRole.MAIN, + ); + + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toBeDefined(); + const lastContent = capturedContents![capturedContents!.length - 1]; + expect(lastContent.role).toBe('user'); + expect(lastContent.parts).toEqual([{ text: 'yes' }]); + }); + + it('should ensure request contents end with a valid user turn when apiHistoryOverride has a thought-only trailing turn', async () => { + const apiHistoryOverride: Content[] = [ + { role: 'user', parts: [{ text: 'Initial question' }] }, + { role: 'model', parts: [{ text: 'Initial answer' }] }, + { + role: 'user', + parts: [{ text: 'internal thought only', thought: true } as Part], + }, + ]; + + let capturedContents: Content[] | undefined; + vi.mocked(mockContentGenerator.generateContentStream).mockImplementation( + async (params) => { + capturedContents = params.contents as Content[]; + return (async function* (): AsyncGenerator { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'Response' }], + }, + finishReason: 'STOP' as unknown as undefined, + }, + ], + } as unknown as GenerateContentResponse; + })(); + }, + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.5-pro' }, + 'Follow-up question', + 'prompt-override', + new AbortController().signal, + LlmRole.MAIN, + undefined, + apiHistoryOverride, + ); + + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toBeDefined(); + const lastContent = capturedContents![capturedContents!.length - 1]; + expect(lastContent.role).toBe('user'); + expect(lastContent.parts?.length).toBeGreaterThan(0); + }); + + it('should synthesize functionResponse with generic_tool fallback when trailing functionCall has missing or whitespace name', async () => { + chat.setHistory([ + { role: 'user', parts: [{ text: 'Run tool' }] }, + { + role: 'model', + parts: [ + { + functionCall: { + id: 'call_fallback', + name: ' ', + args: {}, + }, + }, + ], + }, + ]); + + let capturedContents: Content[] | undefined; + vi.mocked(mockContentGenerator.generateContentStream).mockImplementation( + async (params) => { + capturedContents = params.contents as Content[]; + return (async function* (): AsyncGenerator { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'Response' }], + }, + finishReason: 'STOP' as unknown as undefined, + }, + ], + } as unknown as GenerateContentResponse; + })(); + }, + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.5-pro' }, + [{ text: '' }], + 'prompt-tool-fallback', + new AbortController().signal, + LlmRole.MAIN, + ); + + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toBeDefined(); + const lastTurn = capturedContents![capturedContents!.length - 1]; + expect(lastTurn.role).toBe('user'); + expect(lastTurn.parts).toEqual([ + { + functionResponse: { + name: 'generic_tool', + id: 'call_fallback', + response: { + error: 'Response was lost or interrupted.', + }, + }, + }, + ]); + }); + }); }); diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 1fe4a305275..1c8192d7d47 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -70,6 +70,7 @@ import { } from '../availability/policyHelpers.js'; import { coreEvents } from '../utils/events.js'; import type { AgentLoopContext } from '../config/agent-loop-context.js'; +import { debugLogger } from '../utils/debugLogger.js'; export enum StreamEventType { /** A regular content chunk from the API. */ @@ -559,6 +560,7 @@ export class GeminiChat { const lastTurn = history[history.length - 1]; if ( !lastTurn || + lastTurn.content.role !== 'user' || partListUnionToString(lastTurn.content.parts || []) !== userMessageContent ) { @@ -630,6 +632,7 @@ export class GeminiChat { const lastTurn = history[history.length - 1]; if ( !lastTurn || + lastTurn.content.role !== 'user' || partListUnionToString(lastTurn.content.parts || []) !== partListUnionToString(userContent.parts || []) ) { @@ -1079,10 +1082,70 @@ export class GeminiChat { const finalContents = stripToolCallIdPrefixes(contentsToUse); + let contentsToDispatch = finalContents; + const lastContentTurn = + contentsToDispatch.length > 0 + ? contentsToDispatch[contentsToDispatch.length - 1] + : null; + + if ( + !lastContentTurn || + lastContentTurn.role !== 'user' || + !lastContentTurn.parts?.length + ) { + debugLogger.warn( + 'Final contents do not end with a valid user turn. Normalizing contents to satisfy Gemini API invariant.', + ); + const cloned: Content[] = contentsToDispatch.map((item) => + structuredClone(item), + ); + const lastTurn = cloned.length > 0 ? cloned[cloned.length - 1] : null; + if (lastTurn && lastTurn.role === 'model') { + const hasFunctionCall = lastTurn.parts?.some( + (p) => p && p.functionCall, + ); + if (hasFunctionCall) { + const missingResponses: Part[] = []; + for (const part of lastTurn.parts || []) { + if (part && part.functionCall) { + missingResponses.push({ + functionResponse: { + name: part.functionCall.name?.trim() || 'generic_tool', + id: part.functionCall.id, + response: { + error: 'Response was lost or interrupted.', + }, + }, + }); + } + } + cloned.push({ + role: 'user', + parts: missingResponses, + }); + } else { + cloned.push({ + role: 'user', + parts: [{ text: 'Please continue.' }], + }); + } + } else if (!lastTurn) { + cloned.push({ + role: 'user', + parts: [{ text: 'Please continue.' }], + }); + } else if (lastTurn.role === 'user' && !lastTurn.parts?.length) { + lastTurn.parts = [{ text: 'Please continue.' }]; + } + contentsToDispatch = cloned; + } + + lastContentsToUse = contentsToDispatch; + return this.context.config.getContentGenerator().generateContentStream( { model: modelToUse, - contents: finalContents, + contents: contentsToDispatch, config, }, prompt_id, @@ -1754,60 +1817,67 @@ export function isInvalidArgumentError(errorMessage: string): boolean { } export function stripToolCallIdPrefixes(contents: Content[]): Content[] { - return contents.map((content) => { - const 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), - }; + return contents + .map((content) => { + const 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), - }; + 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), + ...(fr.parts ? { parts: fr.parts } : {}), + }; + } } - } - // If there's an empty text key alongside other active properties, remove the empty text key - // so it doesn't trigger "contains empty parts" validation errors on the Gemini API. - const hasOtherKeys = Object.keys(newPart).some( - (key) => key !== 'text' && key !== 'thought' && key !== 'callIndex', - ); - if (newPart.text !== undefined && newPart.text === '' && hasOtherKeys) { - delete newPart.text; - } + // If there's an empty text key alongside other active properties, remove the empty text key + // so it doesn't trigger "contains empty parts" validation errors on the Gemini API. + const hasOtherKeys = Object.keys(newPart).some( + (key) => key !== 'text' && key !== 'thought' && key !== 'callIndex', + ); + if ( + newPart.text !== undefined && + newPart.text === '' && + hasOtherKeys + ) { + delete newPart.text; + } - return newPart; - }) - .filter((part) => { - // Filter out truly empty parts that have only text: '' and no payload - const hasOtherKeys = Object.keys(part).some( - (key) => key !== 'text' && key !== 'thought' && key !== 'callIndex', - ); - if (part.text !== undefined && part.text === '' && !hasOtherKeys) { - return false; - } - return true; - }); + return newPart; + }) + .filter((part) => { + // Filter out truly empty parts that have only text: '' and no payload + const hasOtherKeys = Object.keys(part).some( + (key) => key !== 'text' && key !== 'thought' && key !== 'callIndex', + ); + if (part.text !== undefined && part.text === '' && !hasOtherKeys) { + return false; + } + return true; + }); - return { - ...content, - parts, - }; - }); + return { + ...content, + parts, + }; + }) + .filter((content) => !content.parts || content.parts.length > 0); } export function coalesceConsecutiveRoles( From 976256ff00da23300efbaea94d5997a12c32bf26 Mon Sep 17 00:00:00 2001 From: Luis Felipe Quevedo Date: Tue, 6 Oct 2026 11:45:38 +0000 Subject: [PATCH 2/5] fix(core,cli): address review comments on history normalization, tool response handling, and rewind stats - In historyHardening, preserve and recursively normalize nested functionResponse.parts to retain multimodal parts. - In GeminiChat, synthesize missing function responses when following an unclosed model functionCall turn to preserve proper tool call pairing. - In GeminiChat, populate trailing empty model turns following an unclosed tool response with an interrupted response placeholder to avoid role fusion. - In GeminiChat, synchronize agentHistory with durable synthetic messages when normalizing dangling turns. - In GeminiChat, preserve pre-stripping request contents for AfterModel hooks by removing redundant assignment. - In GeminiChat, re-coalesce consecutive contents after stripping empty parts to maintain role alternation invariants. - In RewindViewer and rewindFileOps, update synthetic tool response matching to accurately filter turns with sibling parts and track file statistics across multi-step turns. - Add comprehensive regression tests in geminiChat.test.ts and historyHardening.test.ts. --- .../cli/src/ui/components/RewindViewer.tsx | 2 +- packages/cli/src/ui/utils/rewindFileOps.ts | 26 +++- packages/core/src/core/geminiChat.test.ts | 91 ++++++++++++ packages/core/src/core/geminiChat.ts | 140 +++++++++++++++--- .../core/src/utils/historyHardening.test.ts | 41 +++++ packages/core/src/utils/historyHardening.ts | 6 + 6 files changed, 280 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index fb1ad699624..6273deddbf8 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -80,7 +80,7 @@ export const RewindViewer: React.FC = ({ : []; const isToolResponse = parts.length > 0 && - parts.every( + parts.some( (p) => typeof p === 'object' && p !== null && 'functionResponse' in p, ); diff --git a/packages/cli/src/ui/utils/rewindFileOps.ts b/packages/cli/src/ui/utils/rewindFileOps.ts index 7eaebe90ed9..4385d295e3c 100644 --- a/packages/cli/src/ui/utils/rewindFileOps.ts +++ b/packages/cli/src/ui/utils/rewindFileOps.ts @@ -29,6 +29,25 @@ export interface FileChangeStats { details?: FileChangeDetail[]; } +/** + * Determines whether a user message record is a synthetic tool response. + */ +export function isToolResponseMessage(msg: MessageRecord): boolean { + if (msg.type !== 'user') return false; + const content = msg.content; + const parts = Array.isArray(content) + ? content + : content !== undefined && content !== null + ? [content] + : []; + return ( + parts.length > 0 && + parts.some( + (p) => typeof p === 'object' && p !== null && 'functionResponse' in p, + ) + ); +} + /** * Calculates file change statistics for a single turn. * A turn is defined as the sequence of messages starting after the given user message @@ -53,7 +72,12 @@ export function calculateTurnStats( // Look ahead until the next user message (single turn) for (let i = msgIndex + 1; i < conversation.messages.length; i++) { const msg = conversation.messages[i]; - if (msg.type === 'user') break; // Stop at next user message + if (msg.type === 'user') { + if (isToolResponseMessage(msg)) { + continue; + } + break; // Stop at next user message + } if (msg.type === 'gemini' && msg.toolCalls) { for (const toolCall of msg.toolCalls) { diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 9a5d98d47b3..bfd0e0952cb 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -5401,5 +5401,96 @@ describe('GeminiChat', () => { }, ]); }); + + it('should synthesize matching functionResponse before follow-up user text prompt when preceding turn has unclosed functionCall', async () => { + chat.setHistory([ + { role: 'user', parts: [{ text: 'Please read the file' }] }, + { + role: 'model', + parts: [ + { + functionCall: { + id: 'call_read', + name: 'read_file', + args: { path: 'foo.ts' }, + }, + }, + ], + }, + ]); + + let capturedContents: Content[] | undefined; + vi.mocked(mockContentGenerator.generateContentStream).mockImplementation( + async (params) => { + capturedContents = params.contents as Content[]; + return (async function* (): AsyncGenerator { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'Understood, doing something else.' }], + }, + finishReason: 'STOP' as unknown as undefined, + }, + ], + } as unknown as GenerateContentResponse; + })(); + }, + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.5-pro' }, + [{ text: 'Nevermind, do something else.' }], + 'prompt-followup', + new AbortController().signal, + LlmRole.MAIN, + ); + + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toBeDefined(); + const lastTurn = capturedContents![capturedContents!.length - 1]; + expect(lastTurn.role).toBe('user'); + expect(lastTurn.parts).toEqual([ + { + functionResponse: { + name: 'read_file', + id: 'call_read', + response: { + error: 'Response was lost or interrupted.', + }, + }, + }, + { text: 'Nevermind, do something else.' }, + ]); + }); + + it('should re-coalesce adjacent turns of same role when interior empty user turn is stripped', () => { + const input: Content[] = [ + { + role: 'model', + parts: [{ text: 'Step 1 output' }], + }, + { + role: 'user', + parts: [{ text: '' }], + }, + { + role: 'model', + parts: [{ text: 'Step 2 output' }], + }, + ]; + + const stripped = stripToolCallIdPrefixes(input); + expect(stripped.length).toBe(1); + expect(stripped[0].role).toBe('model'); + expect(stripped[0].parts).toEqual([ + { text: 'Step 1 output' }, + { text: 'Step 2 output' }, + ]); + }); }); }); diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 1c8192d7d47..2dccd5bb0ad 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -512,12 +512,12 @@ export class GeminiChat { let userContent = createUserContent(message); const isOriginalFunctionResponse = isFunctionResponse(userContent); - // A turn can end leaving history on an unanswered tool response: a stream - // error after the response was committed, or a cancelled tool call. Close - // it before recording a genuinely new user message, otherwise the two user - // turns are coalesced into one and the model continues the trailing text - // instead of answering it. + // If history ended on an unanswered model tool call or an unanswered tool response + // (e.g. cancelled tool call, interrupted stream, or user follow-up prompt), close it + // before recording a genuinely new user message so proper tool call pairing and + // role alternation are maintained. if (!isOriginalFunctionResponse) { + this.closeUnansweredToolCallsTurn(userContent); this.closeUnansweredToolResponseTurn(); } @@ -827,26 +827,75 @@ export class GeminiChat { return streamWithRetries.call(this); } + /** + * Synthesizes matching functionResponse parts into userContent when history + * ends with an unclosed model functionCall, so the subsequent user prompt + * preserves proper tool call pairing and role alternation. + */ + private closeUnansweredToolCallsTurn(userContent: Content): void { + const turns = this.agentHistory.get(); + const last = turns[turns.length - 1]; + if ( + last?.content.role !== 'model' || + !last.content.parts?.some((part) => !!part.functionCall) + ) { + return; + } + const missingResponses: Part[] = []; + for (const part of last.content.parts || []) { + if (part && part.functionCall) { + missingResponses.push({ + functionResponse: { + name: part.functionCall.name?.trim() || 'generic_tool', + id: part.functionCall.id, + response: { + error: 'Response was lost or interrupted.', + }, + }, + }); + } + } + if (missingResponses.length > 0) { + const remainingParts = (userContent.parts || []).filter( + (p) => !(p.text !== undefined && p.text === ''), + ); + userContent.parts = [...missingResponses, ...remainingParts]; + } + } + /** * Appends a closing model turn when history ends with an unanswered tool * response, so the next user message stays a turn of its own. */ private closeUnansweredToolResponseTurn(): void { const turns = this.agentHistory.get(); - const last = turns[turns.length - 1]; + let targetTurn = turns[turns.length - 1]; + let hadEmptyTrailingModelTurn = false; if ( - last?.content.role !== 'user' || - !last.content.parts?.some((part) => !!part.functionResponse) + targetTurn?.content.role === 'model' && + (!targetTurn.content.parts || targetTurn.content.parts.length === 0) + ) { + hadEmptyTrailingModelTurn = true; + targetTurn = turns[turns.length - 2]; + } + if ( + targetTurn?.content.role !== 'user' || + !targetTurn.content.parts?.some((part) => !!part.functionResponse) ) { return; } - this.agentHistory.push({ - id: randomUUID(), - content: { - role: 'model', - parts: [{ text: INTERRUPTED_RESPONSE_PLACEHOLDER }], - }, - }); + if (hadEmptyTrailingModelTurn) { + const lastTurn = turns[turns.length - 1]; + lastTurn.content.parts = [{ text: INTERRUPTED_RESPONSE_PLACEHOLDER }]; + } else { + this.agentHistory.push({ + id: randomUUID(), + content: { + role: 'model', + parts: [{ text: INTERRUPTED_RESPONSE_PLACEHOLDER }], + }, + }); + } } private extractBinaryInjections( @@ -1119,29 +1168,50 @@ export class GeminiChat { }); } } - cloned.push({ + const normalizedUserTurn: Content = { role: 'user', parts: missingResponses, - }); + }; + cloned.push(normalizedUserTurn); + const id = this.chatRecordingService.recordSyntheticMessage( + 'user', + missingResponses, + ); + this.agentHistory.push({ id, content: normalizedUserTurn }); } else { - cloned.push({ + const normalizedUserTurn: Content = { role: 'user', parts: [{ text: 'Please continue.' }], - }); + }; + cloned.push(normalizedUserTurn); + const id = this.chatRecordingService.recordSyntheticMessage( + 'user', + normalizedUserTurn.parts!, + ); + this.agentHistory.push({ id, content: normalizedUserTurn }); } } else if (!lastTurn) { - cloned.push({ + const normalizedUserTurn: Content = { role: 'user', parts: [{ text: 'Please continue.' }], - }); + }; + cloned.push(normalizedUserTurn); + const id = this.chatRecordingService.recordSyntheticMessage( + 'user', + normalizedUserTurn.parts!, + ); + this.agentHistory.push({ id, content: normalizedUserTurn }); } else if (lastTurn.role === 'user' && !lastTurn.parts?.length) { lastTurn.parts = [{ text: 'Please continue.' }]; + const historyTurns = this.agentHistory.get(); + const lastHistoryTurn = historyTurns[historyTurns.length - 1]; + if (lastHistoryTurn && lastHistoryTurn.content.role === 'user') { + lastHistoryTurn.content.parts = [{ text: 'Please continue.' }]; + } } contentsToDispatch = cloned; } - lastContentsToUse = contentsToDispatch; - return this.context.config.getContentGenerator().generateContentStream( { model: modelToUse, @@ -1817,7 +1887,7 @@ export function isInvalidArgumentError(errorMessage: string): boolean { } export function stripToolCallIdPrefixes(contents: Content[]): Content[] { - return contents + const stripped = contents .map((content) => { const parts = (content.parts || []) .map((part) => { @@ -1878,6 +1948,28 @@ export function stripToolCallIdPrefixes(contents: Content[]): Content[] { }; }) .filter((content) => !content.parts || content.parts.length > 0); + + return coalesceConsecutiveContents(stripped); +} + +export function coalesceConsecutiveContents(contents: Content[]): Content[] { + const result: Content[] = []; + for (const item of contents) { + const lastIdx = result.length - 1; + const last = result[lastIdx]; + if (last && last.role === item.role) { + const hasParts = last.parts || item.parts; + result[lastIdx] = { + ...last, + parts: hasParts + ? [...(last.parts || []), ...(item.parts || [])] + : undefined, + }; + } else { + result.push({ ...item }); + } + } + return result; } export function coalesceConsecutiveRoles( diff --git a/packages/core/src/utils/historyHardening.test.ts b/packages/core/src/utils/historyHardening.test.ts index 90577e1e4b2..49bef68da05 100644 --- a/packages/core/src/utils/historyHardening.test.ts +++ b/packages/core/src/utils/historyHardening.test.ts @@ -578,4 +578,45 @@ describe('scrubHistory', () => { { text: 'World' }, ]); }); + + it('should preserve nested parts array within functionResponse', () => { + const history: HistoryTurn[] = [ + { + id: '1', + content: { + role: 'user', + parts: [ + { + functionResponse: { + name: 'readFile', + id: 'call-1', + response: { mimeType: 'image/png' }, + parts: [ + { + inlineData: { + mimeType: 'image/png', + data: 'base64data', + }, + }, + ], + } as unknown as Part['functionResponse'], + } as unknown as Part, + ], + }, + }, + ]; + + const scrubbed = scrubHistory(history); + expect(scrubbed.length).toBe(1); + const fr = scrubbed[0].content.parts![0].functionResponse; + expect(fr).toBeDefined(); + expect((fr as unknown as Record)['parts']).toEqual([ + { + inlineData: { + mimeType: 'image/png', + data: 'base64data', + }, + }, + ]); + }); }); diff --git a/packages/core/src/utils/historyHardening.ts b/packages/core/src/utils/historyHardening.ts index 8a4e8a2f5cc..d58274161f6 100644 --- a/packages/core/src/utils/historyHardening.ts +++ b/packages/core/src/utils/historyHardening.ts @@ -485,6 +485,12 @@ export function scrubPart(part: Part): Part { if (part.functionResponse.id) { scrubbedResp['id'] = part.functionResponse.id; } + if ( + 'parts' in part.functionResponse && + Array.isArray(part.functionResponse.parts) + ) { + scrubbedResp['parts'] = part.functionResponse.parts.map(scrubPart); + } scrubbed['functionResponse'] = scrubbedResp; } if ('fileData' in part) { From d58cc9364f1fbd5a4181fd1969222c7c1720bff5 Mon Sep 17 00:00:00 2001 From: Luis Felipe Quevedo Date: Tue, 6 Oct 2026 12:26:28 +0000 Subject: [PATCH 3/5] fix(core,cli): persist normalized history mutations and reuse tool response helper - In GeminiChat, persist mutated history to chatRecordingService when filling empty trailing model turns with an interrupted response placeholder. - In GeminiChat, persist mutated history to chatRecordingService when replacing empty trailing user turns with continuation text. - In RewindViewer, reuse isToolResponseMessage from rewindFileOps to avoid duplicate synthetic tool response checking logic. --- packages/cli/src/ui/components/RewindViewer.tsx | 15 ++------------- packages/core/src/core/geminiChat.ts | 2 ++ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 6273deddbf8..2b222221fdb 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -19,6 +19,7 @@ import { useKeypress } from '../hooks/useKeypress.js'; import { useRewind } from '../hooks/useRewind.js'; import { RewindConfirmation, RewindOutcome } from './RewindConfirmation.js'; import { stripReferenceContent } from '../utils/formatters.js'; +import { isToolResponseMessage } from '../utils/rewindFileOps.js'; import { Command } from '../key/keyMatchers.js'; import { CliSpinner } from './CliSpinner.js'; import { ExpandableText } from './shared/ExpandableText.js'; @@ -72,19 +73,7 @@ export const RewindViewer: React.FC = ({ () => conversation.messages.filter((msg) => { if (msg.type !== 'user') return false; - const content = msg.content; - const parts = Array.isArray(content) - ? content - : content !== undefined && content !== null - ? [content] - : []; - const isToolResponse = - parts.length > 0 && - parts.some( - (p) => - typeof p === 'object' && p !== null && 'functionResponse' in p, - ); - return !isToolResponse; + return !isToolResponseMessage(msg); }), [conversation.messages], ); diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 2dccd5bb0ad..9606c4608e5 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -887,6 +887,7 @@ export class GeminiChat { if (hadEmptyTrailingModelTurn) { const lastTurn = turns[turns.length - 1]; lastTurn.content.parts = [{ text: INTERRUPTED_RESPONSE_PLACEHOLDER }]; + this.chatRecordingService.updateMessagesFromHistory(turns); } else { this.agentHistory.push({ id: randomUUID(), @@ -1207,6 +1208,7 @@ export class GeminiChat { const lastHistoryTurn = historyTurns[historyTurns.length - 1]; if (lastHistoryTurn && lastHistoryTurn.content.role === 'user') { lastHistoryTurn.content.parts = [{ text: 'Please continue.' }]; + this.chatRecordingService.updateMessagesFromHistory(historyTurns); } } contentsToDispatch = cloned; From 709cf4af34f299fd0ef0757f8d6037a222fb406e Mon Sep 17 00:00:00 2001 From: Luis Felipe Quevedo Date: Tue, 6 Oct 2026 20:03:45 +0000 Subject: [PATCH 4/5] refactor(core,cli): keep request normalization pure and close tool turns with dedicated history turns - core: replace the stateful normalization inside the retryable apiCall closure with a pure ensureTerminalUserTurn helper, so retries, per-call history overrides and hook-modified contents no longer write to agentHistory or the session log; AfterModel hooks now receive the normalized contents. - core: replace an empty trailing user turn once per send (outside the retry loop) and keep the session log in sync. - core: closeUnansweredToolCallsTurn now records a dedicated synthetic functionResponse turn instead of merging it into the user's prompt; extract buildInterruptedToolResponseParts and named constants. - core: add GeminiChat/GeminiClient.discardTrailingUnansweredToolCallTurn, which removes only the trailing unanswered call turn and keeps turn ids. - core: share one empty-part predicate between prefix stripping and normalization, spread functionCall/functionResponse fields when stripping prefixes, drop a dead parts check and skip coalescing role-less contents. - cli: when a whole tool batch is declined, roll back only the trailing unanswered call turn, keeping the user prompt and completed tool rounds. - cli: isToolResponseMessage now requires function responses with no user-authored text (binary siblings allowed); flatten the calculateTurnStats guard and simplify the RewindViewer filter. - tests: cover the new helpers, retries with overrides, the empty trailing model turn branch, multi-round turn stats and the declined batch rollback with a realistic sendMessageStream mock. --- .../cli/src/ui/components/RewindViewer.tsx | 7 +- .../cli/src/ui/hooks/useGeminiStream.test.tsx | 87 +++- packages/cli/src/ui/hooks/useGeminiStream.ts | 21 +- .../cli/src/ui/utils/rewindFileOps.test.ts | 135 ++++++ packages/cli/src/ui/utils/rewindFileOps.ts | 38 +- packages/core/src/core/client.ts | 12 + packages/core/src/core/geminiChat.test.ts | 456 +++++++++++++++++- packages/core/src/core/geminiChat.ts | 310 +++++++----- 8 files changed, 860 insertions(+), 206 deletions(-) diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 2b222221fdb..935de9459b7 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -71,10 +71,9 @@ export const RewindViewer: React.FC = ({ const interactions = useMemo( () => - conversation.messages.filter((msg) => { - if (msg.type !== 'user') return false; - return !isToolResponseMessage(msg); - }), + conversation.messages.filter( + (msg) => msg.type === 'user' && !isToolResponseMessage(msg), + ), [conversation.messages], ); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index eb7e6dbda1a..9ad079cbb94 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -92,6 +92,19 @@ const MockedGeminiClientClass = vi.hoisted(() => this.setHistory = vi.fn().mockImplementation((newHistory: any[]) => { mockHistory = [...newHistory]; }); + this.discardTrailingUnansweredToolCallTurn = vi + .fn() + .mockImplementation(() => { + const last = mockHistory[mockHistory.length - 1]; + if ( + last?.role !== 'model' || + !last.parts?.some((part: any) => !!part.functionCall) + ) { + return false; + } + mockHistory = mockHistory.slice(0, -1); + return true; + }); this.generateContent = vi.fn().mockResolvedValue({ candidates: [ { content: { parts: [{ text: 'Got it. Focusing on tests only.' }] } }, @@ -1132,7 +1145,7 @@ describe('useGeminiStream', () => { ), ); - // Call submitQuery to populate the user turn and set historyLengthAfterUserPromptRef + // Call submitQuery to populate the user turn await act(async () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises result.current.submitQuery('User prompt'); @@ -1167,7 +1180,7 @@ describe('useGeminiStream', () => { }); }); - it('should keep the rollback anchor at the original user prompt when a continuation turn is cancelled', async () => { + it('should keep the user prompt and completed tool rounds when a later tool batch is declined', async () => { const cancelledToolCalls: TrackedToolCall[] = [ { request: { @@ -1193,7 +1206,22 @@ describe('useGeminiStream', () => { } as any, ]; const client = new MockedGeminiClientClass(mockConfig); - client.setHistory([{ role: 'user', parts: [{ text: 'User prompt' }] }]); + const priorTurn = [ + { role: 'user', parts: [{ text: 'Earlier prompt' }] }, + { role: 'model', parts: [{ text: 'Earlier answer' }] }, + ]; + client.setHistory(priorTurn); + // Model the real sendMessageStream contract: the user turn is recorded in + // the client history when the request is sent, not before submitQuery. + mockSendMessageStream.mockImplementation((query: PartListUnion) => { + const parts = (Array.isArray(query) ? query : [query]).map((part) => + typeof part === 'string' ? { text: part } : part, + ); + client.setHistory([...client.getHistory(), { role: 'user', parts }]); + return (async function* () { + yield { type: ServerGeminiEventType.Content, value: 'Working on it' }; + })(); + }); let capturedOnComplete: | ((completedTools: TrackedToolCall[]) => Promise) @@ -1233,13 +1261,13 @@ describe('useGeminiStream', () => { ), ); - // The initial user prompt anchors the rollback index at length 1. + // Turn 1 starts: sendMessageStream records the user prompt. await act(async () => { await result.current.submitQuery('User prompt'); }); - // A first tool round completes and its response is submitted back to the - // model as a continuation turn. The continuation must not move the anchor. + // The model answers with a first tool call, which completes and is sent + // back to the model as a continuation turn. const firstRoundResponse: Part[] = [ { functionResponse: { @@ -1250,12 +1278,11 @@ describe('useGeminiStream', () => { }, ]; client.setHistory([ - { role: 'user', parts: [{ text: 'User prompt' }] }, + ...client.getHistory(), { role: 'model', - parts: [{ functionCall: { name: 'testTool', args: {} } }], + parts: [{ functionCall: { name: 'testTool', id: '1', args: {} } }], }, - { role: 'user', parts: firstRoundResponse }, ]); await act(async () => { await result.current.submitQuery(firstRoundResponse, { @@ -1268,11 +1295,12 @@ describe('useGeminiStream', () => { ...client.getHistory(), { role: 'model', - parts: [{ functionCall: { name: 'testTool', args: {} } }], + parts: [{ functionCall: { name: 'testTool', id: '2', args: {} } }], }, ]); + vi.mocked(client.setHistory).mockClear(); - // The second tool call is cancelled. + // The second tool call is declined. await act(async () => { if (capturedOnComplete) { await new Promise((resolve) => setTimeout(resolve, 0)); @@ -1280,16 +1308,31 @@ describe('useGeminiStream', () => { } }); - await waitFor(() => { - expect(mockMarkToolsAsSubmitted).toHaveBeenCalledWith(['2']); - expect(client.addHistory).not.toHaveBeenCalled(); - // The whole cancelled exchange is removed back to the original user - // prompt instead of stopping at the intermediate continuation point, - // which would leave the history ending on a model turn. - expect(client.getHistory()).toEqual([ - { role: 'user', parts: [{ text: 'User prompt' }] }, - ]); - }); + try { + await waitFor(() => { + expect(mockMarkToolsAsSubmitted).toHaveBeenCalledWith(['2']); + expect(client.addHistory).not.toHaveBeenCalled(); + // Only the trailing unanswered call is removed, without re-setting + // the whole history (which would re-record every turn). + expect( + client.discardTrailingUnansweredToolCallTurn, + ).toHaveBeenCalledTimes(1); + expect(client.setHistory).not.toHaveBeenCalled(); + // The previous turn, the user prompt and the completed first round + // are preserved. + expect(client.getHistory()).toEqual([ + ...priorTurn, + { role: 'user', parts: [{ text: 'User prompt' }] }, + { + role: 'model', + parts: [{ functionCall: { name: 'testTool', id: '1', args: {} } }], + }, + { role: 'user', parts: firstRoundResponse }, + ]); + }); + } finally { + mockSendMessageStream.mockImplementation(() => (async function* () {})()); + } }); it('should record tool responses in history when the model was switched due to a quota error', async () => { @@ -1722,7 +1765,7 @@ describe('useGeminiStream', () => { ), ); - // Call submitQuery to populate the user turn and set historyLengthAfterUserPromptRef + // Call submitQuery to populate the user turn await act(async () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises result.current.submitQuery('User prompt'); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index a561011c6f4..7201d614b6d 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -258,7 +258,6 @@ export const useGeminiStream = ( const abortControllerRef = useRef(null); const turnCancelledRef = useRef(false); const activeQueryIdRef = useRef(null); - const historyLengthAfterUserPromptRef = useRef(undefined); const previousApprovalModeRef = useRef( config.getApprovalMode(), ); @@ -1739,11 +1738,6 @@ export const useGeminiStream = ( return; } - if (geminiClient && !options?.isContinuation) { - historyLengthAfterUserPromptRef.current = - geminiClient.getHistory().length; - } - if (!options?.isContinuation) { if (typeof queryToSend === 'string') { // logging the text prompts only for now @@ -2127,17 +2121,10 @@ export const useGeminiStream = ( } setIsResponding(false); - if ( - geminiClient && - historyLengthAfterUserPromptRef.current !== undefined - ) { - const targetLength = historyLengthAfterUserPromptRef.current; - if (geminiClient.getHistory().length > targetLength) { - geminiClient.setHistory( - geminiClient.getHistory().slice(0, targetLength), - ); - } - } + // Only roll back the unanswered model function call turn for this + // cancelled batch. The originating user prompt and any tool rounds + // that already completed (and may have changed files) stay in history. + geminiClient?.discardTrailingUnansweredToolCallTurn(); const callIdsToMarkAsSubmitted = geminiTools.map( (toolCall) => toolCall.request.callId, diff --git a/packages/cli/src/ui/utils/rewindFileOps.test.ts b/packages/cli/src/ui/utils/rewindFileOps.test.ts index 4e693386aba..e6486e7a23e 100644 --- a/packages/cli/src/ui/utils/rewindFileOps.test.ts +++ b/packages/cli/src/ui/utils/rewindFileOps.test.ts @@ -9,6 +9,7 @@ import fs from 'node:fs/promises'; import { calculateTurnStats, calculateRewindImpact, + isToolResponseMessage, revertFileChanges, } from './rewindFileOps.js'; import { @@ -120,6 +121,140 @@ describe('rewindFileOps', () => { removedLines: 3, }); }); + + it('aggregates stats across multiple tool rounds separated by tool responses', async () => { + const { getFileDiffFromResultDisplay, computeModelAddedAndRemovedLines } = + await import('@google/gemini-cli-core'); + vi.mocked(getFileDiffFromResultDisplay).mockImplementation( + (resultDisplay) => + ({ + filePath: String(resultDisplay), + fileName: String(resultDisplay), + originalContent: 'old', + newContent: 'new', + isNewFile: false, + diffStat: { + model_added_lines: 0, + model_removed_lines: 0, + model_added_chars: 0, + model_removed_chars: 0, + user_added_lines: 0, + user_removed_lines: 0, + user_added_chars: 0, + user_removed_chars: 0, + }, + fileDiff: 'diff', + }) as ReturnType, + ); + vi.mocked(computeModelAddedAndRemovedLines).mockReturnValue({ + addedLines: 2, + removedLines: 1, + }); + + const userMsg = { + type: 'user', + content: [{ text: 'Edit both files' }], + } as unknown as MessageRecord; + const toolResponse = (id: string) => + ({ + type: 'user', + content: [ + { functionResponse: { id, name: 'replace', response: {} } }, + ], + }) as unknown as MessageRecord; + const editRound = (file: string) => + ({ + type: 'gemini', + toolCalls: [{ name: 'replace', args: {}, resultDisplay: file }], + }) as unknown as MessageRecord; + const nextUserMsg = { + type: 'user', + content: [{ text: 'Next prompt' }], + } as unknown as MessageRecord; + + const conversation = { + messages: [ + userMsg, + editRound('a.ts'), + toolResponse('1'), + editRound('b.ts'), + toolResponse('2'), + nextUserMsg, + editRound('c.ts'), + ], + }; + + const result = calculateTurnStats( + conversation as unknown as ConversationRecord, + userMsg, + ); + expect(result).toEqual({ + fileCount: 2, + addedLines: 4, + removedLines: 2, + }); + }); + }); + + describe('isToolResponseMessage', () => { + const userMessage = (content: unknown) => + ({ type: 'user', content }) as unknown as MessageRecord; + const functionResponsePart = { + functionResponse: { id: '1', name: 'read_file', response: {} }, + }; + + it('returns true for a message with only functionResponse parts', () => { + expect( + isToolResponseMessage( + userMessage([functionResponsePart, functionResponsePart]), + ), + ).toBe(true); + }); + + it('returns true when functionResponse parts carry binary siblings', () => { + expect( + isToolResponseMessage( + userMessage([ + functionResponsePart, + { inlineData: { mimeType: 'image/png', data: 'abc' } }, + { fileData: { mimeType: 'video/mp4', fileUri: 'gs://x' } }, + ]), + ), + ).toBe(true); + }); + + it('returns false when functionResponse parts are mixed with user text', () => { + expect( + isToolResponseMessage( + userMessage([functionResponsePart, { text: 'Do something else' }]), + ), + ).toBe(false); + }); + + it('returns false for plain text user messages', () => { + expect(isToolResponseMessage(userMessage('hello'))).toBe(false); + expect(isToolResponseMessage(userMessage([{ text: 'hello' }]))).toBe( + false, + ); + }); + + it('returns false for empty or binary-only user messages', () => { + expect(isToolResponseMessage(userMessage([]))).toBe(false); + expect( + isToolResponseMessage( + userMessage([{ inlineData: { mimeType: 'image/png', data: 'a' } }]), + ), + ).toBe(false); + }); + + it('returns false for non-user messages', () => { + expect( + isToolResponseMessage({ + type: 'gemini', + content: [functionResponsePart], + } as unknown as MessageRecord), + ).toBe(false); + }); }); describe('calculateRewindImpact', () => { diff --git a/packages/cli/src/ui/utils/rewindFileOps.ts b/packages/cli/src/ui/utils/rewindFileOps.ts index 4385d295e3c..29ba30a0b59 100644 --- a/packages/cli/src/ui/utils/rewindFileOps.ts +++ b/packages/cli/src/ui/utils/rewindFileOps.ts @@ -29,21 +29,34 @@ export interface FileChangeStats { details?: FileChangeDetail[]; } +function isPartWithKey(part: unknown, key: string): boolean { + return typeof part === 'object' && part !== null && key in part; +} + /** * Determines whether a user message record is a synthetic tool response. + * + * A tool response holds at least one `functionResponse` part, and every part + * is either a `functionResponse` or binary data emitted alongside tool output + * (`inlineData` / `fileData`). Any user-authored text (e.g. a prompt or a + * steering hint sent with the responses) keeps the message a real user turn. */ export function isToolResponseMessage(msg: MessageRecord): boolean { - if (msg.type !== 'user') return false; - const content = msg.content; - const parts = Array.isArray(content) - ? content - : content !== undefined && content !== null - ? [content] - : []; + if ( + msg.type !== 'user' || + !Array.isArray(msg.content) || + msg.content.length === 0 + ) { + return false; + } + const parts: unknown[] = msg.content; return ( - parts.length > 0 && - parts.some( - (p) => typeof p === 'object' && p !== null && 'functionResponse' in p, + parts.some((p) => isPartWithKey(p, 'functionResponse')) && + parts.every( + (p) => + isPartWithKey(p, 'functionResponse') || + isPartWithKey(p, 'inlineData') || + isPartWithKey(p, 'fileData'), ) ); } @@ -72,10 +85,7 @@ export function calculateTurnStats( // Look ahead until the next user message (single turn) for (let i = msgIndex + 1; i < conversation.messages.length; i++) { const msg = conversation.messages[i]; - if (msg.type === 'user') { - if (isToolResponseMessage(msg)) { - continue; - } + if (msg.type === 'user' && !isToolResponseMessage(msg)) { break; // Stop at next user message } diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index c39c9b815c3..67506fc7a85 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -301,6 +301,18 @@ export class GeminiClient { this.forceFullIdeContext = true; } + /** + * Removes the trailing model turn when it only holds unanswered function + * calls. See {@link GeminiChat.discardTrailingUnansweredToolCallTurn}. + */ + discardTrailingUnansweredToolCallTurn(): boolean { + const removed = this.getChat().discardTrailingUnansweredToolCallTurn(); + if (removed) { + this.updateTelemetryTokenCount(); + } + return removed; + } + private lastUsedModelId?: string; async setTools(modelId?: string): Promise { diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index bfd0e0952cb..6a333955b9d 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -28,6 +28,10 @@ import { THINKING_ONLY_NUDGE_MESSAGE, NO_RESPONSE_TEXT_NUDGE_MESSAGE, applyRetryNudge, + coalesceConsecutiveContents, + ensureTerminalUserTurn, + CONTINUE_PROMPT_TEXT, + INTERRUPTED_TOOL_RESPONSE_ERROR, } from './geminiChat.js'; import { type CompletedToolCall, @@ -5387,19 +5391,25 @@ describe('GeminiChat', () => { } expect(capturedContents).toBeDefined(); - const lastTurn = capturedContents![capturedContents!.length - 1]; - expect(lastTurn.role).toBe('user'); - expect(lastTurn.parts).toEqual([ - { - functionResponse: { - name: 'generic_tool', - id: 'call_fallback', - response: { - error: 'Response was lost or interrupted.', + expect(capturedContents![2]).toEqual({ + role: 'user', + parts: [ + { + functionResponse: { + name: 'generic_tool', + id: 'call_fallback', + response: { + error: INTERRUPTED_TOOL_RESPONSE_ERROR, + }, }, }, - }, - ]); + ], + }); + const lastTurn = capturedContents![capturedContents!.length - 1]; + expect(lastTurn).toEqual({ + role: 'user', + parts: [{ text: CONTINUE_PROMPT_TEXT }], + }); }); it('should synthesize matching functionResponse before follow-up user text prompt when preceding turn has unclosed functionCall', async () => { @@ -5439,6 +5449,15 @@ describe('GeminiChat', () => { }, ); + const recordMessageSpy = vi.spyOn( + chat.getChatRecordingService(), + 'recordMessage', + ); + const recordSyntheticMessageSpy = vi.spyOn( + chat.getChatRecordingService(), + 'recordSyntheticMessage', + ); + const stream = await chat.sendMessageStream( { model: 'gemini-2.5-pro' }, [{ text: 'Nevermind, do something else.' }], @@ -5452,20 +5471,59 @@ describe('GeminiChat', () => { } expect(capturedContents).toBeDefined(); - const lastTurn = capturedContents![capturedContents!.length - 1]; - expect(lastTurn.role).toBe('user'); - expect(lastTurn.parts).toEqual([ + expect(capturedContents).toEqual([ + { role: 'user', parts: [{ text: 'Please read the file' }] }, + { + role: 'model', + parts: [ + { + functionCall: { + id: 'call_read', + name: 'read_file', + args: { path: 'foo.ts' }, + }, + }, + ], + }, + { + role: 'user', + parts: [ + { + functionResponse: { + name: 'read_file', + id: 'call_read', + response: { error: INTERRUPTED_TOOL_RESPONSE_ERROR }, + }, + }, + ], + }, + { + role: 'model', + parts: [{ text: INTERRUPTED_RESPONSE_PLACEHOLDER }], + }, + { + role: 'user', + parts: [{ text: 'Nevermind, do something else.' }], + }, + ]); + + // The synthetic response and the user's prompt are recorded as separate + // messages; the prompt is not mixed with functionResponse parts. + expect(recordSyntheticMessageSpy).toHaveBeenCalledWith('user', [ { functionResponse: { name: 'read_file', id: 'call_read', - response: { - error: 'Response was lost or interrupted.', - }, + response: { error: INTERRUPTED_TOOL_RESPONSE_ERROR }, }, }, - { text: 'Nevermind, do something else.' }, ]); + expect(recordMessageSpy).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'user', + content: [{ text: 'Nevermind, do something else.' }], + }), + ); }); it('should re-coalesce adjacent turns of same role when interior empty user turn is stripped', () => { @@ -5492,5 +5550,367 @@ describe('GeminiChat', () => { { text: 'Step 2 output' }, ]); }); + + it('should not add synthetic turns to history or the session log across retries', async () => { + const recordSyntheticMessageSpy = vi.spyOn( + chat.getChatRecordingService(), + 'recordSyntheticMessage', + ); + const apiHistoryOverride: Content[] = [ + { role: 'user', parts: [{ text: 'Initial question' }] }, + { role: 'model', parts: [{ text: 'Initial answer' }] }, + ]; + + const capturedContents: Content[][] = []; + vi.mocked(mockContentGenerator.generateContentStream) + // Attempt 1: thought-only response triggers a mid-stream retry. + .mockImplementationOnce(async (params) => { + capturedContents.push(params.contents as Content[]); + return (async function* () { + yield { + candidates: [ + { + content: { + role: 'model', + parts: [{ thought: true, text: 'thinking' }], + }, + finishReason: 'STOP', + }, + ], + } as unknown as GenerateContentResponse; + })(); + }) + // Attempt 2: valid response. + .mockImplementationOnce(async (params) => { + capturedContents.push(params.contents as Content[]); + return (async function* () { + yield { + candidates: [ + { + content: { role: 'model', parts: [{ text: 'Answer' }] }, + finishReason: 'STOP', + }, + ], + } as unknown as GenerateContentResponse; + })(); + }); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.0-flash' }, + 'Follow-up question', + 'prompt-retry-normalization', + new AbortController().signal, + LlmRole.MAIN, + undefined, + apiHistoryOverride, + ); + for await (const _ of stream) { + // consume + } + + expect(capturedContents).toHaveLength(2); + expect(capturedContents[0].at(-1)).toEqual({ + role: 'user', + parts: [{ text: CONTINUE_PROMPT_TEXT }], + }); + expect(recordSyntheticMessageSpy).not.toHaveBeenCalled(); + expect(chat.getHistory()).toEqual([ + { role: 'user', parts: [{ text: 'Follow-up question' }] }, + { role: 'model', parts: [{ text: 'Answer' }] }, + ]); + expect(apiHistoryOverride).toHaveLength(2); + }); + + it('should populate a trailing empty model turn after a tool response in place', async () => { + chat.setHistory([ + { id: 'u1', content: { role: 'user', parts: [{ text: 'Search' }] } }, + { + id: 'm1', + content: { + role: 'model', + parts: [{ functionCall: { id: 'c1', name: 'grep', args: {} } }], + }, + }, + { + id: 'u2', + content: { + role: 'user', + parts: [ + { + functionResponse: { + id: 'c1', + name: 'grep', + response: { output: 'ok' }, + }, + }, + ], + }, + }, + { id: 'm2', content: { role: 'model', parts: [] } }, + ]); + const updateSpy = vi.spyOn( + chat.getChatRecordingService(), + 'updateMessagesFromHistory', + ); + vi.mocked(mockContentGenerator.generateContentStream).mockResolvedValue( + (async function* () { + yield { + candidates: [ + { + content: { role: 'model', parts: [{ text: 'Done' }] }, + finishReason: 'STOP', + }, + ], + } as unknown as GenerateContentResponse; + })(), + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.0-flash' }, + 'Next prompt', + 'prompt-empty-model-turn', + new AbortController().signal, + LlmRole.MAIN, + ); + for await (const _ of stream) { + // consume + } + + const turns = chat.getHistoryTurns(); + expect(turns.map((t) => t.id).slice(0, 4)).toEqual([ + 'u1', + 'm1', + 'u2', + 'm2', + ]); + expect(turns[3].content.parts).toEqual([ + { text: INTERRUPTED_RESPONSE_PLACEHOLDER }, + ]); + expect(turns[4].content).toEqual({ + role: 'user', + parts: [{ text: 'Next prompt' }], + }); + expect(updateSpy).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + id: 'm2', + content: { + role: 'model', + parts: [{ text: INTERRUPTED_RESPONSE_PLACEHOLDER }], + }, + }), + ]), + ); + }); + + it('should replace an empty trailing user turn once and keep history in sync', async () => { + chat.setHistory([ + { role: 'user', parts: [{ text: 'Hi' }] }, + { role: 'model', parts: [{ text: 'Hello' }] }, + ]); + vi.mocked(mockContentGenerator.generateContentStream).mockResolvedValue( + (async function* () { + yield { + candidates: [ + { + content: { role: 'model', parts: [{ text: 'Continuing' }] }, + finishReason: 'STOP', + }, + ], + } as unknown as GenerateContentResponse; + })(), + ); + + const stream = await chat.sendMessageStream( + { model: 'gemini-2.0-flash' }, + [{ text: '' }], + 'prompt-empty-user-turn', + new AbortController().signal, + LlmRole.MAIN, + ); + for await (const _ of stream) { + // consume + } + + expect(chat.getHistory()).toEqual([ + { role: 'user', parts: [{ text: 'Hi' }] }, + { role: 'model', parts: [{ text: 'Hello' }] }, + { role: 'user', parts: [{ text: CONTINUE_PROMPT_TEXT }] }, + { role: 'model', parts: [{ text: 'Continuing' }] }, + ]); + }); + }); + + describe('ensureTerminalUserTurn', () => { + it('returns the same array when the last turn is a user turn with content', () => { + const contents: Content[] = [ + { role: 'model', parts: [{ text: 'a' }] }, + { role: 'user', parts: [{ text: 'b' }] }, + ]; + expect(ensureTerminalUserTurn(contents)).toBe(contents); + }); + + it('replaces the parts of a trailing user turn without content', () => { + const contents: Content[] = [ + { role: 'model', parts: [{ text: 'a' }] }, + { role: 'user', parts: [{ text: '' }] }, + ]; + const snapshot = structuredClone(contents); + expect(ensureTerminalUserTurn(contents)).toEqual([ + { role: 'model', parts: [{ text: 'a' }] }, + { role: 'user', parts: [{ text: CONTINUE_PROMPT_TEXT }] }, + ]); + expect(contents).toEqual(snapshot); + }); + + it('appends matching responses after a trailing model function call turn', () => { + const contents: Content[] = [ + { role: 'user', parts: [{ text: 'go' }] }, + { + role: 'model', + parts: [ + { text: 'Running tools' }, + { functionCall: { id: 'read_file__1', name: 'read_file' } }, + { functionCall: { id: '2', name: ' ' } }, + ], + }, + ]; + expect(ensureTerminalUserTurn(contents).at(-1)).toEqual({ + role: 'user', + parts: [ + { + functionResponse: { + name: 'read_file', + id: 'read_file__1', + response: { error: INTERRUPTED_TOOL_RESPONSE_ERROR }, + }, + }, + { + functionResponse: { + name: 'generic_tool', + id: '2', + response: { error: INTERRUPTED_TOOL_RESPONSE_ERROR }, + }, + }, + ], + }); + expect(contents).toHaveLength(2); + }); + + it('appends a continuation prompt after a trailing model text turn or for empty contents', () => { + expect( + ensureTerminalUserTurn([{ role: 'model', parts: [{ text: 'a' }] }]), + ).toEqual([ + { role: 'model', parts: [{ text: 'a' }] }, + { role: 'user', parts: [{ text: CONTINUE_PROMPT_TEXT }] }, + ]); + expect(ensureTerminalUserTurn([])).toEqual([ + { role: 'user', parts: [{ text: CONTINUE_PROMPT_TEXT }] }, + ]); + }); + }); + + describe('discardTrailingUnansweredToolCallTurn', () => { + it('removes only the trailing model function call turn and keeps turn ids', () => { + chat.setHistory([ + { id: 'u1', content: { role: 'user', parts: [{ text: 'go' }] } }, + { + id: 'm1', + content: { + role: 'model', + parts: [{ functionCall: { id: 'c1', name: 'a', args: {} } }], + }, + }, + { + id: 'u2', + content: { + role: 'user', + parts: [ + { functionResponse: { id: 'c1', name: 'a', response: {} } }, + ], + }, + }, + { + id: 'm2', + content: { + role: 'model', + parts: [{ functionCall: { id: 'c2', name: 'b', args: {} } }], + }, + }, + ]); + + expect(chat.discardTrailingUnansweredToolCallTurn()).toBe(true); + expect(chat.getHistoryTurns().map((t) => t.id)).toEqual([ + 'u1', + 'm1', + 'u2', + ]); + }); + + it('does nothing when the last turn is not a model function call turn', () => { + chat.setHistory([ + { id: 'u1', content: { role: 'user', parts: [{ text: 'go' }] } }, + { id: 'm1', content: { role: 'model', parts: [{ text: 'done' }] } }, + ]); + + expect(chat.discardTrailingUnansweredToolCallTurn()).toBe(false); + expect(chat.getHistoryTurns().map((t) => t.id)).toEqual(['u1', 'm1']); + }); + }); + + describe('stripToolCallIdPrefixes field preservation', () => { + it('keeps every functionCall and functionResponse field when stripping prefixes', () => { + const result = stripToolCallIdPrefixes([ + { + role: 'model', + parts: [ + { + functionCall: { + id: 'tool__1', + name: 'tool', + args: { a: 1 }, + willContinue: true, + } as Part['functionCall'], + }, + ], + }, + { + role: 'user', + parts: [ + { + functionResponse: { + id: 'tool__1', + name: 'tool', + response: { ok: true }, + willContinue: true, + }, + }, + ], + }, + ]); + + expect(result[0].parts![0].functionCall).toEqual({ + id: '1', + name: 'tool', + args: { a: 1 }, + willContinue: true, + }); + expect(result[1].parts![0].functionResponse).toEqual({ + id: '1', + name: 'tool', + response: { ok: true }, + willContinue: true, + }); + }); + }); + + describe('coalesceConsecutiveContents', () => { + it('does not merge adjacent contents without a role', () => { + const contents: Content[] = [ + { parts: [{ text: 'a' }] }, + { parts: [{ text: 'b' }] }, + ]; + expect(coalesceConsecutiveContents(contents)).toEqual(contents); + }); }); }); diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 9606c4608e5..095a5292c48 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -117,6 +117,19 @@ export const SYNTHETIC_THOUGHT_SIGNATURE = 'skip_thought_signature_validator'; export const INTERRUPTED_RESPONSE_PLACEHOLDER = '[The previous response was interrupted before it completed.]'; +/** + * Error payload used for synthesized function responses that close a model + * function call whose real response never arrived. + */ +export const INTERRUPTED_TOOL_RESPONSE_ERROR = + 'Response was lost or interrupted.'; + +/** + * Text used for a synthesized user turn when the request would otherwise not + * end with a user turn that carries content. + */ +export const CONTINUE_PROMPT_TEXT = 'Please continue.'; + /** * Internal interface for parts that carry the magic 'callIndex' property * used during model response consolidation. @@ -514,10 +527,11 @@ export class GeminiChat { // If history ended on an unanswered model tool call or an unanswered tool response // (e.g. cancelled tool call, interrupted stream, or user follow-up prompt), close it - // before recording a genuinely new user message so proper tool call pairing and - // role alternation are maintained. + // with dedicated synthetic turns before recording a genuinely new user message, so + // tool call pairing and role alternation are maintained and the user's prompt stays + // a turn of its own. if (!isOriginalFunctionResponse) { - this.closeUnansweredToolCallsTurn(userContent); + this.closeUnansweredToolCallsTurn(); this.closeUnansweredToolResponseTurn(); } @@ -641,6 +655,10 @@ export class GeminiChat { } } + // Durable history repair runs once per send, outside the retry loop, so + // retried attempts never append duplicate synthetic turns. + this.replaceEmptyTrailingUserTurn(); + const requestHistory = this.getHistoryTurns(true); const streamWithRetries = async function* ( @@ -828,39 +846,55 @@ export class GeminiChat { } /** - * Synthesizes matching functionResponse parts into userContent when history - * ends with an unclosed model functionCall, so the subsequent user prompt - * preserves proper tool call pairing and role alternation. + * Appends a dedicated synthetic user turn with matching functionResponse + * parts when history ends with an unclosed model functionCall. The + * subsequent user prompt is then recorded as a clean turn of its own (after + * `closeUnansweredToolResponseTurn` adds the closing model turn). */ - private closeUnansweredToolCallsTurn(userContent: Content): void { + private closeUnansweredToolCallsTurn(): void { const turns = this.agentHistory.get(); const last = turns[turns.length - 1]; - if ( - last?.content.role !== 'model' || - !last.content.parts?.some((part) => !!part.functionCall) - ) { + if (last?.content.role !== 'model') { return; } - const missingResponses: Part[] = []; - for (const part of last.content.parts || []) { - if (part && part.functionCall) { - missingResponses.push({ - functionResponse: { - name: part.functionCall.name?.trim() || 'generic_tool', - id: part.functionCall.id, - response: { - error: 'Response was lost or interrupted.', - }, - }, - }); - } + const missingResponses = buildInterruptedToolResponseParts( + last.content.parts ?? [], + ); + if (missingResponses.length === 0) { + return; } - if (missingResponses.length > 0) { - const remainingParts = (userContent.parts || []).filter( - (p) => !(p.text !== undefined && p.text === ''), - ); - userContent.parts = [...missingResponses, ...remainingParts]; + const id = this.chatRecordingService.recordSyntheticMessage( + 'user', + missingResponses, + ); + this.agentHistory.push({ + id, + content: { role: 'user', parts: missingResponses }, + }); + } + + /** + * Replaces the parts of a trailing user turn that carries no content (e.g. + * only empty text parts) with a continuation prompt, keeping the in-memory + * history and the session log in sync. Runs once per send so that the + * persisted history never keeps an empty user turn between model turns. + */ + private replaceEmptyTrailingUserTurn(): void { + const turns = this.agentHistory.get(); + const last = turns[turns.length - 1]; + if (last?.content.role !== 'user' || hasNonEmptyParts(last.content)) { + return; } + this.agentHistory.set([ + ...turns.slice(0, -1), + { + id: last.id, + content: { ...last.content, parts: [{ text: CONTINUE_PROMPT_TEXT }] }, + }, + ]); + this.chatRecordingService.updateMessagesFromHistory( + this.agentHistory.get(), + ); } /** @@ -1125,99 +1159,28 @@ export class GeminiChat { this.tools = await this.onModelChanged(modelToUse); } - // Track final request parameters for AfterModel hooks - lastModelToUse = modelToUse; - lastConfig = config; - lastContentsToUse = contentsToUse; - - const finalContents = stripToolCallIdPrefixes(contentsToUse); - - let contentsToDispatch = finalContents; - const lastContentTurn = - contentsToDispatch.length > 0 - ? contentsToDispatch[contentsToDispatch.length - 1] - : null; - - if ( - !lastContentTurn || - lastContentTurn.role !== 'user' || - !lastContentTurn.parts?.length - ) { + // Normalize the outbound payload so it always ends with a user turn that + // carries content. This is a pure transformation: retries, per-call + // history overrides and hook-modified contents never leak into the + // persistent history or the session log. + const normalizedContents = ensureTerminalUserTurn(contentsToUse); + if (normalizedContents !== contentsToUse) { debugLogger.warn( 'Final contents do not end with a valid user turn. Normalizing contents to satisfy Gemini API invariant.', ); - const cloned: Content[] = contentsToDispatch.map((item) => - structuredClone(item), - ); - const lastTurn = cloned.length > 0 ? cloned[cloned.length - 1] : null; - if (lastTurn && lastTurn.role === 'model') { - const hasFunctionCall = lastTurn.parts?.some( - (p) => p && p.functionCall, - ); - if (hasFunctionCall) { - const missingResponses: Part[] = []; - for (const part of lastTurn.parts || []) { - if (part && part.functionCall) { - missingResponses.push({ - functionResponse: { - name: part.functionCall.name?.trim() || 'generic_tool', - id: part.functionCall.id, - response: { - error: 'Response was lost or interrupted.', - }, - }, - }); - } - } - const normalizedUserTurn: Content = { - role: 'user', - parts: missingResponses, - }; - cloned.push(normalizedUserTurn); - const id = this.chatRecordingService.recordSyntheticMessage( - 'user', - missingResponses, - ); - this.agentHistory.push({ id, content: normalizedUserTurn }); - } else { - const normalizedUserTurn: Content = { - role: 'user', - parts: [{ text: 'Please continue.' }], - }; - cloned.push(normalizedUserTurn); - const id = this.chatRecordingService.recordSyntheticMessage( - 'user', - normalizedUserTurn.parts!, - ); - this.agentHistory.push({ id, content: normalizedUserTurn }); - } - } else if (!lastTurn) { - const normalizedUserTurn: Content = { - role: 'user', - parts: [{ text: 'Please continue.' }], - }; - cloned.push(normalizedUserTurn); - const id = this.chatRecordingService.recordSyntheticMessage( - 'user', - normalizedUserTurn.parts!, - ); - this.agentHistory.push({ id, content: normalizedUserTurn }); - } else if (lastTurn.role === 'user' && !lastTurn.parts?.length) { - lastTurn.parts = [{ text: 'Please continue.' }]; - const historyTurns = this.agentHistory.get(); - const lastHistoryTurn = historyTurns[historyTurns.length - 1]; - if (lastHistoryTurn && lastHistoryTurn.content.role === 'user') { - lastHistoryTurn.content.parts = [{ text: 'Please continue.' }]; - this.chatRecordingService.updateMessagesFromHistory(historyTurns); - } - } - contentsToDispatch = cloned; } + // Track final request parameters for AfterModel hooks + lastModelToUse = modelToUse; + lastConfig = config; + lastContentsToUse = normalizedContents; + + const finalContents = stripToolCallIdPrefixes(normalizedContents); + return this.context.config.getContentGenerator().generateContentStream( { model: modelToUse, - contents: contentsToDispatch, + contents: finalContents, config, }, prompt_id, @@ -1352,6 +1315,33 @@ export class GeminiChat { ensureStableToolIds(this.agentHistory.get() as HistoryTurn[]); } + /** + * Removes the trailing model turn when it holds function calls that were + * never answered (e.g. every call in the batch was declined). Earlier + * completed tool rounds and the originating user prompt are kept, and the + * durable IDs of the remaining turns are preserved. + * + * @returns true when a turn was removed. + */ + discardTrailingUnansweredToolCallTurn(): boolean { + const turns = this.agentHistory.get(); + const last = turns[turns.length - 1]; + if ( + last?.content.role !== 'model' || + !last.content.parts?.some((part) => !!part.functionCall) + ) { + return false; + } + this.agentHistory.rollback(turns.length - 1); + this.lastPromptTokenCount = estimateTokenCountSync( + this.agentHistory.flatMap((c) => c.content.parts || []), + ); + this.chatRecordingService.updateMessagesFromHistory( + this.agentHistory.get(), + ); + return true; + } + setHistory(history: ReadonlyArray): void { const wrappedHistory: HistoryTurn[] = history.map((item) => { if ('id' in item && 'content' in item) { @@ -1888,6 +1878,73 @@ export function isInvalidArgumentError(errorMessage: string): boolean { return errorMessage.includes('Request contains an invalid argument'); } +/** + * Returns true for a part that only holds an empty `text` value (optionally + * alongside bookkeeping keys) and no other payload. + */ +function isEmptyTextPart(part: Part): boolean { + if (part.text !== '') { + return false; + } + return !Object.keys(part).some( + (key) => key !== 'text' && key !== 'thought' && key !== 'callIndex', + ); +} + +/** + * Returns true when the content has at least one part that survives + * `stripToolCallIdPrefixes` (i.e. it is not an empty text part). + */ +function hasNonEmptyParts(content: Content): boolean { + return (content.parts ?? []).some((part) => !isEmptyTextPart(part)); +} + +/** + * Builds functionResponse parts that close every functionCall in `parts` + * whose real response was lost or interrupted. + */ +export function buildInterruptedToolResponseParts( + parts: readonly Part[], +): Part[] { + return parts + .filter((part) => !!part?.functionCall) + .map((part) => ({ + functionResponse: { + name: part.functionCall!.name?.trim() || 'generic_tool', + id: part.functionCall!.id, + response: { error: INTERRUPTED_TOOL_RESPONSE_ERROR }, + }, + })); +} + +/** + * Ensures the request contents end with a user turn that carries content, as + * required by the Gemini API. Pure: returns the same array when no change is + * needed, otherwise a new array; the input is never mutated. + * + * - A trailing user turn without content gets a continuation prompt. + * - A trailing model turn with function calls gets matching responses. + * - Any other trailing turn (or no turn at all) gets a continuation prompt. + */ +export function ensureTerminalUserTurn(contents: Content[]): Content[] { + const last = contents.at(-1); + if (last?.role === 'user' && hasNonEmptyParts(last)) { + return contents; + } + if (last?.role === 'user') { + return [ + ...contents.slice(0, -1), + { ...last, parts: [{ text: CONTINUE_PROMPT_TEXT }] }, + ]; + } + const missingResponses = buildInterruptedToolResponseParts(last?.parts ?? []); + const parts = + missingResponses.length > 0 + ? missingResponses + : [{ text: CONTINUE_PROMPT_TEXT }]; + return [...contents, { role: 'user', parts }]; +} + export function stripToolCallIdPrefixes(contents: Content[]): Content[] { const stripped = contents .map((content) => { @@ -1899,8 +1956,7 @@ export function stripToolCallIdPrefixes(contents: Content[]): Content[] { const name = fc.name?.trim() || 'generic_tool'; if (fc.id && fc.id.startsWith(`${name}__`)) { newPart.functionCall = { - name: fc.name, - args: fc.args, + ...fc, id: fc.id.substring(name.length + 2), }; } @@ -1910,10 +1966,10 @@ export function stripToolCallIdPrefixes(contents: Content[]): Content[] { const name = fr.name?.trim() || 'generic_tool'; if (fr.id && fr.id.startsWith(`${name}__`)) { newPart.functionResponse = { - name: fr.name, - response: fr.response, + // History parts are plain JSON objects, not class instances. + // eslint-disable-next-line @typescript-eslint/no-misused-spread + ...fr, id: fr.id.substring(name.length + 2), - ...(fr.parts ? { parts: fr.parts } : {}), }; } } @@ -1933,23 +1989,15 @@ export function stripToolCallIdPrefixes(contents: Content[]): Content[] { return newPart; }) - .filter((part) => { - // Filter out truly empty parts that have only text: '' and no payload - const hasOtherKeys = Object.keys(part).some( - (key) => key !== 'text' && key !== 'thought' && key !== 'callIndex', - ); - if (part.text !== undefined && part.text === '' && !hasOtherKeys) { - return false; - } - return true; - }); + // Filter out truly empty parts that have only text: '' and no payload + .filter((part) => !isEmptyTextPart(part)); return { ...content, parts, }; }) - .filter((content) => !content.parts || content.parts.length > 0); + .filter((content) => content.parts.length > 0); return coalesceConsecutiveContents(stripped); } @@ -1959,7 +2007,7 @@ export function coalesceConsecutiveContents(contents: Content[]): Content[] { for (const item of contents) { const lastIdx = result.length - 1; const last = result[lastIdx]; - if (last && last.role === item.role) { + if (last && last.role && last.role === item.role) { const hasParts = last.parts || item.parts; result[lastIdx] = { ...last, From a430cf79392c9f8212f6dac3f9f74701d3ca5a54 Mon Sep 17 00:00:00 2001 From: Luis Felipe Quevedo Date: Tue, 6 Oct 2026 20:21:51 +0000 Subject: [PATCH 5/5] fix(core): return early from discardTrailingUnansweredToolCallTurn when chat is not initialized GeminiClient.discardTrailingUnansweredToolCallTurn now checks isInitialized() before calling getChat(), so the CLI cancellation flow cannot hit 'Chat not initialized'. Adds unit tests for the uninitialized, removed and no-op paths. --- packages/core/src/core/client.test.ts | 45 +++++++++++++++++++++++++++ packages/core/src/core/client.ts | 4 +++ 2 files changed, 49 insertions(+) diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 86272c02d17..39d6ee5c102 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -351,6 +351,51 @@ describe('Gemini Client (client.ts)', () => { }); }); + describe('discardTrailingUnansweredToolCallTurn', () => { + it('should return false without throwing when the chat is not initialized', () => { + const uninitializedClient = new GeminiClient( + mockConfig as unknown as AgentLoopContext, + ); + + expect(uninitializedClient.isInitialized()).toBe(false); + expect(() => + uninitializedClient.discardTrailingUnansweredToolCallTurn(), + ).not.toThrow(); + expect(uninitializedClient.discardTrailingUnansweredToolCallTurn()).toBe( + false, + ); + }); + + it('should delegate to the chat and update telemetry when a turn is removed', () => { + const mockChat = { + discardTrailingUnansweredToolCallTurn: vi.fn().mockReturnValue(true), + getLastPromptTokenCount: vi.fn().mockReturnValue(0), + setTools: vi.fn(), + } as unknown as GeminiChat; + client['chat'] = mockChat; + vi.mocked(uiTelemetryService.setLastPromptTokenCount).mockClear(); + + expect(client.discardTrailingUnansweredToolCallTurn()).toBe(true); + expect( + mockChat.discardTrailingUnansweredToolCallTurn, + ).toHaveBeenCalledTimes(1); + expect(uiTelemetryService.setLastPromptTokenCount).toHaveBeenCalled(); + }); + + it('should not update telemetry when no turn is removed', () => { + const mockChat = { + discardTrailingUnansweredToolCallTurn: vi.fn().mockReturnValue(false), + getLastPromptTokenCount: vi.fn().mockReturnValue(0), + setTools: vi.fn(), + } as unknown as GeminiChat; + client['chat'] = mockChat; + vi.mocked(uiTelemetryService.setLastPromptTokenCount).mockClear(); + + expect(client.discardTrailingUnansweredToolCallTurn()).toBe(false); + expect(uiTelemetryService.setLastPromptTokenCount).not.toHaveBeenCalled(); + }); + }); + describe('resumeChat', () => { it('should update telemetry token count when a chat is resumed', async () => { const history: Content[] = [ diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 67506fc7a85..01bcc1e1938 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -304,8 +304,12 @@ export class GeminiClient { /** * Removes the trailing model turn when it only holds unanswered function * calls. See {@link GeminiChat.discardTrailingUnansweredToolCallTurn}. + * Returns false without side effects when the chat is not initialized. */ discardTrailingUnansweredToolCallTurn(): boolean { + if (!this.isInitialized()) { + return false; + } const removed = this.getChat().discardTrailingUnansweredToolCallTurn(); if (removed) { this.updateTelemetryTokenCount();