fix(cli): fix vim mode Esc leak, Enter submit, render lag and implement missing VIM commands

[zzhenyao]

What this PR does

Fixes three vim mode issues and implements missing NORMAL mode commands:

1. Esc key leak: Prevents Esc in INSERT mode from triggering AppContainer's handler, which caused "Press Esc again to clear" prompts, input buffer clearing, and interrupted model responses
2. Enter not sending in NORMAL mode: Adds Enter key handling to submit messages in NORMAL mode
3. Mode indicator render lag: Splits VimModeContext into State and Actions contexts with useMemo to prevent unnecessary re-renders, and adds -- NORMAL -- indicator
4. Missing NORMAL mode commands: Implements undo, replace, case toggle, join lines, indent/unindent, WORD motions, find/till motions, yank/paste with system clipboard, and operator-pending combinations

Fixes: #4675

Why it's needed

Esc key leak: KeypressContext.broadcast has no "consume" mechanism — all subscribers receive every event. When vim's vimHandleInput and AppContainer's useKeypress both receive Esc, the AppContainer handler triggers unwanted behavior.

Enter not sending: The vim handler in NORMAL mode consumed Enter without forwarding it to the submit handler.

Render lag: The original single Context created a new value object on every mode change, forcing all 6 consumers (including AppContainer, SettingsDialog, etc.) to re-render even when they only needed stable callbacks.

Missing commands: Many standard commands were missing or swallowed, making vim mode feel incomplete and inconsistent with user expectations.

Reviewer Test Plan

Evidence (Before & After)

Before:

• INSERT mode: Esc shows "Press Esc again to clear", double-press clears buffer, press during response interrupts streaming
• NORMAL mode: Enter does nothing, no -- NORMAL -- indicator shown
• Mode switching: Footer indicator updates with visible delay

After:

• INSERT mode: Esc only switches to NORMAL mode, no side effects
• NORMAL mode: Enter submits message (if non-empty), -- NORMAL -- indicator shows immediately
• Mode switching: Indicator updates instantly with no perceptible delay

How to Verify

1. Build and run

   npm run build
   npm start

2. Esc key isolation (INSERT mode)

   • Enable vim mode: /vim
   • Type some text, then press Esc to switch from INSERT to NORMAL mode
   • Verify: no "Press Esc again to clear" prompt, buffer not cleared, streaming responses not interrupted
   • Verify: -- NORMAL -- indicator appears in Footer

3. Enter submits in NORMAL mode

   • Type text, press Esc to enter NORMAL mode
   • Press Enter
   • Verify: message is submitted (non-empty text only)
   • Verify: Enter on empty or whitespace-only text does nothing

4. Context split optimization

   • Toggle vim mode on/off
   • Verify: only consumers that actually need mode state re-render
   • AppContainer (uses only toggleVimEnabled), SettingsDialog, Footer, Composer should all work correctly

5. NORMAL mode commands

   • u: undo last change
   • r + char: replace character under cursor
   • ~: toggle case and move forward
   • J: join current line with next
   • >> / <<: indent/unindent
   • W / B / E: WORD motions (whitespace-delimited)
   • f/F/t/T + char: find/till character motions
   • ; / ,: repeat/undo find
   • y + motion / yy / Y: yank to register
   • p / P: paste from register
   • d + motion / c + motion / dd / cc: delete/change with motion

6. System clipboard integration

   • Test yank (yy) and paste (p) on macOS, Windows, and Linux
   • Verify: copied text appears in system clipboard
   • Verify: pasted text from system clipboard appears at cursor

7. Operator-pending combinations

   • Test dw (delete word), cw (change word), yy (yank line), d$ (delete to end of line)
   • Verify: operations work correctly
   • Verify: operator-pending times out after 1 second if no motion provided

8. Regression

   • Non-vim mode: all functionality works as before
   • vim mode disabled: no impact on normal operation
   • Settings dialog: vim mode toggle still works
   • Status line: vim mode indicator updates correctly

Planned follow-ups

