From bf6dae4690a5b796b1ee5086873239c16ff4f90a Mon Sep 17 00:00:00 2001 From: krishdef7 <157892833+krishdef7@users.noreply.github.com> Date: Sat, 28 Mar 2026 05:09:48 +0530 Subject: [PATCH] fix(hooks): prevent SessionEnd from firing twice in non-interactive mode (#22139) Co-authored-by: Tommaso Sciortino --- packages/cli/src/gemini.tsx | 5 -- packages/cli/src/gemini_cleanup.test.tsx | 90 ++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 4b43d7d81b..fa22f59267 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -671,11 +671,6 @@ export async function main() { } } - // Register SessionEnd hook for graceful exit - registerCleanup(async () => { - await config.getHookSystem()?.fireSessionEndEvent(SessionEndReason.Exit); - }); - if (!input) { debugLogger.error( `No input provided via stdin. Input can be provided by piping data into gemini or using the --prompt option.`, diff --git a/packages/cli/src/gemini_cleanup.test.tsx b/packages/cli/src/gemini_cleanup.test.tsx index 382ad3f81f..b2fa2139fd 100644 --- a/packages/cli/src/gemini_cleanup.test.tsx +++ b/packages/cli/src/gemini_cleanup.test.tsx @@ -6,7 +6,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { main } from './gemini.js'; -import { debugLogger, type Config } from '@google/gemini-cli-core'; +import { + debugLogger, + SessionEndReason, + type Config, + type HookSystem, +} from '@google/gemini-cli-core'; vi.mock('@google/gemini-cli-core', async (importOriginal) => { const actual = @@ -197,11 +202,11 @@ describe('gemini.tsx main function cleanup', () => { setValue: vi.fn(), forScope: () => ({ settings: {}, originalSettings: {}, path: '' }), errors: [], - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + } as unknown as ReturnType); vi.mocked(parseArguments).mockResolvedValue({ promptInteractive: false, - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + } as unknown as Awaited>); vi.mocked(loadCliConfig).mockResolvedValue({ isInteractive: vi.fn(() => false), getQuestion: vi.fn(() => 'test'), @@ -238,7 +243,8 @@ describe('gemini.tsx main function cleanup', () => { setTerminalBackground: vi.fn(), refreshAuth: vi.fn(), getRemoteAdminSettings: vi.fn(() => undefined), - } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + getUseAlternateBuffer: vi.fn(() => false), + } as unknown as Config); await main(); @@ -248,4 +254,80 @@ describe('gemini.tsx main function cleanup', () => { expect.objectContaining({ message: 'Cleanup failed' }), ); }); + + it('should register SessionEnd hook exactly once in non-interactive mode', async () => { + const { loadCliConfig, parseArguments } = await import( + './config/config.js' + ); + const { registerCleanup } = await import('./utils/cleanup.js'); + + const mockHookSystem = { + fireSessionEndEvent: vi.fn().mockResolvedValue(undefined), + fireSessionStartEvent: vi.fn().mockResolvedValue(undefined), + } as unknown as HookSystem; + + vi.mocked(parseArguments).mockResolvedValue({ + promptInteractive: false, + } as unknown as Awaited>); + + vi.mocked(loadCliConfig).mockResolvedValue( + buildMockConfig({ + getHookSystem: vi.fn(() => mockHookSystem), + }), + ); + + vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + + await main(); + + const registeredCallbacks = vi + .mocked(registerCleanup) + .mock.calls.map(([fn]) => fn); + for (const fn of registeredCallbacks) await fn(); + expect(mockHookSystem.fireSessionEndEvent).toHaveBeenCalledTimes(1); + expect(mockHookSystem.fireSessionEndEvent).toHaveBeenCalledWith( + SessionEndReason.Exit, + ); + }); + + function buildMockConfig(overrides: Partial = {}): Config { + return { + isInteractive: vi.fn(() => false), + getQuestion: vi.fn(() => 'test'), + getSandbox: vi.fn(() => false), + getDebugMode: vi.fn(() => false), + getPolicyEngine: vi.fn(), + getMessageBus: () => ({ subscribe: vi.fn() }), + getEnableHooks: vi.fn(() => true), + getHookSystem: vi.fn(() => undefined), + initialize: vi.fn(), + storage: { initialize: vi.fn().mockResolvedValue(undefined) }, + getContentGeneratorConfig: vi.fn(), + getMcpClientManager: vi.fn(), + getIdeMode: vi.fn(() => false), + getAcpMode: vi.fn(() => false), + getScreenReader: vi.fn(() => false), + getGeminiMdFileCount: vi.fn(() => 0), + getProjectRoot: vi.fn(() => '/'), + getListExtensions: vi.fn(() => false), + getListSessions: vi.fn(() => false), + getDeleteSession: vi.fn(() => undefined), + getToolRegistry: vi.fn(), + getExtensions: vi.fn(() => []), + getModel: vi.fn(() => 'gemini-pro'), + getEmbeddingModel: vi.fn(() => 'embedding-001'), + getApprovalMode: vi.fn(() => 'default'), + getCoreTools: vi.fn(() => []), + getTelemetryEnabled: vi.fn(() => false), + getTelemetryLogPromptsEnabled: vi.fn(() => false), + getFileFilteringRespectGitIgnore: vi.fn(() => true), + getOutputFormat: vi.fn(() => 'text'), + getUsageStatisticsEnabled: vi.fn(() => false), + setTerminalBackground: vi.fn(), + refreshAuth: vi.fn(), + getRemoteAdminSettings: vi.fn(() => undefined), + getUseAlternateBuffer: vi.fn(() => false), + ...overrides, + } as unknown as Config; + } });