fix(cli): require whitespace before @ to trigger file completion - #4487
Conversation
Input like `cici@192.168.0.160` was incorrectly triggering @ file completion mode, causing the session to become unresponsive because Enter was consumed by the suggestion handler instead of submitting. Add a check that @ must be at position 0 or preceded by a space, matching the existing `isAtCommand()` semantics used at submit time.
📋 Review SummaryThis PR fixes a bug where email-like addresses (e.g., 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Missing regression test — The test plan lists manual checks, but no automated regression test was added. useCommandCompletion.test.ts has thorough positive coverage but zero negative tests — nothing verifies that email@domain.com or word@word stays in CompletionMode.IDLE. Without a regression guard, a future refactor could silently re-introduce this bug. Consider adding a test case like: input 'cici@192.168.0.160' → expect CompletionMode.IDLE.
— qwen3.7-max via Qwen Code /review
Add two test cases to prevent regression: - cici@192.168.0.160 should not trigger AT completion - user@example.com should not trigger AT completion
Use /\s/.test() instead of === ' ' so the completion trigger matches isAtCommand's /\s@/ pattern, covering tabs and other whitespace.
wenshao
left a comment
There was a problem hiding this comment.
No review findings at this commit. Both R1 suggestions (regression tests, \s consistency) are addressed. Downgraded from Approve to Comment: self-PR; CI still running. — qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: self-PR. Both R1 suggestions (regression tests, \s consistency) are addressed. CI all_pass 9/9. — qwen3.7-max via Qwen Code /review
pomelo-nwu
left a comment
There was a problem hiding this comment.
Ran the 3 manual scenarios from the PR description locally in tmux. All pass.
Env: macOS, branch fix/at-completion-requires-whitespace, npm run bundle → node dist/cli.js inside tmux; test cwd has 3 empty files (file.txt, readme.md, sample.js) to seed the completion list.
| # | Input | Expected | Observed |
|---|---|---|---|
| 1 | cici@192.168.0.160 |
no @ completion |
no dropdown, Enter would submit normally |
| 2 | @ at line start |
completion fires | dropdown lists all 3 files |
| 3 | hello @ (space before @) |
completion fires | dropdown lists all 3 files |
The /\s/.test(...) check correctly distinguishes "email/IP-like @" from a real file-completion trigger. LGTM.
中文
本地 tmux 跑了 PR 描述里 Test plan 的 3 个手动场景,全部符合预期:
cici@192.168.0.160:没有弹出文件补全下拉,Enter 可以正常提交(修复前的 bug 现已不复现)。- 行首
@:补全列表正常弹出,列出当前目录下的 file.txt / readme.md / sample.js。 hello @(空格后):补全列表正常弹出。
/\s/.test(prevChar) 这个判断已经能区分"邮箱/IP 里的 @"和"真正想触发文件补全的 @",跟提交期 isAtCommand 用的 /\s@/ 语义对齐,OK。
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The whitespace-before-@ guard aligns with isAtCommand() semantics and the regression tests cover the key cases.
Summary
cici@192.168.0.160was incorrectly triggering@file completion mode, causing the session to become unresponsive (Enter consumed by suggestion handler instead of submitting)useCommandCompletiondetected any@character as file completion trigger, without requiring preceding whitespace — unlikeisAtCommand()used at submit time@must be at position 0 or preceded by a space before activatingCompletionMode.ATTest plan
useCommandCompletiontests pass (24/24)InputPrompttests pass (150/150)cici@192.168.0.160→ no file completion dropdown, Enter submits normally@file.txt→ file completion still workshello @file→ file completion still works