• Documentation: Will update user-facing docs for new vim commands, NORMAL mode, and clipboard integration in a later PR.

Future work (to be discussed)

• Visual mode (v): Visual selection mode for text highlighting and selection. In Vim mode, the “Ctrl+?” shortcut doesn't work. It would be nice if VISUAL mode worked in Vim mode. #2779

中文

此 PR 做了什么

修复三个 vim 模式问题并实现缺失的 NORMAL 模式命令：

1. Esc 键穿透：防止 INSERT 模式下 Esc 触发 AppContainer 的 handler，避免"Press Esc again to clear"提示、输入缓冲区清空和模型回复中断
2. NORMAL 模式下 Enter 无法发送：添加 Enter 键处理以在 NORMAL 模式下提交消息
3. 模式指示器渲染延迟：将 VimModeContext 拆分为 State 和 Actions 两个 Context 并使用 useMemo，防止不必要的重渲染，并添加 -- NORMAL -- 指示器
4. 缺失的 NORMAL 模式命令：实现撤销、替换、大小写切换、合并行、缩进/反缩进、WORD 移动、查找/直到移动、系统剪贴板复制粘贴和 operator-pending 组合

Fixes: #

为什么需要

Esc 穿透：KeypressContext.broadcast 没有"消费"机制，所有订阅者都会收到每个事件。当 vim 的 vimHandleInput 和 AppContainer 的 useKeypress 都收到 Esc 时，AppContainer handler 触发了不必要的行为。

Enter 无法发送：NORMAL 模式下的 vim handler 消费了 Enter 但没有转发给提交 handler。

渲染延迟：原始单一 Context 在每次模式切换时创建新的 value 对象，导致所有 6 个消费者（包括 AppContainer、SettingsDialog 等）即使只需要稳定回调也被迫重渲染。

缺失命令：许多标准命令缺失或被吞掉，使 vim 模式感觉不完整且不符合用户预期。

Reviewer 测试计划

证据（修改前 vs 修改后）

修改前：

• INSERT 模式：Esc 显示"Press Esc again to clear"，双击清空缓冲区，按 Esc 中断流式响应
• NORMAL 模式：Enter 无效，不显示 -- NORMAL -- 指示器
• 模式切换：Footer 指示器有明显延迟

修改后：

• INSERT 模式：Esc 仅切换到 NORMAL 模式，无副作用
• NORMAL 模式：Enter 提交消息（非空文本），-- NORMAL -- 指示器立即显示
• 模式切换：指示器即时更新，无感知延迟

[zzhenyao]

@wenshao Thank you for the thorough review. Here's what I addressed:

Critical bugs (all fixed):

1. >>/<< indent duplication: replaceRange constructs prefix + replacement + suffix, so passing ' ' + line caused duplication. Fixed to pass just ' ' for >> and '' for <<. The outdent case also had the same issue with line.slice(2) / line.slice(1).

2. pendingOperator leak on movement keys: After pressing d, y, c, >, or <, movement keys like h/j/k/l/w/b/e/0/$/^/G would leave the operator in a stuck state. Added if (state.pendingOperator) dispatch({ type: 'SET_PENDING_OPERATOR', operator: null }) to all movement handlers. The >>/<< operators now defer CLEAR_COUNT until the second key arrives so the count is available for multi-line operations.

3. yankRange off-by-one: lines.slice(startRow + 1, endRow) excluded the last line when yanking multi-line ranges. Fixed by conditionally including middle lines only when there are any between start and end rows.

4. Delete/change operations not populating yank register: DELETE_CHAR, DELETE_LINE CHANGE_LINE, DELETE_TO_EOL, CHANGE_TO_EOL all now call SET_YANK_REGISTER and writeClipboard before executing the buffer operation, matching what yy and yw already do.

5. Yank/paste tests with zero assertions: Replaced no-op comments with real assertions buffer.replaceRange calls. Fixed the p/P test expectations to match actual behavior: yw from col 0 yanks 'hello ' (word + trailing space) and pastes at the correct position.

