fix(coding-agent): report edit access failures correctly (#3955)
* fix(coding-agent): report edit access failures correctly closes #3894 - classify edit and edit-preview access errors by errno - add regressions for missing, permission, and fallback cases - document the fix in the coding-agent changelog * chore: get rid of CHANGELOG.md entry * refactor(coding-agent): apply review suggestions - use single error msg * refactor(coding-agent): clean up test cases
This commit is contained in:
@@ -412,8 +412,9 @@ export async function computeEditsDiff(
|
||||
// Check if file exists and is readable
|
||||
try {
|
||||
await access(absolutePath, constants.R_OK);
|
||||
} catch {
|
||||
return { error: `File not found: ${path}` };
|
||||
} catch (error: unknown) {
|
||||
const errorMessage = error instanceof Error && "code" in error ? `Error code: ${error.code}` : String(error);
|
||||
return { error: `Could not write file: ${path}. ${errorMessage}.` };
|
||||
}
|
||||
|
||||
// Read the file
|
||||
|
||||
@@ -341,11 +341,13 @@ export function createEditToolDefinition(
|
||||
// Check if file exists.
|
||||
try {
|
||||
await ops.access(absolutePath);
|
||||
} catch {
|
||||
} catch (error: unknown) {
|
||||
const errorMessage =
|
||||
error instanceof Error && "code" in error ? `Error code: ${error.code}` : String(error);
|
||||
if (signal) {
|
||||
signal.removeEventListener("abort", onAbort);
|
||||
}
|
||||
reject(new Error(`File not found: ${path}`));
|
||||
reject(new Error(`Could not write file: ${path}. ${errorMessage}.`));
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
|
||||
import { chmodSync, existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { executeBashWithOperations } from "../src/core/bash-executor.js";
|
||||
import { createBashTool, createLocalBashOperations } from "../src/core/tools/bash.js";
|
||||
import { computeEditsDiff } from "../src/core/tools/edit-diff.js";
|
||||
import {
|
||||
createEditTool,
|
||||
createFindTool,
|
||||
@@ -253,6 +254,17 @@ describe("Coding Agent Tools", () => {
|
||||
).rejects.toThrow(/Could not find the exact text/);
|
||||
});
|
||||
|
||||
it("should include ENOENT when the edit target does not exist", async () => {
|
||||
const missingFile = join(testDir, "missing.txt");
|
||||
|
||||
await expect(
|
||||
editTool.execute("test-call-6b", {
|
||||
path: missingFile,
|
||||
edits: [{ oldText: "hello", newText: "world" }],
|
||||
}),
|
||||
).rejects.toThrow(`Could not write file: ${missingFile}. Error code: ENOENT.`);
|
||||
});
|
||||
|
||||
it("should fail if text appears multiple times", async () => {
|
||||
const testFile = join(testDir, "edit-test.txt");
|
||||
const originalContent = "foo foo foo";
|
||||
@@ -366,6 +378,55 @@ describe("Coding Agent Tools", () => {
|
||||
|
||||
expect(readFileSync(testFile, "utf-8")).toBe(originalContent);
|
||||
});
|
||||
|
||||
it("should include EACCES for read-only files", async () => {
|
||||
const testFile = join(testDir, "edit-readonly.txt");
|
||||
writeFileSync(testFile, "hello\n");
|
||||
chmodSync(testFile, 0o444);
|
||||
|
||||
await expect(
|
||||
editTool.execute("test-call-14", {
|
||||
path: testFile,
|
||||
edits: [{ oldText: "hello", newText: "world" }],
|
||||
}),
|
||||
).rejects.toThrow(`Could not write file: ${testFile}. Error code: EACCES.`);
|
||||
});
|
||||
|
||||
it("should include the original error message for unknown edit access errors", async () => {
|
||||
const genericFailureTool = createEditTool(testDir, {
|
||||
operations: {
|
||||
access: async () => {
|
||||
throw new Error("disk offline");
|
||||
},
|
||||
readFile: async () => Buffer.from("hello\n", "utf-8"),
|
||||
writeFile: async () => {},
|
||||
},
|
||||
});
|
||||
|
||||
await expect(
|
||||
genericFailureTool.execute("test-call-16", {
|
||||
path: "broken.txt",
|
||||
edits: [{ oldText: "hello", newText: "world" }],
|
||||
}),
|
||||
).rejects.toThrow("Could not write file: broken.txt. Error: disk offline.");
|
||||
});
|
||||
|
||||
it("should include ENOENT in diff preview for missing files", async () => {
|
||||
const missingFile = join(testDir, "missing-preview.txt");
|
||||
const result = await computeEditsDiff(missingFile, [{ oldText: "hello", newText: "world" }], testDir);
|
||||
|
||||
expect(result).toEqual({ error: `Could not write file: ${missingFile}. Error code: ENOENT.` });
|
||||
});
|
||||
|
||||
it("should include EACCES in diff preview for unreadable files", async () => {
|
||||
const unreadableFile = join(testDir, "unreadable-preview.txt");
|
||||
writeFileSync(unreadableFile, "hello\n");
|
||||
chmodSync(unreadableFile, 0o222);
|
||||
|
||||
const result = await computeEditsDiff(unreadableFile, [{ oldText: "hello", newText: "world" }], testDir);
|
||||
|
||||
expect(result).toEqual({ error: `Could not write file: ${unreadableFile}. Error code: EACCES.` });
|
||||
});
|
||||
});
|
||||
|
||||
describe("bash tool", () => {
|
||||
|
||||
Reference in New Issue
Block a user