Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 48 additions & 37 deletions packages/cli/src/ui/hooks/slashCommandProcessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@

import { act, renderHook, waitFor } from '@testing-library/react';
import { vi, describe, it, expect, beforeEach } from 'vitest';
import { useSlashCommandProcessor } from './slashCommandProcessor.js';
import {
useSlashCommandProcessor,
type SlashCommandProcessorActions,
} from './slashCommandProcessor.js';
import type {
CommandContext,
ConfirmShellCommandsActionReturn,
Expand Down Expand Up @@ -121,6 +124,33 @@ describe('useSlashCommandProcessor', () => {
});
const mockSettings = {} as LoadedSettings;

const createMockActions = (): SlashCommandProcessorActions => ({
openAuthDialog: mockOpenAuthDialog,
openArenaDialog: vi.fn(),
openThemeDialog: mockOpenThemeDialog,
openEditorDialog: vi.fn(),
openMemoryDialog: mockOpenMemoryDialog,
openSettingsDialog: vi.fn(),
openModelDialog: mockOpenModelDialog,
openManageModelsDialog: vi.fn(),
openTrustDialog: vi.fn(),
openPermissionsDialog: vi.fn(),
openApprovalModeDialog: vi.fn(),
openResumeDialog: vi.fn(),
handleResume: vi.fn(),
openDeleteDialog: vi.fn(),
quit: mockSetQuittingMessages,
setDebugMessage: vi.fn(),
dispatchExtensionStateUpdate: vi.fn(),
addConfirmUpdateExtensionRequest: vi.fn(),
openSubagentCreateDialog: vi.fn(),
openAgentsManagerDialog: vi.fn(),
openExtensionsManagerDialog: vi.fn(),
openMcpDialog: vi.fn(),
openHooksDialog: vi.fn(),
openRewindSelector: vi.fn(),
});

beforeEach(() => {
vi.clearAllMocks();
vi.mocked(BuiltinCommandLoader).mockClear();
Expand Down Expand Up @@ -154,24 +184,7 @@ describe('useSlashCommandProcessor', () => {
setIsProcessing,
{ current: true }, // isIdleRef
vi.fn(), // setGeminiMdFileCount
{
openAuthDialog: mockOpenAuthDialog,
openThemeDialog: mockOpenThemeDialog,
openEditorDialog: vi.fn(),
openMemoryDialog: mockOpenMemoryDialog,
openSettingsDialog: vi.fn(),
openModelDialog: mockOpenModelDialog,
openTrustDialog: vi.fn(),
openPermissionsDialog: vi.fn(),
openApprovalModeDialog: vi.fn(),
openResumeDialog: vi.fn(),
quit: mockSetQuittingMessages,
setDebugMessage: vi.fn(),
dispatchExtensionStateUpdate: vi.fn(),
addConfirmUpdateExtensionRequest: vi.fn(),
openSubagentCreateDialog: vi.fn(),
openAgentsManagerDialog: vi.fn(),
},
createMockActions(),
new Map(), // extensionsUpdateState
true, // isConfigInitialized
null, // logger
Expand Down Expand Up @@ -270,6 +283,21 @@ describe('useSlashCommandProcessor', () => {
);
});

it('should let slash-prefixed file paths fall through to the model', async () => {
const result = setupProcessorHook();
await waitFor(() => expect(result.current.slashCommands).toBeDefined());

let actionResult;
await act(async () => {
actionResult = await result.current.handleSlashCommand(
'/api/apiFunction/接口的实现',
);
});

expect(actionResult).toBe(false);
expect(mockAddItem).not.toHaveBeenCalled();
});

it('should display help for a parent command invoked without a subcommand', async () => {
const parentCommand: SlashCommand = {
name: 'parent',
Expand Down Expand Up @@ -968,24 +996,7 @@ describe('useSlashCommandProcessor', () => {
vi.fn(), // setIsProcessing
{ current: true }, // isIdleRef
vi.fn(), // setGeminiMdFileCount
{
openAuthDialog: mockOpenAuthDialog,
openThemeDialog: mockOpenThemeDialog,
openEditorDialog: vi.fn(),
openMemoryDialog: mockOpenMemoryDialog,
openSettingsDialog: vi.fn(),
openModelDialog: vi.fn(),
openTrustDialog: vi.fn(),
openPermissionsDialog: vi.fn(),
openApprovalModeDialog: vi.fn(),
openResumeDialog: vi.fn(),
quit: mockSetQuittingMessages,
setDebugMessage: vi.fn(),
dispatchExtensionStateUpdate: vi.fn(),
addConfirmUpdateExtensionRequest: vi.fn(),
openSubagentCreateDialog: vi.fn(),
openAgentsManagerDialog: vi.fn(),
},
createMockActions(),
new Map(), // extensionsUpdateState
true, // isConfigInitialized
null, // logger
Expand Down
10 changes: 8 additions & 2 deletions packages/cli/src/ui/hooks/slashCommandProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ import { FileCommandLoader } from '../../services/FileCommandLoader.js';
import { McpPromptLoader } from '../../services/McpPromptLoader.js';
import { SkillCommandLoader } from '../../services/SkillCommandLoader.js';
import { parseSlashCommand } from '../../utils/commands.js';
import { isBtwCommand } from '../utils/commandUtils.js';
import {
hasSlashCommandPathSeparator,
isBtwCommand,
} from '../utils/commandUtils.js';
import { clearScreen } from '../../utils/stdioHelpers.js';
import { useKeypress } from './useKeypress.js';
import {
Expand Down Expand Up @@ -78,7 +81,7 @@ const SLASH_COMMANDS_SKIP_RECORDING = new Set([
'btw',
]);

interface SlashCommandProcessorActions {
export interface SlashCommandProcessorActions {
openAuthDialog: () => void;
openArenaDialog?: (type: Exclude<ArenaDialogType, null>) => void;
openThemeDialog: () => void;
Expand Down Expand Up @@ -448,6 +451,9 @@ export const useSlashCommandProcessor = (
if (!trimmed.startsWith('/') && !trimmed.startsWith('?')) {
return false;
}
if (trimmed.startsWith('/') && hasSlashCommandPathSeparator(trimmed)) {
Comment thread
yiliang114 marked this conversation as resolved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is unreachable in practice. Every caller of handleSlashCommand already gates on isSlashCommand(...) (useGeminiStream.ts:656, AppContainer.tsx:1257), and isSlashCommand itself returns false for path-like inputs after this PR.

Two options:

  1. Drop the check and rely on the caller-side gate.
  2. Keep it as a safety net but add a one-line comment explaining why it duplicates isSlashCommand's logic.

Without a comment, a future maintainer will likely "DRY it up" and remove it, possibly along with the safety net it was meant to provide.

return false;
}

const recordedItems: Array<Omit<HistoryItem, 'id'>> = [];
const recordItem = (item: Omit<HistoryItem, 'id'>) => {
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/src/ui/utils/commandUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ describe('commandUtils', () => {
expect(isSlashCommand('/*\n * Multi-line comment\n */')).toBe(false);
expect(isSlashCommand('/*comment without space*/')).toBe(false);
});

it('should return false for slash-prefixed file paths', () => {
expect(isSlashCommand('/api/apiFunction/接口的实现')).toBe(false);
expect(isSlashCommand('/Users/me/project/src/index.ts')).toBe(false);
expect(isSlashCommand('/var/log/syslog check this')).toBe(false);
expect(isSlashCommand('/home/user/.qwen/settings.json')).toBe(false);
expect(isSlashCommand('/tmp/test.txt')).toBe(false);
expect(isSlashCommand('/tmp\\test.txt')).toBe(false);
});
});

describe('copyToClipboard', () => {
Expand Down
15 changes: 14 additions & 1 deletion packages/cli/src/ui/utils/commandUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,18 @@ export const isAtCommand = (query: string): boolean =>
// Check if starts with @ OR has a space, then @
query.startsWith('@') || /\s@/.test(query);

const SLASH_PATH_SEPARATOR_RE = /[/\\]/;

const getSlashCommandFirstToken = (query: string): string =>
query.slice(1).trimStart().split(/\s+/)[0] ?? '';
Comment thread
yiliang114 marked this conversation as resolved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: String.prototype.split always returns a non-empty array, so [0] is always a string and the ?? '' never fires. Harmless, but slightly misleading — it suggests split could return [].


export const hasSlashCommandPathSeparator = (query: string): boolean =>
SLASH_PATH_SEPARATOR_RE.test(getSlashCommandFirstToken(query));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstated precondition: getSlashCommandFirstToken calls query.slice(1) and silently strips the first character whether or not it's /. Today the only caller is hasSlashCommandPathSeparator, whose only reachable callers gate on startsWith('/') — so this is safe. But hasSlashCommandPathSeparator is exported, and a future caller passing 'foo/bar' would get back 'oo/bar' and a wrong answer.

Two cheap mitigations:

  • Add a JSDoc note ("expects a /-prefixed query") on hasSlashCommandPathSeparator, or
  • Have hasSlashCommandPathSeparator early-return false if !query.startsWith('/'), removing the precondition entirely.


/**
* Checks if a query string potentially represents an '/' command.
* It triggers if the query starts with '/' but excludes code comments like '//' and '/*'.
* It triggers if the query starts with '/' but excludes code comments like '//'
* and '/*', and file paths where the first token contains a path separator.
*
* @param query The input query string.
* @returns True if the query looks like an '/' command, false otherwise.
Expand All @@ -60,6 +69,10 @@ export const isSlashCommand = (query: string): boolean => {
return false;
}

if (hasSlashCommandPathSeparator(query)) {
return false;
}

return true;
};

Expand Down
30 changes: 30 additions & 0 deletions packages/cli/src/ui/utils/historyMapping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,27 @@ describe('computeApiTruncationIndex', () => {
// Slash '/help' (id=3) should not be counted
expect(computeApiTruncationIndex(ui, 5, api)).toBe(2);
});

it('counts path-like slash prompts that were sent to the model', () => {
const ui: HistoryItem[] = [
userItem(1, 'hello'),
geminiItem(2),
userItem(3, '/api/apiFunction/接口的实现'),
geminiItem(4),
userItem(5, 'world'),
geminiItem(6),
];
const api: Content[] = [
userContent('hello'),
modelContent('response 1'),
userContent('/api/apiFunction/接口的实现'),
modelContent('response 2'),
userContent('world'),
modelContent('response 3'),
];

expect(computeApiTruncationIndex(ui, 5, api)).toBe(4);
});
});

describe('single turn', () => {
Expand All @@ -233,6 +254,15 @@ describe('isRealUserTurn', () => {
expect(isRealUserTurn(userItem(1, '/stats'))).toBe(false);
});

it('returns true for path-like slash prompts', () => {
expect(isRealUserTurn(userItem(1, '/api/apiFunction/接口的实现'))).toBe(
true,
);
expect(isRealUserTurn(userItem(1, '/Users/name/project 帮我安装'))).toBe(
true,
);
});

it('returns false for ? commands', () => {
expect(isRealUserTurn(userItem(1, '?help'))).toBe(false);
});
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/ui/utils/historyMapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import type { HistoryItem } from '../types.js';
import type { Content } from '@google/genai';
import { isSlashCommand } from './commandUtils.js';

/**
* Returns true when the history item represents a real user prompt that was
Expand All @@ -15,7 +16,7 @@ import type { Content } from '@google/genai';
*/
export function isRealUserTurn(item: HistoryItem): boolean {
if (item.type !== 'user' || !item.text) return false;
return !item.text.startsWith('/') && !item.text.startsWith('?');
return !isSlashCommand(item.text) && !item.text.startsWith('?');
}

/**
Expand Down
Loading