fix(vim): prevented partial-insert corruption in vim tool execution
- Added rollback handling for pending INSERT-mode changes whenever a non-final kbd sequence leaves insert mode, and updated the resulting VimInputError with guidance for using `insert` and escaping insert transitions. - Hardened VimTool execution by resetting stale insert state before processing commands and by only applying empty inserts when Vim remains in INSERT mode. - Adjusted Vim search handling to mimic Vim magic escaping and taught `o`/`O` numeric prefixes to act like `Go`/`GO` line inserts, then updated the expected error message test.
This commit is contained in:
@@ -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<Esc>"]}` — 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 `<Esc>`).
|
||||
|
||||
## 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<CR>"]}
|
||||
|
||||
@@ -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 <Esc> 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 <Esc> 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<typeof vimSchema, VimToolDetails> {
|
||||
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<typeof vimSchema, VimToolDetails> {
|
||||
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 <Esc>)
|
||||
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()) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user