From e0be1b2afdfcc2f0ddbc75807a78711e0dc1d703 Mon Sep 17 00:00:00 2001 From: matt korwel Date: Tue, 17 Mar 2026 14:42:40 -0700 Subject: [PATCH] fix(cli): use active sessionId in useLogger and improve resume robustness (#22606) --- packages/cli/src/config/config.ts | 5 +- packages/cli/src/gemini.tsx | 2 +- packages/cli/src/ui/hooks/useLogger.test.tsx | 62 ++++++++++++++++++++ packages/cli/src/ui/hooks/useLogger.ts | 18 ++++-- packages/cli/src/utils/sessionUtils.test.ts | 38 ++++++++++++ packages/cli/src/utils/sessionUtils.ts | 28 ++++++--- 6 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 packages/cli/src/ui/hooks/useLogger.test.tsx diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 957bb6510e..aba827d08e 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -244,10 +244,11 @@ export async function parseArguments( // When --resume passed without a value (`gemini --resume`): value = "" (string) // When --resume not passed at all: this `coerce` function is not called at all, and // `yargsInstance.argv.resume` is undefined. - if (value === '') { + const trimmed = value.trim(); + if (trimmed === '') { return RESUME_LATEST; } - return value; + return trimmed; }, }) .option('list-sessions', { diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 04a370d7e9..4722bb73f3 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -647,7 +647,7 @@ export async function main() { process.exit(ExitCodes.FATAL_INPUT_ERROR); } - const prompt_id = Math.random().toString(16).slice(2); + const prompt_id = sessionId; logUserPrompt( config, new UserPromptEvent( diff --git a/packages/cli/src/ui/hooks/useLogger.test.tsx b/packages/cli/src/ui/hooks/useLogger.test.tsx new file mode 100644 index 0000000000..262dfb5380 --- /dev/null +++ b/packages/cli/src/ui/hooks/useLogger.test.tsx @@ -0,0 +1,62 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook } from '../../test-utils/render.js'; +import { waitFor } from '../../test-utils/async.js'; +import { useLogger } from './useLogger.js'; +import { + sessionId as globalSessionId, + Logger, + type Storage, + type Config, +} from '@google/gemini-cli-core'; +import { ConfigContext } from '../contexts/ConfigContext.js'; +import type React from 'react'; + +// Mock Logger +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + Logger: vi.fn().mockImplementation((id: string) => ({ + initialize: vi.fn().mockResolvedValue(undefined), + sessionId: id, + })), + }; +}); + +describe('useLogger', () => { + const mockStorage = {} as Storage; + const mockConfig = { + getSessionId: vi.fn().mockReturnValue('active-session-id'), + } as unknown as Config; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should initialize with the global sessionId by default', async () => { + const { result } = renderHook(() => useLogger(mockStorage)); + + await waitFor(() => expect(result.current).not.toBeNull()); + expect(Logger).toHaveBeenCalledWith(globalSessionId, mockStorage); + }); + + it('should initialize with the active sessionId from ConfigContext when available', async () => { + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + const { result } = renderHook(() => useLogger(mockStorage), { wrapper }); + + await waitFor(() => expect(result.current).not.toBeNull()); + expect(Logger).toHaveBeenCalledWith('active-session-id', mockStorage); + }); +}); diff --git a/packages/cli/src/ui/hooks/useLogger.ts b/packages/cli/src/ui/hooks/useLogger.ts index b0f43cb11d..2c9309821d 100644 --- a/packages/cli/src/ui/hooks/useLogger.ts +++ b/packages/cli/src/ui/hooks/useLogger.ts @@ -4,17 +4,25 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useEffect } from 'react'; -import { sessionId, Logger, type Storage } from '@google/gemini-cli-core'; +import { useState, useEffect, useContext } from 'react'; +import { + sessionId as globalSessionId, + Logger, + type Storage, +} from '@google/gemini-cli-core'; +import { ConfigContext } from '../contexts/ConfigContext.js'; /** * Hook to manage the logger instance. */ -export const useLogger = (storage: Storage) => { +export const useLogger = (storage: Storage): Logger | null => { const [logger, setLogger] = useState(null); + const config = useContext(ConfigContext); useEffect(() => { - const newLogger = new Logger(sessionId, storage); + const activeSessionId = config?.getSessionId() ?? globalSessionId; + const newLogger = new Logger(activeSessionId, storage); + /** * Start async initialization, no need to await. Using await slows down the * time from launch to see the gemini-cli prompt and it's better to not save @@ -26,7 +34,7 @@ export const useLogger = (storage: Storage) => { setLogger(newLogger); }) .catch(() => {}); - }, [storage]); + }, [storage, config]); return logger; }; diff --git a/packages/cli/src/utils/sessionUtils.test.ts b/packages/cli/src/utils/sessionUtils.test.ts index 7bddde481d..d65c60c41d 100644 --- a/packages/cli/src/utils/sessionUtils.test.ts +++ b/packages/cli/src/utils/sessionUtils.test.ts @@ -239,6 +239,44 @@ describe('SessionSelector', () => { expect(result.sessionData.messages[0].content).toBe('Latest session'); }); + it('should resolve session by UUID with whitespace (trimming)', async () => { + const sessionId = randomUUID(); + + // Create test session files + 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', + 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)}.json`, + ), + JSON.stringify(session, null, 2), + ); + + const sessionSelector = new SessionSelector(config); + + // Test resolving by UUID with leading/trailing spaces + const result = await sessionSelector.resolveSession(` ${sessionId} `); + expect(result.sessionData.sessionId).toBe(sessionId); + expect(result.sessionData.messages[0].content).toBe('Test message'); + }); + it('should deduplicate sessions by ID', async () => { const sessionId = randomUUID(); diff --git a/packages/cli/src/utils/sessionUtils.ts b/packages/cli/src/utils/sessionUtils.ts index 3aa0131ac2..ca6685f47d 100644 --- a/packages/cli/src/utils/sessionUtils.ts +++ b/packages/cli/src/utils/sessionUtils.ts @@ -57,10 +57,14 @@ export class SessionError extends Error { /** * Creates an error for when a session identifier is invalid. */ - static invalidSessionIdentifier(identifier: string): SessionError { + static invalidSessionIdentifier( + identifier: string, + chatsDir?: string, + ): SessionError { + const dirInfo = chatsDir ? ` in ${chatsDir}` : ''; return new SessionError( 'INVALID_SESSION_IDENTIFIER', - `Invalid session identifier "${identifier}".\n Use --list-sessions to see available sessions, then use --resume {number}, --resume {uuid}, or --resume latest.`, + `Invalid session identifier "${identifier}".\n Searched for sessions${dirInfo}.\n Use --list-sessions to see available sessions, then use --resume {number}, --resume {uuid}, or --resume latest.`, ); } } @@ -416,6 +420,7 @@ export class SessionSelector { * @throws Error if the session is not found or identifier is invalid */ async findSession(identifier: string): Promise { + const trimmedIdentifier = identifier.trim(); const sessions = await this.listSessions(); if (sessions.length === 0) { @@ -430,24 +435,28 @@ export class SessionSelector { // Try to find by UUID first const sessionByUuid = sortedSessions.find( - (session) => session.id === identifier, + (session) => session.id === trimmedIdentifier, ); if (sessionByUuid) { return sessionByUuid; } // Parse as index number (1-based) - only allow numeric indexes - const index = parseInt(identifier, 10); + const index = parseInt(trimmedIdentifier, 10); if ( !isNaN(index) && - index.toString() === identifier && + index.toString() === trimmedIdentifier && index > 0 && index <= sortedSessions.length ) { return sortedSessions[index - 1]; } - throw SessionError.invalidSessionIdentifier(identifier); + const chatsDir = path.join( + this.config.storage.getProjectTempDir(), + 'chats', + ); + throw SessionError.invalidSessionIdentifier(trimmedIdentifier, chatsDir); } /** @@ -458,8 +467,9 @@ export class SessionSelector { */ async resolveSession(resumeArg: string): Promise { let selectedSession: SessionInfo; + const trimmedResumeArg = resumeArg.trim(); - if (resumeArg === RESUME_LATEST) { + if (trimmedResumeArg === RESUME_LATEST) { const sessions = await this.listSessions(); if (sessions.length === 0) { @@ -475,7 +485,7 @@ export class SessionSelector { selectedSession = sessions[sessions.length - 1]; } else { try { - selectedSession = await this.findSession(resumeArg); + selectedSession = await this.findSession(trimmedResumeArg); } catch (error) { // SessionError already has detailed messages - just rethrow if (error instanceof SessionError) { @@ -483,7 +493,7 @@ export class SessionSelector { } // Wrap unexpected errors with context throw new Error( - `Failed to find session "${resumeArg}": ${error instanceof Error ? error.message : String(error)}`, + `Failed to find session "${trimmedResumeArg}": ${error instanceof Error ? error.message : String(error)}`, ); } }