Fix: Handle file read errors gracefully in resolving file states

.changeset/fix-file-operation-error-handling.md

@@ -0,0 +1,5 @@
+---
+"@perstack/runtime": patch
+---
+
+Handle file read errors gracefully in resolving PDF and image file states

CODE_REVIEW_REPORT.md

@@ -18,7 +18,7 @@ Overall, the codebase is well-structured with strong architectural decisions. Th
 | ✅ Verified   | 5     | Confirmed not an issue / working correctly |
 | 📝 Documented | 1     | Behavior documented, no code change needed |
 | ⏸️ Deferred   | 8     | Low priority / E2E scope / future work     |
-| 🔴 Open       | 5     | Runtime package issues (2025-12-03)        |
+| 🔴 Open       | 4     | Runtime package issues (2025-12-03)        |
 | 🟡 Low Prio   | 4     | Runtime minor issues (2025-12-03)          |
 
 ---
@@ -698,31 +698,16 @@ experts[expertKey] = toRuntimeExpert(expert)  // Mutates argument
 
 **Status**: 🔴 **Open**
 
+**Status**: ✅ **Fixed** — Commit `796f981`
+
 **Category**: Potential Bug  
 **Severity**: Minor
 
-**Location**: `packages/runtime/src/states/resolving-pdf-file.ts:31`, `resolving-image-file.ts:31`
-
-**Issue**: `readFile(path)` can throw if file doesn't exist or is inaccessible, but errors are not caught:
-
-```typescript
-const file = await readFile(path).then((buffer) => ({
-  encodedData: buffer.toString("base64"),
-  // ...
-}))
-```
+**Location**: `packages/runtime/src/states/resolving-pdf-file.ts`, `resolving-image-file.ts`
 
-**Impact**: File read errors will crash the runtime instead of being fed back to the LLM.
+**Issue**: `readFile(path)` can throw if file doesn't exist or is inaccessible.
 
-**Recommendation**: Wrap in try-catch and return error message to LLM:
-```typescript
-try {
-  const buffer = await readFile(path)
-  // ...
-} catch (error) {
-  files.push({ type: "textPart", text: `Error reading file: ${error.message}` })
-}
-```
+**Resolution**: Added try-catch blocks to handle file read errors gracefully. Errors are now returned as text parts to the LLM instead of crashing.
 
 ---
 
@@ -917,7 +902,7 @@ const metaInstruction = dedent`
 | #34   | Delegate expert not found error        | 🔴 Open         |
 | #35   | Skill object mutation                  | 🔴 Open         |
 | #36   | experts object mutation                | 🔴 Open         |
-| #37   | File operation error handling          | 🔴 Open         |
+| #37   | File operation error handling          | ✅ Fixed        |
 | #38   | RunSetting schema validation           | 🔴 Open         |
 | #39   | closeSkillManagers failure handling    | ✅ Fixed        |
 | #40   | EventEmitter listener errors           | 🟡 Low priority |
@@ -945,7 +930,6 @@ const metaInstruction = dedent`
 **Minor (Should Fix)**:
 - **#34**: Delegate expert not found — improve error message
 - **#35, #36**: Object mutation side effects — defensive copying recommended
-- **#37**: File read errors unhandled — should feed back to LLM
 - **#38**: RunSetting not validated — use Zod schema
 
 **By Design / Needs Clarification**:
