fix(ui): prevent over-eager slash subcommand completion (#20136)

This commit is contained in:
Keith Guerin
2026-03-24 20:11:09 -07:00
committed by GitHub
parent 73526416cf
commit 46aa3fd193
4 changed files with 169 additions and 157 deletions
@@ -1,6 +1,6 @@
/** /**
* @license * @license
* Copyright 2025 Google LLC * Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
@@ -20,7 +20,7 @@ import {
useCommandCompletion, useCommandCompletion,
CompletionMode, CompletionMode,
} from './useCommandCompletion.js'; } from './useCommandCompletion.js';
import type { CommandContext } from '../commands/types.js'; import type { CommandContext, SlashCommand } from '../commands/types.js';
import type { Config } from '@google/gemini-cli-core'; import type { Config } from '@google/gemini-cli-core';
import { useTextBuffer } from '../components/shared/text-buffer.js'; import { useTextBuffer } from '../components/shared/text-buffer.js';
import type { Suggestion } from '../components/SuggestionsDisplay.js'; import type { Suggestion } from '../components/SuggestionsDisplay.js';
@@ -72,7 +72,11 @@ const setupMocks = ({
shellSuggestions = [], shellSuggestions = [],
isLoading = false, isLoading = false,
isPerfectMatch = false, isPerfectMatch = false,
slashCompletionRange = { completionStart: 0, completionEnd: 0 }, slashCompletionRange = {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () => undefined,
},
shellCompletionRange = { shellCompletionRange = {
completionStart: 0, completionStart: 0,
completionEnd: 0, completionEnd: 0,
@@ -85,7 +89,13 @@ const setupMocks = ({
shellSuggestions?: Suggestion[]; shellSuggestions?: Suggestion[];
isLoading?: boolean; isLoading?: boolean;
isPerfectMatch?: boolean; isPerfectMatch?: boolean;
slashCompletionRange?: { completionStart: number; completionEnd: number }; slashCompletionRange?: {
completionStart: number;
completionEnd: number;
getCommandFromSuggestion: (
suggestion: Suggestion,
) => SlashCommand | undefined;
};
shellCompletionRange?: { shellCompletionRange?: {
completionStart: number; completionStart: number;
completionEnd: number; completionEnd: number;
@@ -471,10 +481,15 @@ describe('useCommandCompletion', () => {
}); });
describe('handleAutocomplete', () => { describe('handleAutocomplete', () => {
it('should complete a partial command', async () => { it('should complete a partial command and NOT add a space if it has an action', async () => {
setupMocks({ setupMocks({
slashSuggestions: [{ label: 'memory', value: 'memory' }], slashSuggestions: [{ label: 'memory', value: 'memory' }],
slashCompletionRange: { completionStart: 1, completionEnd: 4 }, slashCompletionRange: {
completionStart: 1,
completionEnd: 4,
getCommandFromSuggestion: () =>
({ action: vi.fn() }) as unknown as SlashCommand,
},
}); });
const { result } = await renderCommandCompletionHook('/mem'); const { result } = await renderCommandCompletionHook('/mem');
@@ -487,12 +502,40 @@ describe('useCommandCompletion', () => {
result.current.handleAutocomplete(0); result.current.handleAutocomplete(0);
}); });
expect(result.current.textBuffer.text).toBe('/memory '); expect(result.current.textBuffer.text).toBe('/memory');
});
it('should complete a partial command and ADD a space if it has NO action (e.g. just a parent)', async () => {
setupMocks({
slashSuggestions: [{ label: 'chat', value: 'chat' }],
slashCompletionRange: {
completionStart: 1,
completionEnd: 5,
getCommandFromSuggestion: () => ({}) as unknown as SlashCommand, // No action
},
});
const { result } = await renderCommandCompletionHook('/chat');
await waitFor(() => {
expect(result.current.suggestions.length).toBe(1);
});
act(() => {
result.current.handleAutocomplete(0);
});
expect(result.current.textBuffer.text).toBe('/chat ');
}); });
it('should complete a file path', async () => { it('should complete a file path', async () => {
setupMocks({ setupMocks({
atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }], atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }],
slashCompletionRange: {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () => undefined,
},
}); });
const { result } = await renderCommandCompletionHook('@src/fi'); const { result } = await renderCommandCompletionHook('@src/fi');
@@ -517,7 +560,11 @@ describe('useCommandCompletion', () => {
insertValue: 'resume list', insertValue: 'resume list',
}, },
], ],
slashCompletionRange: { completionStart: 1, completionEnd: 5 }, slashCompletionRange: {
completionStart: 1,
completionEnd: 5,
getCommandFromSuggestion: () => undefined,
},
}); });
const { result } = await renderCommandCompletionHook('/resu'); const { result } = await renderCommandCompletionHook('/resu');
@@ -539,6 +586,11 @@ describe('useCommandCompletion', () => {
setupMocks({ setupMocks({
atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }], atSuggestions: [{ label: 'src/file1.txt', value: 'src/file1.txt' }],
slashCompletionRange: {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () => undefined,
},
}); });
const { result } = await renderCommandCompletionHook(text, cursorOffset); const { result } = await renderCommandCompletionHook(text, cursorOffset);
@@ -559,6 +611,11 @@ describe('useCommandCompletion', () => {
it('should complete a directory path ending with / without a trailing space', async () => { it('should complete a directory path ending with / without a trailing space', async () => {
setupMocks({ setupMocks({
atSuggestions: [{ label: 'src/components/', value: 'src/components/' }], atSuggestions: [{ label: 'src/components/', value: 'src/components/' }],
slashCompletionRange: {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () => undefined,
},
}); });
const { result } = await renderCommandCompletionHook('@src/comp'); const { result } = await renderCommandCompletionHook('@src/comp');
@@ -579,6 +636,11 @@ describe('useCommandCompletion', () => {
atSuggestions: [ atSuggestions: [
{ label: 'src\\components\\', value: 'src\\components\\' }, { label: 'src\\components\\', value: 'src\\components\\' },
], ],
slashCompletionRange: {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () => undefined,
},
}); });
const { result } = await renderCommandCompletionHook('@src\\comp'); const { result } = await renderCommandCompletionHook('@src\\comp');
@@ -594,6 +656,33 @@ describe('useCommandCompletion', () => {
expect(result.current.textBuffer.text).toBe('@src\\components\\'); expect(result.current.textBuffer.text).toBe('@src\\components\\');
}); });
it('should ADD a space for AT completion even if name matches a command with an action', async () => {
// Setup a mock where getCommandFromSuggestion WOULD return a command with an action
// if it were in SLASH mode.
setupMocks({
atSuggestions: [{ label: 'memory', value: 'memory' }],
slashCompletionRange: {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () =>
({ action: vi.fn() }) as unknown as SlashCommand,
},
});
const { result } = await renderCommandCompletionHook('@mem');
await waitFor(() => {
expect(result.current.suggestions.length).toBe(1);
});
act(() => {
result.current.handleAutocomplete(0);
});
// Should have a space because it's AT mode, not SLASH mode
expect(result.current.textBuffer.text).toBe('@memory ');
});
it('should show ghost text for a single shell completion', async () => { it('should show ghost text for a single shell completion', async () => {
const text = 'l'; const text = 'l';
setupMocks({ setupMocks({
@@ -905,6 +994,11 @@ describe('useCommandCompletion', () => {
it('should complete file path and add trailing space', async () => { it('should complete file path and add trailing space', async () => {
setupMocks({ setupMocks({
atSuggestions: [{ label: 'src/file.txt', value: 'src/file.txt' }], atSuggestions: [{ label: 'src/file.txt', value: 'src/file.txt' }],
slashCompletionRange: {
completionStart: 0,
completionEnd: 0,
getCommandFromSuggestion: () => undefined,
},
}); });
const { result } = await renderCommandCompletionHook('/cmd @src/fi'); const { result } = await renderCommandCompletionHook('/cmd @src/fi');
@@ -1,16 +1,17 @@
/** /**
* @license * @license
* Copyright 2025 Google LLC * Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
import type React from 'react';
import { useCallback, useMemo, useEffect, useState } from 'react'; import { useCallback, useMemo, useEffect, useState } from 'react';
import type { Suggestion } from '../components/SuggestionsDisplay.js'; import type { Suggestion } from '../components/SuggestionsDisplay.js';
import type { CommandContext, SlashCommand } from '../commands/types.js'; import type { CommandContext, SlashCommand } from '../commands/types.js';
import type { TextBuffer } from '../components/shared/text-buffer.js'; import type { TextBuffer } from '../components/shared/text-buffer.js';
import { logicalPosToOffset } from '../components/shared/text-buffer.js'; import { logicalPosToOffset } from '../components/shared/text-buffer.js';
import { isSlashCommand } from '../utils/commandUtils.js';
import { toCodePoints } from '../utils/textUtils.js'; import { toCodePoints } from '../utils/textUtils.js';
import { isSlashCommand } from '../utils/commandUtils.js';
import { useAtCompletion } from './useAtCompletion.js'; import { useAtCompletion } from './useAtCompletion.js';
import { useSlashCompletion } from './useSlashCompletion.js'; import { useSlashCompletion } from './useSlashCompletion.js';
import { useShellCompletion } from './useShellCompletion.js'; import { useShellCompletion } from './useShellCompletion.js';
@@ -436,10 +437,23 @@ export function useCommandCompletion({
const lineCodePoints = toCodePoints(buffer.lines[cursorRow] || ''); const lineCodePoints = toCodePoints(buffer.lines[cursorRow] || '');
const charAfterCompletion = lineCodePoints[end]; const charAfterCompletion = lineCodePoints[end];
let shouldAddSpace = true;
if (completionMode === CompletionMode.SLASH) {
const command =
slashCompletionRange.getCommandFromSuggestion(suggestion);
// Don't add a space if the command has an action (can be executed)
// and doesn't have a completion function (doesn't REQUIRE more arguments)
const isExecutableCommand = !!(command && command.action);
const requiresArguments = !!(command && command.completion);
shouldAddSpace = !isExecutableCommand || requiresArguments;
}
if ( if (
charAfterCompletion !== ' ' && charAfterCompletion !== ' ' &&
!suggestionText.endsWith('/') && !suggestionText.endsWith('/') &&
!suggestionText.endsWith('\\') !suggestionText.endsWith('\\') &&
shouldAddSpace
) { ) {
suggestionText += ' '; suggestionText += ' ';
} }
@@ -1,6 +1,6 @@
/** /**
* @license * @license
* Copyright 2025 Google LLC * Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
@@ -464,7 +464,7 @@ describe('useSlashCompletion', () => {
() => () =>
useTestHarnessForSlashCompletion( useTestHarnessForSlashCompletion(
true, true,
'/chat', '/chat ',
slashCommands, slashCommands,
mockCommandContext, mockCommandContext,
), ),
@@ -484,7 +484,7 @@ describe('useSlashCompletion', () => {
() => () =>
useTestHarnessForSlashCompletion( useTestHarnessForSlashCompletion(
true, true,
'/resume', '/resume ',
slashCommands, slashCommands,
mockCommandContext, mockCommandContext,
), ),
@@ -513,53 +513,6 @@ describe('useSlashCompletion', () => {
unmountResume(); unmountResume();
}); });
it('should show the grouped /resume menu for unique /resum prefix input', async () => {
const slashCommands = [
createTestCommand({
name: 'resume',
description: 'Resume command',
action: vi.fn(),
subCommands: [
createTestCommand({
name: 'list',
description: 'List checkpoints',
suggestionGroup: 'checkpoints',
}),
createTestCommand({
name: 'save',
description: 'Save checkpoint',
suggestionGroup: 'checkpoints',
}),
],
}),
];
const { result, unmount } = await renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/resum',
slashCommands,
mockCommandContext,
),
);
await resolveMatch();
await waitFor(() => {
expect(result.current.suggestions[0]).toMatchObject({
label: 'list',
sectionTitle: 'auto',
submitValue: '/resume',
});
expect(result.current.isPerfectMatch).toBe(false);
expect(result.current.suggestions.slice(1).map((s) => s.label)).toEqual(
expect.arrayContaining(['list', 'save']),
);
});
unmount();
});
it('should sort exact altName matches to the top', async () => { it('should sort exact altName matches to the top', async () => {
const slashCommands = [ const slashCommands = [
createTestCommand({ createTestCommand({
@@ -594,7 +547,7 @@ describe('useSlashCompletion', () => {
unmount(); unmount();
}); });
it('should suggest subcommands when a parent command is fully typed without a trailing space', async () => { it('should suggest the command itself instead of subcommands when a parent command is fully typed without a trailing space', async () => {
const slashCommands = [ const slashCommands = [
createTestCommand({ createTestCommand({
name: 'chat', name: 'chat',
@@ -618,18 +571,47 @@ describe('useSlashCompletion', () => {
await resolveMatch(); await resolveMatch();
await waitFor(() => { await waitFor(() => {
// Should show the auto-session entry plus subcommands of 'chat' // Should show 'chat' as the suggestion, NOT its subcommands
expect(result.current.suggestions).toHaveLength(3); expect(result.current.suggestions).toHaveLength(1);
expect(result.current.suggestions[0]).toMatchObject({ expect(result.current.suggestions[0].label).toBe('chat');
label: 'list', // completionStart should be at 1 (to replace 'chat')
sectionTitle: 'auto', expect(result.current.completionStart).toBe(1);
submitValue: '/chat', });
}); unmount();
expect(result.current.suggestions.map((s) => s.label)).toEqual( });
expect.arrayContaining(['list', 'save']),
); it('should NOT suggest subcommands when a parent command is fully typed without a trailing space (fix for over-eager completion)', async () => {
// completionStart should be at the end of '/chat' to append subcommands const slashCommands = [
expect(result.current.completionStart).toBe(5); createTestCommand({
name: 'stats',
description: 'Check session stats',
action: vi.fn(), // Has action
subCommands: [
createTestCommand({
name: 'session',
description: 'Show session-specific usage statistics',
}),
],
}),
];
const { result, unmount } = await renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/stats',
slashCommands,
mockCommandContext,
),
);
await resolveMatch();
await waitFor(() => {
// Should show 'stats' as the suggestion, NOT 'session'
expect(result.current.suggestions).toHaveLength(1);
expect(result.current.suggestions[0].label).toBe('stats');
// isPerfectMatch should be true because it has an action
expect(result.current.isPerfectMatch).toBe(true);
}); });
unmount(); unmount();
}); });
@@ -1,6 +1,6 @@
/** /**
* @license * @license
* Copyright 2025 Google LLC * Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
@@ -54,8 +54,6 @@ interface CommandParserResult {
partial: string; partial: string;
currentLevel: readonly SlashCommand[] | undefined; currentLevel: readonly SlashCommand[] | undefined;
leafCommand: SlashCommand | null; leafCommand: SlashCommand | null;
exactMatchAsParent: SlashCommand | undefined;
usedPrefixParentDescent: boolean;
isArgumentCompletion: boolean; isArgumentCompletion: boolean;
} }
@@ -71,8 +69,6 @@ function useCommandParser(
partial: '', partial: '',
currentLevel: slashCommands, currentLevel: slashCommands,
leafCommand: null, leafCommand: null,
exactMatchAsParent: undefined,
usedPrefixParentDescent: false,
isArgumentCompletion: false, isArgumentCompletion: false,
}; };
} }
@@ -90,7 +86,6 @@ function useCommandParser(
let currentLevel: readonly SlashCommand[] | undefined = slashCommands; let currentLevel: readonly SlashCommand[] | undefined = slashCommands;
let leafCommand: SlashCommand | null = null; let leafCommand: SlashCommand | null = null;
let usedPrefixParentDescent = false;
for (const part of commandPathParts) { for (const part of commandPathParts) {
if (!currentLevel) { if (!currentLevel) {
@@ -115,60 +110,6 @@ function useCommandParser(
} }
} }
let exactMatchAsParent: SlashCommand | undefined;
if (!hasTrailingSpace && currentLevel) {
exactMatchAsParent = currentLevel.find(
(cmd) => matchesCommand(cmd, partial) && cmd.subCommands,
);
if (exactMatchAsParent) {
// Only descend if there are NO other matches for the partial at this level.
// This ensures that typing "/memory" still shows "/memory-leak" if it exists.
const otherMatches = currentLevel.filter(
(cmd) =>
cmd !== exactMatchAsParent &&
(cmd.name.toLowerCase().startsWith(partial.toLowerCase()) ||
cmd.altNames?.some((alt) =>
alt.toLowerCase().startsWith(partial.toLowerCase()),
)),
);
if (otherMatches.length === 0) {
leafCommand = exactMatchAsParent;
currentLevel = exactMatchAsParent.subCommands as
| readonly SlashCommand[]
| undefined;
partial = '';
}
}
// Phase-one alias UX: allow unique prefix descent for /chat and /resume
// so `/cha` and `/resum` expose the same grouped menu immediately.
if (!exactMatchAsParent && partial && currentLevel) {
const prefixParentMatches = currentLevel.filter(
(cmd) =>
!!cmd.subCommands &&
(cmd.name.toLowerCase().startsWith(partial.toLowerCase()) ||
cmd.altNames?.some((alt) =>
alt.toLowerCase().startsWith(partial.toLowerCase()),
)),
);
if (prefixParentMatches.length === 1) {
const candidate = prefixParentMatches[0];
if (candidate.name === 'chat' || candidate.name === 'resume') {
exactMatchAsParent = candidate;
leafCommand = candidate;
usedPrefixParentDescent = true;
currentLevel = candidate.subCommands as
| readonly SlashCommand[]
| undefined;
partial = '';
}
}
}
}
const depth = commandPathParts.length; const depth = commandPathParts.length;
const isArgumentCompletion = !!( const isArgumentCompletion = !!(
leafCommand?.completion && leafCommand?.completion &&
@@ -182,8 +123,6 @@ function useCommandParser(
partial, partial,
currentLevel, currentLevel,
leafCommand, leafCommand,
exactMatchAsParent,
usedPrefixParentDescent,
isArgumentCompletion, isArgumentCompletion,
}; };
}, [query, slashCommands]); }, [query, slashCommands]);
@@ -343,19 +282,9 @@ function useCommandSuggestions(
}); });
const finalSuggestions = sortedSuggestions.map((cmd) => { const finalSuggestions = sortedSuggestions.map((cmd) => {
const canonicalParentName =
parserResult.usedPrefixParentDescent &&
leafCommand &&
(leafCommand.name === 'chat' || leafCommand.name === 'resume')
? leafCommand.name
: undefined;
const suggestion: Suggestion = { const suggestion: Suggestion = {
label: cmd.name, label: cmd.name,
value: cmd.name, value: cmd.name,
insertValue: canonicalParentName
? `${canonicalParentName} ${cmd.name}`
: undefined,
description: cmd.description, description: cmd.description,
commandKind: cmd.kind, commandKind: cmd.kind,
}; };
@@ -384,7 +313,7 @@ function useCommandSuggestions(
description: 'Browse auto-saved chats', description: 'Browse auto-saved chats',
commandKind: CommandKind.BUILT_IN, commandKind: CommandKind.BUILT_IN,
sectionTitle: 'auto', sectionTitle: 'auto',
submitValue: `/${leafCommand.name}`, submitValue: `/${canonicalParentName}`,
}; };
setSuggestions([autoSectionSuggestion, ...finalSuggestions]); setSuggestions([autoSectionSuggestion, ...finalSuggestions]);
return; return;
@@ -427,12 +356,10 @@ function useCompletionPositions(
return { start: -1, end: -1 }; return { start: -1, end: -1 };
} }
const { hasTrailingSpace, partial, exactMatchAsParent } = parserResult; const { hasTrailingSpace, partial } = parserResult;
// Set completion start/end positions // Set completion start/end positions
if (parserResult.usedPrefixParentDescent) { if (hasTrailingSpace) {
return { start: 1, end: query.length };
} else if (hasTrailingSpace || exactMatchAsParent) {
return { start: query.length, end: query.length }; return { start: query.length, end: query.length };
} else if (partial) { } else if (partial) {
if (parserResult.isArgumentCompletion) { if (parserResult.isArgumentCompletion) {
@@ -461,12 +388,7 @@ function usePerfectMatch(
return { isPerfectMatch: false }; return { isPerfectMatch: false };
} }
if ( if (leafCommand && partial === '' && leafCommand.action) {
leafCommand &&
partial === '' &&
leafCommand.action &&
!parserResult.usedPrefixParentDescent
) {
return { isPerfectMatch: true }; return { isPerfectMatch: true };
} }