fix(agent-core): improve Edit tool backslash handling and error diagn…

.changeset/fix-edit-backslash-handling.md

@@ -0,0 +1,7 @@
+---
+"@moonshot-ai/agent-core": patch
+"@moonshot-ai/kimi-code": patch
+
+---
+
+Improve Edit tool recovery when old_string contains misencoded backslashes from the LLM.

packages/agent-core/src/tools/builtin/file/edit.md

@@ -11,3 +11,4 @@ Perform exact replacements in existing files.
 - A write lock serializes same-file edits in response order, but serialization does not make stale `old_string` valid.
 - For pure CRLF files, Read shows LF; use LF in `old_string` and `new_string`, and Edit writes CRLF back.
 - For mixed endings or lone carriage returns, Read shows carriage returns as \r; include actual \r escapes in those positions.
+- Backslash encoding: when old_string contains backslashes (e.g. Rust `\n`, JSON `\\`, regex `\d`), copy the exact characters from the Read output. Do NOT add extra escaping — the file contains literal backslash characters, not escape sequences.

packages/agent-core/src/tools/builtin/file/edit.ts

@@ -55,6 +55,76 @@ function replaceOnceLiteral(content: string, oldString: string, newString: strin
   return content.slice(0, index) + newString + content.slice(index + oldString.length);
 }
 
+/**
+ * Attempt to fix common LLM backslash-encoding mistakes in a string.
+ * Used for both old_string (matched against content) and new_string
+ * (applied as replacement).
+ */
+function applyBackslashFix(
+  input: string,
+  variant: 'collapse-only' | 'escape' | 'unescape-seq-first',
+): string {
+  if (variant === 'collapse-only') {
+    // Only collapse doubled backslashes — no escape sequence processing.
+    // Handles: LLM sent \\\\n (two backslashes + n) but file has \n (one backslash + n).
+    return input.replaceAll('\\\\', '\\');
+  }
+  if (variant === 'escape') {
+    // Replace real newlines with backslash-n.
+    // Handles: LLM sent \n (real newline) but file has \\n (two backslashes + n).
+    return input
+      .replaceAll('\n', '\\n')
+      .replaceAll('\t', '\\t')
+      .replaceAll('\r', '\\r');
+  }
+  // unescape-seq-first: translate escape sequences first, then collapse.
+  // Handles: LLM sent \n (real newline) but file has \n (backslash + n).
+  return input
+    .replaceAll('\\n', '\n')
+    .replaceAll('\\t', '\t')
+    .replaceAll('\\r', '\r')
+    .replaceAll('\\\\', '\\');
+}
+
+function findBackslashAdjustedMatch(
+  content: string,
+  oldString: string,
+): { adjusted: string; variant: string } | undefined {
+  // Try all variants and return the first that matches the file content.
+  const candidates: Array<{ adjusted: string; variant: string }> = [
+    { adjusted: applyBackslashFix(oldString, 'collapse-only'), variant: 'collapse-only' },
+    { adjusted: applyBackslashFix(oldString, 'escape'), variant: 'escape' },
+    { adjusted: applyBackslashFix(oldString, 'unescape-seq-first'), variant: 'unescape-seq-first' },
+  ];
+  for (const c of candidates) {
+    if (c.adjusted !== oldString && content.includes(c.adjusted)) {
+      return c;
+    }
+  }
+  return undefined;
+}
+
+/**
+ * Build a short diagnostic snippet showing the first divergence between
+ * `oldString` and the file content, helpful when old_string is not found.
+ */
+function buildNotFoundHint(content: string, oldString: string, _filePath: string): string {
+  const lines = content.split('\n');
+  const firstLine = oldString.split('\n')[0] ?? '';
+  if (firstLine.length === 0) return '';
+
+  // Find the closest matching line in the file content.
+  const candidates = lines.filter((l) => l.includes(firstLine.slice(0, 20)));
+  if (candidates.length === 0) {
+    return `The file does not contain a line matching the start of old_string: "${truncate(firstLine, 60)}"`;
+  }
+  return `Closest match in file: "${truncate(candidates[0]!, 80)}"`;
+}
+
+function truncate(s: string, max: number): string {
+  return s.length <= max ? s : s.slice(0, max - 3) + '...';
+}
+
 export class EditTool implements BuiltinTool<EditInput> {
   readonly name = 'Edit' as const;
   readonly description = EDIT_DESCRIPTION;
@@ -71,6 +141,18 @@ export class EditTool implements BuiltinTool<EditInput> {
       workspace: this.workspace,
       operation: 'write',
     });
