Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
170 changes: 168 additions & 2 deletions packages/cli/src/ui/hooks/useCommandCompletion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,12 @@ describe('useCommandCompletion', () => {
kind: CommandKind.BUILT_IN,
modelInvocable: false,
};
const fileCommand: SlashCommand = {
name: 'store-notes',
description: 'Store notes',
kind: CommandKind.FILE,
modelInvocable: true,
};

setupMocks({
slashSuggestions: [
Expand All @@ -529,7 +535,7 @@ describe('useCommandCompletion', () => {
useCommandCompletion(
useTextBufferForTest('please /store'),
testRootDir,
[skillCommand, builtInCommand],
[skillCommand, builtInCommand, fileCommand],
mockCommandContext,
false,
mockConfig,
Expand All @@ -544,11 +550,171 @@ describe('useCommandCompletion', () => {
expect.objectContaining({
enabled: true,
query: '/store',
slashCommands: [skillCommand],
slashCommands: [skillCommand, fileCommand],
}),
);
});

it.each([
{
input: '/review /sto',
expected: '/review /front-end-store-rules ',
},
{
input: '/review\n/sto',
expected: '/review\n/front-end-store-rules ',
},
])(
'should complete a repeated skill in $input',
async ({ input, expected }) => {
const firstSkill: SlashCommand = {
name: 'review',
description: 'Review changes',
kind: CommandKind.SKILL,
modelInvocable: true,
};
const secondSkill: SlashCommand = {
name: 'front-end-store-rules',
description: 'Store rules',
kind: CommandKind.SKILL,
modelInvocable: true,
};
const userOnlySkill: SlashCommand = {
name: 'store-locally',
description: 'Store locally',
kind: CommandKind.SKILL,
modelInvocable: false,
};
const fileCommand: SlashCommand = {
name: 'store-notes',
description: 'Store notes',
kind: CommandKind.FILE,
modelInvocable: true,
};

setupMocks({
slashSuggestions: [
{ label: 'front-end-store-rules', value: 'front-end-store-rules' },
],
slashCompletionRange: { completionStart: 1, completionEnd: 4 },
});

const { result } = renderHook(() => {
const textBuffer = useTextBufferForTest(input);
const completion = useCommandCompletion(
textBuffer,
testRootDir,
[firstSkill, secondSkill, userOnlySkill, fileCommand],
mockCommandContext,
false,
mockConfig,
);
return { ...completion, textBuffer };
});

await waitFor(() => {
expect(result.current.showSuggestions).toBe(true);
});

expect(useSlashCompletion).toHaveBeenLastCalledWith(
expect.objectContaining({
enabled: true,
query: '/sto',
slashCommands: [firstSkill, secondSkill, userOnlySkill],
}),
);

act(() => {
result.current.handleAutocomplete(0);
});

expect(result.current.textBuffer.text).toBe(expected);
},
);

it.each(['/stats /sto', '/unknown /sto', '/unknown\n/sto'])(
'should not treat an invalid stacked prefix as mid-input completion: %s',
async (input) => {
const skillCommand: SlashCommand = {
name: 'store-rules',
description: 'Store rules',
kind: CommandKind.SKILL,
modelInvocable: true,
};
const builtInCommand: SlashCommand = {
name: 'stats',
description: 'Show stats',
kind: CommandKind.BUILT_IN,
modelInvocable: false,
};

const { result } = renderHook(() =>
useCommandCompletion(
useTextBufferForTest(input),
testRootDir,
[skillCommand, builtInCommand],
mockCommandContext,
false,
mockConfig,
),
);

expect(result.current.midInputGhostText).toBeNull();

await waitFor(() => {
if (input.includes('\n')) {
expect(useSlashCompletion).toHaveBeenLastCalledWith(
expect.objectContaining({ enabled: false }),
);
} else {
expect(useSlashCompletion).toHaveBeenLastCalledWith(
expect.objectContaining({
enabled: true,
query: input,
slashCommands: [skillCommand, builtInCommand],
}),
);
}
});
},
);

it('should keep an indented first command in line-start completion', async () => {
const skillCommand: SlashCommand = {
name: 'front-end-store-rules',
description: 'Store rules',
kind: CommandKind.SKILL,
modelInvocable: true,
};
const builtInCommand: SlashCommand = {
name: 'stats',
description: 'Show stats',
kind: CommandKind.BUILT_IN,
modelInvocable: false,
};

renderHook(() =>
useCommandCompletion(
useTextBufferForTest(' /sto'),
testRootDir,
[skillCommand, builtInCommand],
mockCommandContext,
false,
mockConfig,
),
);

await waitFor(() => {
expect(useSlashCompletion).toHaveBeenLastCalledWith(
expect.objectContaining({
enabled: true,
query: ' /sto',
slashCommands: [skillCommand, builtInCommand],
}),
);
});
});

it('should complete a file path when @ appears after a slash command', async () => {
setupMocks({
atSuggestions: [{ label: 'src/index.ts', value: 'src/index.ts' }],
Expand Down
108 changes: 77 additions & 31 deletions packages/cli/src/ui/hooks/useCommandCompletion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import type {
Suggestion,
SuggestionCategory,
} from '../components/SuggestionsDisplay.js';
import type { CommandContext, SlashCommand } from '../commands/types.js';
import {
CommandKind,
type CommandContext,
type SlashCommand,
} from '../commands/types.js';
import type { TextBuffer } from '../components/shared/text-buffer.js';
import { logicalPosToOffset } from '../components/shared/text-buffer.js';
import {
Expand All @@ -26,14 +30,23 @@ import {
} from './useSlashCompletion.js';
import type { Config } from '@qwen-code/qwen-code-core';
import { useCompletion } from './useCompletion.js';
import { parseSlashCommand } from '../../utils/commands.js';
import {
isValidStackedSkillPrefix,
parseSlashCommand,
} from '../../utils/commands.js';

export enum CompletionMode {
IDLE = 'IDLE',
AT = 'AT',
SLASH = 'SLASH',
}

enum SlashCompletionContext {
LINE_START = 'line-start',
MID_INPUT = 'mid-input',
STACKED_SKILL = 'stacked-skill',
}

function isExactMidInputModelInvocableCommand(
partialCommand: string,
slashCommands: readonly SlashCommand[],
Expand All @@ -48,6 +61,12 @@ function isExactMidInputModelInvocableCommand(
);
}

function isStackedSkillCompletableCommand(cmd: SlashCommand): boolean {
return (
cmd.kind === CommandKind.SKILL && cmd.userInvocable !== false && !cmd.hidden
);
}

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.

[Suggestion] Duplicated predicate — the three-condition check here (kind === SKILL && userInvocable !== false && !hidden) is duplicated inline inside isValidStackedSkillPrefix at commands.ts:46-50. Both answer the same question but live in different modules.

Failure scenario: if a maintainer adds a fourth condition (e.g. cmd.deprecated) to one site without discovering the other, the dropdown would offer commands the prefix validator rejects — silent no-op completions.

Concrete fix: export this predicate from commandUtils.ts (alongside the existing isMidInputCompletableCommand), then import it in both useCommandCompletion.tsx and commands.ts.

Suggested change
function isStackedSkillCompletableCommand(cmd: SlashCommand): boolean {
return (
cmd.kind === CommandKind.SKILL && cmd.userInvocable !== false && !cmd.hidden
);
}
function isStackedSkillCompletableCommand(cmd: SlashCommand): boolean {
return (
cmd.kind === CommandKind.SKILL &&
cmd.userInvocable !== false &&
!cmd.hidden
);
}
中文说明

[Suggestion] 重复的判断条件 —— 此处的三条件检查在 commands.ts:46-50isValidStackedSkillPrefix 中被内联重复。两者回答同一个问题,却分处不同模块。

失败场景:如果维护者向其中一个位置添加了第四个条件(例如 cmd.deprecated),而未发现另一个,则下拉菜单会提供前缀验证器会拒绝的命令 —— 产生静默的无效补全。

建议修复:将此谓函数导出到 commandUtils.ts(与现有的 isMidInputCompletableCommand 并列),然后在 useCommandCompletion.tsxcommands.ts 中导入它。

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e4855cf. I kept the shared predicate in src/utils/commands.ts so the lower-level command utility does not depend on ui/utils. Both call sites now use it.


export interface UseCommandCompletionReturn {
suggestions: Suggestion[];
activeSuggestionIndex: number;
Expand Down Expand Up @@ -97,7 +116,7 @@ export function useCommandCompletion(
query,
completionStart,
completionEnd,
isMidInputSlashCompletion,
slashCompletionContext,
} = useMemo(() => {
const currentLine = buffer.lines[cursorRow] || '';

Expand Down Expand Up @@ -137,59 +156,82 @@ export function useCommandCompletion(
query: partialPath,
completionStart: pathStart,
completionEnd: end,
isMidInputSlashCompletion: false,
slashCompletionContext: null,
};
}
}

if (cursorRow === 0 && isSlashCommand(currentLine.trim())) {
return {
completionMode: CompletionMode.SLASH,
query: currentLine,
completionStart: 0,
completionEnd: currentLine.length,
isMidInputSlashCompletion: false,
};
}

const cursorOffset = logicalPosToOffset(buffer.lines, cursorRow, cursorCol);
const midCmd = findMidInputSlashCommand(buffer.text, cursorOffset);
if (
midCmd &&
!isExactMidInputModelInvocableCommand(
midCmd.partialCommand,
slashCommands,
)
) {
if (midCmd) {
const lineStartOffset = logicalPosToOffset(buffer.lines, cursorRow, 0);
const startOnLine = midCmd.startPos - lineStartOffset;
if (startOnLine >= 0) {
const isInitialCommandOnFirstLine =
cursorRow === 0 &&
isSlashCommand(currentLine.trim()) &&
startOnLine >= 0 &&
codePoints.slice(0, startOnLine).join('').trim().length === 0;
const inputCodePoints = toCodePoints(buffer.text);
const prefix = inputCodePoints.slice(0, midCmd.startPos).join('');
const isStackedSkill =
!isInitialCommandOnFirstLine &&
isValidStackedSkillPrefix(prefix, slashCommands);
const isSlashLedInput = prefix.trimStart().startsWith('/');
const isRegularMidInput =
!isInitialCommandOnFirstLine && !isSlashLedInput;
if (
startOnLine >= 0 &&
(isStackedSkill ||
(isRegularMidInput &&
!isExactMidInputModelInvocableCommand(
midCmd.partialCommand,
slashCommands,
)))
) {
return {
completionMode: CompletionMode.SLASH,
query: midCmd.token,
completionStart: startOnLine,
completionEnd: startOnLine + midCmd.token.length,
isMidInputSlashCompletion: true,
slashCompletionContext: isStackedSkill
? SlashCompletionContext.STACKED_SKILL
: SlashCompletionContext.MID_INPUT,
};
}
}

if (cursorRow === 0 && isSlashCommand(currentLine.trim())) {
return {
completionMode: CompletionMode.SLASH,
query: currentLine,
completionStart: 0,
completionEnd: currentLine.length,
slashCompletionContext: SlashCompletionContext.LINE_START,
};
}

return {
completionMode: CompletionMode.IDLE,
query: null,
completionStart: -1,
completionEnd: -1,
isMidInputSlashCompletion: false,
slashCompletionContext: null,
};
}, [cursorRow, cursorCol, buffer.lines, buffer.text, slashCommands]);

const slashCommandsForCompletion = useMemo(
() =>
isMidInputSlashCompletion
? slashCommands.filter(isMidInputCompletableCommand)
: slashCommands,
[isMidInputSlashCompletion, slashCommands],
);
const isMidInputSlashCompletion =
slashCompletionContext === SlashCompletionContext.MID_INPUT ||
slashCompletionContext === SlashCompletionContext.STACKED_SKILL;

const slashCommandsForCompletion = useMemo(() => {
if (slashCompletionContext === SlashCompletionContext.STACKED_SKILL) {
return slashCommands.filter(isStackedSkillCompletableCommand);
}
if (slashCompletionContext === SlashCompletionContext.MID_INPUT) {
return slashCommands.filter(isMidInputCompletableCommand);
}
return slashCommands;
}, [slashCompletionContext, slashCommands]);

const {
suggestions,
Expand Down Expand Up @@ -342,6 +384,10 @@ export function useCommandCompletion(
const midCmd = findMidInputSlashCommand(buffer.text, cursorOffset);
if (midCmd) {
if (isMidInputSlashCompletion) return null;
const prefix = toCodePoints(buffer.text)
.slice(0, midCmd.startPos)
.join('');
if (prefix.trimStart().startsWith('/')) return null;
const match = getBestSlashCommandMatch(
midCmd.partialCommand,
slashCommands,
Expand Down
Loading