From 9ab44ea9d675cd9d560e22fba50d057f1764f25c Mon Sep 17 00:00:00 2001 From: Harold Mciver Date: Wed, 16 Jul 2025 22:40:56 -0400 Subject: [PATCH] updated `/quit` to use new slash command arch (#4259) Co-authored-by: Abhi --- .../cli/src/services/CommandService.test.ts | 8 ++- packages/cli/src/services/CommandService.ts | 2 + .../cli/src/test-utils/mockCommandContext.ts | 10 ++-- .../cli/src/ui/commands/quitCommand.test.ts | 55 ++++++++++++++++++ packages/cli/src/ui/commands/quitCommand.ts | 35 ++++++++++++ packages/cli/src/ui/commands/types.ts | 8 +++ .../ui/hooks/slashCommandProcessor.test.ts | 56 +------------------ .../cli/src/ui/hooks/slashCommandProcessor.ts | 45 +++------------ 8 files changed, 120 insertions(+), 99 deletions(-) create mode 100644 packages/cli/src/ui/commands/quitCommand.test.ts create mode 100644 packages/cli/src/ui/commands/quitCommand.ts diff --git a/packages/cli/src/services/CommandService.test.ts b/packages/cli/src/services/CommandService.test.ts index 8f5b1421..084f603b 100644 --- a/packages/cli/src/services/CommandService.test.ts +++ b/packages/cli/src/services/CommandService.test.ts @@ -25,6 +25,7 @@ import { compressCommand } from '../ui/commands/compressCommand.js'; import { mcpCommand } from '../ui/commands/mcpCommand.js'; import { editorCommand } from '../ui/commands/editorCommand.js'; import { bugCommand } from '../ui/commands/bugCommand.js'; +import { quitCommand } from '../ui/commands/quitCommand.js'; // Mock the command modules to isolate the service from the command implementations. vi.mock('../ui/commands/memoryCommand.js', () => ({ @@ -75,9 +76,12 @@ vi.mock('../ui/commands/editorCommand.js', () => ({ vi.mock('../ui/commands/bugCommand.js', () => ({ bugCommand: { name: 'bug', description: 'Mock Bug' }, })); +vi.mock('../ui/commands/quitCommand.js', () => ({ + quitCommand: { name: 'quit', description: 'Mock Quit' }, +})); describe('CommandService', () => { - const subCommandLen = 16; + const subCommandLen = 17; let mockConfig: Mocked; beforeEach(() => { @@ -144,6 +148,7 @@ describe('CommandService', () => { const commandNames = tree.map((cmd) => cmd.name); expect(commandNames).toContain('ide'); expect(commandNames).toContain('editor'); + expect(commandNames).toContain('quit'); }); it('should overwrite any existing commands when called again', async () => { @@ -183,6 +188,7 @@ describe('CommandService', () => { mcpCommand, memoryCommand, privacyCommand, + quitCommand, statsCommand, themeCommand, toolsCommand, diff --git a/packages/cli/src/services/CommandService.ts b/packages/cli/src/services/CommandService.ts index 31914556..773f5b31 100644 --- a/packages/cli/src/services/CommandService.ts +++ b/packages/cli/src/services/CommandService.ts @@ -23,6 +23,7 @@ import { toolsCommand } from '../ui/commands/toolsCommand.js'; import { compressCommand } from '../ui/commands/compressCommand.js'; import { ideCommand } from '../ui/commands/ideCommand.js'; import { bugCommand } from '../ui/commands/bugCommand.js'; +import { quitCommand } from '../ui/commands/quitCommand.js'; const loadBuiltInCommands = async ( config: Config | null, @@ -42,6 +43,7 @@ const loadBuiltInCommands = async ( mcpCommand, memoryCommand, privacyCommand, + quitCommand, statsCommand, themeCommand, toolsCommand, diff --git a/packages/cli/src/test-utils/mockCommandContext.ts b/packages/cli/src/test-utils/mockCommandContext.ts index 88da4a32..899d5747 100644 --- a/packages/cli/src/test-utils/mockCommandContext.ts +++ b/packages/cli/src/test-utils/mockCommandContext.ts @@ -76,15 +76,13 @@ export const createMockCommandContext = ( const targetValue = output[key]; if ( - sourceValue && - typeof sourceValue === 'object' && - !Array.isArray(sourceValue) && - targetValue && - typeof targetValue === 'object' && - !Array.isArray(targetValue) + // We only want to recursivlty merge plain objects + Object.prototype.toString.call(sourceValue) === '[object Object]' && + Object.prototype.toString.call(targetValue) === '[object Object]' ) { output[key] = merge(targetValue, sourceValue); } else { + // If not, we do a direct assignment. This preserves Date objects and others. output[key] = sourceValue; } } diff --git a/packages/cli/src/ui/commands/quitCommand.test.ts b/packages/cli/src/ui/commands/quitCommand.test.ts new file mode 100644 index 00000000..e67723fd --- /dev/null +++ b/packages/cli/src/ui/commands/quitCommand.test.ts @@ -0,0 +1,55 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { quitCommand } from './quitCommand.js'; +import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; +import { formatDuration } from '../utils/formatters.js'; + +vi.mock('../utils/formatters.js'); + +describe('quitCommand', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2025-01-01T01:00:00Z')); + vi.mocked(formatDuration).mockReturnValue('1h 0m 0s'); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); + }); + + it('returns a QuitActionReturn object with the correct messages', () => { + const mockContext = createMockCommandContext({ + session: { + stats: { + sessionStartTime: new Date('2025-01-01T00:00:00Z'), + }, + }, + }); + + if (!quitCommand.action) throw new Error('Action is not defined'); + const result = quitCommand.action(mockContext, 'quit'); + + expect(formatDuration).toHaveBeenCalledWith(3600000); // 1 hour in ms + expect(result).toEqual({ + type: 'quit', + messages: [ + { + type: 'user', + text: '/quit', + id: expect.any(Number), + }, + { + type: 'quit', + duration: '1h 0m 0s', + id: expect.any(Number), + }, + ], + }); + }); +}); diff --git a/packages/cli/src/ui/commands/quitCommand.ts b/packages/cli/src/ui/commands/quitCommand.ts new file mode 100644 index 00000000..48daf8c2 --- /dev/null +++ b/packages/cli/src/ui/commands/quitCommand.ts @@ -0,0 +1,35 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { formatDuration } from '../utils/formatters.js'; +import { type SlashCommand } from './types.js'; + +export const quitCommand: SlashCommand = { + name: 'quit', + altName: 'exit', + description: 'exit the cli', + action: (context) => { + const now = Date.now(); + const { sessionStartTime } = context.session.stats; + const wallDuration = now - sessionStartTime.getTime(); + + return { + type: 'quit', + messages: [ + { + type: 'user', + text: `/quit`, // Keep it consistent, even if /exit was used + id: now - 1, + }, + { + type: 'quit', + duration: formatDuration(wallDuration), + id: now, + }, + ], + }; + }, +}; diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index a61a29f2..d3d5ee8a 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -9,6 +9,7 @@ import { HistoryItemWithoutId } from '../types.js'; import { Config, GitService, Logger } from '@google/gemini-cli-core'; import { LoadedSettings } from '../../config/settings.js'; import { UseHistoryManagerReturn } from '../hooks/useHistoryManager.js'; +import type { HistoryItem } from '../types.js'; import { SessionStatsState } from '../contexts/SessionContext.js'; // Grouped dependencies for clarity and easier mocking @@ -56,6 +57,12 @@ export interface ToolActionReturn { toolArgs: Record; } +/** The return type for a command action that results in the app quitting. */ +export interface QuitActionReturn { + type: 'quit'; + messages: HistoryItem[]; +} + /** * The return type for a command action that results in a simple message * being displayed to the user. @@ -87,6 +94,7 @@ export interface LoadHistoryActionReturn { export type SlashCommandActionReturn = | ToolActionReturn | MessageActionReturn + | QuitActionReturn | OpenDialogActionReturn | LoadHistoryActionReturn; // The standardized contract for any command in the system. diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 71c18dd7..38d17fc9 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -54,16 +54,7 @@ vi.mock('../../utils/version.js', () => ({ })); import { act, renderHook } from '@testing-library/react'; -import { - vi, - describe, - it, - expect, - beforeEach, - afterEach, - beforeAll, - Mock, -} from 'vitest'; +import { vi, describe, it, expect, beforeEach, beforeAll, Mock } from 'vitest'; import open from 'open'; import { useSlashCommandProcessor } from './slashCommandProcessor.js'; import { SlashCommandProcessorResult } from '../types.js'; @@ -203,8 +194,6 @@ describe('useSlashCommandProcessor', () => { ); }; - const getProcessor = () => getProcessorHook().result.current; - describe('New command registry', () => { let ActualCommandService: typeof CommandService; @@ -451,47 +440,4 @@ describe('useSlashCommandProcessor', () => { ); }); }); - - describe('/quit and /exit commands', () => { - beforeEach(() => { - vi.useFakeTimers(); - }); - - afterEach(() => { - vi.useRealTimers(); - }); - - it.each([['/quit'], ['/exit']])( - 'should handle %s, set quitting messages, and exit the process', - async (command) => { - const { handleSlashCommand } = getProcessor(); - const mockDate = new Date('2025-01-01T01:02:03.000Z'); - vi.setSystemTime(mockDate); - - await act(async () => { - handleSlashCommand(command); - }); - - expect(mockAddItem).not.toHaveBeenCalled(); - expect(mockSetQuittingMessages).toHaveBeenCalledWith([ - { - type: 'user', - text: command, - id: expect.any(Number), - }, - { - type: 'quit', - duration: '1h 2m 3s', - id: expect.any(Number), - }, - ]); - - // Fast-forward timers to trigger process.exit - await act(async () => { - vi.advanceTimersByTime(100); - }); - expect(mockProcessExit).toHaveBeenCalledWith(0); - }, - ); - }); }); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 35371265..125d051e 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -20,7 +20,6 @@ import { } from '../types.js'; import { promises as fs } from 'fs'; import path from 'path'; -import { formatDuration } from '../utils/formatters.js'; import { LoadedSettings } from '../../config/settings.js'; import { type CommandContext, @@ -202,33 +201,6 @@ export const useSlashCommandProcessor = ( toggleCorgiMode(); }, }, - { - name: 'quit', - altName: 'exit', - description: 'exit the cli', - action: async (mainCommand, _subCommand, _args) => { - const now = new Date(); - const { sessionStartTime } = session.stats; - const wallDuration = now.getTime() - sessionStartTime.getTime(); - - setQuittingMessages([ - { - type: 'user', - text: `/${mainCommand}`, - id: now.getTime() - 1, - }, - { - type: 'quit', - duration: formatDuration(wallDuration), - id: now.getTime(), - }, - ]); - - setTimeout(() => { - process.exit(0); - }, 100); - }, - }, ]; if (config?.getCheckpointingEnabled()) { @@ -352,15 +324,7 @@ export const useSlashCommandProcessor = ( }); } return commands; - }, [ - addMessage, - toggleCorgiMode, - config, - session, - gitService, - loadHistory, - setQuittingMessages, - ]); + }, [addMessage, toggleCorgiMode, config, gitService, loadHistory]); const handleSlashCommand = useCallback( async ( @@ -470,6 +434,12 @@ export const useSlashCommandProcessor = ( }); return { type: 'handled' }; } + case 'quit': + setQuittingMessages(result.messages); + setTimeout(() => { + process.exit(0); + }, 100); + return { type: 'handled' }; default: { const unhandled: never = result; throw new Error(`Unhandled slash command result: ${unhandled}`); @@ -549,6 +519,7 @@ export const useSlashCommandProcessor = ( openThemeDialog, openPrivacyNotice, openEditorDialog, + setQuittingMessages, ], );