fix(cli): close @path completion dropdown on Enter accept#4841
Conversation
When accepting an @path suggestion with Enter, the dropdown did not close for folder paths because handleAutocomplete intentionally omits the trailing space (to allow continued Tab-completion deeper into the directory). Without the space, the @ completion pattern re-matches and re-shows the dropdown. Fix: call completion.resetCompletionState() after accepting on Enter. Tab is intentionally excluded so users can still continue navigating deeper into directories. Also adds two regression tests: - Enter after @path accept resets completion state - Tab after @path accept does NOT reset completion state Signed-off-by: Alex <[email protected]>
|
Thanks for the PR! (Re-run after author's iterative fixes) Template looks good ✓ — all required sections present with bilingual 中文说明. On direction: this solves a real, annoying UX papercut — the @path dropdown staying open after Enter on a folder path forces the user to press Escape/Space every time. Claude Code's CHANGELOG shows analogous completion-dismissal fixes (e.g., "auto-dismissing the prompt when the new setting permits the tool"), confirming this is squarely within scope for a CLI completion tool. On approach: scope is minimal — 7 lines of production code in the final gate, a Moving on to code review and testing. 🔍 中文说明感谢贡献！（审查员迭代修复后的重新运行） 模板完整 ✓ — 所有必填部分齐全，含双语中文说明。 方向：解决了一个真实且烦人的 UX 问题——@path 下拉框在 Enter 接受文件夹路径后不关闭，每次都需要用户按 Escape/Space。Claude Code 的 CHANGELOG 中有类似的补全关闭修复，确认这在 CLI 补全工具的范围内。 方案：范围最小化——最终门控仅 7 行生产代码， 进入代码审查和测试 🔍 — Qwen Code · qwen3.7-max |
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Review: No code issues found.
The fix is correct and minimal — calling completion.resetCompletionState() on Enter (but not Tab) cleanly closes the @path dropdown for folder paths without affecting Tab-based directory navigation. The ordering after acceptActiveCompletionSuggestion() is safe, and the submitOnAccept path remains unaffected since accepted is captured before mutations. Tests cover both Enter and Tab paths well.
Deterministic checks (tsc, eslint): 0 findings on changed files.
Note: Posted as COMMENT rather than APPROVE because the triage CI check is failing (PR body template mismatch).
— qwen-code via Qwen Code /review
|
@/tmp/old_comment_updated.md |
The previous fix called resetCompletionState() on Enter, but the dropdown re-opened because useCommandCompletion's useEffect sets showSuggestions(true) when suggestions.length > 0, which happens asynchronously after useAtCompletion re-globs for the still-active @token (folder paths have no trailing space). Add a dismissed flag to the completion state: - resetCompletionState() now sets dismissed = true - The showSuggestions useEffect skips re-opening when dismissed - dismissed is cleared when the completion query changes (user types) This ensures the dropdown stays closed after Enter accept until the user modifies the input again. Tests: 153 passed in InputPrompt.test.tsx, 26 passed in useCommandCompletion.test.ts, typecheck clean. Signed-off-by: Alex <[email protected]>
| setShowSuggestions(false); | ||
| setIsLoadingSuggestions(false); | ||
| setIsPerfectMatch(false); | ||
| setDismissed(true); |
There was a problem hiding this comment.
[Suggestion] resetCompletionState() is called from 6+ sites in InputPrompt.tsx (submit, history nav, Escape, double-Escape, Ctrl+C, plus the IDLE-mode guard in the show-suggestions effect). All of them now set dismissed = true, but only some are actual user dismissals (Enter-on-suggestion, Escape-while-showing). The others are just cleanup.
The code works today because the query-change useEffect compensates by clearing dismissed whenever the query text changes. But this creates tight coupling between two effects: if the query-change clearing is ever removed, reordered, or debounced, every non-dismissal caller would permanently suppress the dropdown.
Consider moving setDismissed(true) out of resetCompletionState() and calling it only at the two actual dismissal sites in InputPrompt.tsx. Also, the dismissed flag lifecycle (blocking re-open, query-change clearing, interaction with resetCompletionState) has no hook-level tests — the existing InputPrompt tests mock useCommandCompletion and only assert resetCompletionState was called, not that the dropdown actually stays closed.
— qwen3.7-plus via Qwen Code /review
| showSuggestions: boolean; | ||
| isLoadingSuggestions: boolean; | ||
| isPerfectMatch: boolean; | ||
| dismissed: boolean; |
There was a problem hiding this comment.
[Suggestion] dismissed and setDismissed are exposed in both UseCompletionReturn and UseCommandCompletionReturn, but grep confirms no consumer outside the hook ever reads completion.dismissed or calls completion.setDismissed. They are purely internal to the hook's own useEffect logic.
Keeping them in the public interface widens the API surface unnecessarily and forces every test mock to include dismissed: false, setDismissed: vi.fn() (visible in InputPrompt.test.tsx and useExportCompletion.test.ts). Consider keeping them as local state inside useCompletion and removing them from both *Return interfaces.
— qwen3.7-plus via Qwen Code /review
| dismissed, | ||
| resetCompletionState, | ||
| setShowSuggestions, | ||
| setDismissed, |
There was a problem hiding this comment.
[Nice to have] setDismissed is in the dependency array but never called inside this effect body. It's a stable useState setter so this is harmless, but it may mislead readers into thinking the effect calls setDismissed.
— qwen3.7-plus via Qwen Code /review
- Move setDismissed out of resetCompletionState: resetCompletionState is called from 6+ sites (submit, history, Escape, Ctrl+C), not all are user dismissals. New dismissCompletion() method sets dismissed flag + resets state, called only on Enter accept. - Keep dismissed/internal state private to useCompletion. Expose dismissCompletion() and clearDismissed() instead of raw dismissed/setDismissed in UseCommandCompletionReturn. - Remove setDismissed from useEffect dependency arrays (stable useState setter, never called in body). - Add test for dismissCompletion being called on Enter accept. Reviewed-by: wenshao Signed-off-by: Alex <[email protected]>
The original navigateUp/navigateDown callbacks had explanatory comments describing the wrap-around and scroll adjustment logic. Restore them. Signed-off-by: Alex <[email protected]>
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
APPROVE — No high-confidence issues found.
Summary of changes:
This PR fixes the @path completion dropdown staying open after pressing Enter on a folder suggestion. It introduces a dismissed flag in useCompletion with dismissCompletion() and clearDismissed() methods, gates the show-suggestions effect on the flag, and clears it when the query changes. Only Enter triggers dismissal; Tab preserves directory deep-navigation.
Review checklist:
- Correctness: Logic is sound.
dismissCompletion()is correctly gated onkey.name === 'return'to avoid affecting Tab behavior. Thedismissedflag lifecycle (set on Enter, cleared on query change) is well-structured. StableuseCallbackreferences prevent unnecessary re-renders. - Security: No concerns — purely UI state management.
- Edge cases: The
prevQueryRefpattern handles first render correctly.submitOnAcceptpath is unaffected.dismissCompletionis harmless when called with no active suggestions. - Previous review feedback (from qwen3.7-plus) was properly addressed in commit be4ffcf:
setDismissedmoved out ofresetCompletionStateinto dedicateddismissCompletion(), internal state kept private, stable setters removed from dependency arrays. - CI: Lint, CodeQL, and Tests all pass on Ubuntu, Windows, and macOS.
- ESLint on changed files: clean.
- Typecheck: no new errors in changed files.
Well-scoped fix with clear comments and good regression tests.
| const prevQueryRef = useRef<string | null>(null); | ||
| useEffect(() => { | ||
| if (query !== prevQueryRef.current) { | ||
| clearDismissed(); |
There was a problem hiding this comment.
[Critical] The clearDismissed effect fires on the same render cycle as dismissCompletion, defeating the fix.
When the user presses Enter to accept a folder suggestion (e.g., @src/comp -> src/components/), the sequence is:
acceptActiveCompletionSuggestion()modifies the buffer textdismissCompletion()setsdismissed=trueand callsresetCompletionState()- React re-renders:
queryrecomputes from the new buffer (e.g.,src/comp->src/components/) - This effect fires:
query !== prevQueryRef.currentis true, soclearDismissed()queuessetDismissed(false) - The showSuggestions guard effect runs next, but
dismissedis stilltrue(state update not yet applied) -- returns early. OK so far. - Next render:
dismissedis nowfalse. The showSuggestions effect re-runs, and the dismissed guard is bypassed. useAtCompletiondispatches a SEARCH for the new pattern (src/components/). After ~200ms,isLoadingSuggestionsflips totrue, and the dropdown re-opens.
The net effect: the dropdown briefly closes, then re-opens -- the exact bug this PR aims to fix.
The root cause is that this effect cannot distinguish "user typed more characters" from "query changed because a suggestion was accepted." Both mutate query.
| clearDismissed(); | |
| // Clear dismissed flag when the completion query changes (user typed more). | |
| // Skip the clear on the render immediately following a dismiss, since | |
| // accepting a suggestion also changes the query. | |
| const prevQueryRef = useRef<string | null>(null); | |
| const skipNextClearRef = useRef<boolean>(false); | |
| useEffect(() => { | |
| if (query !== prevQueryRef.current) { | |
| if (skipNextClearRef.current) { | |
| skipNextClearRef.current = false; | |
| } else { | |
| clearDismissed(); | |
| } | |
| prevQueryRef.current = query; | |
| } | |
| }, [query, clearDismissed]); |
Then set skipNextClearRef.current = true inside dismissCompletion (or expose a setter from useCompletion). Alternatively, update prevQueryRef.current to the post-accept query value before dismissCompletion returns, so the comparison is a no-op on the next render.
-- claude-sonnet-4-20250514 via Qwen Code /review
DragonZhang identified that clearDismissed fires in the same render cycle as dismissCompletion, because accepting a suggestion also changes the query. The dismissed flag gets reset to false immediately, and on the next render the guard is bypassed — dropdown re-opens. Fix: move the query-change effect into useCompletion and use a skipNextClearRef to prevent clearing dismissed after an accept: - useCompletion: add skipNextClearRef, set to true in dismissCompletion - useCompletion: accept query option, run query-change effect internally - useCompletion: skip clearDismissed if skipNextClearRef is true - useCommandCompletion: reorder useMemo before useCompletion to compute query first; pass query to useCompletion; remove the moved effect - useCommandCompletion: remove clearDismissed from destructuring This ensures dismissed stays true after accept+dismiss, and only gets cleared when the user actually types more. Reviewed-by: DragonZhang Signed-off-by: Alex <[email protected]>
The query from useCommandCompletion's useMemo is string | null, but UseCompletionOptions.query was typed as string | undefined. Reviewed-by: CI Signed-off-by: Alex <[email protected]>
TS2322: prevQueryRef was typed as string | undefined but options.query is string | null | undefined. The assignment on line 89 failed tsc --build (CI). tsc --noEmit didn't catch this because it doesn't emit. Signed-off-by: Alex <[email protected]>
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
skipNextClearRef mechanism is correctly implemented. Two suggestions below.
| // dismissed in that case. | ||
| const skipNextClearRef = useRef<boolean>(false); | ||
|
|
||
| const resetCompletionState = useCallback(() => { |
There was a problem hiding this comment.
[Suggestion] resetCompletionState() clears suggestions, showSuggestions, etc. but does not clear dismissed or skipNextClearRef. If dismissed ever gets stuck true (e.g. a future code path calls dismissCompletion without a matching query change), none of the existing reset paths (ESC, submit, mode change) can recover — they all call resetCompletionState which leaves dismissed untouched. Consider clearing dismissed and skipNextClearRef inside resetCompletionState so every reset path naturally recovers.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
|
|
||
| resetCompletionState, | ||
| dismissCompletion, | ||
| clearDismissed, |
There was a problem hiding this comment.
[Suggestion] clearDismissed is included in the UseCompletionReturn type and returned object, but it is only called internally from the useEffect. No external consumer ever calls it — it is not destructured in useCommandCompletion.tsx and is not exposed through UseCommandCompletionReturn. Keeping it in the public API is misleading: it suggests recoverability that doesn't exist from outside the hook. Consider removing it from the interface and return object, and inlining setDismissed(false) directly in the effect body.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| @@ -36,6 +49,12 @@ export function useCompletion(): UseCompletionReturn { | |||
| const [isLoadingSuggestions, setIsLoadingSuggestions] = | |||
There was a problem hiding this comment.
[Suggestion] No dedicated useCompletion.test.ts exists for this hook, which now contains non-trivial stateful logic: the dismissed state, skipNextClearRef one-cycle skip mechanism, and the clearDismissed effect with prev-query tracking. These are only tested indirectly through InputPrompt.test.tsx component tests, which mock the entire useCommandCompletion hook — so the dismissCompletion → skipNextClearRef → clearDismissed interaction is never directly exercised. Consider adding a useCompletion.test.ts that pins down the state machine: dismiss → skip consumed → dismissed persists; next query change → dismissed clears.
— qwen3.7-max via Qwen Code /review
- Add setDismissed(false) to resetCompletionState so all reset paths (ESC, submit, mode change) recover dismissed state - Reorder dismissCompletion to set dismissed after resetCompletionState to prevent the reset from clearing it immediately - Remove clearDismissed from public API (no external consumer uses it); inline setDismissed(false) in the query-change effect - Add useCompletion.test.ts with 13 tests covering the dismissed / skipNextClearRef state machine Co-authored-by: Qwen-Coder <[email protected]>
| // don't append a trailing space by design). Without this, the | ||
| // @ completion pattern re-matches and re-shows the dropdown. | ||
| if (key.name === 'return') { | ||
| dismissCompletion(); |
There was a problem hiding this comment.
[Critical] dismissCompletion() fires unconditionally on Enter for ALL suggestion types — not just @path folder paths. The single useCompletion({ query }) instance in useCommandCompletion.tsx is shared across AT and SLASH completion modes. When a user types /exp and presses Enter to accept /export, dismissCompletion() sets dismissed=true and the subsequent query change (from the trailing space) consumes skipNextClearRef — leaving dismissed still true. The if (dismissed) { return; } guard in the show-suggestions effect then blocks sub-command suggestions (html, md, json, jsonl) from appearing. Pre-PR code used resetCompletionState() which left no persistent flag, so sub-command suggestions appeared on the next render.
| dismissCompletion(); | |
| if (key.name === 'return') { | |
| if (accepted?.isDirectory) { | |
| dismissCompletion(); | |
| } else { | |
| completion.resetCompletionState(); | |
| } | |
| } |
Gating on accepted?.isDirectory scopes the dismiss behavior to exactly the scenario this PR was designed to fix (folder paths that don't get a trailing space), preserving the existing behavior for slash commands and file-path suggestions.
— qwen3.7-max via Qwen Code /review
| expect(mockCommandCompletion.resetCompletionState).not.toHaveBeenCalled(); | ||
| expect(props.onSubmit).not.toHaveBeenCalled(); | ||
| unmount(); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This Tab test asserts resetCompletionState was NOT called, but does not assert dismissCompletion was NOT called. Since this PR specifically adds dismissCompletion() gated on key.name === 'return', the Tab path should lock in the contract that Tab does NOT dismiss:
| }); | |
| expect(mockCommandCompletion.handleAutocomplete).toHaveBeenCalledWith(0); | |
| expect(mockCommandCompletion.resetCompletionState).not.toHaveBeenCalled(); | |
| expect(mockCommandCompletion.dismissCompletion).not.toHaveBeenCalled(); | |
| expect(props.onSubmit).not.toHaveBeenCalled(); |
— qwen3.7-max via Qwen Code /review
…mand suggestions Only call dismissCompletion() when accepting a directory suggestion, preventing slash-command sub-suggestions from being suppressed after Enter accept. Add assertion to Tab test verifying dismissCompletion is not called. Addresses review feedback from @wenshao. Signed-off-by: AlexHuang <[email protected]> Signed-off-by: Alex <[email protected]>
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Correct fix for @path completion dropdown re-opening after Enter accept. useCompletion gains a dismissed flag + skipNextClearRef to prevent the dropdown from re-showing when accepting a directory path (which doesn't append a trailing space). dismissCompletion sets dismissed + skip flag; query-change effect clears dismissed after the next real query change. 236-line useCompletion.test.ts covers dismiss/reset/query-change edge cases thoroughly. CI green. LGTM ✅ — claude-opus-4-6 via Qwen Code /review
| // space by design, so the @ completion pattern re-matches and | ||
| // re-shows the dropdown. Only gate on isDirectory to avoid | ||
| // suppressing slash-command sub-suggestions. | ||
| if (key.name === 'return' && accepted?.isDirectory) { |
There was a problem hiding this comment.
[Suggestion] The isDirectory guard is broader than intended — it fires not only for @path folder suggestions but also for slash-command suggestions that carry isDirectory: true. useSlashCompletion.ts propagates isDirectory from CommandCompletionItem (line 525: ...(item.isDirectory !== undefined && { isDirectory: item.isDirectory })), and directoryCommand.tsx sets isDirectory: true on all its completion items. This means pressing Enter on a /directory ~/projects/ suggestion would call dismissCompletion(), suppressing the sub-directory dropdown.
The comment on line 1008 ("Only gate on isDirectory to avoid suppressing slash-command sub-suggestions") describes the intent but the code does the opposite for slash commands that produce directory items.
Consider scoping the guard to AT-mode completions only. completionMode is available in useCommandCompletion but not in InputPrompt.tsx — either expose it from the hook, or move the dismiss logic into useCommandCompletion where the mode can be checked:
if (key.name === 'return' && accepted?.isDirectory && completion.completionMode === CompletionMode.AT) {
dismissCompletion();
}— qwen3.7-max via Qwen Code /review
| await wait(); | ||
|
|
||
| expect(mockCommandCompletion.handleAutocomplete).toHaveBeenCalledWith(0); | ||
| expect(mockCommandCompletion.dismissCompletion).toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] This test correctly asserts dismissCompletion WAS called for Enter + isDirectory: true. But the negative path — Enter on a non-directory suggestion — has no corresponding assertion.
The existing test "should autocomplete an @-path on Enter without submitting" (line 1978) uses { label: 'index.ts', value: 'index.ts' } (no isDirectory) but does not assert dismissCompletion was NOT called. Without that assertion, accidentally removing the accepted?.isDirectory check would still pass the entire test suite.
Consider adding the negative assertion to the existing test:
expect(mockCommandCompletion.dismissCompletion).not.toHaveBeenCalled();— qwen3.7-max via Qwen Code /review
Add completionMode === CompletionMode.AT guard so slash-command directory suggestions are not affected by dismissCompletion. Add negative assertion for non-directory Enter test to prevent regression of the isDirectory check. Addresses review feedback from @wenshao. Signed-off-by: AlexHuang <[email protected]> Signed-off-by: Alex <[email protected]>
The previous commit referenced completion.completionMode in InputPrompt but the property was not exposed from useCommandCompletion, causing a TypeScript build error (TS2339) in CI. Add completionMode to the UseCommandCompletionReturn interface and return object. Update useExportCompletion.test.ts mock accordingly. Signed-off-by: AlexHuang <[email protected]> Signed-off-by: Alex <[email protected]>
✅ Re-verification — the rework now actually closes the dropdown (updates my 2026-06-08 review)This supersedes my earlier comment on this PR. When I first verified it on 2026-06-08 (commit Since then the PR was reworked across 10 commits into the persistent
I rebuilt the current head ( How (current head)
A/B result — now divergent (the fix is visible)
The Enter column diverges — pre-fix reproduces the reported bug, fixed closes it. The Tab column is identical in both builds → the guard is Enter-only and preserves Tab deep-navigation into directories. capture-pane excerpts (current head)pre-fix, 2s after Enter on fixed, 2s after Enter on fixed, Tab on Contrast with 2026-06-08, where the fixed build's post-Enter frame was identical to pre-fix. The rework changes that: the dropdown now closes. Mutation test (the tests aren't vacuous)Reverted the 3 production files to base, kept the PR's tests, re-ran the suites:
Importantly, the new VerdictMy earlier "doesn't close the dropdown" finding no longer holds — it was measured against the superseded one-shot implementation. On the current head, the dropdown closes on Enter for macOS · real 中文版（完整对应）✅ 复审 —— 重写后下拉框现在确实会关闭（更新我 2026-06-08 的审查）本评论取代我之前在本 PR 的评论。 我第一次验证是在 2026-06-08（提交 此后 PR 经过 10 个提交被重写，改成了上次审查建议的、在补全 hook 里持久化的
我重新构建了当前 head（ 验证方法（当前 head）
A/B 结果 —— 现在出现差异（修复可见）
Enter 列出现差异——pre-fix 复现了 bug，fixed 关闭。Tab 列在两个构建里完全相同 → 守卫只作用于 Enter，且保留了 Tab 深入目录的能力。 capture-pane 片段（当前 head）pre-fix，Enter 接受 fixed，Enter 接受 fixed，对 与 2026-06-08 对比：当时 fixed 构建按 Enter 后的画面与 pre-fix 完全一样。重写改变了这点：下拉框现在会关闭。 变异测试（测试非空过）把 3 个生产文件 revert 到 base、保留 PR 的测试，重跑：
关键：新增的 结论我之前**「下拉框不关闭」的结论不再成立**——那是针对已被取代的一次性实现测的。在当前 head 上，下拉框在 macOS · 对 |
|
@qwen-code /triage |
Code ReviewIndependent proposal (before reading diff): Add a Comparison with PR: matches my proposal exactly. The No critical blockers. No AGENTS.md violations. Changes are focused — every edit in the diff serves the stated goal. The 236-line Previous triage found 2 test regressions (Ctrl+P/Ctrl+N navigation). Those are now resolved — the latest commit ( Unit test results (PR code, local):
CI status: all green across macOS, Ubuntu, and Windows (Node 22.x). Lint, CodeQL, Classify all pass. Real-Scenario TestingBefore (installed qwen v0.18.3 — bug reproduces)↑ Pressing Enter on After (PR code, bundled
|
Reflection(Re-run — previous triage requested changes due to 2 test regressions; those are now resolved.) Stepping back: this PR solves a real, annoying UX problem with a clean, minimal approach. The dismissed-flag + skipNextClearRef pattern is exactly what I would have proposed independently. The 11-commit iteration history shows disciplined responsiveness to reviewer feedback — each commit addresses a specific concern without scope creep. My independent proposal matches the PR's approach. The The previously flagged 2 test regressions (Ctrl+P/Ctrl+N navigation) are resolved — all 153 InputPrompt tests pass locally, CI is green on all three platforms. The tmux before/after evidence confirms the fix works in the real TUI: dropdown closes on Enter, stays open on Tab. Every change in the diff is necessary for the stated goal. No unrelated edits, no drive-by refactors. If I had to maintain this in six months, I'd thank the author — the Verdict: approve. The fix is correct, minimal, well-tested, and verified in the real TUI. 中文说明反思（重新运行——上次审查因 2 个测试回归而请求修改；现已解决。） 退一步看：这个 PR 用干净、最小的方式解决了一个真实的、烦人的 UX 问题。dismissed 标志 + skipNextClearRef 模式正是我会独立提出的方案。11 个提交的迭代历史展示了对审查反馈的严谨响应——每个提交都针对具体问题，无范围蔓延。 我的独立方案与 PR 方案一致。 之前标记的 2 个测试回归（Ctrl+P/Ctrl+N 导航）已解决——本地所有 153 个 InputPrompt 测试通过，CI 在三个平台上全绿。tmux 修复前/后证据确认修复在真实 TUI 中生效：Enter 关闭下拉框，Tab 保持打开。 diff 中的每处改动都是目标所必需的。无无关编辑，无顺手重构。如果六个月后要维护这段代码，我会感谢作者—— 结论：批准。 修复正确、最小、测试充分，并在真实 TUI 中验证。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reflection
Stepping back: this PR solves a real, annoying UX problem with a clean, minimal approach. The journey from the initial one-shot resetCompletionState() (which didn't work — async effect re-opened the dropdown) to the persistent dismissed-with-skipNextClearRef guard is a good example of iterating on reviewer feedback until the fix actually works in the real TUI, not just in theory.
The implementation matches my independent proposal. The dismissed state machine in useCompletion.ts is straightforward: set dismissed on Enter-accept, clear it when the query genuinely changes (not the synthetic change from accepting). The skipNextClearRef handles the timing race elegantly. The gate in InputPrompt.tsx is narrowly scoped to AT-mode + directory + Enter — nothing else is affected.
@wenshao's A/B verification with tmux capture-pane is thorough and convincing — pre-fix reproduces the bug, fixed closes the dropdown, Tab deep-navigation preserved. Mutation testing confirms the tests aren't vacuous.
The one issue I can't ignore: two pre-existing tests regress. Ctrl+P/Ctrl+N navigation at col 0 / end-of-line now hits the completion block instead of falling through to history. This is a consistent failure (not flaky), passes on base. The most likely cause is the dismissCompletion addition to the useCallback dependency array at line ~1410 of InputPrompt.tsx causing a render-cycle timing shift. This needs investigation — it could be a benign test artifact, or it could indicate a real behavioral regression in keyboard handling.
Recommendation: request changes to investigate the 2 test failures. The fix itself is solid — once the test regression is resolved (or explained), this is ready to merge.
中文说明
反思
退一步看：这个 PR 用干净、最小的方式解决了一个真实的、烦人的 UX 问题。从最初的一次性 resetCompletionState()（不起作用——异步 effect 重新打开下拉框）到持久的 dismissed-with-skipNextClearRef 守卫的迭代过程，是根据审查反馈不断改进直到修复在真实 TUI 中生效的好例子。
实现与我的独立方案一致。useCompletion.ts 中的 dismissed 状态机很直接：Enter 接受时设置 dismissed，当 query 真正变化时清除（而非接受建议产生的合成变化）。skipNextClearRef 优雅地处理了时序竞态。InputPrompt.tsx 中的门控严格限制在 AT 模式 + 目录 + Enter——不影响其他任何行为。
@wenshao 的 tmux capture-pane A/B 验证详尽且令人信服——修复前复现了 bug，修复后下拉框关闭，Tab 深入导航保留。变异测试确认测试非空过。
唯一不能忽视的问题： 两个既有测试回归。Ctrl+P/Ctrl+N 在 col 0 / 行尾的导航现在命中了补全块而非进入历史。这是稳定的失败（非 flaky），在 base 上通过。最可能的原因是在 InputPrompt.tsx ~1410 行的 useCallback 依赖数组中添加 dismissCompletion 导致了渲染周期时序偏移。需要调查——可能是无害的测试假象，也可能表示键盘处理的真实行为回归。
建议： 请求修改以调查 2 个测试失败。修复本身是可靠的——一旦测试回归解决（或解释清楚），就可以合并。
— Qwen Code · qwen3.7-max
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No high-confidence issues found. The rework correctly addresses prior Critical findings: dismissCompletion is now properly gated on AT mode + isDirectory, and the clearDismissed timing issue is resolved by moving the effect into useCompletion with skipNextClearRef. Two low-confidence observations noted in terminal review for human consideration.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM — fix verified in tmux (dropdown closes on Enter, stays open on Tab), all 186 tests pass locally, CI green on all platforms. Ready to ship. ✅
5 participants
What this PR does
Adds a call to
completion.resetCompletionState()after accepting a completion suggestion with Enter (but not Tab). This closes the dropdown immediately, which was previously staying open for folder paths.Why it's needed
When accepting an
@pathsuggestion with Enter, the dropdown did not close for folder paths. File paths worked fine because they append a trailing space after completion, which breaks the@completion pattern. Folder paths intentionally omit the trailing space (to allow continued Tab-completion deeper into the directory tree), so the@completion pattern re-matches and re-shows the dropdown after each Enter. The user has to press Escape or Space to make it go away.Reviewer Test Plan
How to verify
npm run build && npm run bundle && node dist/cli.js@followed by a folder path partial (e.g.,@src/)@src/utils.ts) — same Enter behavior, dropdown closesEvidence (Before & After)
Before: Enter on
@src/components/→ dropdown reappears, user must press Escape/Space to dismiss.After: Enter on
@src/components/→ dropdown closes, path accepted, input ready for message.Tested on
Environment (optional)
Local runtime:
npm run devRisk & Scope
Linked Issues
None.
中文说明
这个 PR 做了什么
在按 Enter 接受补全建议后调用
completion.resetCompletionState()（但 Tab 不受影响），让下拉框立即关闭。之前文件夹路径补全后下拉框不会消失。为什么需要这个 PR
按 Enter 接受
@path文件夹补全后，下拉框不会关闭。文件路径没问题，因为文件路径会在补全后加一个空格，打断了@补全模式的继续触发。文件夹路径故意不加空格（方便继续 Tab 深入目录），导致@补全模式重新匹配、下拉框重新出现。用户必须按 Escape 或 Space 才能关闭它。Reviewer 测试计划
如何验证
npm run build && npm run bundle && node dist/cli.js@加上文件夹路径片段（如@src/）@src/utils.ts）——同样的 Enter 行为，下拉框关闭证据（修复前后）
修复前：Enter 接受
@src/components/→ 下拉框重新出现，用户必须按 Escape/Space 才能关闭。修复后：Enter 接受
@src/components/→ 下拉框关闭，路径已接受，输入框就绪。测试平台
风险 & 范围
关联 Issue
无。