6. Esc guard missing integration test: Added test that renders AppContainer with vimEnabled: true, vimMode: 'INSERT', fires Esc via the captured keypress handler, and asserts cancelOngoingRequest is not called.

Suggestions:

7. Clipboard blocking: Switched from execSync to execFileSync (avoids shell overhead, injection-safe). Reduced timeout from 1000ms to 200ms. Added module-level caching for Linux clipboard tools (linuxReadCmd / linuxWriteCmd) so the fallback probe only runs once at startup. Async fire-and-forget for writes deferred to a follow-up PR — the current changes reduce worst-case blocking from ~3s to ~200ms on first call, then ~50ms after caching.

8. repeatCount not respected: Added count support to ~ (toggle N chars, advance cursor), J (join N lines), f/F/t/T (find Nth occurrence), ;/, (repeat find N times ), r (replace N chars), p/P (paste N times), and >>/<< (indent/outdent N lines). Note: u intentionally ignores count in real vim, so left as-is.

[zzhenyao]

@wenshao Thanks for the continued review! All R8 comments have been addressed:

• Fixed vim_change_movement for j and k off-by-one: changed count to count + 1 in line range calculation, matching the fix already applied to vim_delete_movement
• Fixed dj/yj yank text line count mismatch in handleDeleteMovement, handleYankMovement, and executeCommand DELETE_MOVEMENT/YANK_MOVEMENT cases: yank now captures count + 1 lines (4 locations), matching the actual deletion range
• Replaced text.length with cpLen(text) in characterwise P handler for correct cursor positioning with non-BMP characters (emoji, math symbols)
• Replaced String.prototype.slice() with cpSlice() across 15 locations in vim.ts (yankRange, executeCommand DELETE_CHAR / DELETE_TO_EOL / CHANGE_TO_EOL / DELETE_MOVEMENT / YANK_MOVEMENT / TOGGLE_CASE, handleDeleteMovement, handleYankMovement, and ~ handler) to use code-point-based slicing consistent with the cursor system
• Fixed p/P linewise paste cursor position: p now uses vimMoveToLine(row + 2) (1-based indexing), P now calls vimMoveToLine(row + 1) before vimMoveToLineStart() so cursor lands on the first pasted line instead of the original line
• Fixed J (join lines) cursor position for multi-line joins: cursor now lands at the last join point (e.g. 3J on ["hello","world","foo"] places cursor at column 12 before "foo") instead of the first join point, matching real vim behavior
• Replaced state.lastCommand?.char with stateRef.current.lastCommand?.char in executeCommand REPLACE_CHAR case and removed it from the useCallback dependency array, preventing unnecessary function recreation on every r command

[zzhenyao]

@wenshao Thanks for the review! All R9 comments addressed:

• Fixed J (join lines) cursor position in two handlers (CMD_TYPES.JOIN_LINES and the main keypress handler): joinCol now tracks the join-space position using cpLen(joined) - cpLen(trimmed) - 1 instead of accumulating the full joined length
• Fixed REPLACE_CHAR (r command) bounds check: replaced line.length with cpLen(line) for Unicode/supplementary-plane support
• Updated stale comments for paste cursor: clarified that vimMoveToLine(row + 2) uses 1-based indexing (maps to 0-based row + 1, the first pasted line)

[wenshao]

[Critical] p linewise paste on the last line of the buffer places cursor one line too low (line 1824 — posted as body because the line already has an existing comment).

In the last-line branch (row + 1 >= buffer.lines.length), replaceRange(lastRow, lastLineLen, lastRow, lastLineLen, '\n' + repeated.replace(/\n$/, '')) appends pasted text starting at 0-based row row + 1. But vimMoveToLine(row + 2) maps to 0-based row + 1 via Math.min(lineNumber - 1, ...), which IS the first pasted line. However, after the paste adds N new lines, vimMoveToLine(row + 2) with Math.min(2 - 1, state.lines.length - 1) = Math.min(1, ...) → 0-based row 1 for a 2-line buffer. Wait — for buffer ["hello", "world"] with cursor on row 1: after paste the buffer becomes ["hello", "world", "world"] (3 lines). vimMoveToLine(3) → Math.min(3 - 1, 3 - 1) = 2 → 0-based row 2 (the NEW "world"). But the first pasted line is at row 1 (original "world").

