diff --git a/packages/cli/src/acp/acpRpcDispatcher.test.ts b/packages/cli/src/acp/acpRpcDispatcher.test.ts index 6dff01c751a..a08be48470b 100644 --- a/packages/cli/src/acp/acpRpcDispatcher.test.ts +++ b/packages/cli/src/acp/acpRpcDispatcher.test.ts @@ -336,4 +336,16 @@ describe('GeminiAgent - RPC Dispatcher', () => { }), ).rejects.toThrow('Session not found: unknown'); }); + + it('should delegate dispose to sessionManager', async () => { + const disposeMock = vi.fn().mockResolvedValue(undefined); + (agent as unknown as { sessionManager: { dispose: Mock } }).sessionManager = + { + dispose: disposeMock, + }; + + await agent.dispose(); + + expect(disposeMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/cli/src/acp/acpRpcDispatcher.ts b/packages/cli/src/acp/acpRpcDispatcher.ts index a7d7d26e61d..8d30d8f8825 100644 --- a/packages/cli/src/acp/acpRpcDispatcher.ts +++ b/packages/cli/src/acp/acpRpcDispatcher.ts @@ -33,8 +33,8 @@ export class GeminiAgent { this.sessionManager = new AcpSessionManager(settings, argv, connection); } - dispose(): void { - this.sessionManager.dispose(); + async dispose(): Promise { + await this.sessionManager.dispose(); } async initialize( diff --git a/packages/cli/src/acp/acpSession.test.ts b/packages/cli/src/acp/acpSession.test.ts index 96ddff304d2..eb97ca78ee1 100644 --- a/packages/cli/src/acp/acpSession.test.ts +++ b/packages/cli/src/acp/acpSession.test.ts @@ -1366,4 +1366,18 @@ describe('Session', () => { ); }); }); + + describe('dispose', () => { + it('should safely dispose without throwing when config.dispose is undefined', async () => { + delete (mockConfig as { dispose?: unknown }).dispose; + await expect(session.dispose()).resolves.toBeUndefined(); + }); + + it('should catch rejection when config.dispose rejects', async () => { + mockConfig.dispose = vi + .fn() + .mockRejectedValue(new Error('Disposal failed')); + await expect(session.dispose()).resolves.toBeUndefined(); + }); + }); }); diff --git a/packages/cli/src/acp/acpSession.ts b/packages/cli/src/acp/acpSession.ts index 1c00d5cc08f..1c674df96b0 100644 --- a/packages/cli/src/acp/acpSession.ts +++ b/packages/cli/src/acp/acpSession.ts @@ -188,12 +188,19 @@ export class Session { } }; - dispose(): void { + async dispose(): Promise { coreEvents.off( CoreEvent.ApprovalModeChanged, this.handleApprovalModeChanged, ); this.disposeController.abort(); + if (this.context.config?.dispose) { + try { + await this.context.config.dispose(); + } catch (err) { + debugLogger.error(`Error disposing config: ${err}`); + } + } } async cancelPendingPrompt(): Promise { diff --git a/packages/cli/src/acp/acpSessionManager.test.ts b/packages/cli/src/acp/acpSessionManager.test.ts index adeb07e1e4b..b218e5de026 100644 --- a/packages/cli/src/acp/acpSessionManager.test.ts +++ b/packages/cli/src/acp/acpSessionManager.test.ts @@ -15,13 +15,18 @@ import { type Mocked, } from 'vitest'; import { AcpSessionManager } from './acpSessionManager.js'; -import type * as acp from '@agentclientprotocol/sdk'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import * as acp from '@agentclientprotocol/sdk'; import { AuthType, type Config, + CoreEvent, + coreEvents, GEMINI_MODEL_ALIAS_AUTO, type MessageBus, - type Storage, + Storage, } from '@google/gemini-cli-core'; import type { LoadedSettings } from '../config/settings.js'; import { loadCliConfig, type CliArgs } from '../config/config.js'; @@ -56,6 +61,7 @@ describe('AcpSessionManager', () => { mockConfig = { refreshAuth: vi.fn(), initialize: vi.fn(), + dispose: vi.fn(), waitForMcpInit: vi.fn(), getFileSystemService: vi.fn(), setFileSystemService: vi.fn(), @@ -64,6 +70,8 @@ describe('AcpSessionManager', () => { getModel: vi.fn().mockReturnValue('gemini-pro'), getGeminiClient: vi.fn().mockReturnValue({ startChat: vi.fn().mockResolvedValue({}), + resumeChat: vi.fn().mockResolvedValue(undefined), + getChat: vi.fn().mockReturnValue({}), }), getMessageBus: vi.fn().mockReturnValue({ publish: vi.fn(), @@ -379,4 +387,313 @@ describe('AcpSessionManager', () => { expect(startAutoMemoryIfEnabledMock).toHaveBeenCalledWith(mockConfig); }); + + it('should successfully load an ACP session created by newSession without resumable content filters', async () => { + const testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'acp-load-test-')); + const sessionId = 'test-session-uuid-123'; + const storage = new Storage(testDir, sessionId); + await storage.initialize(); + const chatsDir = path.join(storage.getProjectTempDir(), 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + // Initial header written by ChatRecordingService on newSession (hasResumableContent is false) + const initialRecord = { + sessionId, + projectHash: 'test-hash', + startTime: new Date().toISOString(), + lastUpdated: new Date().toISOString(), + kind: 'main', + messages: [], + }; + await fs.writeFile( + path.join(chatsDir, `session-2026-09-30-${sessionId.slice(0, 8)}.jsonl`), + JSON.stringify(initialRecord) + '\n', + ); + + const response = await manager.loadSession( + { + sessionId, + cwd: testDir, + mcpServers: [], + }, + {}, + ); + + expect(response).toBeDefined(); + expect(response.modes).toBeDefined(); + expect(response.models).toBeDefined(); + expect(mockConfig.getGeminiClient().resumeChat).toHaveBeenCalledWith( + [], + expect.objectContaining({ + conversation: expect.objectContaining({ sessionId }), + }), + ); + }); + + it('should successfully load an ACP session with conversational content', async () => { + const testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'acp-load-chat-')); + const sessionId = 'test-session-with-chat'; + const storage = new Storage(testDir, sessionId); + await storage.initialize(); + const chatsDir = path.join(storage.getProjectTempDir(), 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const sessionRecord = { + sessionId, + projectHash: 'test-hash', + startTime: new Date().toISOString(), + lastUpdated: new Date().toISOString(), + kind: 'main', + messages: [ + { type: 'user', content: 'hello' }, + { type: 'gemini', content: 'world' }, + ], + }; + await fs.writeFile( + path.join(chatsDir, `session-2026-09-30-${sessionId.slice(0, 8)}.jsonl`), + JSON.stringify(sessionRecord) + '\n', + ); + + const response = await manager.loadSession( + { + sessionId, + cwd: testDir, + mcpServers: [], + }, + {}, + ); + + expect(response).toBeDefined(); + expect(mockConfig.getGeminiClient().resumeChat).toHaveBeenCalled(); + }); + + it('should reject loading an invalid session identifier without leaking event listeners', async () => { + const testDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'acp-load-invalid-'), + ); + const initialListenerCount = coreEvents.listenerCount( + CoreEvent.ModelChanged, + ); + + // Call loadSession with an invalid/non-existent session ID 15 times + for (let i = 0; i < 15; i++) { + await expect( + manager.loadSession( + { + sessionId: `non-existent-id-${i}`, + cwd: testDir, + mcpServers: [], + }, + {}, + ), + ).rejects.toThrow('Invalid session identifier'); + } + + // Verify no listeners were leaked + expect(coreEvents.listenerCount(CoreEvent.ModelChanged)).toBe( + initialListenerCount, + ); + }); + + it('should reject loading a session identifier containing path traversal without performing file operations', async () => { + const testDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'acp-load-traversal-'), + ); + + await expect( + manager.loadSession( + { + sessionId: '../../evil', + cwd: testDir, + mcpServers: [], + }, + {}, + ), + ).rejects.toSatisfy((error) => { + expect(error).toBeInstanceOf(acp.RequestError); + expect((error as acp.RequestError).code).toBe(-32602); + expect((error as acp.RequestError).message).toBe( + 'Invalid session identifier format.', + ); + return true; + }); + + await expect( + manager.loadSession( + { + sessionId: 'path/with/slash', + cwd: testDir, + mcpServers: [], + }, + {}, + ), + ).rejects.toThrow('Invalid session identifier format.'); + + await expect( + manager.loadSession( + { + sessionId: 'path\\with\\backslash', + cwd: testDir, + mcpServers: [], + }, + {}, + ), + ).rejects.toThrow('Invalid session identifier format.'); + }); + + it('should dispose an existing session before initializing new config when reloading a session', async () => { + const testDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'acp-reload-test-'), + ); + const sessionId = 'test-session-reload-123'; + const storage = new Storage(testDir, sessionId); + await storage.initialize(); + const chatsDir = path.join(storage.getProjectTempDir(), 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const sessionRecord = { + sessionId, + projectHash: 'test-hash', + startTime: new Date().toISOString(), + lastUpdated: new Date().toISOString(), + kind: 'main', + messages: [{ type: 'user', content: 'hello' }], + }; + await fs.writeFile( + path.join(chatsDir, `session-2026-09-30-${sessionId.slice(0, 8)}.jsonl`), + JSON.stringify(sessionRecord) + '\n', + ); + + // First load + await manager.loadSession( + { + sessionId, + cwd: testDir, + mcpServers: [], + }, + {}, + ); + + const firstSession = manager.getSession(sessionId); + expect(firstSession).toBeDefined(); + const disposeSpy = vi.spyOn(firstSession!, 'dispose'); + + // Second load with the same sessionId + await manager.loadSession( + { + sessionId, + cwd: testDir, + mcpServers: [], + }, + {}, + ); + + expect(disposeSpy).toHaveBeenCalledTimes(1); + const secondSession = manager.getSession(sessionId); + expect(secondSession).toBeDefined(); + expect(secondSession).not.toBe(firstSession); + }); + + it('should dispose config when newSession initialization fails', async () => { + mockConfig.getContentGeneratorConfig = vi.fn().mockReturnValue({ + apiKey: 'test-key', + }); + mockConfig.initialize = vi.fn().mockRejectedValue(new Error('Init failed')); + + await expect( + manager.newSession( + { + cwd: '/tmp', + mcpServers: [], + }, + {}, + ), + ).rejects.toThrow('Init failed'); + + expect(mockConfig.dispose).toHaveBeenCalled(); + }); + + it('should await session disposals and clear sessions on dispose', async () => { + mockConfig.getContentGeneratorConfig = vi.fn().mockReturnValue({ + apiKey: 'test-key', + }); + const response = await manager.newSession( + { + cwd: '/tmp', + mcpServers: [], + }, + {}, + ); + + const session = manager.getSession(response.sessionId); + expect(session).toBeDefined(); + const disposeSpy = vi.spyOn(session!, 'dispose'); + + await manager.dispose(); + + expect(disposeSpy).toHaveBeenCalledTimes(1); + expect(manager.getSession(response.sessionId)).toBeUndefined(); + }); + + it('should dispose session and remove from manager when newSession fails after session instantiation', async () => { + mockConfig.getContentGeneratorConfig = vi.fn().mockReturnValue({ + apiKey: 'test-key', + }); + mockConfig.getModel = vi.fn().mockImplementation(() => { + throw new Error('Post-session failure'); + }); + + await expect( + manager.newSession( + { + cwd: '/tmp', + mcpServers: [], + }, + {}, + ), + ).rejects.toThrow('Post-session failure'); + + expect(mockConfig.dispose).toHaveBeenCalled(); + expect(manager.getSession('test-session-id')).toBeUndefined(); + }); + + it('should dispose session and remove from manager when loadSession fails after session instantiation', async () => { + const testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-test-')); + const sessionId = '11111111-2222-3333-4444-555555555555'; + const storage = new Storage(testDir); + await storage.initialize(); + const chatsDir = path.join(storage.getProjectTempDir(), 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const sessionRecord = { + sessionId, + projectHash: 'test-hash', + startTime: new Date().toISOString(), + lastUpdated: new Date().toISOString(), + kind: 'main', + messages: [{ type: 'user', content: 'hello' }], + }; + await fs.writeFile( + path.join(chatsDir, `session-2026-09-30-${sessionId.slice(0, 8)}.jsonl`), + JSON.stringify(sessionRecord) + '\n', + ); + + mockConfig.getModel = vi.fn().mockImplementation(() => { + throw new Error('Post-session load failure'); + }); + + await expect( + manager.loadSession( + { + sessionId, + cwd: testDir, + mcpServers: [], + }, + {}, + ), + ).rejects.toThrow('Post-session load failure'); + + expect(mockConfig.dispose).toHaveBeenCalled(); + expect(manager.getSession(sessionId)).toBeUndefined(); + }); }); diff --git a/packages/cli/src/acp/acpSessionManager.ts b/packages/cli/src/acp/acpSessionManager.ts index f1a76a01dba..55c97938075 100644 --- a/packages/cli/src/acp/acpSessionManager.ts +++ b/packages/cli/src/acp/acpSessionManager.ts @@ -12,6 +12,7 @@ import { startupProfiler, convertSessionToClientHistory, createPolicyUpdater, + Storage, } from '@google/gemini-cli-core'; import * as acp from '@agentclientprotocol/sdk'; import { randomUUID } from 'node:crypto'; @@ -48,10 +49,17 @@ export class AcpSessionManager { return this.sessions.get(sessionId); } - dispose(): void { - for (const session of this.sessions.values()) { - session.dispose(); - } + async dispose(): Promise { + const disposePromises = Array.from(this.sessions.entries()).map( + async ([sessionId, session]) => { + try { + await session.dispose(); + } catch (err) { + debugLogger.error(`Error disposing session ${sessionId}: ${err}`); + } + }, + ); + await Promise.all(disposePromises); this.sessions.clear(); } @@ -103,135 +111,206 @@ export class AcpSessionManager { } if (!isAuthenticated) { + try { + await config?.dispose?.(); + } catch (disposeError) { + debugLogger.error(`Error disposing config: ${disposeError}`); + } throw new acp.RequestError( -32000, authErrorMessage || 'Authentication required.', ); } - if (this.clientCapabilities?.fs) { - const acpFileSystemService = new AcpFileSystemService( - this.connection, - sessionId, - this.clientCapabilities.fs, - config.getFileSystemService(), - cwd, - ); - config.setFileSystemService(acpFileSystemService); - } + let session: Session | undefined; + try { + if (this.clientCapabilities?.fs) { + const acpFileSystemService = new AcpFileSystemService( + this.connection, + sessionId, + this.clientCapabilities.fs, + config.getFileSystemService(), + cwd, + ); + config.setFileSystemService(acpFileSystemService); + } - await config.initialize(); - startupProfiler.flush(config); - startAutoMemoryIfEnabled(config); + await config.initialize(); + startupProfiler.flush(config); + startAutoMemoryIfEnabled(config); - const geminiClient = config.getGeminiClient(); + const geminiClient = config.getGeminiClient(); - const chat = geminiClient.isInitialized?.() - ? geminiClient.getChat() - : await geminiClient.startChat(); + const chat = geminiClient.isInitialized?.() + ? geminiClient.getChat() + : await geminiClient.startChat(); - const session = new Session( - sessionId, - chat, - config, - this.connection, - this.settings, - ); - this.sessions.set(sessionId, session); - - setTimeout(() => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - session.sendAvailableCommands(); - }, 0); + session = new Session( + sessionId, + chat, + config, + this.connection, + this.settings, + ); + this.sessions.set(sessionId, session); - const { availableModels, currentModelId } = buildAvailableModels( - config, - loadedSettings, - ); + const { availableModels, currentModelId } = buildAvailableModels( + config, + loadedSettings, + ); - const response = { - sessionId, - modes: { - availableModes: buildAvailableModes(config.isPlanEnabled()), - currentModeId: config.getApprovalMode(), - }, - models: { - availableModels, - currentModelId, - }, - }; - return response; + const response = { + sessionId, + modes: { + availableModes: buildAvailableModes(config.isPlanEnabled()), + currentModeId: config.getApprovalMode(), + }, + models: { + availableModels, + currentModelId, + }, + }; + + setTimeout(() => { + session?.sendAvailableCommands().catch((err) => { + debugLogger.error(`Error sending available commands: ${err}`); + }); + }, 0); + + return response; + } catch (error) { + if (session) { + this.sessions.delete(sessionId); + try { + await session.dispose(); + } catch (disposeError) { + debugLogger.error( + `Error disposing session in newSession: ${disposeError}`, + ); + } + } else if (config) { + try { + await config.dispose?.(); + } catch (disposeError) { + debugLogger.error( + `Error disposing config in newSession: ${disposeError}`, + ); + } + } + throw error; + } } async loadSession( { sessionId, cwd, mcpServers }: acp.LoadSessionRequest, authDetails: AuthDetails, ): Promise { - 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); + if (!/^[a-zA-Z0-9-_]+$/.test(sessionId)) { + throw new acp.RequestError(-32602, 'Invalid session identifier format.'); + } - const geminiClient = config.getGeminiClient(); - await geminiClient.resumeChat(clientHistory, { - conversation: sessionData, - filePath: sessionPath, - }); + const storage = new Storage(cwd); + await storage.initialize(); + const sessionSelector = new SessionSelector(storage); - const session = new Session( + const { sessionData, sessionPath } = await sessionSelector.resolveSession( sessionId, - geminiClient.getChat(), - config, - this.connection, - this.settings, + { allowEmpty: true }, ); const existingSession = this.sessions.get(sessionId); if (existingSession) { - existingSession.dispose(); + try { + await existingSession.dispose(); + } catch (err) { + debugLogger.error( + `Error disposing existing session ${sessionId}: ${err}`, + ); + } finally { + this.sessions.delete(sessionId); + } } - this.sessions.set(sessionId, session); + let config: Config | undefined; + let session: Session | undefined; + try { + config = await this.prepareSessionConfig( + sessionId, + cwd, + mcpServers, + authDetails, + ); + + await config.initialize(); + startupProfiler.flush(config); + startAutoMemoryIfEnabled(config); - // Stream history back to client - // eslint-disable-next-line @typescript-eslint/no-floating-promises - session.streamHistory(sessionData.messages); + const messages = sessionData.messages ?? []; + const clientHistory = convertSessionToClientHistory(messages); - setTimeout(() => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - session.sendAvailableCommands(); - }, 0); + const geminiClient = config.getGeminiClient(); + await geminiClient.resumeChat(clientHistory, { + conversation: sessionData, + filePath: sessionPath, + }); - const { availableModels, currentModelId } = buildAvailableModels( - config, - this.settings, - ); + session = new Session( + sessionId, + geminiClient.getChat(), + config, + this.connection, + this.settings, + ); - const response = { - modes: { - availableModes: buildAvailableModes(config.isPlanEnabled()), - currentModeId: config.getApprovalMode(), - }, - models: { - availableModels, - currentModelId, - }, - }; - return response; + this.sessions.set(sessionId, session); + + const { availableModels, currentModelId } = buildAvailableModels( + config, + this.settings, + ); + + const response = { + modes: { + availableModes: buildAvailableModes(config.isPlanEnabled()), + currentModeId: config.getApprovalMode(), + }, + models: { + availableModels, + currentModelId, + }, + }; + + // Stream history back to client + session.streamHistory(messages).catch((err) => { + debugLogger.error(`Error streaming history: ${err}`); + }); + + setTimeout(() => { + session?.sendAvailableCommands().catch((err) => { + debugLogger.error(`Error sending available commands: ${err}`); + }); + }, 0); + + return response; + } catch (error) { + if (session) { + this.sessions.delete(sessionId); + try { + await session.dispose(); + } catch (disposeError) { + debugLogger.error( + `Error disposing session in loadSession: ${disposeError}`, + ); + } + } else if (config) { + try { + await config.dispose?.(); + } catch (disposeError) { + debugLogger.error(`Error disposing config: ${disposeError}`); + } + } + throw error; + } } private async prepareSessionConfig( @@ -265,19 +344,33 @@ export class AcpSessionManager { ); } catch (e) { debugLogger.error(`Authentication failed: ${e}`); + try { + await config?.dispose?.(); + } catch (disposeError) { + debugLogger.error(`Error disposing config: ${disposeError}`); + } throw acp.RequestError.authRequired(); } // 3. Set the ACP FileSystemService (if supported) before config initialization - if (this.clientCapabilities?.fs) { - const acpFileSystemService = new AcpFileSystemService( - this.connection, - sessionId, - this.clientCapabilities.fs, - config.getFileSystemService(), - cwd, - ); - config.setFileSystemService(acpFileSystemService); + try { + if (this.clientCapabilities?.fs) { + const acpFileSystemService = new AcpFileSystemService( + this.connection, + sessionId, + this.clientCapabilities.fs, + config.getFileSystemService(), + cwd, + ); + config.setFileSystemService(acpFileSystemService); + } + } catch (e) { + try { + await config?.dispose?.(); + } catch (disposeError) { + debugLogger.error(`Error disposing config: ${disposeError}`); + } + throw e; } return config; diff --git a/packages/cli/src/utils/sessionUtils.test.ts b/packages/cli/src/utils/sessionUtils.test.ts index 5677da57272..f9358714fbe 100644 --- a/packages/cli/src/utils/sessionUtils.test.ts +++ b/packages/cli/src/utils/sessionUtils.test.ts @@ -803,6 +803,226 @@ describe('SessionSelector', () => { }, ); }); + + describe('resolveSessionById', () => { + it('should resolve session with no messages / no resumable content', async () => { + const sessionId = randomUUID(); + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const session = { + sessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + lastUpdated: '2024-01-01T10:00:00.000Z', + kind: 'main', + messages: [], + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${sessionId.slice(0, 8)}.jsonl`, + ), + JSON.stringify(session) + '\n', + ); + + const sessionSelector = new SessionSelector(storage); + const result = await sessionSelector.resolveSessionById(sessionId); + + expect(result.sessionData.sessionId).toBe(sessionId); + expect(result.sessionData.messages).toEqual([]); + expect(result.displayInfo).toContain('Empty conversation'); + }); + + it('should resolve session with messages', async () => { + const sessionId = randomUUID(); + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const session = { + sessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + lastUpdated: '2024-01-01T10:30:00.000Z', + kind: 'main', + messages: [ + { + type: 'user', + content: 'Hello ACP', + id: 'msg1', + timestamp: '2024-01-01T10:00:00.000Z', + }, + ], + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${sessionId.slice(0, 8)}.jsonl`, + ), + JSON.stringify(session) + '\n', + ); + + const sessionSelector = new SessionSelector(storage); + const result = await sessionSelector.resolveSessionById(sessionId); + + expect(result.sessionData.sessionId).toBe(sessionId); + expect(result.sessionData.messages).toHaveLength(1); + }); + + it('should format displayInfo using startTime fallback when lastUpdated is missing', async () => { + const sessionId = randomUUID(); + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const session = { + sessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + messages: [ + { + type: 'user', + content: 'Hello without lastUpdated', + id: 'msg1', + timestamp: '2024-01-01T10:00:00.000Z', + }, + ], + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${sessionId.slice(0, 8)}.jsonl`, + ), + JSON.stringify(session) + '\n', + ); + + const sessionSelector = new SessionSelector(storage); + const result = await sessionSelector.resolveSessionById(sessionId); + + expect(result.sessionData.sessionId).toBe(sessionId); + expect(result.displayInfo).toContain( + `Session ${sessionId}: Hello without lastUpdated`, + ); + expect(result.displayInfo).not.toContain('Invalid Date'); + }); + + it('should throw INVALID_SESSION_IDENTIFIER if session does not exist on disk', async () => { + const nonExistentId = randomUUID(); + const sessionSelector = new SessionSelector(storage); + + await expect( + sessionSelector.resolveSessionById(nonExistentId), + ).rejects.toSatisfy((error) => { + expect(error).toBeInstanceOf(SessionError); + expect((error as SessionError).code).toBe('INVALID_SESSION_IDENTIFIER'); + return true; + }); + }); + + it('should not resolve subagent sessions via resolveSessionById', async () => { + const subagentId = randomUUID(); + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const session = { + sessionId: subagentId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + lastUpdated: '2024-01-01T10:00:00.000Z', + kind: 'subagent', + messages: [], + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${subagentId.slice(0, 8)}.jsonl`, + ), + JSON.stringify(session) + '\n', + ); + + const sessionSelector = new SessionSelector(storage); + await expect( + sessionSelector.resolveSessionById(subagentId), + ).rejects.toThrow(SessionError); + }); + + it('should allow resolving empty session via resolveSession with allowEmpty: true', async () => { + const sessionId = randomUUID(); + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const session = { + sessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + lastUpdated: '2024-01-01T10:00:00.000Z', + kind: 'main', + messages: [], + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${sessionId.slice(0, 8)}.jsonl`, + ), + JSON.stringify(session) + '\n', + ); + + const sessionSelector = new SessionSelector(storage); + + // With allowEmpty: true, it succeeds + const result = await sessionSelector.resolveSession(sessionId, { + allowEmpty: true, + }); + expect(result.sessionData.sessionId).toBe(sessionId); + + // Without allowEmpty, it rejects (preserving human terminal CLI semantics) + await expect(sessionSelector.resolveSession(sessionId)).rejects.toThrow( + SessionError, + ); + }); + + it('should continue to resolve via index rather than resolveSessionById when passing numeric index with allowEmpty: true', async () => { + const sessionId = randomUUID(); + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + const session = { + sessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + lastUpdated: '2024-01-01T10:00:00.000Z', + messages: [ + { + type: 'user', + content: 'test message', + id: 'msg1', + timestamp: '2024-01-01T10:00:00.000Z', + }, + ], + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${sessionId.slice(0, 8)}.jsonl`, + ), + JSON.stringify(session) + '\n', + ); + + const sessionSelector = new SessionSelector(storage); + + const result = await sessionSelector.resolveSession('1', { + allowEmpty: true, + }); + + expect(result.sessionData.sessionId).toBe(sessionId); + expect(result.sessionData.messages[0].content).toBe('test message'); + }); + }); }); describe('extractFirstUserMessage', () => { diff --git a/packages/cli/src/utils/sessionUtils.ts b/packages/cli/src/utils/sessionUtils.ts index 2830451aa00..0bcd8768d10 100644 --- a/packages/cli/src/utils/sessionUtils.ts +++ b/packages/cli/src/utils/sessionUtils.ts @@ -230,6 +230,17 @@ export interface GetSessionOptions { includeFullContent?: boolean; } +/** + * Options for resolving sessions. + */ +export interface ResolveSessionOptions { + /** + * Whether to allow resolving sessions that have no resumable content yet + * (e.g., newly established ACP sessions). + */ + allowEmpty?: boolean; +} + /** * Loads all session files (including corrupted ones) from the chats directory. * @returns Array of session file entries, with sessionInfo null for corrupted files @@ -491,16 +502,104 @@ export class SessionSelector { throw SessionError.invalidSessionIdentifier(trimmedIdentifier, chatsDir); } + /** + * Resolves a session directly by its full UUID, bypassing interactive terminal list + * filtering (such as `hasResumableContent: false`). + * + * @param id - Full session UUID + * @returns Promise resolving to session selection result + * @throws SessionError if the session file does not exist or is invalid + */ + async resolveSessionById(id: string): Promise { + const trimmedId = id.trim(); + const chatsDir = path.join(this.storage.getProjectTempDir(), 'chats'); + const files = await fs.readdir(chatsDir).catch(() => []); + + const shortId = trimmedId.slice(0, 8); + const candidateFiles = files.filter( + (f) => + f.startsWith(SESSION_FILE_PREFIX) && + (f.endsWith(`-${shortId}.json`) || f.endsWith(`-${shortId}.jsonl`)), + ); + + const matches: Array<{ + filePath: string; + sessionData: ConversationRecord; + }> = []; + + for (const fileName of candidateFiles) { + try { + const sessionPath = path.join(chatsDir, fileName); + const sessionData = await loadConversationRecord(sessionPath); + if ( + sessionData && + sessionData.sessionId === trimmedId && + sessionData.kind !== 'subagent' + ) { + matches.push({ filePath: sessionPath, sessionData }); + } + } catch { + // Ignore unparseable files + } + } + + if (matches.length === 0) { + throw SessionError.invalidSessionIdentifier(trimmedId, chatsDir); + } + + // If duplicate records exist, choose the most recently updated one + matches.sort((a, b) => { + const getTime = (dateStr: string | undefined) => { + if (!dateStr) return 0; + const t = new Date(dateStr).getTime(); + return isNaN(t) ? 0 : t; + }; + const timeA = getTime( + a.sessionData.lastUpdated?.trim() || a.sessionData.startTime, + ); + const timeB = getTime( + b.sessionData.lastUpdated?.trim() || b.sessionData.startTime, + ); + return timeB - timeA; + }); + + const { filePath, sessionData } = matches[0]; + const messages = sessionData.messages ?? []; + const firstUserMsg = extractFirstUserMessage(messages); + const messageCount = messages.length; + const timestamp = + sessionData.lastUpdated?.trim() || + sessionData.startTime?.trim() || + new Date().toISOString(); + const displayInfo = `Session ${sessionData.sessionId}: ${firstUserMsg} (${messageCount} messages, ${formatRelativeTime(timestamp)})`; + + return { + sessionPath: filePath, + sessionData, + displayInfo, + }; + } + /** * Resolves a resume argument to a specific session. * * @param resumeArg - Can be "latest", a full UUID, or an index number (1-based) + * @param options - Optional resolution options (e.g. allowEmpty to bypass resumable content filtering for exact UUIDs) * @returns Promise resolving to session selection result */ - async resolveSession(resumeArg: string): Promise { - let selectedSession: SessionInfo; + async resolveSession( + resumeArg: string, + options?: ResolveSessionOptions, + ): Promise { const trimmedResumeArg = resumeArg.trim(); + const isIndex = /^\d+$/.test(trimmedResumeArg); + if (options?.allowEmpty && trimmedResumeArg !== RESUME_LATEST && !isIndex) { + return this.resolveSessionById(trimmedResumeArg); + } + + let selectedSession: SessionInfo; + if (trimmedResumeArg === RESUME_LATEST) { const sessions = await this.listSessions();