feat(ui): improve discoverability of MCP slash commands (#6080)

Co-authored-by: Rinil Kunhiraman <rinilkunhiraman@users.noreply.github.com>
Co-authored-by: Allen Hutchison <adh@google.com>
This commit is contained in:
Rinil Kunhiraman
2025-09-05 20:15:40 -07:00
committed by GitHub
parent a027010097
commit d2b5b4f129
4 changed files with 231 additions and 509 deletions
+30 -21
View File
@@ -7,7 +7,7 @@
import type React from 'react'; import type React from 'react';
import { Box, Text } from 'ink'; import { Box, Text } from 'ink';
import { Colors } from '../colors.js'; import { Colors } from '../colors.js';
import type { SlashCommand } from '../commands/types.js'; import { type SlashCommand, CommandKind } from '../commands/types.js';
interface Help { interface Help {
commands: readonly SlashCommand[]; commands: readonly SlashCommand[];
@@ -64,27 +64,32 @@ export const Help: React.FC<Help> = ({ commands }) => (
<Text bold color={Colors.Foreground}> <Text bold color={Colors.Foreground}>
Commands: Commands:
</Text> </Text>
{commands.map((command: SlashCommand) => ( {commands
<Box key={command.name} flexDirection="column"> .filter((command) => command.description)
<Text color={Colors.Foreground}> .map((command: SlashCommand) => (
<Text bold color={Colors.AccentPurple}> <Box key={command.name} flexDirection="column">
{' '} <Text color={Colors.Foreground}>
/{command.name} <Text bold color={Colors.AccentPurple}>
</Text> {' '}
{command.description && ' - ' + command.description} /{command.name}
</Text>
{command.subCommands &&
command.subCommands.map((subCommand) => (
<Text key={subCommand.name} color={Colors.Foreground}>
<Text bold color={Colors.AccentPurple}>
{' '}
{subCommand.name}
</Text>
{subCommand.description && ' - ' + subCommand.description}
</Text> </Text>
))} {command.kind === CommandKind.MCP_PROMPT && (
</Box> <Text color={Colors.Gray}> [MCP]</Text>
))} )}
{command.description && ' - ' + command.description}
</Text>
{command.subCommands &&
command.subCommands.map((subCommand) => (
<Text key={subCommand.name} color={Colors.Foreground}>
<Text bold color={Colors.AccentPurple}>
{' '}
{subCommand.name}
</Text>
{subCommand.description && ' - ' + subCommand.description}
</Text>
))}
</Box>
))}
<Text color={Colors.Foreground}> <Text color={Colors.Foreground}>
<Text bold color={Colors.AccentPurple}> <Text bold color={Colors.AccentPurple}>
{' '} {' '}
@@ -92,6 +97,10 @@ export const Help: React.FC<Help> = ({ commands }) => (
</Text> </Text>
- shell command - shell command
</Text> </Text>
<Text color={Colors.Foreground}>
<Text color={Colors.Gray}>[MCP]</Text> - Model Context Protocol command
(from external servers)
</Text>
<Box height={1} /> <Box height={1} />
@@ -7,12 +7,13 @@
import { Box, Text } from 'ink'; import { Box, Text } from 'ink';
import { Colors } from '../colors.js'; import { Colors } from '../colors.js';
import { PrepareLabel } from './PrepareLabel.js'; import { PrepareLabel } from './PrepareLabel.js';
import { isSlashCommand } from '../utils/commandUtils.js'; import { CommandKind } from '../commands/types.js';
export interface Suggestion { export interface Suggestion {
label: string; label: string;
value: string; value: string;
description?: string; description?: string;
matchedIndex?: number; matchedIndex?: number;
commandKind?: CommandKind;
} }
interface SuggestionsDisplayProps { interface SuggestionsDisplayProps {
suggestions: Suggestion[]; suggestions: Suggestion[];
@@ -53,21 +54,6 @@ export function SuggestionsDisplay({
); );
const visibleSuggestions = suggestions.slice(startIndex, endIndex); const visibleSuggestions = suggestions.slice(startIndex, endIndex);
const isSlashCommandMode = isSlashCommand(userInput);
let commandNameWidth = 0;
if (isSlashCommandMode) {
const maxLabelLength = visibleSuggestions.length
? Math.max(...visibleSuggestions.map((s) => s.label.length))
: 0;
const maxAllowedWidth = Math.floor(width * 0.35);
commandNameWidth = Math.max(
15,
Math.min(maxLabelLength + 2, maxAllowedWidth),
);
}
return ( return (
<Box flexDirection="column" paddingX={1} width={width}> <Box flexDirection="column" paddingX={1} width={width}>
{scrollOffset > 0 && <Text color={Colors.Foreground}></Text>} {scrollOffset > 0 && <Text color={Colors.Foreground}></Text>}
@@ -88,31 +74,33 @@ export function SuggestionsDisplay({
return ( return (
<Box key={`${suggestion.value}-${originalIndex}`} width={width}> <Box key={`${suggestion.value}-${originalIndex}`} width={width}>
<Box flexDirection="row"> <Box flexDirection="row">
{isSlashCommandMode ? ( {(() => {
<> const isSlashCommand = userInput.startsWith('/');
<Box width={commandNameWidth} flexShrink={0}> return (
{labelElement} <>
</Box> {isSlashCommand ? (
{suggestion.description ? ( <Box flexShrink={0} paddingRight={2}>
<Box flexGrow={1} marginLeft={1}> {labelElement}
<Text color={textColor} wrap="wrap"> {suggestion.commandKind === CommandKind.MCP_PROMPT && (
{suggestion.description} <Text color={Colors.Gray}> [MCP]</Text>
</Text> )}
</Box> </Box>
) : null} ) : (
</> labelElement
) : ( )}
<> {suggestion.description && (
{labelElement} <Box
{suggestion.description ? ( flexGrow={1}
<Box flexGrow={1} marginLeft={1}> paddingLeft={isSlashCommand ? undefined : 1}
<Text color={textColor} wrap="wrap"> >
{suggestion.description} <Text color={textColor} wrap="truncate">
</Text> {suggestion.description}
</Box> </Text>
) : null} </Box>
</> )}
)} </>
);
})()}
</Box> </Box>
</Box> </Box>
); );
@@ -226,7 +226,12 @@ describe('useSlashCompletion', () => {
await waitFor(() => { await waitFor(() => {
expect(result.current.suggestions).toEqual([ expect(result.current.suggestions).toEqual([
{ label: 'memory', value: 'memory', description: 'Manage memory' }, {
label: 'memory',
value: 'memory',
description: 'Manage memory',
commandKind: CommandKind.BUILT_IN,
},
]); ]);
}); });
}); });
@@ -254,6 +259,7 @@ describe('useSlashCompletion', () => {
label: 'stats', label: 'stats',
value: 'stats', value: 'stats',
description: 'check session stats. Usage: /stats [model|tools]', description: 'check session stats. Usage: /stats [model|tools]',
commandKind: CommandKind.BUILT_IN,
}, },
]); ]);
}); });
@@ -368,8 +374,18 @@ describe('useSlashCompletion', () => {
expect(result.current.suggestions).toHaveLength(2); expect(result.current.suggestions).toHaveLength(2);
expect(result.current.suggestions).toEqual( expect(result.current.suggestions).toEqual(
expect.arrayContaining([ expect.arrayContaining([
{ label: 'show', value: 'show', description: 'Show memory' }, {
{ label: 'add', value: 'add', description: 'Add to memory' }, label: 'show',
value: 'show',
description: 'Show memory',
commandKind: CommandKind.BUILT_IN,
},
{
label: 'add',
value: 'add',
description: 'Add to memory',
commandKind: CommandKind.BUILT_IN,
},
]), ]),
); );
}); });
@@ -397,8 +413,18 @@ describe('useSlashCompletion', () => {
expect(result.current.suggestions).toHaveLength(2); expect(result.current.suggestions).toHaveLength(2);
expect(result.current.suggestions).toEqual( expect(result.current.suggestions).toEqual(
expect.arrayContaining([ expect.arrayContaining([
{ label: 'show', value: 'show', description: 'Show memory' }, {
{ label: 'add', value: 'add', description: 'Add to memory' }, label: 'show',
value: 'show',
description: 'Show memory',
commandKind: CommandKind.BUILT_IN,
},
{
label: 'add',
value: 'add',
description: 'Add to memory',
commandKind: CommandKind.BUILT_IN,
},
]), ]),
); );
}); });
@@ -425,7 +451,12 @@ describe('useSlashCompletion', () => {
await waitFor(() => { await waitFor(() => {
expect(result.current.suggestions).toEqual([ expect(result.current.suggestions).toEqual([
{ label: 'add', value: 'add', description: 'Add to memory' }, {
label: 'add',
value: 'add',
description: 'Add to memory',
commandKind: CommandKind.BUILT_IN,
},
]); ]);
}); });
}); });
@@ -574,473 +605,166 @@ describe('useSlashCompletion', () => {
}); });
}); });
describe('Fuzzy Matching', () => { describe('Command Kind Information', () => {
const fuzzyTestCommands = [ it('should include commandKind for MCP commands in suggestions', async () => {
createTestCommand({ const slashCommands = [
name: 'help', {
altNames: ['?'], name: 'summarize',
description: 'Show help', description: 'Summarize content',
}), kind: CommandKind.MCP_PROMPT,
createTestCommand({ action: vi.fn(),
name: 'history', },
description: 'Show command history', {
}), name: 'help',
createTestCommand({ name: 'hello', description: 'Hello world command' }), description: 'Show help',
createTestCommand({ kind: CommandKind.BUILT_IN,
name: 'config', action: vi.fn(),
altNames: ['configure'], },
description: 'Configure settings', ] as SlashCommand[];
}),
createTestCommand({ name: 'clear', description: 'Clear the screen' }),
];
it('should match commands with fuzzy search for partial queries', async () => {
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/he',
fuzzyTestCommands,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toEqual(expect.arrayContaining(['help', 'hello']));
});
it('should handle case-insensitive fuzzy matching', async () => {
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/HeLp',
fuzzyTestCommands,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toContain('help');
});
it('should provide typo-tolerant matching', async () => {
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/hlp',
fuzzyTestCommands,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toContain('help');
});
it('should match against alternative names with fuzzy search', async () => {
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/conf',
fuzzyTestCommands,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toContain('config');
});
it('should fallback to prefix matching when AsyncFzf find fails', async () => {
// Mock console.error to avoid noise in test output
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Import the mocked AsyncFzf
const { AsyncFzf } = await import('fzf');
// Create a failing find method for this specific test
const mockFind = vi
.fn()
.mockRejectedValue(new Error('AsyncFzf find failed'));
// Mock AsyncFzf to return an instance with failing find
vi.mocked(AsyncFzf).mockImplementation(
(_items, _options) =>
({
finder: vi.fn(),
find: mockFind,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any,
);
const testCommands = [
createTestCommand({ name: 'clear', description: 'Clear the screen' }),
createTestCommand({
name: 'config',
description: 'Configure settings',
}),
createTestCommand({ name: 'chat', description: 'Start chat' }),
];
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/cle',
testCommands,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
// Should still get suggestions via prefix matching fallback
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toContain('clear');
expect(labels).not.toContain('config'); // Doesn't start with 'cle'
expect(labels).not.toContain('chat'); // Doesn't start with 'cle'
// Verify the error was logged
await waitFor(() => {
expect(consoleErrorSpy).toHaveBeenCalledWith(
'[Fuzzy search - falling back to prefix matching]',
expect.any(Error),
);
});
consoleErrorSpy.mockRestore();
// Reset AsyncFzf mock to default behavior for other tests
vi.mocked(AsyncFzf).mockImplementation(createDefaultAsyncFzfMock());
});
it('should show all commands for empty partial query', async () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useTestHarnessForSlashCompletion( useTestHarnessForSlashCompletion(
true, true,
'/', '/',
fuzzyTestCommands,
mockCommandContext,
),
);
expect(result.current.suggestions.length).toBe(fuzzyTestCommands.length);
});
it('should handle AsyncFzf errors gracefully and fallback to prefix matching', async () => {
// Mock console.error to avoid noise in test output
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Import the mocked AsyncFzf
const { AsyncFzf } = await import('fzf');
// Create a failing find method for this specific test
const mockFind = vi
.fn()
.mockRejectedValue(new Error('AsyncFzf error in find'));
// Mock AsyncFzf to return an instance with failing find
vi.mocked(AsyncFzf).mockImplementation(
(_items, _options) =>
({
finder: vi.fn(),
find: mockFind,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any,
);
const testCommands = [
{ name: 'test', description: 'Test command' },
{ name: 'temp', description: 'Temporary command' },
] as unknown as SlashCommand[];
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/te',
testCommands,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
// Should get suggestions via prefix matching fallback
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toEqual(expect.arrayContaining(['test', 'temp']));
// Verify the error was logged
await waitFor(() => {
expect(consoleErrorSpy).toHaveBeenCalledWith(
'[Fuzzy search - falling back to prefix matching]',
expect.any(Error),
);
});
consoleErrorSpy.mockRestore();
// Reset AsyncFzf mock to default behavior for other tests
vi.mocked(AsyncFzf).mockImplementation(createDefaultAsyncFzfMock());
});
it('should cache AsyncFzf instances for performance', async () => {
// Reset constructor call count and ensure mock is set up correctly
resetConstructorCallCount();
// Import the mocked AsyncFzf
const { AsyncFzf } = await import('fzf');
vi.mocked(AsyncFzf).mockImplementation(createDefaultAsyncFzfMock());
const { result, rerender } = renderHook(
({ query }) =>
useTestHarnessForSlashCompletion(
true,
query,
fuzzyTestCommands,
mockCommandContext,
),
{ initialProps: { query: '/he' } },
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const firstResults = result.current.suggestions.map((s) => s.label);
const callCountAfterFirst = getConstructorCallCount();
expect(callCountAfterFirst).toBeGreaterThan(0);
// Rerender with same query - should use cached instance
rerender({ query: '/he' });
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const secondResults = result.current.suggestions.map((s) => s.label);
const callCountAfterSecond = getConstructorCallCount();
// Should have same number of constructor calls (reused cached instance)
expect(callCountAfterSecond).toBe(callCountAfterFirst);
expect(secondResults).toEqual(firstResults);
// Different query should still use same cached instance for same command set
rerender({ query: '/hel' });
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const thirdCallCount = getConstructorCallCount();
expect(thirdCallCount).toBe(callCountAfterFirst); // Same constructor call count
});
it('should not return duplicate suggestions when query matches both name and altNames', async () => {
const commandsWithAltNames = [
createTestCommand({
name: 'config',
altNames: ['configure', 'conf'],
description: 'Configure settings',
}),
createTestCommand({
name: 'help',
altNames: ['?'],
description: 'Show help',
}),
];
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/con',
commandsWithAltNames,
mockCommandContext,
),
);
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
const labels = result.current.suggestions.map((s) => s.label);
const uniqueLabels = new Set(labels);
// Should not have duplicates
expect(labels.length).toBe(uniqueLabels.size);
expect(labels).toContain('config');
});
});
describe('Race Condition Handling', () => {
it('should handle rapid input changes without race conditions', async () => {
const mockDelayedCompletion = vi
.fn()
.mockImplementation(
async (_context: CommandContext, partialArg: string) => {
// Simulate network delay with different delays for different inputs
const delay = partialArg.includes('slow') ? 200 : 50;
await new Promise((resolve) => setTimeout(resolve, delay));
return [`suggestion-for-${partialArg}`];
},
);
const slashCommands = [
createTestCommand({
name: 'test',
description: 'Test command',
completion: mockDelayedCompletion,
}),
];
const { result, rerender } = renderHook(
({ query }) =>
useTestHarnessForSlashCompletion(
true,
query,
slashCommands,
mockCommandContext,
),
{ initialProps: { query: '/test slowquery' } },
);
// Quickly change to a faster query
rerender({ query: '/test fastquery' });
await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0);
});
// Should show suggestions for the latest query only
const labels = result.current.suggestions.map((s) => s.label);
expect(labels).toContain('suggestion-for-fastquery');
expect(labels).not.toContain('suggestion-for-slowquery');
});
it('should not update suggestions if component unmounts during async operation', async () => {
let resolveCompletion: (value: string[]) => void;
const mockCompletion = vi.fn().mockImplementation(
async () =>
new Promise<string[]>((resolve) => {
resolveCompletion = resolve;
}),
);
const slashCommands = [
createTestCommand({
name: 'test',
description: 'Test command',
completion: mockCompletion,
}),
];
const { unmount } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/test query',
slashCommands, slashCommands,
mockCommandContext, mockCommandContext,
), ),
); );
// Start the async operation expect(result.current.suggestions).toEqual(
await waitFor(() => { expect.arrayContaining([
expect(mockCompletion).toHaveBeenCalled(); {
}); label: 'summarize',
value: 'summarize',
// Unmount before completion resolves description: 'Summarize content',
unmount(); commandKind: CommandKind.MCP_PROMPT,
},
// Now resolve the completion {
resolveCompletion!(['late-suggestion']); label: 'help',
value: 'help',
// Wait a bit to ensure any pending updates would have been processed description: 'Show help',
await new Promise((resolve) => setTimeout(resolve, 100)); commandKind: CommandKind.BUILT_IN,
},
// Since the component is unmounted, suggestions should remain empty ]),
// and no state update errors should occur );
expect(true).toBe(true); // Test passes if no errors are thrown
}); });
});
describe('Error Logging', () => { it('should include commandKind when filtering MCP commands by prefix', async () => {
it('should log errors to the console', async () => { const slashCommands = [
// Mock console.error to capture log calls {
const consoleErrorSpy = vi name: 'summarize',
.spyOn(console, 'error') description: 'Summarize content',
.mockImplementation(() => {}); kind: CommandKind.MCP_PROMPT,
action: vi.fn(),
// Import the mocked AsyncFzf },
const { AsyncFzf } = await import('fzf'); {
name: 'settings',
// Create a failing find method with error containing sensitive-looking data description: 'Open settings',
const sensitiveError = new Error( kind: CommandKind.BUILT_IN,
'Database connection failed: user=admin, pass=secret123', action: vi.fn(),
); },
const mockFind = vi.fn().mockRejectedValue(sensitiveError); ] as SlashCommand[];
// Mock AsyncFzf to return an instance with failing find
vi.mocked(AsyncFzf).mockImplementation(
(_items, _options) =>
({
find: mockFind,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
}) as any,
);
const testCommands = [
createTestCommand({ name: 'test', description: 'Test command' }),
];
const { result } = renderHook(() => const { result } = renderHook(() =>
useTestHarnessForSlashCompletion( useTestHarnessForSlashCompletion(
true, true,
'/test', '/summ',
testCommands, slashCommands,
mockCommandContext, mockCommandContext,
), ),
); );
await waitFor(() => { await waitFor(() => {
expect(result.current.suggestions.length).toBeGreaterThan(0); expect(result.current.suggestions).toEqual([
{
label: 'summarize',
value: 'summarize',
description: 'Summarize content',
commandKind: CommandKind.MCP_PROMPT,
},
]);
}); });
});
// Should get fallback suggestions it('should include commandKind for sub-commands', async () => {
const labels = result.current.suggestions.map((s) => s.label); const slashCommands = [
expect(labels).toContain('test'); {
name: 'memory',
description: 'Manage memory',
kind: CommandKind.BUILT_IN,
subCommands: [
{
name: 'show',
description: 'Show memory',
kind: CommandKind.BUILT_IN,
action: vi.fn(),
},
{
name: 'add',
description: 'Add to memory',
kind: CommandKind.MCP_PROMPT,
action: vi.fn(),
},
],
},
] as SlashCommand[];
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/memory',
slashCommands,
mockCommandContext,
),
);
expect(result.current.suggestions).toEqual(
expect.arrayContaining([
{
label: 'show',
value: 'show',
description: 'Show memory',
commandKind: CommandKind.BUILT_IN,
},
{
label: 'add',
value: 'add',
description: 'Add to memory',
commandKind: CommandKind.MCP_PROMPT,
},
]),
);
});
it('should include commandKind for file commands', async () => {
const slashCommands = [
{
name: 'custom-script',
description: 'Run custom script',
kind: CommandKind.FILE,
action: vi.fn(),
},
] as SlashCommand[];
const { result } = renderHook(() =>
useTestHarnessForSlashCompletion(
true,
'/custom',
slashCommands,
mockCommandContext,
),
);
// Verify error logging occurred
await waitFor(() => { await waitFor(() => {
expect(consoleErrorSpy).toHaveBeenCalledWith( expect(result.current.suggestions).toEqual([
'[Fuzzy search - falling back to prefix matching]', {
sensitiveError, label: 'custom-script',
); value: 'custom-script',
description: 'Run custom script',
commandKind: CommandKind.FILE,
},
]);
}); });
consoleErrorSpy.mockRestore();
// Reset AsyncFzf mock to default behavior
vi.mocked(AsyncFzf).mockImplementation(createDefaultAsyncFzfMock());
}); });
}); });
}); });
@@ -267,6 +267,7 @@ function useCommandSuggestions(
label: cmd.name, label: cmd.name,
value: cmd.name, value: cmd.name,
description: cmd.description, description: cmd.description,
commandKind: cmd.kind,
})); }));
setSuggestions(finalSuggestions); setSuggestions(finalSuggestions);