buffer.vimMoveToLine(row + 1);

— qwen3.7-max via Qwen Code /review

[zzhenyao]

@wenshao Thanks for the review! All R10 comments addressed:

• Fixed p linewise paste cursor on last line: vimMoveToLine(row + 2) → row + 1 — the old code placed the cursor one line below the first pasted line
• Fixed .length vs cpLen mismatch in replaceRange endCol: >>, <<, and p last-line paste now all use cpLen(lines[endRow] ?? '') instead of .length. The >>/<< handlers were also refactored to route through executeCommand() so the fix lives in one place
• Made findCharInLine and findCharInLineReverse code-point-aware: replaced line.indexOf / line[i] with [...line] array iteration so f/F/t/T and their ;/, repeats work correctly on lines with emoji or supplementary-plane characters
• Made word-boundary helpers (findNextWordCol, findPrevWordCol, findWordEndCol) code-point-aware via [...line] spread — same pattern as the char-find fix above
• Fixed stateRef optimization leak: getCurrentCount and handleCharRead now read from stateRef.current.count instead of closing over state.count, so digit keypresses no longer rebuild the entire callback tree
• Extracted extractMovementText helper to deduplicate the h/j/k/l text-extraction logic across handleDeleteMovement, handleYankMovement, and executeCommand (DELETE_MOVEMENT / YANK_MOVEMENT) — was ~96 lines of near-identical code. Also had J/>>/<< delegate to executeCommand like dd/yy already do

[zzhenyao]

Thanks for the review! @wenshao All R11 comments addressed:

• Fixed missing dispatches for J, >>, <<: added SET_LAST_COMMAND, CLEAR_COUNT, and SET_PENDING_OPERATOR(null) so dot-repeat (.) works, count doesn't leak into the next command, and pendingOperator is properly cleared
• Removed redundant SET_PENDING_OPERATOR(null) from executeCommand JOIN_LINES case (now handled at call sites)
• Fixed p linewise paste cursor on last line: vimMoveToLine(row + 1) → row + 2 — the old code placed the cursor on the original last line instead of the first pasted line
• Fixed T command bounds check: line.length - 1 → cpLen(line) - 1 to properly handle supplementary characters
• Fixed r command bounds check: line.length → cpLen(line) for consistency with code-point indexing

[wenshao]

All R11 findings correctly addressed in this increment. J/>>/<< now dispatch SET_LAST_COMMAND + CLEAR_COUNT + SET_PENDING_OPERATOR for dot-repeat support; p last-line cursor fixed to vimMoveToLine(row + 2) (1-based API); T and r bounds checks converted to cpLen(line) for unicode correctness. Build passes, 206 tests pass. No new high-confidence issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

[wenshao]

Verification Report

Branch: fix/vim-key-conflict
Base: main
Environment: macOS Darwin 25.4.0, Node.js v22.17.0

Test Results

Check	Command	Result
vim.test.ts	vitest run vim.test.ts	✅ 132 tests passed (460ms)
VimModeContext.test.tsx	vitest run VimModeContext.test.tsx	✅ 4 tests passed (26ms)
vim-buffer-actions.test.ts	vitest run vim-buffer-actions.test.ts	✅ 74 tests passed (14ms)
AppContainer.test.tsx	vitest run AppContainer.test.tsx	✅ 78 tests passed (972ms)
Composer.test.tsx	vitest run Composer.test.tsx	✅ 20 tests passed (53ms)
SettingsDialog.test.tsx	vitest run SettingsDialog.test.tsx	✅ 53 tests passed (1083ms)
useStatusLine.test.ts	vitest run useStatusLine.test.ts	✅ 67 tests passed (3301ms)
Total	7 test files	428 tests passed
ESLint	eslint on 9 changed source files	✅ No errors
CLI Type Check	npm run typecheck --workspace=packages/cli	⚠️ 44 errors — none in PR files (all pre-existing)
Core Build	npm run build --workspace=packages/core	✅ Success
CLI Build	npm run build --workspace=packages/cli	✅ Success
Whitespace	git diff --check	✅ Clean

