diff --git a/packages/cli/src/acp/acpResume.test.ts b/packages/cli/src/acp/acpResume.test.ts index c38b23d7151..c59a73205ba 100644 --- a/packages/cli/src/acp/acpResume.test.ts +++ b/packages/cli/src/acp/acpResume.test.ts @@ -310,4 +310,67 @@ describe('GeminiAgent Session Resume', () => { ); }); }); + + it('should resolve session before initializing config or fresh chat recording', async () => { + const callOrder: string[] = []; + const resolveSessionMock = vi.fn().mockImplementation(async () => { + callOrder.push('resolveSession'); + return { + sessionData: { + sessionId: 'same-minute-session-id', + messages: [{ type: 'user', content: [{ text: 'Hello' }] }], + }, + sessionPath: '/path/to/session.jsonl', + }; + }); + + (SessionSelector as unknown as Mock).mockImplementation(() => ({ + resolveSession: resolveSessionMock, + })); + mockConfig.initialize.mockImplementation(async () => { + callOrder.push('config.initialize'); + }); + vi.mocked(mockConfig.getGeminiClient().resumeChat).mockImplementation( + async () => { + callOrder.push('geminiClient.resumeChat'); + }, + ); + (convertSessionToClientHistory as unknown as Mock).mockReturnValue([ + { role: 'user', parts: [{ text: 'Hello' }] }, + ]); + + await agent.loadSession({ + sessionId: 'same-minute-session-id', + cwd: '/tmp', + mcpServers: [], + }); + + expect(callOrder).toEqual([ + 'resolveSession', + 'config.initialize', + 'geminiClient.resumeChat', + ]); + expect(mockConfig.getGeminiClient().initialize).not.toHaveBeenCalled(); + }); + + it('should not initialize config if session resolution fails', async () => { + (SessionSelector as unknown as Mock).mockImplementation(() => ({ + resolveSession: vi + .fn() + .mockRejectedValue( + new Error('No previous sessions found for this project.'), + ), + })); + + await expect( + agent.loadSession({ + sessionId: 'missing-session-id', + cwd: '/tmp', + mcpServers: [], + }), + ).rejects.toThrow('No previous sessions found for this project.'); + + expect(mockConfig.initialize).not.toHaveBeenCalled(); + expect(mockConfig.getGeminiClient().resumeChat).not.toHaveBeenCalled(); + }); }); diff --git a/packages/cli/src/acp/acpSessionManager.ts b/packages/cli/src/acp/acpSessionManager.ts index cfa7037a24b..f1a76a01dba 100644 --- a/packages/cli/src/acp/acpSessionManager.ts +++ b/packages/cli/src/acp/acpSessionManager.ts @@ -126,7 +126,9 @@ export class AcpSessionManager { const geminiClient = config.getGeminiClient(); - const chat = await geminiClient.startChat(); + const chat = geminiClient.isInitialized?.() + ? geminiClient.getChat() + : await geminiClient.startChat(); const session = new Session( sessionId, @@ -165,22 +167,26 @@ export class AcpSessionManager { { sessionId, cwd, mcpServers }: acp.LoadSessionRequest, authDetails: AuthDetails, ): Promise { - const config = await this.initializeSessionConfig( + const config = await this.prepareSessionConfig( sessionId, cwd, mcpServers, authDetails, ); + await config.storage?.initialize?.(); const sessionSelector = new SessionSelector(config.storage); const { sessionData, sessionPath } = await sessionSelector.resolveSession(sessionId); + await config.initialize(); + startupProfiler.flush(config); + startAutoMemoryIfEnabled(config); + const clientHistory = convertSessionToClientHistory(sessionData.messages); const geminiClient = config.getGeminiClient(); - await geminiClient.initialize(); await geminiClient.resumeChat(clientHistory, { conversation: sessionData, filePath: sessionPath, @@ -228,7 +234,7 @@ export class AcpSessionManager { return response; } - private async initializeSessionConfig( + private async prepareSessionConfig( sessionId: string, cwd: string, mcpServers: acp.McpServer[], @@ -274,12 +280,6 @@ export class AcpSessionManager { config.setFileSystemService(acpFileSystemService); } - // 4. Now that we are authenticated, it is safe to initialize the config - // which starts the MCP servers and other heavy resources. - await config.initialize(); - startupProfiler.flush(config); - startAutoMemoryIfEnabled(config); - return config; } diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 6dc4b60fda3..c39c9b815c3 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -340,6 +340,20 @@ export class GeminiClient { history: ReadonlyArray, resumedSessionData?: ResumedSessionData, ): Promise { + if (resumedSessionData?.filePath) { + const previousRecordingService = this.chat?.getChatRecordingService?.(); + if ( + previousRecordingService && + previousRecordingService.getConversationFilePath?.() !== + resumedSessionData.filePath + ) { + try { + await previousRecordingService.deleteCurrentSessionIfNotResumableAsync?.(); + } catch { + // Best-effort cleanup of abandoned startup-only session file + } + } + } this.chat = await this.startChat(history, resumedSessionData); this.updateTelemetryTokenCount(); } diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index 8a63e3e541e..6a9209919a0 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -213,6 +213,49 @@ describe('ChatRecordingService', () => { expect(files[0]).toMatch(/^session-.*-test-ses\.jsonl$/); }); + it('should not append to or poison an existing session file in the same UTC minute', async () => { + await chatRecordingService.initialize(); + chatRecordingService.recordMessage({ + type: 'user', + content: 'Reply with exactly: alpha', + model: 'gemini-pro', + }); + chatRecordingService.recordMessage({ + type: 'gemini', + content: 'alpha', + model: 'gemini-pro', + }); + + const originalFilePath = chatRecordingService.getConversationFilePath()!; + expect(fs.existsSync(originalFilePath)).toBe(true); + + // A second fresh initialization in the same UTC minute for the same sessionId + // (e.g. during eager config initialization before resumeChat) must not append a + // context-only checkpoint onto the existing session file. + const secondRecordingService = new ChatRecordingService(mockConfig); + await secondRecordingService.initialize(); + secondRecordingService.updateMessagesFromHistory([ + { + id: 'ctx-1', + content: { + role: 'user', + parts: [{ text: 'env' }], + }, + } as HistoryTurn, + ]); + + const secondFilePath = secondRecordingService.getConversationFilePath()!; + expect(secondFilePath).not.toBe(originalFilePath); + expect(path.basename(secondFilePath)).toMatch( + /^session-.*-1-test-ses\.jsonl$/, + ); + + const reloadedOriginal = await loadConversationRecord(originalFilePath); + expect(reloadedOriginal).not.toBeNull(); + expect(reloadedOriginal?.hasResumableContent).toBe(true); + expect(reloadedOriginal?.messages).toHaveLength(2); + }); + it('should include the conversation kind when specified', async () => { await chatRecordingService.initialize(undefined, 'subagent'); chatRecordingService.recordMessage({ diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 186282eb1d0..7bd6156b06a 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -511,10 +511,12 @@ export class ChatRecordingService { if (this.kind === 'subagent') { filename = `${safeSessionId}.jsonl`; } else { - filename = `${SESSION_FILE_PREFIX}${timestamp}-${safeSessionId.slice( - 0, - 8, - )}.jsonl`; + const shortId = safeSessionId.slice(0, 8); + filename = `${SESSION_FILE_PREFIX}${timestamp}-${shortId}.jsonl`; + let collisionIndex = 1; + while (fs.existsSync(path.join(chatsDir, filename))) { + filename = `${SESSION_FILE_PREFIX}${timestamp}-${collisionIndex++}-${shortId}.jsonl`; + } } this.conversationFile = path.join(chatsDir, filename);