Fix read/edit tool error handling so upstream errors surface (#163)

Fixes #162:
- guard both tool callbacks so they return the host-provided error
instead of crashing on missing content
- skip writeTextFile in the edit tool when the initial read fails,
mirroring the read tool behavior
- add regression tests

---------

Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>

src/mcp-server.ts

@@ -101,15 +101,19 @@ Usage:
             };
           }
 
-          const content = await agent.readTextFile({
+          const readResponse = await agent.readTextFile({
             sessionId,
             path: input.file_path,
             line: input.offset,
             limit: input.limit,
           });
 
+          if (typeof readResponse?.content !== "string") {
+            throw new Error(`No file contents for ${input.file_path}.`);
+          }
+
           // Extract lines with byte limit enforcement
-          const result = extractLinesWithByteLimit(content.content, defaults.maxFileSize);
+          const result = extractLinesWithByteLimit(readResponse.content, defaults.maxFileSize);
 
           // Construct informative message about what was read
           let readInfo = "";
@@ -253,47 +257,62 @@ Usage:
         },
       },
       async (input) => {
-        const session = agent.sessions[sessionId];
-        if (!session) {
+        try {
+          const session = agent.sessions[sessionId];
+          if (!session) {
+            return {
+              content: [
+                {
+                  type: "text",
+                  text: "The user has left the building",
+                },
+              ],
+            };
+          }
+
+          const readResponse = await agent.readTextFile({
+            sessionId,
+            path: input.file_path,
+          });
+
+          if (typeof readResponse?.content !== "string") {
+            throw new Error(`No file contents for ${input.file_path}.`);
+          }
+
+          const { newContent } = replaceAndCalculateLocation(readResponse.content, [
+            {
+              oldText: input.old_string,
+              newText: input.new_string,
+              replaceAll: input.replace_all,
+            },
+          ]);
+
+          const patch = diff.createPatch(input.file_path, readResponse.content, newContent);
+
+          await agent.writeTextFile({
+            sessionId,
+            path: input.file_path,
+            content: newContent,
+          });
+
           return {
             content: [
               {
                 type: "text",
-                text: "The user has left the building",
+                text: patch,
+              },
+            ],
+          };
+        } catch (error: any) {
+          return {
+            content: [
+              {
+                type: "text",
+                text: "Editing file failed: " + (error?.message ?? String(error)),
               },
             ],
           };
         }
-
-        const { content } = await agent.readTextFile({
-          sessionId,
-          path: input.file_path,
-        });
-
-        const { newContent } = replaceAndCalculateLocation(content, [
-          {
-            oldText: input.old_string,
-            newText: input.new_string,
-            replaceAll: input.replace_all,
-          },
-        ]);
-
-        const patch = diff.createPatch(input.file_path, content, newContent);
-
-        await agent.writeTextFile({
-          sessionId,
-          path: input.file_path,
-          content: newContent,
-        });
-
-        return {
-          content: [
-            {
-              type: "text",
-              text: patch,
-            },
-          ],
-        };
       },
     );
   }

src/tests/mcp-server-read.test.ts

@@ -0,0 +1,72 @@
+import { describe, expect, it, vi } from "vitest";
+import { createMcpServer } from "../mcp-server.js";
+
+const SESSION_ID = "session-id";
+
+function getTool(server: any, name: string) {
+  const tools = server?._registeredTools;
+  if (!tools || !tools[name]) {
+    throw new Error(`Tool ${name} not registered`);
+  }
+  return tools[name];
+}
+
+function createAgent(overrides: Record<string, unknown> = {}) {
+  return {
+    sessions: {
+      [SESSION_ID]: {},
+    },
+    readTextFile: vi.fn(),
+    writeTextFile: vi.fn(),
+    backgroundTerminals: {},
+    clientCapabilities: {},
+    client: {
+      sessionUpdate: vi.fn(),
+      createTerminal: vi.fn(),
+      requestPermission: vi.fn(),
+    },
+    ...overrides,
+  };
+}
+
+describe("MCP server read-related tools", () => {
+  it("reports missing file contents instead of crashing the read tool", async () => {
+    const agent = createAgent();
+    agent.readTextFile.mockResolvedValue({});
+
+    const server = createMcpServer(agent as any, SESSION_ID, {
+      fs: { readTextFile: true },
+    });
+
+    const readTool = getTool(server, "Read");
+
+    const result = await readTool.callback({
+      file_path: "/tmp/file.txt",
+      offset: 1,
+      limit: 2000,
+    });
+
+    expect(result.content[0].text).toBe("Reading file failed: No file contents for /tmp/file.txt.");
+  });
+
+  it("reports missing contents inside the edit tool and skips writes", async () => {
+    const agent = createAgent();
+    agent.readTextFile.mockResolvedValue({});
+
+    const server = createMcpServer(agent as any, SESSION_ID, {
+      fs: { readTextFile: true, writeTextFile: true },
+    });
+
+    const editTool = getTool(server, "Edit");
+
+    const result = await editTool.callback({
+      file_path: "/tmp/file.txt",
+      old_string: "foo",
+      new_string: "bar",
+      replace_all: false,
+    });
+
+    expect(result.content[0].text).toBe("Editing file failed: No file contents for /tmp/file.txt.");
+    expect(agent.writeTextFile).not.toHaveBeenCalled();
+  });
+});