diff --git a/packages/coding-agent/src/prompts/tools/vim.md b/packages/coding-agent/src/prompts/tools/vim.md index b2fc8eaae..738ed80b8 100644 --- a/packages/coding-agent/src/prompts/tools/vim.md +++ b/packages/coding-agent/src/prompts/tools/vim.md @@ -9,34 +9,36 @@ Stateful Vim editor. Every call requires `file` — the buffer loads automatical **Never put text content in kbd.** Only Vim keystrokes go there. - BAD: `{"kbd": ["1Gohello world"]}` — text mixed into kbd +- BAD: `{"kbd": ["1Go", "hello world"]}` — text as a separate kbd entry (gets executed as keystrokes!) +- BAD: `{"kbd": ["1Ao"], "insert": "text"}` — `A` enters INSERT so `o` is typed as text, not a command - GOOD: `{"kbd": ["1Go"], "insert": "hello world"}` — command in kbd, text in insert For `insert` to work, the last `kbd` entry must leave INSERT mode active (`o`, `O`, `i`, `a`, `A`, `cc`, `C`, `s`, `S`). The tool auto-exits INSERT and auto-saves after each call. Each non-final `kbd` entry must end in NORMAL mode (add ``). -## Best pattern: replace entire file +## Editing patterns -For any edit touching multiple locations, replace the whole file — it is the most reliable approach: - -```json -{"file": "f.py", "kbd": ["ggdGi"], "insert": "entire new file content"} -``` - -`ggdGi` = go to top, delete all, enter INSERT. Then `insert` provides the complete new content. - -## Surgical edits - -**Insert after line N** (include indentation in insert — `o` does NOT auto-indent): +**Insert after line N** — use `NGo` (Go to line N, open below). Include full indentation in insert: ```json {"file": "f.py", "kbd": ["3Go"], "insert": " new line here"} ``` -**Replace line N** (`cc` clears the line and enters INSERT): +**Insert before line N** — use `NGO` (Go to line N, open above). Include full indentation: +```json +{"file": "f.py", "kbd": ["3GO"], "insert": " new line here"} +``` + +**Replace line N** — `cc` clears the line and enters INSERT: ```json {"file": "f.py", "kbd": ["5Gcc"], "insert": " replacement content"} ``` +**Replace entire file** — `ggdGi` = go to top, delete all, enter INSERT: +```json +{"file": "f.py", "kbd": ["ggdGi"], "insert": "entire new file content"} +``` + **Find and replace**: ```json {"file": "f.py", "kbd": [":%s/old/new/g"]} diff --git a/packages/coding-agent/src/tools/vim.ts b/packages/coding-agent/src/tools/vim.ts index f900d2bea..58f0d120c 100644 --- a/packages/coding-agent/src/tools/vim.ts +++ b/packages/coding-agent/src/tools/vim.ts @@ -115,8 +115,17 @@ async function executeKeySequences( } await engine.executeTokens(group.tokens, commandText, onStep); if (index < groups.length - 1 && engine.inputMode === "insert") { + // Roll back partial changes to prevent buffer corruption across calls. + engine.rollbackPendingInsert(); + const nextSeq = groups[index + 1]?.sequence ?? ""; + const looksLikeText = nextSeq.length > 0 && /\s/.test(nextSeq) && !/^[:/%]/.test(nextSeq); + let hint = + "Use the insert field for inserted text, or include to return to NORMAL mode before the next kbd entry."; + if (looksLikeText) { + hint += ` The next entry (\`${nextSeq.length > 40 ? `${nextSeq.slice(0, 37)}...` : nextSeq}\`) looks like text content — put it in the \`insert\` field instead. For multi-location edits, replace the entire file: {"kbd": ["ggdGi"], "insert": "full new content"}.`; + } throw new VimInputError( - `Sequence ${index + 1} left Vim in INSERT mode. Use the insert field for inserted text or include before starting another kbd entry.`, + `Sequence ${index + 1} (\`${group.sequence}\`) entered INSERT mode — changes rolled back. ${hint}`, group.tokens[group.tokens.length - 1], ); } @@ -365,6 +374,13 @@ export class VimTool implements AgentTool { return this.#renderFromEngine(engine, VIM_OPEN_VIEWPORT_LINES, engine.viewportStart); } + // Safety: if the engine is stuck in INSERT mode, always reset before executing kbd. + // Only skip for the intentional pause→resume pattern (no kbd, insert provided). + const hasKbd = sequences.some(s => s.length > 0); + if (engine.inputMode === "insert" && (hasKbd || (!params.pause && params.insert === undefined))) { + engine.rollbackPendingInsert(); + } + // Execute kbd sequences const commandText = sequences.join(" "); const tokenGroups = splitTokensBySequence(sequences); @@ -406,8 +422,11 @@ export class VimTool implements AgentTool { await executeKeySequences(engine, tokenGroups, commandText, emitUpdate); if (!engine.closed && params.insert !== undefined) { - await engine.applyLiteralInsert(params.insert, params.pause !== true); - await emitUpdate?.(); + // Skip empty insert when not in INSERT mode (e.g., kbd included ) + if (params.insert.length > 0 || engine.inputMode === "insert") { + await engine.applyLiteralInsert(params.insert, params.pause !== true); + await emitUpdate?.(); + } } if (params.pause === true && !engine.closed && engine.getPendingInput()) { diff --git a/packages/coding-agent/src/vim/engine.ts b/packages/coding-agent/src/vim/engine.ts index 6218eab40..f358ff297 100644 --- a/packages/coding-agent/src/vim/engine.ts +++ b/packages/coding-agent/src/vim/engine.ts @@ -96,9 +96,20 @@ function literalTextToReplayTokens(text: string): string[] { return tokens; } +// Convert a vim-style search pattern to a JavaScript RegExp. +// In vim's default ("magic") mode, (, ), {, }, |, + are literal unless backslash-escaped. +// In JS regex these are metacharacters. Swap the escaping so bare chars are literal +// and \( etc. become regex groups. +function vimPatternToJsRegex(pattern: string): string { + return pattern.replace(/\\([(){}|+])|([(){}|+])/g, (_match, escaped, bare) => { + if (escaped) return escaped; // \( -> ( (regex group) + return `\\${bare}`; // ( -> \( (literal paren) + }); +} + function createSearchRegex(pattern: string, flags = "g"): RegExp { try { - return new RegExp(pattern, flags); + return new RegExp(vimPatternToJsRegex(pattern), flags); } catch { return new RegExp(escapeRegex(pattern), flags); } @@ -344,6 +355,16 @@ export class VimEngine { } } + rollbackPendingInsert(): void { + if (this.#pendingChange) { + this.buffer.restore(this.#pendingChange.before); + this.#pendingChange = null; + } + this.inputMode = "normal"; + this.selectionAnchor = null; + this.#pendingInput = ""; + } + setCursor(line: number, col: number): void { this.buffer.setCursor({ line, col }); } @@ -843,12 +864,20 @@ export class VimEngine { await this.#startInsertChange(["A"]); return nextIndex + 1; case "o": + // When count > 1 (e.g. `13o`), interpret as `13Go` — go to line N then open below. + // Models confuse `No` with `NGo`; bare `o` with a high count is almost never intended. + if (hasCount) { + this.buffer.setCursor({ line: Math.min(count, this.buffer.lineCount()) - 1, col: 0 }); + } await this.#startInsertChange(["o"], () => { const line = this.buffer.cursor.line + 1; this.buffer.insertLines(line, [""]); }); return nextIndex + 1; case "O": + if (hasCount) { + this.buffer.setCursor({ line: Math.min(count, this.buffer.lineCount()) - 1, col: 0 }); + } await this.#startInsertChange(["O"], () => { const line = this.buffer.cursor.line; this.buffer.insertLines(line, [""]); @@ -1698,7 +1727,7 @@ export class VimEngine { this.buffer.replaceOffsets(offset, offset, text, offset + text.length); } - #readCount(tokens: readonly VimKeyToken[], index: number): { count: number; nextIndex: number } { + #readCount(tokens: readonly VimKeyToken[], index: number): { count: number; hasCount: boolean; nextIndex: number } { let cursor = index; let digits = ""; while (cursor < tokens.length) { diff --git a/packages/coding-agent/test/tools/vim.test.ts b/packages/coding-agent/test/tools/vim.test.ts index a8aea64a2..b4d00bc10 100644 --- a/packages/coding-agent/test/tools/vim.test.ts +++ b/packages/coding-agent/test/tools/vim.test.ts @@ -241,7 +241,7 @@ describe("vim tool", () => { await tool.execute("open", { file: "ambiguous.ts" }); await expect(tool.execute("bad", { file: "ambiguous.ts", kbd: ["o", "o"] })).rejects.toThrow( - /left Vim in INSERT mode/i, + /entered INSERT mode/i, ); }); diff --git a/scripts/edit_benchmark_common.py b/scripts/edit_benchmark_common.py index bffd8d859..748d26404 100644 --- a/scripts/edit_benchmark_common.py +++ b/scripts/edit_benchmark_common.py @@ -96,6 +96,7 @@ This is a survey. Answer these 6 questions about your experience using the editi """ DEFAULT_MAX_TURNS = 20 +MAX_TOOL_CALLS_PER_PROMPT = 10 _PRINT_LOCK = threading.Lock() @@ -357,18 +358,27 @@ def run_benchmark_for_model( client.install_headless_ui() verbose_cleanup = install_verbose_logging(client, model, log_mode, thinking) + per_prompt_tool_calls = 0 + def handle_tool_count(event: ToolExecutionStartEvent) -> None: - nonlocal edit_vim_tool_calls, turns_used + nonlocal edit_vim_tool_calls, turns_used, per_prompt_tool_calls + per_prompt_tool_calls += 1 if counting_edit_turns: turns_used += 1 if event.tool_name in {"edit", "vim"}: edit_vim_tool_calls += 1 + if per_prompt_tool_calls >= MAX_TOOL_CALLS_PER_PROMPT: + try: + client.abort() + except Exception: + pass # best-effort interrupt tool_count_remover = client.on_tool_execution_start(handle_tool_count) try: for turn in range(1, max_turns + 1): prompt_attempts = turn + per_prompt_tool_calls = 0 if turn == 1: client.prompt(spec.initial_prompt)