diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index b970093f150..af83e46e028 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1079,6 +1079,7 @@ export const AppContainer = (props: AppContainerProps) => { extensionsUpdateStateInternal, isConfigInitialized, logger, + historyManager.updateItem, setSessionName, ); diff --git a/packages/cli/src/ui/auth/useAuth.ts b/packages/cli/src/ui/auth/useAuth.ts index 255a3d22027..dc954fd137d 100644 --- a/packages/cli/src/ui/auth/useAuth.ts +++ b/packages/cli/src/ui/auth/useAuth.ts @@ -17,7 +17,7 @@ import type { LoadedSettings } from '../../config/settings.js'; import { getPersistScopeForModelSelection } from '../../config/modelProvidersScope.js'; import { useQwenAuth } from '../hooks/useQwenAuth.js'; import { AuthState, MessageType } from '../types.js'; -import type { HistoryItem } from '../types.js'; +import type { HistoryItemWithoutId } from '../types.js'; import { t } from '../../i18n/index.js'; import { applyProviderInstallPlan } from '../../auth/install/applyProviderInstallPlan.js'; @@ -110,7 +110,7 @@ export type AuthController = { export const useAuthCommand = ( settings: LoadedSettings, config: Config, - addItem: (item: Omit, timestamp: number) => void, + addItem: (item: HistoryItemWithoutId, timestamp: number) => void, onAuthChange?: () => void, ) => { const unAuthenticated = config.getAuthType() === undefined; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 82e2beecd59..682e23c6408 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -12,6 +12,7 @@ import { } from './slashCommandProcessor.js'; import type { CommandContext, + ConfirmActionReturn, ConfirmShellCommandsActionReturn, SlashCommand, } from '../commands/types.js'; @@ -29,8 +30,14 @@ import { makeFakeConfig, } from '@qwen-code/qwen-code-core'; -const { logSlashCommand } = vi.hoisted(() => ({ +const { logSlashCommand, debugLoggerMock } = vi.hoisted(() => ({ logSlashCommand: vi.fn(), + debugLoggerMock: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, })); vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { @@ -39,6 +46,7 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { return { ...original, logSlashCommand, + createDebugLogger: () => debugLoggerMock, getIdeInstaller: vi.fn().mockReturnValue(null), }; }); @@ -110,6 +118,7 @@ function createTestCommand( describe('useSlashCommandProcessor', () => { const mockAddItem = vi.fn(); + const mockUpdateItem = vi.fn(); const mockClearItems = vi.fn(); const mockLoadHistory = vi.fn(); const mockOpenThemeDialog = vi.fn(); @@ -156,6 +165,8 @@ describe('useSlashCommandProcessor', () => { beforeEach(() => { vi.clearAllMocks(); + let nextHistoryItemId = 1; + mockAddItem.mockImplementation(() => nextHistoryItemId++); vi.mocked(BuiltinCommandLoader).mockClear(); mockBuiltinLoadCommands.mockResolvedValue([]); mockFileLoadCommands.mockResolvedValue([]); @@ -192,6 +203,7 @@ describe('useSlashCommandProcessor', () => { new Map(), // extensionsUpdateState true, // isConfigInitialized null, // logger + mockUpdateItem, ), ); @@ -299,6 +311,15 @@ describe('useSlashCommandProcessor', () => { }); expect(actionResult).toBe(false); + + let absPathResult; + await act(async () => { + absPathResult = await result.current.handleSlashCommand( + '/Users/zhoushuo/Desktop/dw-operator-skill 帮我安装', + ); + }); + + expect(absPathResult).toBe(false); expect(mockAddItem).not.toHaveBeenCalled(); }); @@ -631,9 +652,21 @@ describe('useSlashCommandProcessor', () => { }); expect(mockAddItem).toHaveBeenCalledWith( - { type: MessageType.USER, text: '/filecmd' }, + { type: MessageType.USER, text: '/filecmd', sentToModel: false }, expect.any(Number), ); + expect(mockUpdateItem).toHaveBeenCalledWith(1, { sentToModel: true }); + expect(debugLoggerMock.debug).toHaveBeenCalledWith( + 'Marked slash command invocation as model-sent: /filecmd', + ); + const recorder = mockConfig.getChatRecordingService() as unknown as { + recordSlashCommand: ReturnType; + }; + expect(recorder.recordSlashCommand).toHaveBeenCalledWith({ + phase: 'invocation', + rawCommand: '/filecmd', + sentToModel: true, + }); }); it('should handle "submit_prompt" action returned from a mcp-based command', async () => { @@ -663,9 +696,10 @@ describe('useSlashCommandProcessor', () => { }); expect(mockAddItem).toHaveBeenCalledWith( - { type: MessageType.USER, text: '/mcpcmd' }, + { type: MessageType.USER, text: '/mcpcmd', sentToModel: false }, expect.any(Number), ); + expect(mockUpdateItem).toHaveBeenCalledWith(1, { sentToModel: true }); }); }); @@ -797,6 +831,107 @@ describe('useSlashCommandProcessor', () => { expect(finalContext.session.sessionShellAllowlist.size).toBe(0); }); + it('should not duplicate user history when a confirmed command submits a prompt', async () => { + mockCommandAction + .mockResolvedValueOnce({ + type: 'confirm_shell_commands', + commandsToConfirm: ['rm -rf /'], + originalInvocation: { raw: '/shellcmd' }, + } as ConfirmShellCommandsActionReturn) + .mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: 'run approved command' }], + }); + + const result = setupProcessorHook([shellCommand]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + act(() => { + result.current.handleSlashCommand('/shellcmd'); + }); + await waitFor(() => { + expect(result.current.shellConfirmationRequest).not.toBeNull(); + }); + + await act(async () => { + result.current.shellConfirmationRequest?.onConfirm( + ToolConfirmationOutcome.ProceedOnce, + ['rm -rf /'], + ); + }); + + await waitFor(() => { + expect(mockCommandAction).toHaveBeenCalledTimes(2); + }); + const userInvocationCalls = mockAddItem.mock.calls.filter( + ([item]) => item.type === MessageType.USER && item.text === '/shellcmd', + ); + expect(userInvocationCalls).toHaveLength(1); + expect(mockUpdateItem).toHaveBeenCalledWith(1, { sentToModel: true }); + + const recorder = mockConfig.getChatRecordingService() as unknown as { + recordSlashCommand: ReturnType; + }; + expect(recorder.recordSlashCommand).toHaveBeenCalledTimes(2); + expect(recorder.recordSlashCommand).toHaveBeenCalledWith({ + phase: 'invocation', + rawCommand: '/shellcmd', + sentToModel: true, + }); + }); + + it('should not duplicate user history when a confirmed action submits a prompt', async () => { + const action = vi + .fn() + .mockResolvedValueOnce({ + type: 'confirm_action', + prompt: 'Continue?', + originalInvocation: { raw: '/actioncmd' }, + } as ConfirmActionReturn) + .mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: 'run confirmed action' }], + }); + const command = createTestCommand({ + name: 'actioncmd', + action, + }); + + const result = setupProcessorHook([command]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + act(() => { + result.current.handleSlashCommand('/actioncmd'); + }); + await waitFor(() => { + expect(result.current.confirmationRequest).not.toBeNull(); + }); + + await act(async () => { + result.current.confirmationRequest?.onConfirm(true); + }); + + await waitFor(() => { + expect(action).toHaveBeenCalledTimes(2); + }); + const userInvocationCalls = mockAddItem.mock.calls.filter( + ([item]) => + item.type === MessageType.USER && item.text === '/actioncmd', + ); + expect(userInvocationCalls).toHaveLength(1); + expect(mockUpdateItem).toHaveBeenCalledWith(1, { sentToModel: true }); + + const recorder = mockConfig.getChatRecordingService() as unknown as { + recordSlashCommand: ReturnType; + }; + expect(recorder.recordSlashCommand).toHaveBeenCalledTimes(2); + expect(recorder.recordSlashCommand).toHaveBeenCalledWith({ + phase: 'invocation', + rawCommand: '/actioncmd', + sentToModel: true, + }); + }); + it('should re-run command and update session allowlist on "Proceed Always"', async () => { const result = setupProcessorHook([shellCommand]); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); @@ -998,7 +1133,7 @@ describe('useSlashCommandProcessor', () => { // It should be added to the history. expect(mockAddItem).toHaveBeenCalledWith( - { type: MessageType.USER, text: '/exit' }, + { type: MessageType.USER, text: '/exit', sentToModel: false }, expect.any(Number), ); }); @@ -1024,6 +1159,7 @@ describe('useSlashCommandProcessor', () => { new Map(), // extensionsUpdateState true, // isConfigInitialized null, // logger + mockUpdateItem, ), ); @@ -1066,6 +1202,7 @@ describe('useSlashCommandProcessor', () => { new Map(), true, null, + mockUpdateItem, ), ); @@ -1134,6 +1271,7 @@ describe('useSlashCommandProcessor', () => { new Map(), isConfigInitialized, null, + mockUpdateItem, ); }, { initialProps: { isConfigInitialized: false } }, @@ -1192,6 +1330,7 @@ describe('useSlashCommandProcessor', () => { new Map(), isConfigInitialized, null, + mockUpdateItem, ), { initialProps: { isConfigInitialized: false } }, ); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 0607778cddf..b36b392c5a5 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -62,7 +62,7 @@ type SerializableHistoryItem = Record; const debugLogger = createDebugLogger('SLASH_COMMAND_PROCESSOR'); function serializeHistoryItemForRecording( - item: Omit, + item: HistoryItemWithoutId, ): SerializableHistoryItem { const clone: SerializableHistoryItem = { ...item }; if ('timestamp' in clone && clone['timestamp'] instanceof Date) { @@ -132,6 +132,7 @@ export const useSlashCommandProcessor = ( extensionsUpdateState: Map, isConfigInitialized: boolean, logger: Logger | null, + updateItem: UseHistoryManagerReturn['updateItem'], setSessionName?: (name: string | null) => void, ) => { const { stats: sessionStats, startNewSession } = useSessionStats(); @@ -511,6 +512,7 @@ export const useSlashCommandProcessor = ( rawQuery: PartListUnion, oneTimeShellAllowlist?: Set, overwriteConfirmed?: boolean, + existingInvocationItemId?: number, ): Promise => { if (typeof rawQuery !== 'string') { return false; @@ -524,8 +526,8 @@ export const useSlashCommandProcessor = ( return false; } - const recordedItems: Array> = []; - const recordItem = (item: Omit) => { + const recordedItems: HistoryItemWithoutId[] = []; + const recordItem = (item: HistoryItemWithoutId) => { recordedItems.push(item); }; const addItemWithRecording: UseHistoryManagerReturn['addItem'] = ( @@ -543,14 +545,17 @@ export const useSlashCommandProcessor = ( abortControllerRef.current = abortController; const userMessageTimestamp = Date.now(); - if (!isBtwCommand(trimmed)) { - addItemWithRecording( - { type: MessageType.USER, text: trimmed }, + let invocationItemId = existingInvocationItemId; + let invocationSentToModel = false; + if (!isBtwCommand(trimmed) && invocationItemId === undefined) { + invocationItemId = addItemWithRecording( + { type: MessageType.USER, text: trimmed, sentToModel: false }, userMessageTimestamp, ); } let hasError = false; + let delegatedToRecursiveInvocation = false; const { commandToExecute, args, @@ -765,6 +770,18 @@ export const useSlashCommandProcessor = ( return { type: 'handled' }; case 'submit_prompt': + if (invocationItemId !== undefined) { + invocationSentToModel = true; + debugLogger.debug( + `Marked slash command invocation as model-sent: /${resolvedCommandPath.join( + ' ', + )}`, + ); + // React applies this update asynchronously. No same-turn + // logic reads the UI history classification; rewind/resume + // consumers observe it after state has rendered. + updateItem(invocationItemId, { sentToModel: true }); + } return { type: 'submit_prompt', content: result.content, @@ -804,10 +821,13 @@ export const useSlashCommandProcessor = ( ); } + delegatedToRecursiveInvocation = true; return await handleSlashCommand( result.originalInvocation.raw, // Pass the approved commands as a one-time grant for this execution. new Set(approvedCommands), + undefined, + invocationItemId, ); } case 'confirm_action': { @@ -834,10 +854,12 @@ export const useSlashCommandProcessor = ( return { type: 'handled' }; } + delegatedToRecursiveInvocation = true; return await handleSlashCommand( result.originalInvocation.raw, undefined, true, + invocationItemId, ); } case 'stream_messages': { @@ -904,15 +926,17 @@ export const useSlashCommandProcessor = ( const chatRecorder = config.getChatRecordingService(); const primaryCommand = resolvedCommandPath[0] || - trimmed.replace(/^[/?]/, '').split(/\s+/)[0] || + trimmed.replace(/^[/?]/, '').split(/\s+/u)[0] || trimmed; const shouldRecord = + !delegatedToRecursiveInvocation && !SLASH_COMMANDS_SKIP_RECORDING.has(primaryCommand); try { if (shouldRecord) { chatRecorder?.recordSlashCommand({ phase: 'invocation', rawCommand: trimmed, + sentToModel: invocationSentToModel, }); const outputItems = recordedItems .filter((item) => item.type !== 'user') @@ -930,7 +954,12 @@ export const useSlashCommandProcessor = ( ); } } - if (config && resolvedCommandPath[0] && !hasError) { + if ( + config && + resolvedCommandPath[0] && + !hasError && + !delegatedToRecursiveInvocation + ) { const event = makeSlashCommandEvent({ command: resolvedCommandPath[0], subcommand, @@ -952,6 +981,7 @@ export const useSlashCommandProcessor = ( setSessionShellAllowlist, setIsProcessing, setConfirmationRequest, + updateItem, ], ); diff --git a/packages/cli/src/ui/hooks/useEditorSettings.test.ts b/packages/cli/src/ui/hooks/useEditorSettings.test.ts index fa3cf98b71f..8059e4b2a6a 100644 --- a/packages/cli/src/ui/hooks/useEditorSettings.test.ts +++ b/packages/cli/src/ui/hooks/useEditorSettings.test.ts @@ -18,7 +18,7 @@ import { renderHook } from '@testing-library/react'; import { useEditorSettings } from './useEditorSettings.js'; import type { LoadedSettings } from '../../config/settings.js'; import { SettingScope } from '../../config/settings.js'; -import { MessageType, type HistoryItem } from '../types.js'; +import { MessageType, type HistoryItemWithoutId } from '../types.js'; import { type EditorType, checkHasEditorType, @@ -41,7 +41,7 @@ describe('useEditorSettings', () => { let mockLoadedSettings: LoadedSettings; let mockSetEditorError: MockedFunction<(error: string | null) => void>; let mockAddItem: MockedFunction< - (item: Omit, timestamp: number) => void + (item: HistoryItemWithoutId, timestamp: number) => void >; beforeEach(() => { diff --git a/packages/cli/src/ui/hooks/useEditorSettings.ts b/packages/cli/src/ui/hooks/useEditorSettings.ts index 5d6a5a371b2..4903240b418 100644 --- a/packages/cli/src/ui/hooks/useEditorSettings.ts +++ b/packages/cli/src/ui/hooks/useEditorSettings.ts @@ -6,7 +6,7 @@ import { useState, useCallback } from 'react'; import type { LoadedSettings, SettingScope } from '../../config/settings.js'; -import { type HistoryItem, MessageType } from '../types.js'; +import { type HistoryItemWithoutId, MessageType } from '../types.js'; import type { EditorType } from '@qwen-code/qwen-code-core'; import { allowEditorTypeInSandbox, @@ -26,7 +26,7 @@ interface UseEditorSettingsReturn { export const useEditorSettings = ( loadedSettings: LoadedSettings, setEditorError: (error: string | null) => void, - addItem: (item: Omit, timestamp: number) => void, + addItem: (item: HistoryItemWithoutId, timestamp: number) => void, ): UseEditorSettingsReturn => { const [isEditorDialogOpen, setIsEditorDialogOpen] = useState(false); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.test.ts b/packages/cli/src/ui/hooks/useHistoryManager.test.ts index c6f600323e3..ec9bd1ef31e 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.test.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.test.ts @@ -4,12 +4,29 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import { useHistory } from './useHistoryManager.js'; -import type { HistoryItem } from '../types.js'; +import type { HistoryItemWithoutId } from '../types.js'; + +const { debugLoggerMock } = vi.hoisted(() => ({ + debugLoggerMock: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +vi.mock('@qwen-code/qwen-code-core', () => ({ + createDebugLogger: () => debugLoggerMock, +})); describe('useHistoryManager', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + it('should initialize with an empty history', () => { const { result } = renderHook(() => useHistory()); expect(result.current.history).toEqual([]); @@ -18,7 +35,7 @@ describe('useHistoryManager', () => { it('should add an item to history with a unique ID', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const itemData: Omit = { + const itemData: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Hello', }; @@ -41,11 +58,11 @@ describe('useHistoryManager', () => { it('should generate unique IDs for items added with the same base timestamp', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const itemData1: Omit = { + const itemData1: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'First', }; - const itemData2: Omit = { + const itemData2: HistoryItemWithoutId = { type: 'gemini', // Replaced HistoryItemType.Gemini text: 'Second', }; @@ -69,7 +86,7 @@ describe('useHistoryManager', () => { it('should update an existing history item', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const initialItem: Omit = { + const initialItem: HistoryItemWithoutId = { type: 'gemini', // Replaced HistoryItemType.Gemini text: 'Initial content', }; @@ -95,7 +112,7 @@ describe('useHistoryManager', () => { it('should not change history if updateHistoryItem is called with a nonexistent ID', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const itemData: Omit = { + const itemData: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Hello', }; @@ -105,22 +122,27 @@ describe('useHistoryManager', () => { }); const originalHistory = [...result.current.history]; // Clone before update attempt + const originalHistoryRef = result.current.history; act(() => { result.current.updateItem(99999, { text: 'Should not apply' }); // Nonexistent ID }); expect(result.current.history).toEqual(originalHistory); + expect(result.current.history).toBe(originalHistoryRef); + expect(debugLoggerMock.debug).toHaveBeenCalledWith( + 'Skipped history update; item 99999 was not found.', + ); }); it('should clear the history', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const itemData1: Omit = { + const itemData1: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'First', }; - const itemData2: Omit = { + const itemData2: HistoryItemWithoutId = { type: 'gemini', // Replaced HistoryItemType.Gemini text: 'Second', }; @@ -142,19 +164,19 @@ describe('useHistoryManager', () => { it('should not add consecutive duplicate user messages', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const itemData1: Omit = { + const itemData1: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Duplicate message', }; - const itemData2: Omit = { + const itemData2: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Duplicate message', }; - const itemData3: Omit = { + const itemData3: HistoryItemWithoutId = { type: 'gemini', // Replaced HistoryItemType.Gemini text: 'Gemini response', }; - const itemData4: Omit = { + const itemData4: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Another user message', }; @@ -175,15 +197,15 @@ describe('useHistoryManager', () => { it('should add duplicate user messages if they are not consecutive', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); - const itemData1: Omit = { + const itemData1: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Message 1', }; - const itemData2: Omit = { + const itemData2: HistoryItemWithoutId = { type: 'gemini', // Replaced HistoryItemType.Gemini text: 'Gemini response', }; - const itemData3: Omit = { + const itemData3: HistoryItemWithoutId = { type: 'user', // Replaced HistoryItemType.User text: 'Message 1', // Duplicate text, but not consecutive }; diff --git a/packages/cli/src/ui/hooks/useHistoryManager.ts b/packages/cli/src/ui/hooks/useHistoryManager.ts index 1d3fc8de116..0b79768843c 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.ts @@ -5,19 +5,22 @@ */ import { useState, useRef, useCallback, useMemo } from 'react'; -import type { HistoryItem } from '../types.js'; +import { createDebugLogger } from '@qwen-code/qwen-code-core'; +import type { HistoryItem, HistoryItemWithoutId } from '../types.js'; // Type for the updater function passed to updateHistoryItem type HistoryItemUpdater = ( prevItem: HistoryItem, -) => Partial>; +) => Partial; + +const debugLogger = createDebugLogger('HISTORY_MANAGER'); export interface UseHistoryManagerReturn { history: HistoryItem[]; - addItem: (itemData: Omit, baseTimestamp: number) => number; // Returns the generated ID + addItem: (itemData: HistoryItemWithoutId, baseTimestamp: number) => number; // Returns the generated ID updateItem: ( id: number, - updates: Partial> | HistoryItemUpdater, + updates: Partial | HistoryItemUpdater, ) => void; clearItems: () => void; loadHistory: (newHistory: HistoryItem[]) => void; @@ -46,7 +49,7 @@ export function useHistory(): UseHistoryManagerReturn { // Adds a new item to the history state with a unique ID. const addItem = useCallback( - (itemData: Omit, baseTimestamp: number): number => { + (itemData: HistoryItemWithoutId, baseTimestamp: number): number => { const id = getNextMessageId(baseTimestamp); const newItem: HistoryItem = { ...itemData, id } as HistoryItem; @@ -79,19 +82,28 @@ export function useHistory(): UseHistoryManagerReturn { const updateItem = useCallback( ( id: number, - updates: Partial> | HistoryItemUpdater, + updates: Partial | HistoryItemUpdater, ) => { - setHistory((prevHistory) => - prevHistory.map((item) => { + setHistory((prevHistory) => { + let updated = false; + const nextHistory = prevHistory.map((item) => { if (item.id === id) { + updated = true; // Apply updates based on whether it's an object or a function const newUpdates = typeof updates === 'function' ? updates(item) : updates; return { ...item, ...newUpdates } as HistoryItem; } return item; - }), - ); + }); + if (!updated) { + debugLogger.debug( + `Skipped history update; item ${id} was not found.`, + ); + return prevHistory; + } + return nextHistory; + }); }, [], ); diff --git a/packages/cli/src/ui/hooks/useResumeCommand.ts b/packages/cli/src/ui/hooks/useResumeCommand.ts index e52d000dde1..c37b81aec02 100644 --- a/packages/cli/src/ui/hooks/useResumeCommand.ts +++ b/packages/cli/src/ui/hooks/useResumeCommand.ts @@ -13,7 +13,7 @@ import { import { buildResumedHistoryItems } from '../utils/resumeHistoryUtils.js'; import { restoreGoalFromHistory } from '../utils/restoreGoal.js'; import type { UseHistoryManagerReturn } from './useHistoryManager.js'; -import { MessageType, type HistoryItem } from '../types.js'; +import { MessageType, type HistoryItemWithoutId } from '../types.js'; import { hasBlockingBackgroundWork, resetBackgroundStateForSessionSwitch, @@ -82,13 +82,11 @@ export function useResumeCommand( if (hasBlockingBackgroundWork(config)) { closeResumeDialog(); - addItem?.( - { - type: MessageType.ERROR, - text: BACKGROUND_WORK_SWITCH_BLOCKED_MESSAGE, - } as Omit, - Date.now(), - ); + const blockedMessage: HistoryItemWithoutId = { + type: MessageType.ERROR, + text: BACKGROUND_WORK_SWITCH_BLOCKED_MESSAGE, + }; + addItem?.(blockedMessage, Date.now()); return; } @@ -139,15 +137,13 @@ export function useResumeCommand( const recovered = await config.loadPausedBackgroundAgents(sessionId); if (recovered.length > 0) { - addItem?.( - { - type: MessageType.INFO, - text: config - .getBackgroundAgentResumeService() - .buildRecoveredBackgroundAgentsNotice(recovered.length), - } as Omit, - Date.now(), - ); + const recoveredMessage: HistoryItemWithoutId = { + type: MessageType.INFO, + text: config + .getBackgroundAgentResumeService() + .buildRecoveredBackgroundAgentsNotice(recovered.length), + }; + addItem?.(recoveredMessage, Date.now()); } // SessionStart hook is handled during chat initialization so its diff --git a/packages/cli/src/ui/hooks/useThemeCommand.ts b/packages/cli/src/ui/hooks/useThemeCommand.ts index b7b61384a62..e55cf1377fa 100644 --- a/packages/cli/src/ui/hooks/useThemeCommand.ts +++ b/packages/cli/src/ui/hooks/useThemeCommand.ts @@ -7,7 +7,7 @@ import { useState, useCallback } from 'react'; import { themeManager, AUTO_THEME_NAME } from '../themes/theme-manager.js'; import type { LoadedSettings, SettingScope } from '../../config/settings.js'; // Import LoadedSettings, AppSettings, MergedSetting -import { type HistoryItem, MessageType } from '../types.js'; +import { type HistoryItemWithoutId, MessageType } from '../types.js'; import process from 'node:process'; import { t } from '../../i18n/index.js'; @@ -24,7 +24,7 @@ interface UseThemeCommandReturn { export const useThemeCommand = ( loadedSettings: LoadedSettings, setThemeError: (error: string | null) => void, - addItem: (item: Omit, timestamp: number) => void, + addItem: (item: HistoryItemWithoutId, timestamp: number) => void, initialThemeError: string | null, ): UseThemeCommandReturn => { const [isThemeDialogOpen, setIsThemeDialogOpen] = diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 5a180130b53..dd6c70cd475 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -96,6 +96,16 @@ export type HistoryItemUser = HistoryItemBase & { type: 'user'; text: string; promptId?: string; + /** + * Whether this UI history item represents a user turn that reached the model. + * + * NOTE: This is set explicitly by slash command processing because visible + * slash-command invocations may be handled locally without entering API + * history. Regular user messages leave this undefined and are classified by + * the legacy lexical fallback in isRealUserTurn. New user-item paths with + * ambiguous model-history behavior must set this explicitly. + */ + sentToModel?: boolean; }; export type HistoryItemGemini = HistoryItemBase & { diff --git a/packages/cli/src/ui/utils/commandUtils.ts b/packages/cli/src/ui/utils/commandUtils.ts index 0e851372ce1..d4055751366 100644 --- a/packages/cli/src/ui/utils/commandUtils.ts +++ b/packages/cli/src/ui/utils/commandUtils.ts @@ -42,7 +42,7 @@ export const isAtCommand = (query: string): boolean => const SLASH_PATH_SEPARATOR_RE = /[/\\]/; const getSlashCommandFirstToken = (query: string): string => - query.slice(1).trimStart().split(/\s+/)[0] ?? ''; + query.slice(1).trimStart().split(/\s+/u)[0] ?? ''; export const hasSlashCommandPathSeparator = (query: string): boolean => SLASH_PATH_SEPARATOR_RE.test(getSlashCommandFirstToken(query)); @@ -52,6 +52,10 @@ export const hasSlashCommandPathSeparator = (query: string): boolean => * It triggers if the query starts with '/' but excludes code comments like '//' * and '/*', and file paths where the first token contains a path separator. * + * WARNING: This lexical classifier is also used as the legacy fallback for + * UI history items that do not have explicit sentToModel metadata. Coordinate + * changes here with isRealUserTurn in historyMapping.ts. + * * @param query The input query string. * @returns True if the query looks like an '/' command, false otherwise. */ diff --git a/packages/cli/src/ui/utils/historyMapping.test.ts b/packages/cli/src/ui/utils/historyMapping.test.ts index 8f6426a6d95..2879d8231a3 100644 --- a/packages/cli/src/ui/utils/historyMapping.test.ts +++ b/packages/cli/src/ui/utils/historyMapping.test.ts @@ -39,8 +39,17 @@ function startupPair(): [Content, Content] { ]; } -function userItem(id: number, text = `prompt ${id}`): HistoryItem { - return { type: 'user', id, text } as HistoryItem; +function userItem( + id: number, + text = `prompt ${id}`, + sentToModel?: boolean, +): HistoryItem { + return { + type: 'user', + id, + text, + ...(sentToModel === undefined ? {} : { sentToModel }), + } as HistoryItem; } function geminiItem(id: number): HistoryItem { @@ -229,6 +238,27 @@ describe('computeApiTruncationIndex', () => { expect(computeApiTruncationIndex(ui, 5, api)).toBe(4); }); + + it('counts slash command invocations explicitly marked as sent to the model', () => { + const ui: HistoryItem[] = [ + userItem(1, 'hello'), + geminiItem(2), + userItem(3, '/filecmd', true), + geminiItem(4), + userItem(5, 'world'), + geminiItem(6), + ]; + const api: Content[] = [ + userContent('hello'), + modelContent('response 1'), + userContent('expanded file command prompt'), + modelContent('response 2'), + userContent('world'), + modelContent('response 3'), + ]; + + expect(computeApiTruncationIndex(ui, 5, api)).toBe(4); + }); }); describe('single turn', () => { @@ -254,6 +284,22 @@ describe('isRealUserTurn', () => { expect(isRealUserTurn(userItem(1, '/stats'))).toBe(false); }); + it('uses explicit model-sent metadata for slash commands', () => { + expect(isRealUserTurn(userItem(1, '/filecmd', true))).toBe(true); + expect(isRealUserTurn(userItem(1, '/help', false))).toBe(false); + }); + + it('ignores corrupted non-boolean sentToModel metadata', () => { + const item = { + type: 'user', + id: 1, + text: '/filecmd', + sentToModel: 'true', + } as unknown as HistoryItem; + + expect(isRealUserTurn(item)).toBe(false); + }); + it('returns true for path-like slash prompts', () => { expect(isRealUserTurn(userItem(1, '/api/apiFunction/接口的实现'))).toBe( true, diff --git a/packages/cli/src/ui/utils/historyMapping.ts b/packages/cli/src/ui/utils/historyMapping.ts index 4c7d9875e4b..76395561578 100644 --- a/packages/cli/src/ui/utils/historyMapping.ts +++ b/packages/cli/src/ui/utils/historyMapping.ts @@ -17,6 +17,11 @@ import { isSlashCommand } from './commandUtils.js'; */ export function isRealUserTurn(item: HistoryItem): boolean { if (item.type !== 'user' || !item.text) return false; + if (typeof item.sentToModel === 'boolean') return item.sentToModel; + // Legacy resumed sessions do not have sentToModel, so this fallback is + // intentionally coupled to isSlashCommand's current lexical classifier. + // Changes to slash-command classification must account for old sessions that + // still rely on this inference. return !isSlashCommand(item.text) && !item.text.startsWith('?'); } diff --git a/packages/cli/src/ui/utils/resumeHistoryUtils.test.ts b/packages/cli/src/ui/utils/resumeHistoryUtils.test.ts index 7fae821e4fa..8493da4a460 100644 --- a/packages/cli/src/ui/utils/resumeHistoryUtils.test.ts +++ b/packages/cli/src/ui/utils/resumeHistoryUtils.test.ts @@ -325,4 +325,110 @@ describe('resumeHistoryUtils', () => { { id: 8, type: 'gemini', text: 'Follow-up' }, ]); }); + + it('preserves model-sent slash command metadata on resume', () => { + const conversation = { + messages: [ + { + type: 'system', + subtype: 'slash_command', + systemPayload: { + phase: 'invocation', + rawCommand: '/filecmd', + sentToModel: true, + }, + }, + { + type: 'assistant', + message: { parts: [{ text: 'Follow-up' } as Part] }, + }, + ], + } as unknown as ConversationRecord; + + const session: ResumedSessionData = { + conversation, + } as ResumedSessionData; + + const items = buildResumedHistoryItems(session, makeConfig({}), 20); + + expect(items).toEqual([ + { id: 21, type: 'user', text: '/filecmd', sentToModel: true }, + { id: 22, type: 'gemini', text: 'Follow-up' }, + ]); + }); + + it('preserves local-only slash command metadata on resume', () => { + const conversation = { + messages: [ + { + type: 'system', + subtype: 'slash_command', + systemPayload: { + phase: 'invocation', + rawCommand: '/about', + sentToModel: false, + }, + }, + ], + } as unknown as ConversationRecord; + + const session: ResumedSessionData = { + conversation, + } as ResumedSessionData; + + const items = buildResumedHistoryItems(session, makeConfig({}), 30); + + expect(items).toEqual([ + { id: 31, type: 'user', text: '/about', sentToModel: false }, + ]); + }); + + it('omits sentToModel for legacy slash command records', () => { + const conversation = { + messages: [ + { + type: 'system', + subtype: 'slash_command', + systemPayload: { + phase: 'invocation', + rawCommand: '/legacy', + }, + }, + ], + } as unknown as ConversationRecord; + + const session: ResumedSessionData = { + conversation, + } as ResumedSessionData; + + const items = buildResumedHistoryItems(session, makeConfig({}), 40); + + expect(items).toEqual([{ id: 41, type: 'user', text: '/legacy' }]); + expect(items[0]).not.toHaveProperty('sentToModel'); + }); + + it('omits corrupted non-boolean sentToModel metadata on resume', () => { + const conversation = { + messages: [ + { + type: 'system', + subtype: 'slash_command', + systemPayload: { + phase: 'invocation', + rawCommand: '/filecmd', + sentToModel: 'true', + }, + }, + ], + } as unknown as ConversationRecord; + + const session: ResumedSessionData = { + conversation, + } as ResumedSessionData; + + const items = buildResumedHistoryItems(session, makeConfig({}), 50); + + expect(items).toEqual([{ id: 51, type: 'user', text: '/filecmd' }]); + expect(items[0]).not.toHaveProperty('sentToModel'); + }); }); diff --git a/packages/cli/src/ui/utils/resumeHistoryUtils.ts b/packages/cli/src/ui/utils/resumeHistoryUtils.ts index edb0ac6a487..57b14511427 100644 --- a/packages/cli/src/ui/utils/resumeHistoryUtils.ts +++ b/packages/cli/src/ui/utils/resumeHistoryUtils.ts @@ -237,7 +237,15 @@ function convertToHistoryItems( | undefined; if (!payload) continue; if (payload.phase === 'invocation' && payload.rawCommand) { - items.push({ type: 'user', text: payload.rawCommand }); + const sentToModel = + typeof payload.sentToModel === 'boolean' + ? payload.sentToModel + : undefined; + items.push({ + type: 'user', + text: payload.rawCommand, + ...(sentToModel === undefined ? {} : { sentToModel }), + }); } if (payload.phase === 'result') { const outputs = payload.outputHistoryItems ?? []; diff --git a/packages/cli/src/utils/handleAutoUpdate.ts b/packages/cli/src/utils/handleAutoUpdate.ts index 2552f404865..05dd62c3de9 100644 --- a/packages/cli/src/utils/handleAutoUpdate.ts +++ b/packages/cli/src/utils/handleAutoUpdate.ts @@ -8,7 +8,7 @@ import type { UpdateObject } from '../ui/utils/updateCheck.js'; import type { LoadedSettings } from '../config/settings.js'; import { getInstallationInfo } from './installationInfo.js'; import { updateEventEmitter } from './updateEventEmitter.js'; -import type { HistoryItem } from '../ui/types.js'; +import type { HistoryItemWithoutId } from '../ui/types.js'; import { MessageType } from '../ui/types.js'; import { spawnWrapper } from './spawnWrapper.js'; import type { spawn } from 'node:child_process'; @@ -84,14 +84,14 @@ export function handleAutoUpdate( } export function setUpdateHandler( - addItem: (item: Omit, timestamp: number) => void, + addItem: (item: HistoryItemWithoutId, timestamp: number) => void, setUpdateInfo: (info: UpdateObject | null) => void, isIdleRef: { current: boolean } = { current: true }, ) { let successfullyInstalled = false; - const pendingNotifications: Array> = []; + const pendingNotifications: HistoryItemWithoutId[] = []; - const addItemOrDefer = (item: Omit) => { + const addItemOrDefer = (item: HistoryItemWithoutId) => { if (isIdleRef.current) { addItem(item, Date.now()); } else { diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 05c06c19294..ecbdae9b27c 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -358,6 +358,8 @@ export interface SlashCommandRecordPayload { phase: 'invocation' | 'result'; /** Raw user-entered slash command (e.g., "/about"). */ rawCommand: string; + /** Whether the visible slash-command invocation reached model history. */ + sentToModel?: boolean; /** * History items the UI displayed for this command, in the same shape used by * the CLI (without IDs). Stored as plain objects for replay on resume.