@@ -972,3 +956,4 @@ const metaInstruction = dedent`
 | `5490ebb` | Refactor: Split SkillManager into separate classes                 |
 | `ced1aa6` | Fix: maxSteps off-by-one error in finishing step                   |
 | `dac71b5` | Fix: Handle individual close failures in closeSkillManagers        |
+| `796f981` | Fix: Handle file read errors gracefully in resolving file states   |

packages/runtime/src/states/resolving-image-file.test.ts

@@ -1,16 +1,22 @@
 import { createId } from "@paralleldrive/cuid2"
-import { describe, expect, it, vi } from "vitest"
+import { readFile } from "node:fs/promises"
+import { beforeEach, describe, expect, it, vi } from "vitest"
 import { createCheckpoint, createRunSetting, createStep } from "../../test/run-params.js"
 import { StateMachineLogics } from "../index.js"
 
 vi.mock("node:fs/promises", () => ({
-  readFile: vi.fn().mockImplementation(() => {
-    return Promise.resolve(Buffer.from("encoded_image"))
-  }),
+  readFile: vi.fn(),
 }))
 
+const mockReadFile = vi.mocked(readFile)
+
 describe("@perstack/runtime: StateMachineLogic['ResolvingImageFile']", () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
   it("processes image file tool result correctly", async () => {
+    mockReadFile.mockResolvedValue(Buffer.from("encoded_image"))
     const imageInfo = {
       path: "/test/image.png",
       mimeType: "image/png",
@@ -77,4 +83,50 @@ describe("@perstack/runtime: StateMachineLogic['ResolvingImageFile']", () => {
       ],
     })
   })
+
+  it("handles file read error gracefully", async () => {
+    mockReadFile.mockRejectedValue(new Error("ENOENT: no such file or directory"))
+    const imageInfo = {
+      path: "/nonexistent.png",
+      mimeType: "image/png",
+      size: 1024,
+    }
+    const setting = createRunSetting()
+    const checkpoint = createCheckpoint()
+    const step = createStep({
+      toolCall: {
+        id: "tc_123",
+        skillName: "@perstack/base",
+        toolName: "readImageFile",
+        args: { path: "/nonexistent.png" },
+      },
+      toolResult: {
+        id: "tr_123",
+        skillName: "@perstack/base",
+        toolName: "readImageFile",
+        result: [
+          {
+            type: "textPart" as const,
+            text: JSON.stringify(imageInfo),
+            id: createId(),
+          },
+        ],
+      },
+    })
+    const result = await StateMachineLogics.ResolvingImageFile({
+      setting,
+      checkpoint,
+      step,
+      eventListener: async () => {},
+      skillManagers: {},
+    })
+    expect(result.type).toBe("finishToolCall")
+    if (result.type !== "finishToolCall") throw new Error("Unexpected event type")
+    const toolResultPart = result.newMessages[0].contents[0]
+    if (toolResultPart.type !== "toolResultPart") throw new Error("Unexpected part type")
+    expect(toolResultPart.contents[0]).toMatchObject({
+      type: "textPart",
+      text: expect.stringContaining('Failed to read image file "/nonexistent.png"'),
+    })
+  })
 })

packages/runtime/src/states/resolving-image-file.ts

@@ -28,16 +28,19 @@ export async function resolvingImageFileLogic({
       continue
     }
     const { path, mimeType, size } = imageInfo
-    const file = await readFile(path).then((buffer) => ({
-      encodedData: buffer.toString("base64"),
-      mimeType,
-      size,
-    }))
-    files.push({
-      type: "imageInlinePart",
-      encodedData: file.encodedData,
-      mimeType: file.mimeType,
-    })
+    try {
+      const buffer = await readFile(path)
+      files.push({
+        type: "imageInlinePart",
+        encodedData: buffer.toString("base64"),
+        mimeType,
+      })
+    } catch (error) {
+      files.push({
+        type: "textPart",
+        text: `Failed to read image file "${path}": ${error instanceof Error ? error.message : String(error)}`,
+      })
+    }
   }
   return finishToolCall(setting, checkpoint, {
     newMessages: [

packages/runtime/src/states/resolving-pdf-file.test.ts

@@ -1,16 +1,22 @@
 import { createId } from "@paralleldrive/cuid2"
-import { describe, expect, it, vi } from "vitest"
+import { readFile } from "node:fs/promises"
+import { beforeEach, describe, expect, it, vi } from "vitest"
 import { createCheckpoint, createRunSetting, createStep } from "../../test/run-params.js"
 import { StateMachineLogics } from "../index.js"
 
 vi.mock("node:fs/promises", () => ({
-  readFile: vi.fn().mockImplementation(() => {
-    return Promise.resolve(Buffer.from("encoded_pdf_content"))
-  }),
+  readFile: vi.fn(),
 }))
 
+const mockReadFile = vi.mocked(readFile)
+
 describe("@perstack/runtime: StateMachineLogic['ResolvingPdfFile']", () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
   it("processes PDF file tool result correctly", async () => {
+    mockReadFile.mockResolvedValue(Buffer.from("encoded_pdf_content"))
     const pdfInfo = {
       path: "/test.pdf",
       mimeType: "application/pdf",
@@ -88,4 +94,49 @@ describe("@perstack/runtime: StateMachineLogic['ResolvingPdfFile']", () => {
       ],
     })
   })
+
+  it("handles file read error gracefully", async () => {
+    mockReadFile.mockRejectedValue(new Error("ENOENT: no such file or directory"))
+    const pdfInfo = {
+      path: "/nonexistent.pdf",
+      mimeType: "application/pdf",
+      size: 2048,
+    }
+    const setting = createRunSetting()
+    const checkpoint = createCheckpoint()
+    const step = createStep({
+      toolCall: {
+        id: "tc_123",
+        skillName: "@perstack/base",
+        toolName: "readPdfFile",
+        args: { path: "/nonexistent.pdf" },
+      },
+      toolResult: {
+        id: "tr_123",
+        skillName: "@perstack/base",
+        toolName: "readPdfFile",
+        result: [
+          {
+            type: "textPart" as const,
+            text: JSON.stringify(pdfInfo),
+            id: createId(),
+          },
+        ],
+      },
+    })
+    const result = await StateMachineLogics.ResolvingPdfFile({
+      setting,
+      checkpoint,
+      step,
+      eventListener: async () => {},
+      skillManagers: {},
+    })
+    expect(result.type).toBe("finishToolCall")
+    if (result.type !== "finishToolCall") throw new Error("Unexpected event type")
+    const userMessage = result.newMessages[1]
+    expect(userMessage.contents[0]).toMatchObject({
+      type: "textPart",
+      text: expect.stringContaining('Failed to read PDF file "/nonexistent.pdf"'),
+    })
+  })
 })

packages/runtime/src/states/resolving-pdf-file.ts

@@ -28,16 +28,19 @@ export async function resolvingPdfFileLogic({
       continue
     }
     const { path, mimeType, size } = pdfInfo
-    const file = await readFile(path).then((buffer) => ({
-      encodedData: buffer.toString("base64"),
-      mimeType,
-      size,
-    }))
-    files.push({
-      type: "fileInlinePart",
-      encodedData: file.encodedData,
-      mimeType: file.mimeType,
-    })
+    try {
+      const buffer = await readFile(path)
+      files.push({
+        type: "fileInlinePart",
+        encodedData: buffer.toString("base64"),
+        mimeType,
+      })
+    } catch (error) {
+      files.push({
+        type: "textPart",
+        text: `Failed to read PDF file "${path}": ${error instanceof Error ? error.message : String(error)}`,
+      })
+    }
   }
   return finishToolCall(setting, checkpoint, {
     newMessages: [