Skip to content

Commit bf77769

Browse files
committed
🤖 refactor: split file edit tool errors into user/agent messages
- Add 'note' field to file edit error results for agent-specific guidance - Introduce EDIT_FAILED_NOTE_PREFIX constant for consistent messaging - Remove redundant WRITE_DENIED_PREFIX from error messages - Simplify UI error detection to check success field directly - Update tool descriptions to warn that edits may fail Agent notes provide clear failure signals and recovery steps: - 'EDIT FAILED - file was NOT modified' - Specific problem explanation - Concrete remediation (read file, retry with correct params) This addresses the issue where agents continue after file edit failures by making failures more explicit upfront (tool descriptions) and providing targeted recovery guidance (note field) separate from user-facing errors. _Generated with `cmux`_
1 parent 8b6d39a commit bf77769

9 files changed

Lines changed: 111 additions & 30 deletions

File tree

src/components/tools/FileEditToolCall.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { useCopyToClipboard } from "@/hooks/useCopyToClipboard";
2323
import { TooltipWrapper, Tooltip } from "../Tooltip";
2424
import { DiffContainer, DiffRenderer, SelectableDiffRenderer } from "../shared/DiffRenderer";
2525
import { KebabMenu, type KebabMenuItem } from "../KebabMenu";
26-
import { WRITE_DENIED_PREFIX } from "@/types/tools";
26+
2727

