fix(cli): prevent file paths from being treated as slash commands - #3743
Conversation
…enLM#1804) When users input file paths starting with '/' (e.g. '/api/apiFunction/...', '/Users/name/path'), they were incorrectly parsed as slash commands, resulting in "Unknown command" errors. The input was discarded instead of being sent to the model for processing. Root cause: isSlashCommand() only checked for a '/' prefix without validating whether the first token actually looks like a command name. Any '/' prefix triggered the slash command flow, and when no matching command was found, the error was shown with no fallback. Fix: Add looksLikeCommandName() that validates command names contain only [a-zA-Z0-9:_-]. Both isSlashCommand() and handleSlashCommand() now check the first token — if it contains path separators, dots, or non-ASCII characters, the input falls through to normal model processing instead of the command dispatcher. Closes QwenLM#1804
Address review feedback: - Allow '.' in looksLikeCommandName() regex to support extension-qualified commands like gcp.deploy (CommandService renames conflicts as ext.cmd) - Add regression tests for dot-named commands in both commandUtils and slashCommandProcessor - Fix prettier formatting in slashCommandProcessor test file
Review follow-upAddressed review feedback in 776a2ab:
Test resultsTerminal capture (local)Recorded via
7 screenshots + animated GIF generated in |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Typecheck/build verification found two blocking issues that cannot be anchored to diff lines because GitHub does not expose those unchanged test-helper lines in the PR diff.
[Critical] packages/cli/src/ui/hooks/slashCommandProcessor.test.ts:157 — The setupProcessorHook helper passes an incomplete SlashCommandProcessorActions object to useSlashCommandProcessor. The interface now requires additional actions such as openManageModelsDialog, handleResume, openDeleteDialog, openExtensionsManagerDialog, openMcpDialog, openHooksDialog, and openRewindSelector, so TypeScript reports TS2345 and the PR fails typecheck/build verification. Add vi.fn() stubs for the missing required fields.
[Critical] packages/cli/src/ui/hooks/slashCommandProcessor.test.ts:1071 — The inline SlashCommandProcessorActions object in the lifecycle test has the same issue and omits the required action properties. Add the same missing stubs here as well, or reuse a typed helper so future interface additions only need to be updated in one place.
— gpt-5.5 via Qwen Code /review
Review follow-up validationI rechecked the review feedback rather than applying it mechanically. The final behavior keeps command-like Unicode names and aliases valid, and only falls back to model input for path-like values when they are not known loaded commands or aliases. Fresh local verification on Additional focused regression run: Full workspace test totals from I also reran the terminal-capture style smoke flow. The runner completed, but its automated slash-input submission is noisy for |
Re-review: PR #3743 — Prevent file paths from being treated as slash commandsOverviewCloses #1804. When users type Files: Code quality
Verification of prior review concerns
Suggestions
Risks
RecommendationApprove once the brittle hardcoded path-roots set is replaced (or at least clearly justified). The behavior is right for the cases users will actually hit; the implementation can just be simpler and more general. 中文版本重新审查：PR #3743 — 防止文件路径被识别为斜杠命令概述修复 #1804。当用户输入 涉及文件： 代码质量
对前次评审顾虑的核实
改进建议
风险
建议结论在硬编码路径根集合被替换（或至少给出明确说明）后可以批准。功能行为对用户实际遇到的场景是正确的，实现还可以更简洁、更通用。 — Claude Opus 4.7 via Claude Code /review |
|
Follow-up commit Fresh local verification:
Focused regression:
Full workspace test totals:
Build still reports the existing VSCode companion lint warnings and Browserslist notice; no build errors. |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [typecheck] packages/cli/src/ui/hooks/slashCommandProcessor.test.ts:157 and line 1113 — Mock objects are not assignable to SlashCommandProcessorActions. tsc --noEmit reports TS2345 at both locations. The mock factories need to be updated to match the current interface signature.
There was a problem hiding this comment.
Pull request overview
This PR fixes a CLI UX bug where slash-prefixed absolute file paths (e.g. /Users/name/path or /api/apiFunction/接口的实现) were incorrectly routed into slash-command handling and turned into Unknown command errors instead of being sent to the model as normal prompts.
Changes:
- Add
looksLikeCommandName()and use it to distinguish command-like/tokeninputs from path-like or metacharacter-prefixed inputs. - Update slash-command processing to fall back to normal prompt handling for unknown
/token ...argsinputs (while still giving loaded commands/aliases precedence). - Fix message queue draining to preserve original typed order by draining only the leading plain-text run (instead of filtering/reordering).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/ui/utils/commandUtils.ts | Adds looksLikeCommandName() and tightens isSlashCommand() to ignore path-like first tokens. |
| packages/cli/src/ui/utils/commandUtils.test.ts | Adds focused tests for command-name classification and regression coverage for issue #1804. |
| packages/cli/src/ui/hooks/useMessageQueue.ts | Changes draining logic to stop at the first slash command, preserving queue order. |
| packages/cli/src/ui/hooks/useMessageQueue.test.ts | Updates/extends tests to cover preserved ordering and slash-like prompt behavior. |
| packages/cli/src/ui/hooks/slashCommandProcessor.ts | Adds early fall-through for path-like tokens and unknown /token ...args prompts. |
| packages/cli/src/ui/hooks/slashCommandProcessor.test.ts | Adds regression tests for bare /, alias matching, Unicode commands, and path-like fall-through. |
| packages/cli/src/ui/AppContainer.tsx | Updates comment to match the new “drain leading plain prompts” queue behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [typecheck] tsc --noEmit 报告两处 TS2345 错误（行 157 和 1137）：mock 对象不可赋值给 SlashCommandProcessorActions。Mock factory 缺少接口要求的某些属性，阻塞 typecheck。建议补全 mock 对象中缺失的属性或使用 vi.fn() 打桩。
— glm-5.1 via Qwen Code /review
|
@wenshao You were right - the regular package typecheck excludes Verified:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [review] isRealUserTurn 在 packages/cli/src/ui/utils/historyMapping.ts:21 误判新路径产生的斜杠前缀用户轮次，导致 rewind 截断索引错误。
此 PR 使 /data foo 等输入通过 isSlashCommand() → handleSlashCommand() 返回 false → 作为常规提示发送给模型，创建文本以 / 开头的 USER 历史条目。但 isRealUserTurn 假定所有 / 前缀文本都是未到达模型的斜杠命令，返回 false。这导致 computeApiTruncationIndex 的 UI 用户轮次计数比实际少，rewind 索引指向错误位置。
复现： 输入 hello → /data foo → world → /rewind 选择 world → /data foo 交换在 UI 中可见但 API 历史已被截断，后续模型对话在缺失上下文的情况下继续。
建议修复： 在历史条目中添加 sentToModel 标志（在 useGeminiStream 的 fallthrough 路径设置），并更新 isRealUserTurn 检查该标志。
[Suggestion] [review] drainQueue+popNextSegment 的 drain→pop→drain 组合循环（AppContainer 实际使用的模式）未在 useMessageQueue.test.ts 中测试。建议添加覆盖完整 drain→pop→drain 序列的测试。
|
@wenshao Follow-up pushed in This fixes the rewind/history mapping issue by recording explicit model-turn provenance instead of inferring it only from the prompt prefix:
I also handled the smaller review items around the Design note: I kept single-token unknown commands as Verified locally:
CI has started on |
- BerriAI/litellm#27059 (Grok 4.20 azure_ai metadata) merge-after-nits - QwenLM/qwen-code#3743 (path-vs-slash-command classifier) merge-after-nits - QwenLM/qwen-code#3767 (capture actual wire request in OpenAI logger) merge-after-nits - google-gemini/gemini-cli#26306 (bound retry fallback to prevent infinite loop) merge-after-nits - google-gemini/gemini-cli#26305 (/mcp remove slash command) merge-after-nits
|
@wenshao Following the latest review discussion, I split the scope:
So for this PR, I’d like to keep the review focused on the small #1804 behavior. The broader design questions can continue in #3826 without blocking this targeted fix. |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [review] isRealUserTurn in packages/cli/src/ui/utils/historyMapping.ts:18 still uses startsWith('/') to exclude slash commands from rewind. After this PR, file-path inputs like /api/apiFunction/接口的实现 are sent to the model but isRealUserTurn still returns false for them. This causes rewind selector to skip file-path prompts (RewindSelector.tsx:29), API truncation index to miscount (historyMapping.ts:95), and rewind target turn index to be wrong (AppContainer.tsx:1718).
Fix: replace startsWith('/') with isSlashCommand().
export function isRealUserTurn(item: HistoryItem): boolean {
if (item.type !== 'user' || !item.text) return false;
return !isSlashCommand(item.text) && !item.text.startsWith('?');
}— deepseek-v4-pro via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Verification:
- Typecheck: PASS
- ESLint: PASS (0 errors on changed files)
- Tests: 90/90 PASS
- 10-agent review: no Critical or high-confidence findings
The isRealUserTurn fix from the previous review is correctly applied, and the scope is appropriately narrowed to the #1804 path-separator fix.
— deepseek-v4-pro via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Fixes #1804: prompts like /api/apiFunction/接口的实现 or /Users/me/path were being routed to the slash-command handler and rejected as Unknown command. The fix adds a path-detector that classifies inputs whose first whitespace-delimited token contains another / or \ as regular prompts. Out-of-scope items are sensibly documented.
What's good
- Single source of truth.
hasSlashCommandPathSeparator()is reused byisSlashCommand(), the inner check inhandleSlashCommand, and (transitively) byhistoryMapping.isRealUserTurn. No drift between detectors. - Correct rewind/history behavior. Switching
isRealUserTurnfromstartsWith('/')toisSlashCommandis the right call — a path-like prompt was actually sent to the model, so it must count as a real user turn forcomputeApiTruncationIndex. The newhistoryMapping.test.tscase verifies this; without it, rewinds would land on the wrong API turn. - Test mock fix is genuinely needed.
SlashCommandProcessorActionshad grown several fields (openArenaDialog,openManageModelsDialog,handleResume,openDeleteDialog,openExtensionsManagerDialog,openMcpDialog,openHooksDialog,openRewindSelector) that the test mock didn't supply. Hoisting the mock into acreateMockActions()factory and exporting the interface tightens types and removes duplication. - Edge-case coverage. Tests cover Unicode, Windows backslash (
/tmp\\test.txt), absolute paths, hidden config paths, and a path-with-trailing-args (/var/log/syslog check this)./,/help, and/?still round-trip as slash commands. - Positive side effect on the queue.
useMessageQueue.drainQueue()partitions onisSlashCommand. Pre-fix, a queued/api/foowould be retained as a "slash command" and never drained — a latent bug. Post-fix, it drains alongside other prompts. Worth calling out in the PR description even though it's listed as out of scope.
Other notes
- Trailing-slash single segments.
/foo/(first token ='foo/') is now classified as a path, but/foois not, so they take different routes. Consistent with the documented out-of-scope list, but worth flagging in the follow-up issue for the broader/tokenrouting question.
Risk assessment
- No regression for real commands. Built-in commands and aliases use simple identifiers; none contain
/or\in the first token.parseSlashCommanditself splits on whitespace, not slashes (commands.ts:29), so subcommand routing is unaffected. - Args containing paths still work.
/help foo/bar→ first token is'help', classified as slash command. - Performance. One short regex test per
isSlashCommandcall (called on each completion keystroke). Negligible.
Verdict
LGTM with optional cleanup — the fix is correct, well-scoped, and the tests cover the right surface. None of the inline suggestions are blocking; the biggest is the dead defensive check in handleSlashCommand — either annotate or remove it before merge so it doesn't bit-rot.
| if (!trimmed.startsWith('/') && !trimmed.startsWith('?')) { | ||
| return false; | ||
| } | ||
| if (trimmed.startsWith('/') && hasSlashCommandPathSeparator(trimmed)) { |
There was a problem hiding this comment.
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:
- Drop the check and rely on the caller-side gate.
- 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.
| query.slice(1).trimStart().split(/\s+/)[0] ?? ''; | ||
|
|
||
| export const hasSlashCommandPathSeparator = (query: string): boolean => | ||
| SLASH_PATH_SEPARATOR_RE.test(getSlashCommandFirstToken(query)); |
There was a problem hiding this comment.
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") onhasSlashCommandPathSeparator, or - Have
hasSlashCommandPathSeparatorearly-returnfalseif!query.startsWith('/'), removing the precondition entirely.
| const SLASH_PATH_SEPARATOR_RE = /[/\\]/; | ||
|
|
||
| const getSlashCommandFirstToken = (query: string): string => | ||
| query.slice(1).trimStart().split(/\s+/)[0] ?? ''; |
There was a problem hiding this comment.
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 [].
) * fix(cli): prevent file paths from being treated as slash commands (#1804) When users input file paths starting with '/' (e.g. '/api/apiFunction/...', '/Users/name/path'), they were incorrectly parsed as slash commands, resulting in "Unknown command" errors. The input was discarded instead of being sent to the model for processing. Root cause: isSlashCommand() only checked for a '/' prefix without validating whether the first token actually looks like a command name. Any '/' prefix triggered the slash command flow, and when no matching command was found, the error was shown with no fallback. Fix: Add looksLikeCommandName() that validates command names contain only [a-zA-Z0-9:_-]. Both isSlashCommand() and handleSlashCommand() now check the first token — if it contains path separators, dots, or non-ASCII characters, the input falls through to normal model processing instead of the command dispatcher. Closes #1804 * fix(cli): allow dots in command names and fix prettier formatting Address review feedback: - Allow '.' in looksLikeCommandName() regex to support extension-qualified commands like gcp.deploy (CommandService renames conflicts as ext.cmd) - Add regression tests for dot-named commands in both commandUtils and slashCommandProcessor - Fix prettier formatting in slashCommandProcessor test file * fix(cli): handle slash command review edge cases * docs(cli): align slash command validation comment * fix(cli): preserve slash prompt ordering * fix(cli): reject shell-metacharacter slash tokens * test(cli): align slash command action mocks * fix(cli): track model-sent user turns * fix(cli): narrow slash path handling scope * fix(cli): count slash path prompts in history * test(cli): type slash command action mocks
…enLM#3743) * fix(cli): prevent file paths from being treated as slash commands (QwenLM#1804) When users input file paths starting with '/' (e.g. '/api/apiFunction/...', '/Users/name/path'), they were incorrectly parsed as slash commands, resulting in "Unknown command" errors. The input was discarded instead of being sent to the model for processing. Root cause: isSlashCommand() only checked for a '/' prefix without validating whether the first token actually looks like a command name. Any '/' prefix triggered the slash command flow, and when no matching command was found, the error was shown with no fallback. Fix: Add looksLikeCommandName() that validates command names contain only [a-zA-Z0-9:_-]. Both isSlashCommand() and handleSlashCommand() now check the first token — if it contains path separators, dots, or non-ASCII characters, the input falls through to normal model processing instead of the command dispatcher. Closes QwenLM#1804 * fix(cli): allow dots in command names and fix prettier formatting Address review feedback: - Allow '.' in looksLikeCommandName() regex to support extension-qualified commands like gcp.deploy (CommandService renames conflicts as ext.cmd) - Add regression tests for dot-named commands in both commandUtils and slashCommandProcessor - Fix prettier formatting in slashCommandProcessor test file * fix(cli): handle slash command review edge cases * docs(cli): align slash command validation comment * fix(cli): preserve slash prompt ordering * fix(cli): reject shell-metacharacter slash tokens * test(cli): align slash command action mocks * fix(cli): track model-sent user turns * fix(cli): narrow slash path handling scope * fix(cli): count slash path prompts in history * test(cli): type slash command action mocks
Summary
Fixes #1804.
Slash-prefixed path inputs whose first token contains a path separator, such as
/api/apiFunction/接口的实现and/Users/name/path, should be sent to themodel as normal prompts instead of being consumed as unknown slash commands.
This PR intentionally keeps the scope narrow: it does not change the behavior
for single-token unknown slash commands or broader unknown
/token ...argsrouting.
Root Cause
isSlashCommand()previously treated every/-prefixed input as a slashcommand, except JavaScript-style comments. A path-like prompt such as
/api/apiFunction/接口的实现therefore entered slash-command handling and endedas
Unknown command.Fix
contains
/or\.handleSlashCommand()so direct slashprocessing and lexical slash detection agree.
bare
/.mapping by using
isSlashCommand()instead of a rawstartsWith('/')heuristic.
SlashCommandProcessorActionsinterface.Out Of Scope
/data,/README.md, or/data foo.sentToModelprovenance for slash commands that submit prompts tothe model.
Those are separate routing/history design questions and should be reviewed in a
follow-up PR rather than mixed into this targeted #1804 fix.
Reviewer Test Plan
/api/apiFunction/接口的实现is sent to the model rather than producingUnknown command./Users/name/path 帮我看一下is sent to the model rather than producingUnknown command./help,/?, and bare/still stay in the slash-command flow.user turn.
Verification
cd packages/cli && npx vitest run src/ui/utils/commandUtils.test.ts src/ui/utils/historyMapping.test.ts src/ui/hooks/slashCommandProcessor.test.tstsccheck forsrc/ui/hooks/slashCommandProcessor.test.tsnpm run typechecknpm run lint