From 800bd02f27ba76986aa1a679f12cc2b8554be88b Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 21 Sep 2026 06:48:09 +0900 Subject: [PATCH 1/3] fix(read-file): propagate beyond-EOF error instead of reporting file as empty --- src/core/tools/ReadFileTool.ts | 2 +- src/core/tools/__tests__/readFileTool.spec.ts | 27 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 2107cfe21b..2c87c5e826 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -322,7 +322,7 @@ export class ReadFileTool extends BaseTool<"read_file"> { To read more: Use the read_file tool with offset=${nextOffset} and limit=${limit}. ${result.content}` - } else if (result.returnedLines === 0) { + } else if (result.returnedLines === 0 && result.totalLines === 0) { output = "Note: File is empty" } diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index 6c9e177d38..f5210691a8 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -17,6 +17,7 @@ import path from "path" import { isBinaryFile } from "isbinaryfile" import { readFileTool, ReadFileTool } from "../ReadFileTool" +import type { Task } from "../../task/Task" import { formatResponse } from "../../prompts/responses" import { validateImageForProcessing, @@ -581,7 +582,31 @@ describe("ReadFileTool", () => { await readFileTool.execute({ path: "empty.ts" }, mockTask as any, callbacks) - expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("empty")) + expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("Note: File is empty")) + }) + + it("should propagate beyond-EOF error instead of reporting file as empty", async () => { + const mockTask = createMockTask() + const callbacks = createMockCallbacks() + + mockedFsReadFile.mockResolvedValue(Buffer.from("line1\nline2\nline3\nline4\nline5")) + mockedReadWithSlice.mockReturnValue({ + content: "Error: offset 99 is beyond file end (5 lines)", + returnedLines: 0, + totalLines: 5, + wasTruncated: false, + includedRanges: [], + }) + + await readFileTool.execute( + { path: "test.ts", mode: "slice", offset: 100 }, + mockTask as unknown as Task, + callbacks, + ) + + expect(mockedReadWithSlice).toHaveBeenCalledWith(expect.any(String), 99, expect.any(Number)) + expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("beyond file end")) + expect(callbacks.pushToolResult).not.toHaveBeenCalledWith(expect.stringContaining("Note: File is empty")) }) }) From 5e4c7b961e74c53ba8067e1c9e72ea6a110d7542 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Fri, 9 Oct 2026 08:52:34 +0900 Subject: [PATCH 2/3] fix(read-file): report 1-based offset in beyond-EOF error What: readWithSlice now prints offset + 1 in its beyond-file-end error and both spec fixtures assert the 1-based value. Why: read_file takes a 1-based offset from the model, but the error text showed the 0-based value passed to readWithSlice (offset 100 read back as "offset 99"), disagreeing with indentation mode which reports 1-based lines. Impact: the model sees the same line number it asked for, so both read modes report consistent line numbers in errors. --- src/core/tools/__tests__/readFileTool.spec.ts | 2 +- src/integrations/misc/__tests__/indentation-reader.spec.ts | 2 +- src/integrations/misc/indentation-reader.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index f5210691a8..72dc1d24f6 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -591,7 +591,7 @@ describe("ReadFileTool", () => { mockedFsReadFile.mockResolvedValue(Buffer.from("line1\nline2\nline3\nline4\nline5")) mockedReadWithSlice.mockReturnValue({ - content: "Error: offset 99 is beyond file end (5 lines)", + content: "Error: offset 100 is beyond file end (5 lines)", returnedLines: 0, totalLines: 5, wasTruncated: false, diff --git a/src/integrations/misc/__tests__/indentation-reader.spec.ts b/src/integrations/misc/__tests__/indentation-reader.spec.ts index d46cb54277..27057e148a 100644 --- a/src/integrations/misc/__tests__/indentation-reader.spec.ts +++ b/src/integrations/misc/__tests__/indentation-reader.spec.ts @@ -283,7 +283,7 @@ describe("readWithSlice", () => { const result = readWithSlice(SIMPLE_CODE, 1000, 10) expect(result.returnedLines).toBe(0) - expect(result.content).toContain("Error") + expect(result.content).toContain("Error: offset 1001 is beyond file end") }) it("should handle negative offset", () => { diff --git a/src/integrations/misc/indentation-reader.ts b/src/integrations/misc/indentation-reader.ts index aecabd5982..f244498326 100644 --- a/src/integrations/misc/indentation-reader.ts +++ b/src/integrations/misc/indentation-reader.ts @@ -443,7 +443,7 @@ export function readWithSlice( if (offset < 0) offset = 0 if (offset >= totalLines) { return { - content: `Error: offset ${offset} is beyond file end (${totalLines} lines)`, + content: `Error: offset ${offset + 1} is beyond file end (${totalLines} lines)`, includedRanges: [], totalLines, returnedLines: 0, From 5cc4220b017463ea5d2fe5163bee0e5e524c054a Mon Sep 17 00:00:00 2001 From: myk1yt Date: Fri, 30 Oct 2026 11:36:46 +0900 Subject: [PATCH 3/3] test(read-file): add beyond-EOF integration case through real readWithSlice producer What: add one integration test to readFileTool.spec.ts that unmocks the indentation-reader, fs/promises, path and isbinaryfile modules, builds a ReadFileTool against a real temp file with 5 lines, and calls it with the 1-based offset 100. Why: the existing beyond-EOF regression test mocks readWithSlice, so it only proved that the tool forwards whatever string the stub returns. The 1-based offset in the user-facing message ("offset 100 is beyond file end") was asserted against a hand-written stub whose shape had to be kept in sync with indentation-reader.ts by hand. A change to the real producer would not have failed that test. Impact: the beyond-EOF error path is now covered end-to-end through the real producer against a real file. Verified by mutating indentation-reader.ts to emit the 0-based offset: the new test fails with "offset 99", and passes again once reverted. Test count 84 -> 85 in the core vitest config. No production code changed. --- src/core/tools/__tests__/readFileTool.spec.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index fee5fe2b37..52e04415ea 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -12,6 +12,7 @@ * - Output structure formatting */ +import os from "os" import path from "path" import type { ToolUse } from "../../../shared/tools" @@ -607,6 +608,47 @@ describe("ReadFileTool", () => { expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("beyond file end")) expect(callbacks.pushToolResult).not.toHaveBeenCalledWith(expect.stringContaining("Note: File is empty")) }) + + // Integration counterpart to the mocked case above: the real readWithSlice + // producer runs against a real file, so the error text and its 1-based + // offset are proven end-to-end instead of being asserted against a stub. + it("should propagate the real beyond-EOF error from the producer for a real file", async () => { + const realFs = await vi.importActual("fs/promises") + const realPath = await vi.importActual("path") + const tempDir = await realFs.mkdtemp(realPath.join(os.tmpdir(), "read-file-beyond-eof-")) + const fileName = "sample.txt" + await realFs.writeFile(realPath.join(tempDir, fileName), "line1\nline2\nline3\nline4\nline5", "utf8") + + vi.doUnmock("fs/promises") + vi.doUnmock("path") + vi.doUnmock("isbinaryfile") + vi.doUnmock("../../../integrations/misc/indentation-reader") + vi.resetModules() + + try { + const { ReadFileTool } = await import("../ReadFileTool") + const mockTask = { ...createMockTask(), cwd: tempDir } + const callbacks = createMockCallbacks() + + await new ReadFileTool().execute( + { path: fileName, mode: "slice", offset: 100 }, + mockTask as unknown as Task, + callbacks, + ) + + expect(mockedReadWithSlice).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).toHaveBeenCalledTimes(1) + expect(callbacks.pushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Error: offset 100 is beyond file end (5 lines)"), + ) + expect(callbacks.pushToolResult).not.toHaveBeenCalledWith( + expect.stringContaining("Note: File is empty"), + ) + } finally { + vi.resetModules() + await realFs.rm(tempDir, { recursive: true, force: true }).catch(() => {}) + } + }) }) describe("approval flow", () => {