From 43ee9b77ed6836860b11eed52e333cd15caa432d Mon Sep 17 00:00:00 2001 From: Ramiz Wachtler Date: Thu, 30 Apr 2026 12:31:29 +0200 Subject: [PATCH] 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 --- .../coding-agent/src/core/tools/edit-diff.ts | 5 +- packages/coding-agent/src/core/tools/edit.ts | 6 +- packages/coding-agent/test/tools.test.ts | 63 ++++++++++++++++++- 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/src/core/tools/edit-diff.ts b/packages/coding-agent/src/core/tools/edit-diff.ts index bc47f56e..a7d4bb70 100644 --- a/packages/coding-agent/src/core/tools/edit-diff.ts +++ b/packages/coding-agent/src/core/tools/edit-diff.ts @@ -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 diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 24106544..567c593a 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -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; } diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index dace48b2..56f607b1 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -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", () => {