2828
type FileEditOperationArgs =
2929
| FileEditReplaceStringToolArgs
@@ -99,9 +99,9 @@ export const FileEditToolCall: React.FC<FileEditToolCallProps> = ({
9999
status = "pending",
100100
onReviewNote,
101101
}) => {
102-
// Collapse WRITE DENIED errors by default since they're common and expected
103-
const isWriteDenied = result && !result.success && result.error?.startsWith(WRITE_DENIED_PREFIX);
104-
const initialExpanded = !isWriteDenied;
102+
// Collapse failed edits by default since they're common and expected
103+
const isFailed = result && !result.success;
104+
const initialExpanded = !isFailed;
105105

106106
const { expanded, toggleExpanded } = useToolExpansion(initialExpanded);
107107
const [showRaw, setShowRaw] = React.useState(false);

src/services/tools/fileCommon.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { createPatch } from "diff";
33
import type { FileStat, Runtime } from "@/runtime/Runtime";
44
import { SSHRuntime } from "@/runtime/SSHRuntime";
55

6-
// WRITE_DENIED_PREFIX moved to @/types/tools for frontend/backend sharing
7-
86
/**
97
* Maximum file size for file operations (1MB)
108
* Files larger than this should be processed with system tools like grep, sed, etc.

src/services/tools/file_edit_insert.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { FileEditInsertToolResult } from "@/types/tools";
33
import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools";
44
import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions";
55
import { validatePathInCwd, validateNoRedundantPrefix } from "./fileCommon";
6-
import { WRITE_DENIED_PREFIX } from "@/types/tools";
6+
import { EDIT_FAILED_NOTE_PREFIX } from "@/types/tools";
77
import { executeFileEditOperation } from "./file_edit_operation";
88
import { RuntimeError } from "@/runtime/Runtime";
99
import { fileExists } from "@/utils/runtime/fileExists";
@@ -40,14 +40,15 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
4040
if (pathValidation) {
4141
return {
4242
success: false,
43-
error: `${WRITE_DENIED_PREFIX} ${pathValidation.error}`,
43+
error: pathValidation.error,
4444
};
4545
}
4646

4747
if (line_offset < 0) {
4848
return {
4949
success: false,
50-
error: `${WRITE_DENIED_PREFIX} line_offset must be non-negative (got ${line_offset})`,
50+
error: `line_offset must be non-negative (got ${line_offset})`,
51+
note: `${EDIT_FAILED_NOTE_PREFIX} The line_offset must be >= 0.`,
5152
};
5253
}
5354

@@ -61,7 +62,8 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
6162
if (!create) {
6263
return {
6364
success: false,
64-
error: `${WRITE_DENIED_PREFIX} File not found: ${file_path}. To create it, set create: true`,
65+
error: `File not found: ${file_path}. To create it, set create: true`,
66+
note: `${EDIT_FAILED_NOTE_PREFIX} File does not exist. Set create: true to create it, or check the file path.`,
6567
};
6668
}
6769

@@ -72,7 +74,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
7274
if (err instanceof RuntimeError) {
7375
return {
7476
success: false,
75-
error: `${WRITE_DENIED_PREFIX} ${err.message}`,
77+
error: err.message,
7678
};
7779
}
7880
throw err;
@@ -90,6 +92,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
9092
return {
9193
success: false,
9294
error: `line_offset ${line_offset} is beyond file length (${lines.length} lines)`,
95+
note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lines.length} lines. Read the file to get current content, then retry.`,
9396
};
9497
}
9598

@@ -107,14 +110,14 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
107110
if (error && typeof error === "object" && "code" in error && error.code === "EACCES") {
108111
return {
109112
success: false,
110-
error: `${WRITE_DENIED_PREFIX} Permission denied: ${file_path}`,
113+
error: `Permission denied: ${file_path}`,
111114
};
112115
}
113116

114117
const message = error instanceof Error ? error.message : String(error);
115118
return {
116119
success: false,
117-
error: `${WRITE_DENIED_PREFIX} Failed to insert content: ${message}`,
120+
error: `Failed to insert content: ${message}`,
118121
};
119122
}
120123
},

src/services/tools/file_edit_operation.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { describe, test, expect, jest } from "@jest/globals";
22
import { executeFileEditOperation } from "./file_edit_operation";
3-
import { WRITE_DENIED_PREFIX } from "@/types/tools";
43
import type { Runtime } from "@/runtime/Runtime";
54

65
import { createTestToolConfig, getTestDeps } from "./testHelpers";
@@ -25,7 +24,7 @@ describe("executeFileEditOperation", () => {
2524

2625
expect(result.success).toBe(false);
2726
if (!result.success) {
28-
expect(result.error.startsWith(WRITE_DENIED_PREFIX)).toBe(true);
27+
expect(result.error).toContain("File operations are restricted to the workspace directory");
2928
}
3029
});
3130

src/services/tools/file_edit_operation.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/types/tools";
2-
import { WRITE_DENIED_PREFIX } from "@/types/tools";
32
import type { ToolConfiguration } from "@/utils/tools/tools";
43
import {
54
generateDiff,
@@ -19,6 +18,7 @@ type FileEditOperationResult<TMetadata> =
1918
| {
2019
success: false;
2120
error: string;
21+
note?: string; // Agent-only message (not displayed in UI)
2222
};
2323

2424
interface ExecuteFileEditOperationOptions<TMetadata> {
@@ -52,15 +52,15 @@ export async function executeFileEditOperation<TMetadata>({
5252
if (redundantPrefixValidation) {
5353
return {
5454
success: false,
55-
error: `${WRITE_DENIED_PREFIX} ${redundantPrefixValidation.error}`,
55+
error: redundantPrefixValidation.error,
5656
};
5757
}
5858

5959
const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime);
6060
if (pathValidation) {
6161
return {
6262
success: false,
63-
error: `${WRITE_DENIED_PREFIX} ${pathValidation.error}`,
63+
error: pathValidation.error,
6464
};
6565
}
6666

@@ -76,7 +76,7 @@ export async function executeFileEditOperation<TMetadata>({
7676
if (err instanceof RuntimeError) {
7777
return {
7878
success: false,
79-
error: `${WRITE_DENIED_PREFIX} ${err.message}`,
79+
error: err.message,
8080
};
8181
}
8282
throw err;
@@ -85,15 +85,15 @@ export async function executeFileEditOperation<TMetadata>({
8585
if (fileStat.isDirectory) {
8686
return {
8787
success: false,
88-
error: `${WRITE_DENIED_PREFIX} Path is a directory, not a file: ${resolvedPath}`,
88+
error: `Path is a directory, not a file: ${resolvedPath}`,
8989
};
9090
}
9191

9292
const sizeValidation = validateFileSize(fileStat);
9393
if (sizeValidation) {
9494
return {
9595
success: false,
96-
error: `${WRITE_DENIED_PREFIX} ${sizeValidation.error}`,
96+
error: sizeValidation.error,
9797
};
9898
}
9999

@@ -105,7 +105,7 @@ export async function executeFileEditOperation<TMetadata>({
105105
if (err instanceof RuntimeError) {
106106
return {
107107
success: false,
108-
error: `${WRITE_DENIED_PREFIX} ${err.message}`,
108+
error: err.message,
109109
};
110110
}
111111
throw err;
@@ -115,7 +115,8 @@ export async function executeFileEditOperation<TMetadata>({
115115
if (!operationResult.success) {
116116
return {
117117
success: false,
118-
error: `${WRITE_DENIED_PREFIX} ${operationResult.error}`,
118+
error: operationResult.error,
119+
note: operationResult.note, // Pass through agent-only message
119120
};
120121
}
121122

@@ -126,7 +127,7 @@ export async function executeFileEditOperation<TMetadata>({
126127
if (err instanceof RuntimeError) {
127128
return {
128129
success: false,
129-
error: `${WRITE_DENIED_PREFIX} ${err.message}`,
130+
error: err.message,
130131
};
131132
}
132133
throw err;
@@ -145,22 +146,22 @@ export async function executeFileEditOperation<TMetadata>({
145146
if (nodeError.code === "ENOENT") {
146147
return {
147148
success: false,
148-
error: `${WRITE_DENIED_PREFIX} File not found: ${filePath}`,
149+
error: `File not found: ${filePath}`,
149150
};
150151
}
151152

152153
if (nodeError.code === "EACCES") {
153154
return {
154155
success: false,
155-
error: `${WRITE_DENIED_PREFIX} Permission denied: ${filePath}`,
156+
error: `Permission denied: ${filePath}`,
156157
};
157158
}
158159
}
159160

160161
const message = error instanceof Error ? error.message : String(error);
161162
return {
162163
success: false,
163-
error: `${WRITE_DENIED_PREFIX} Failed to edit file: ${message}`,
164+
error: `Failed to edit file: ${message}`,
164165
};
165166
}
166167
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { test, expect } from "bun:test";
2+
import { handleStringReplace, handleLineReplace } from "./file_edit_replace_shared";
3+
4+
test("file_edit_replace_string error includes agent note field", () => {
5+
const result = handleStringReplace(
6+
{
7+
file_path: "test.ts",
8+
old_string: "nonexistent",
9+
new_string: "replacement",
10+
},
11+
"some file content"
12+
);
13+
14+
expect(result.success).toBe(false);
15+
if (!result.success) {
16+
expect(result.error).toContain("old_string not found");
17+
expect(result.note).toBeDefined();
18+
expect(result.note).toContain("EDIT FAILED");
19+
expect(result.note).toContain("file was NOT modified");
20+
}
21+
});
22+
23+
test("file_edit_replace_string ambiguous match error includes note", () => {
24+
const result = handleStringReplace(
25+
{
26+
file_path: "test.ts",
27+
old_string: "duplicate",
28+
new_string: "replacement",
29+
},
30+
"duplicate text with duplicate word"
31+
);
32+
33+
expect(result.success).toBe(false);
34+
if (!result.success) {
35+
expect(result.error).toContain("appears 2 times");
36+
expect(result.note).toBeDefined();
37+
expect(result.note).toContain("EDIT FAILED");
38+
expect(result.note).toContain("file was NOT modified");
39+
}
40+
});
41+
42+
test("file_edit_replace_lines validation error includes note", () => {
43+
const result = handleLineReplace(
44+
{
45+
file_path: "test.ts",
46+
start_line: 10,
47+
end_line: 9,
48+
new_lines: ["new content"],
49+
},
50+
"line 1\nline 2"
51+
);
52+
53+
expect(result.success).toBe(false);
54+
if (!result.success) {
55+
expect(result.error).toContain("end_line must be >= start_line");
56+
expect(result.note).toBeDefined();
57+
expect(result.note).toContain("EDIT FAILED");
58+
expect(result.note).toContain("file was NOT modified");
59+
}
60+
});

src/services/tools/file_edit_replace_shared.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* providing the core logic while keeping the tool definitions simple for AI providers.
66
*/
77

8+
import { EDIT_FAILED_NOTE_PREFIX } from "@/types/tools";
9+
810
interface OperationMetadata {
911
edits_applied: number;
1012
lines_replaced?: number;
@@ -20,6 +22,7 @@ export interface OperationResult {
2022
export interface OperationError {
2123
success: false;
2224
error: string;
25+
note?: string; // Agent-only message (not displayed in UI)
2326
}
2427

2528
export type OperationOutcome = OperationResult | OperationError;
@@ -53,6 +56,7 @@ export function handleStringReplace(
5356
success: false,
5457
error:
5558
"old_string not found in file. The text to replace must exist exactly as written in the file.",
59+
note: `${EDIT_FAILED_NOTE_PREFIX} The old_string does not exist in the file. Read the file first to get the exact current content, then retry.`,
5660
};
5761
}
5862

@@ -63,13 +67,15 @@ export function handleStringReplace(
6367
return {
6468
success: false,
6569
error: `old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`,
70+
note: `${EDIT_FAILED_NOTE_PREFIX} The old_string matched ${occurrences} locations. Add more surrounding context to make it unique, or set replace_count=${occurrences} to replace all occurrences.`,
6671
};
6772
}
6873

6974
if (replaceCount > occurrences && replaceCount !== -1) {
7075
return {
7176
success: false,
7277
error: `replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`,
78+
note: `${EDIT_FAILED_NOTE_PREFIX} The replace_count=${replaceCount} is too high. Retry with replace_count=${occurrences} or -1.`,
7379
};
7480
}
7581

@@ -123,13 +129,15 @@ export function handleLineReplace(
123129
return {
124130
success: false,
125131
error: `start_line must be >= 1 (received ${args.start_line}).`,
132+
note: `${EDIT_FAILED_NOTE_PREFIX} Line numbers must be >= 1.`,
126133
};
127134
}
128135

129136
if (args.end_line < args.start_line) {
130137
return {
131138
success: false,
132139
error: `end_line must be >= start_line (received start ${args.start_line}, end ${args.end_line}).`,
140+
note: `${EDIT_FAILED_NOTE_PREFIX} The end_line must be >= start_line.`,
133141
};
134142
}
135143

@@ -139,6 +147,7 @@ export function handleLineReplace(
139147
return {
140148
success: false,
141149
error: `start_line ${args.start_line} exceeds current file length (${lines.length}).`,
150+
note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lines.length} lines. Read the file to get current content, then retry.`,
142151
};
143152
}
144153

@@ -149,6 +158,7 @@ export function handleLineReplace(
149158
return {
150159
success: false,
151160
error: `expected_lines validation failed. Current lines [${currentRange.join("\n")}] differ from expected [${args.expected_lines.join("\n")}].`,
161+
note: `${EDIT_FAILED_NOTE_PREFIX} The file content changed since you last read it. Read the file again and retry.`,
152162
};
153163
}
154164

src/types/tools.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export interface FileEditDiffSuccessBase {
6666
export interface FileEditErrorResult {
6767
success: false;
6868
error: string;
69+
note?: string; // Agent-only message (not displayed in UI)
6970
}
7071

7172
export interface FileEditReplaceStringToolArgs {
@@ -125,6 +126,12 @@ export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorRe
125126
*/
126127
export const WRITE_DENIED_PREFIX = "WRITE DENIED, FILE UNMODIFIED:";
127128

129+
/**
130+
* Prefix for edit failure notes (agent-only messages).
131+
* This prefix signals to the agent that the file was not modified.
132+
*/
133+
export const EDIT_FAILED_NOTE_PREFIX = "EDIT FAILED - file was NOT modified.";
134+
128135
export type FileEditToolArgs =
129136
| FileEditReplaceStringToolArgs
130137
| FileEditReplaceLinesToolArgs

0 commit comments

Comments
 (0)