diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index 8a63e3e541e..5f6a51e1b72 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -1374,7 +1374,7 @@ describe('ChatRecordingService', () => { }); }); - it('should preserve multi-modal sibling parts during sync', async () => { + it('should sync only the matching function response during sync', async () => { await chatRecordingService.initialize(); const modelMsgId = chatRecordingService.recordMessage({ type: 'gemini', @@ -1440,12 +1440,10 @@ describe('ChatRecordingService', () => { type: 'gemini'; }; const result = lastMsg.toolCalls![0].result as Part[]; - expect(result).toHaveLength(2); + expect(result).toHaveLength(1); expect(result[0].functionResponse!.response).toEqual({ output: maskedSnippet, }); - expect(result[1].inlineData).toBeDefined(); - expect(result[1].inlineData!.mimeType).toBe('image/png'); }); it('should handle parts appearing BEFORE the functionResponse in a content block', async () => { @@ -1503,9 +1501,78 @@ describe('ChatRecordingService', () => { type: 'gemini'; }; const result = lastMsg.toolCalls![0].result as Part[]; - expect(result).toHaveLength(2); - expect(result[0].text).toBe('Prefix metadata or text'); - expect(result[1].functionResponse!.id).toBe(callId); + expect(result).toEqual([ + { + functionResponse: { + name: 'read_file', + id: callId, + response: { output: 'file content' }, + }, + }, + ]); + }); + + it('keeps parallel tool results isolated to their matching call', async () => { + const modelMsgId = chatRecordingService.recordMessage({ + type: 'gemini', + content: '', + model: 'gemini-pro', + }); + const calls = ['call-a', 'call-b'].map((id) => ({ + id, + name: 'read_file', + args: { path: `${id}.txt` }, + result: [], + status: CoreToolCallStatus.Success, + timestamp: new Date().toISOString(), + })); + chatRecordingService.recordToolCalls('gemini-pro', calls); + + chatRecordingService.updateMessagesFromHistory([ + { id: modelMsgId, content: { role: 'model', parts: [] } }, + { + id: 'responses', + content: { + role: 'user', + parts: calls.map((call) => ({ + functionResponse: { + id: call.id, + name: call.name, + response: { output: call.id }, + }, + })), + }, + }, + ]); + + const conversation = (await loadConversationRecord( + chatRecordingService.getConversationFilePath()!, + )) as ConversationRecord; + const toolCalls = ( + conversation.messages[0] as MessageRecord & { + type: 'gemini'; + } + ).toolCalls!; + expect(toolCalls.map((call) => call.result)).toEqual([ + [ + { + functionResponse: { + id: 'call-a', + name: 'read_file', + response: { output: 'call-a' }, + }, + }, + ], + [ + { + functionResponse: { + id: 'call-b', + name: 'read_file', + response: { output: 'call-b' }, + }, + }, + ], + ]); }); it('should not write to disk when no tool calls match', async () => { diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 186282eb1d0..e9964a17417 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -1003,13 +1003,17 @@ export class ChatRecordingService { if (geminiMsg && geminiMsg.type === 'gemini') { const tc = geminiMsg.toolCalls!.find((tc) => tc.id === callId); if (tc) { - // If the history version is different (e.g. masked), sync it into the record - // We sync the entire parts array of the user turn to ensure sibling parts are preserved + // If the history version is different (e.g. masked), sync it into the record. + // Do not copy responses for sibling calls: doing so causes each + // ToolCallRecord to replay every response in a parallel turn. + const matchingPart = (turn.content.parts || []).find( + (candidate) => candidate.functionResponse?.id === callId, + ); if ( - JSON.stringify(tc.result) !== - JSON.stringify(turn.content.parts) + matchingPart && + JSON.stringify(tc.result) !== JSON.stringify([matchingPart]) ) { - tc.result = turn.content.parts || []; + tc.result = [matchingPart]; updated = true; } } diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index f519785ce19..3114a48bb91 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -28,6 +28,7 @@ import { ExecutionLifecycleService } from './executionLifecycleService.js'; import type { AnsiOutput, AnsiToken } from '../utils/terminalSerializer.js'; // Hoisted Mocks +const mockRealpathSync = vi.hoisted(() => vi.fn()); const mockPtySpawn = vi.hoisted(() => vi.fn()); const mockCpSpawn = vi.hoisted(() => vi.fn()); const mockIsBinary = vi.hoisted(() => vi.fn()); @@ -71,12 +72,14 @@ vi.mock('node:fs', async (importOriginal) => { statSync: mockStatSync, fstatSync: mockFstatSync, closeSync: mockCloseSync, + realpathSync: mockRealpathSync, }, mkdirSync: mockMkdirSync, createWriteStream: mockCreateWriteStream, statSync: mockStatSync, fstatSync: mockFstatSync, closeSync: mockCloseSync, + realpathSync: mockRealpathSync, }; }); vi.mock('../utils/shell-utils.js', async (importOriginal) => { @@ -1464,6 +1467,7 @@ describe('ShellExecutionService', () => { rdev: targetRdev, isCharacterDevice: () => true, }); + mockRealpathSync.mockReturnValue('/dev/ttys001'); mockFstatSync.mockImplementation((fd: number) => { if (fd === 12) { return { rdev: targetRdev, isCharacterDevice: () => true }; @@ -1473,6 +1477,7 @@ describe('ShellExecutionService', () => { const result = closeOrphanSlaveFd(10, '/dev/ttys001'); expect(result).toBe(12); + expect(mockCloseSync).toHaveBeenCalledTimes(1); expect(mockCloseSync).toHaveBeenCalledWith(12); }); diff --git a/packages/core/src/utils/sessionUtils.test.ts b/packages/core/src/utils/sessionUtils.test.ts index 141efab5723..3defb8f8175 100644 --- a/packages/core/src/utils/sessionUtils.test.ts +++ b/packages/core/src/utils/sessionUtils.test.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ import { describe, it, expect } from 'vitest'; +import { type Part } from '@google/genai'; import { convertSessionToClientHistory } from './sessionUtils.js'; import { type ConversationRecord } from '../services/chatRecordingService.js'; import { CoreToolCallStatus } from '../scheduler/types.js'; @@ -186,6 +187,75 @@ describe('convertSessionToClientHistory', () => { ]); }); + it('does not replay tool results already stored as durable user responses', () => { + const responsePart: Part = { + functionResponse: { + id: 'call123', + name: 'ls', + response: { output: 'file.txt' }, + }, + }; + const messages: ConversationRecord['messages'] = [ + { + id: 'msg1', + type: 'gemini', + timestamp: '2024-01-01T10:01:00Z', + content: 'Let me check.', + toolCalls: [ + { + id: 'call123', + name: 'ls', + args: { dir: '.' }, + status: CoreToolCallStatus.Success, + timestamp: '2024-01-01T10:01:05Z', + result: [responsePart], + }, + ], + }, + { + id: 'msg1_response', + type: 'user', + timestamp: '2024-01-01T10:01:06Z', + content: [responsePart], + }, + ]; + + const history = convertSessionToClientHistory(messages); + const responses = history.flatMap((turn) => + (turn.content.parts || []).filter((part) => part.functionResponse), + ); + + expect(responses).toEqual([responsePart]); + }); + + it('keeps only one response per call ID in already duplicated sessions', () => { + const responsePart: Part = { + functionResponse: { + id: 'call123', + name: 'ls', + response: { output: 'file.txt' }, + }, + }; + const messages: ConversationRecord['messages'] = [ + { + id: 'response-1', + type: 'user', + timestamp: '2024-01-01T10:01:06Z', + content: [responsePart], + }, + { + id: 'response-2', + type: 'user', + timestamp: '2024-01-01T10:01:07Z', + content: [responsePart], + }, + ]; + + const history = convertSessionToClientHistory(messages); + expect(history).toHaveLength(1); + expect(history[0].content.parts).toEqual([responsePart]); + }); + it('should preserve multi-modal parts (inlineData)', () => { const messages: ConversationRecord['messages'] = [ { diff --git a/packages/core/src/utils/sessionUtils.ts b/packages/core/src/utils/sessionUtils.ts index 9dd30c2e890..a1dfdae13c6 100644 --- a/packages/core/src/utils/sessionUtils.ts +++ b/packages/core/src/utils/sessionUtils.ts @@ -111,6 +111,19 @@ export function convertSessionToClientHistory( messages: ConversationRecord['messages'], ): HistoryTurn[] { const clientHistory: HistoryTurn[] = []; + // Modern recordings persist a tool response both on the ToolCallRecord and as + // a durable user turn. Prefer the latter when it exists; regenerating the + // former would answer the same function call twice on resume. + const recordedResponseIds = new Set( + messages.flatMap((message) => + message.type === 'user' + ? ensurePartArray(message.content).flatMap((part) => + part.functionResponse?.id ? [part.functionResponse.id] : [], + ) + : [], + ), + ); + const emittedResponseIds = new Set(); for (const msg of messages) { if (msg.type === 'info' || msg.type === 'error' || msg.type === 'warning') { @@ -124,13 +137,26 @@ export function convertSessionToClientHistory( continue; } - clientHistory.push({ - id: msg.id, - content: { - role: 'user', - parts: ensurePartArray(msg.content), - }, + // Some already-resumed sessions contain synthetic response turns in + // addition to the original durable response turn. Retain the first + // response for a call ID so those sessions can be recovered as well. + const parts = ensurePartArray(msg.content).filter((part) => { + const responseId = part.functionResponse?.id; + if (!responseId) return true; + if (emittedResponseIds.has(responseId)) return false; + emittedResponseIds.add(responseId); + return true; }); + + if (parts.length > 0) { + clientHistory.push({ + id: msg.id, + content: { + role: 'user', + parts, + }, + }); + } } else if (msg.type === 'gemini') { const modelParts: Part[] = []; @@ -186,7 +212,7 @@ export function convertSessionToClientHistory( if (msg.toolCalls && msg.toolCalls.length > 0) { const functionResponseParts: Part[] = []; for (const toolCall of msg.toolCalls) { - if (toolCall.result) { + if (toolCall.result && !recordedResponseIds.has(toolCall.id)) { let responseData: Part; if (typeof toolCall.result === 'string') { @@ -200,7 +226,14 @@ export function convertSessionToClientHistory( }, }; } else if (Array.isArray(toolCall.result)) { - functionResponseParts.push(...ensurePartArray(toolCall.result)); + // A result belongs only to its matching call. In particular, + // do not replay sibling responses that may have been copied + // into this result by an older session checkpoint. + functionResponseParts.push( + ...ensurePartArray(toolCall.result).filter( + (part) => part.functionResponse?.id === toolCall.id, + ), + ); continue; } else { responseData = toolCall.result;