Test Coverage Summary (428 tests)

Test Suite	Tests	Coverage
vim.test.ts (132)	Esc key isolation (INSERT→NORMAL without AppContainer side effects), Enter submit in NORMAL mode, basic motions (h/j/k/l/w/b/e/0/$), WORD motions (W/B/E), find/till motions (f/F/t/T/;/,), insert commands (i/a/o/O/A/I), delete/change operators (d/c + motions, dd/cc), undo (u), replace (r), case toggle (~), join lines (J), indent/unindent (>>/<<), yank/paste (y/yy/Y/p/P), operator-pending timeout, count prefixes
VimModeContext.test.tsx (4)	Context split into State + Actions, useMemo prevents re-renders, toggleVimEnabled callback stability
vim-buffer-actions.test.ts (74)	Buffer-level word/WORD motions, find/till character search, replace, case toggle, join lines, indent/unindent, yank/paste with register, undo history stack
AppContainer.test.tsx (78)	Esc key handling with vim mode enabled (no "Press Esc again" prompt, no buffer clearing), integration with VimModeContext
Composer.test.tsx (20)	-- NORMAL -- indicator display, mode-aware key routing
SettingsDialog.test.tsx (53)	Vim mode toggle via settings dialog uses Actions context
useStatusLine.test.ts (67)	Vim mode indicator in status line

Code Review Notes

• Esc key isolation is correctly implemented: vimHandleInput returns { consumed: true } when Esc is pressed in INSERT mode, and AppContainer.useKeypress checks this flag to skip its own Esc handling. This fixes the root cause (KeypressContext.broadcast has no consume mechanism).
• Context split (State vs Actions) is a clean optimization: VimModeStateContext provides { isVimMode, mode } (changes on mode switch), VimModeActionsContext provides { toggleVimEnabled, setMode } (stable references via useMemo). This prevents AppContainer, SettingsDialog, etc. from re-rendering on every mode change when they only need callbacks.
• NORMAL mode commands are comprehensive: the 132 vim tests cover basic motions, WORD motions, find/till, operators with motions, count prefixes, yank/paste with register, undo, replace, case toggle, join, indent — well beyond the original basic set.
• Operator-pending timeout (1 second) is a good UX choice — prevents the UI from getting stuck if user enters an operator (d/c/y) but no motion.
• System clipboard integration uses pbcopy/pbpaste (macOS), xclip (Linux), clip.exe/powershell (Windows) — correct cross-platform approach.
• tsconfig.json change: Removes one entry — verified this is the removal of an unused path mapping, no impact.

Verdict

✅ Ready to merge — all 428 tests pass across 7 test suites, lint is clean, build succeeds. No typecheck errors in any PR-touched files. Comprehensive vim command coverage with proper Esc key isolation and context optimization.

---

Verified by wenshao

[zzhenyao]

Thanks for the review and approval! 🙏 @wenshao

[tanzhenxin]

Review

Re-reviewed against the latest commit. The linewise-paste regression from the previous round is resolved — pasting a yanked line onto a single-line or bottom-of-buffer prompt now works correctly.

One narrow edge case remains: submitting from vim NORMAL mode (Esc, then Enter) with a pending image attachment or large paste skips the normal submit processing, so the attachment is dropped or the placeholder marker is sent literally. It only affects the NORMAL-mode Enter path and is acceptable for now — a follow-up can route it through the same submit path INSERT mode uses.

Verdict

APPROVE — the named fixes are correct, the paste regression is resolved, and the remaining edge case is narrow enough to handle as a follow-up.