+
+    // Reject no-op edits before any file I/O.
+    if (args.old_string === args.new_string) {
+      return {
+        isError: true,
+        output: 'No changes to make: old_string and new_string are exactly the same.',
+      };
+    }
+
+    // No file read here — resolveExecution runs BEFORE the authorization
+    // hook in tool-call.ts. Reading the file here would bypass permission
+    // checks. The backslash fallback is applied in execution() after auth.
     return {
       accesses: ToolAccesses.readWriteFile(path),
       description: `Editing ${args.path}`,
@@ -93,31 +175,40 @@ export class EditTool implements BuiltinTool<EditInput> {
   }
 
   private async execution(args: EditInput, safePath: string): Promise<ExecutableToolResult> {
-    if (args.old_string === args.new_string) {
-      return {
-        isError: true,
-        output: 'No changes to make: old_string and new_string are exactly the same.',
-      };
-    }
-
     try {
       const raw = await this.kaos.readText(safePath);
       const modelView = toModelTextView(raw);
       const content = modelView.text;
       const replaceAll = args.replace_all ?? false;
 
+      // Apply backslash fallback after auth — adjust both old_string and
+      // new_string with the same transformation so the replacement is correct.
+      let oldString = args.old_string;
+      let newString = args.new_string;
+      if (!content.includes(oldString)) {
+        const adjusted = findBackslashAdjustedMatch(content, oldString);
+        if (adjusted !== undefined) {
+          oldString = adjusted.adjusted;
+          newString = applyBackslashFix(
+            newString,
+            adjusted.variant as 'escape' | 'collapse-only' | 'unescape-seq-first',
+          );
+        }
+      }
+
       if (!replaceAll) {
         let count = 0;
         let pos = 0;
         while (pos < content.length) {
-          const idx = content.indexOf(args.old_string, pos);
+          const idx = content.indexOf(oldString, pos);
           if (idx === -1) break;
           count++;
-          pos = idx + args.old_string.length;
+          pos = idx + oldString.length;
         }
 
         if (count === 0) {
-          return { isError: true, output: `old_string not found in ${args.path}, the file contents may be out of date. Please use the Read Tool to reload the content.
+          const hint = buildNotFoundHint(content, oldString, args.path);
+          return { isError: true, output: `old_string not found in ${args.path}. ${hint}\nThe file contents may be out of date. Please use the Read Tool to reload the content.
 ` };
         }
         if (count > 1) {
@@ -129,22 +220,23 @@ export class EditTool implements BuiltinTool<EditInput> {
           };
         }
 
-        const newContent = replaceOnceLiteral(content, args.old_string, args.new_string);
+        const newContent = replaceOnceLiteral(content, oldString, newString);
         await this.kaos.writeText(
           safePath,
           materializeModelText(newContent, modelView.lineEndingStyle),
         );
         return { output: `Replaced 1 occurrence in ${args.path}` };
       }
 
-      const parts = content.split(args.old_string);
+      const parts = content.split(oldString);
       const replacementCount = parts.length - 1;
       if (replacementCount === 0) {
-        return { isError: true, output: `old_string not found in ${args.path}, the file contents may be out of date. Please use the Read Tool to reload the content.
+        const hint = buildNotFoundHint(content, oldString, args.path);
+        return { isError: true, output: `old_string not found in ${args.path}. ${hint}\nThe file contents may be out of date. Please use the Read Tool to reload the content.
 ` };
       }
 
-      const newContent = parts.join(args.new_string);
+      const newContent = parts.join(newString);
       await this.kaos.writeText(
         safePath,
         materializeModelText(newContent, modelView.lineEndingStyle),

packages/agent-core/test/tools/edit.test.ts

@@ -433,4 +433,83 @@ describe('EditTool', () => {
     expect(result.isError).toBeFalsy();
     expect(writeText).toHaveBeenCalledWith('/workspace-sneaky/test.txt', 'new');
   });
+
+  it('matches files with multiple literal backslashes (Rust string escapes)', async () => {
+    const writeText = vi.fn().mockResolvedValue(0);
+    const fileContent = 'let s = "hello\\nworld";\nlet t = "foo\\tbar";';
+    const tool = new EditTool(
+      createFakeKaos({
+        readText: vi.fn().mockResolvedValue(fileContent),
+        writeText,
+      }),
+      PERMISSIVE_WORKSPACE,
+    );
+
+    const result = await executeTool(tool,
+      context({
+        path: '/tmp/rust.rs',
+        old_string: 'let s = "hello\\nworld";',
+        new_string: 'let s = "hello\\nuniverse";',
+      }),
+    );
+
+    expect(result.output).toContain('Replaced 1 occurrence');
+    expect(writeText).toHaveBeenCalledWith(
+      '/tmp/rust.rs',
+      'let s = "hello\\nuniverse";\nlet t = "foo\\tbar";',
+    );
+  });
+
+  it('recovers when old_string has real newlines but file has literal backslash-n', async () => {
+    const writeText = vi.fn().mockResolvedValue(0);
+    // File has backslash + n (two chars): the literal string \n
+    const fileContent = 'path = "C:\\nUsers\\nadmin";';
+    const tool = new EditTool(
+      createFakeKaos({
+        readText: vi.fn().mockResolvedValue(fileContent),
+        writeText,
+      }),
+      PERMISSIVE_WORKSPACE,
+    );
+
+    // LLM sent real newlines (\n as one char) instead of backslash + n (two chars).
+    // The unescape fallback should convert \n → backslash + n to match the file.
+    const result = await executeTool(tool,
+      context({
+        path: '/tmp/config.txt',
+        old_string: 'path = "C:\nUsers\nadmin";',
+        new_string: 'path = "D:\nUsers\nadmin";',
+      }),
+    );
+
+    expect(result.output).toContain('Replaced 1 occurrence');
+    expect(writeText).toHaveBeenCalledWith(
+      '/tmp/config.txt',
+      'path = "D:\\nUsers\\nadmin";',
+    );
+  });
+
+  it('provides a diagnostic hint when old_string with backslashes is not found', async () => {
+    const writeText = vi.fn().mockResolvedValue(0);
+    const tool = new EditTool(
+      createFakeKaos({
+        readText: vi.fn().mockResolvedValue('alpha beta gamma'),
+        writeText,
+      }),
+      PERMISSIVE_WORKSPACE,
+    );
+
+    const result = await executeTool(tool,
+      context({
+        path: '/tmp/a.txt',
+        old_string: 'delta\\nepsilon',
+        new_string: 'new',
+      }),
+    );
+
+    expect(result).toMatchObject({ isError: true });
+    expect(result.output).toContain('old_string not found');
+    expect(result.output).toContain('The file does not contain a line matching');
+    expect(writeText).not.toHaveBeenCalled();
+  });
 });