Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-edit-backslash-handling.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions packages/agent-core/src/tools/builtin/file/edit.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
97 changes: 89 additions & 8 deletions packages/agent-core/src/tools/builtin/file/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,73 @@ 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 `old_string`.
*
* Models sometimes double-escape backslashes when encoding old_string in
* JSON (e.g. sending `\\\\n` when the file contains `\\n`). This helper
* tries progressively less-escaped variants and returns the first one
* that matches in `content`, or `undefined` if none match.
*/
function findBackslashAdjustedMatch(
content: string,
oldString: string,
): { adjusted: string; variant: string } | undefined {
const variants: Array<{ adjusted: string; variant: string }> = [];

// Variant 1: the LLM double-escaped — unescape one level.
// "\\\\n" in JS string → "\\n" (backslash + n), but the file has "\n" (backslash + n).
// If old_string contains sequences like "\\n" that the file doesn't have,
// try replacing "\\n" → "\n" etc.
const unescaped = oldString
.replaceAll('\\n', '\n')
.replaceAll('\\t', '\t')
.replaceAll('\\r', '\r')
.replaceAll('\\\\', '\\');
Comment thread
LifeJiggy marked this conversation as resolved.
Outdated
if (unescaped !== oldString) {
variants.push({ adjusted: unescaped, variant: 'unescape-sequences' });
}

// Variant 2: the LLM under-escaped — the file has literal double-backslash
// but old_string has single. Try the reverse.
const overEscaped = oldString
.replaceAll(/\\n/g, '\\n')
.replaceAll(/\\t/g, '\\t')
.replaceAll(/\\r/g, '\\r');
// Only try this if it actually changed something and differs from variant 1.
if (overEscaped !== oldString && overEscaped !== unescaped) {
variants.push({ adjusted: overEscaped, variant: 'escape-sequences' });
}

for (const v of variants) {
if (content.includes(v.adjusted)) {
return v;
}
}
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;
Expand Down Expand Up @@ -106,18 +173,31 @@ export class EditTool implements BuiltinTool<EditInput> {
const content = modelView.text;
const replaceAll = args.replace_all ?? false;

// Try the original old_string first; if not found, attempt backslash
// encoding fixes that cover common LLM mis-encodings.
let oldString = args.old_string;
let backslashHint: string | undefined;
if (!content.includes(oldString)) {
const adjusted = findBackslashAdjustedMatch(content, oldString);
if (adjusted !== undefined) {
oldString = adjusted.adjusted;
backslashHint = ` (matched after ${adjusted.variant} adjustment)`;
}
}

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) {
Expand All @@ -129,18 +209,19 @@ export class EditTool implements BuiltinTool<EditInput> {
};
}

const newContent = replaceOnceLiteral(content, args.old_string, args.new_string);
const newContent = replaceOnceLiteral(content, oldString, args.new_string);
await this.kaos.writeText(
safePath,
materializeModelText(newContent, modelView.lineEndingStyle),
);
return { output: `Replaced 1 occurrence in ${args.path}` };
return { output: `Replaced 1 occurrence in ${args.path}${backslashHint ?? ''}` };
}

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.
` };
}

Expand All @@ -149,7 +230,7 @@ export class EditTool implements BuiltinTool<EditInput> {
safePath,
materializeModelText(newContent, modelView.lineEndingStyle),
);
return { output: `Replaced ${String(replacementCount)} occurrences in ${args.path}` };
return { output: `Replaced ${String(replacementCount)} occurrences in ${args.path}${backslashHint ?? ''}` };
} catch (error) {
const code = (error as { code?: unknown } | null)?.code;
if (code === 'EISDIR') {
Expand Down
79 changes: 79 additions & 0 deletions packages/agent-core/test/tools/edit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 double-escaped backslashes from LLM JSON encoding', async () => {
const writeText = vi.fn().mockResolvedValue(0);
// File has a literal backslash followed by 'n' (two chars).
const fileContent = 'path = "C:\\nUsers\\nadmin";';
const tool = new EditTool(
createFakeKaos({
readText: vi.fn().mockResolvedValue(fileContent),
writeText,
}),
PERMISSIVE_WORKSPACE,
);

// LLM sent double-escaped: in JS string this is "\\n" (backslash + n)
// but it was meant to match the literal "\n" in 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();
});
});
Loading