From 893c525578d84228d2ce4cee3f94f887e55e45da Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:48:01 -0400 Subject: [PATCH] fix(read-tool): clamp offset to file bounds instead of throwing (#3007) (#3042) When an agent requests read(file, offset: 30) on a 13-line file, the read tool threw "Offset 30 is beyond end of file" which propagated as invalid JSON downstream during milestone completion. Now clamps the offset to the last line and prepends a notice, allowing the agent to continue with valid content. Fixes both read.ts and hashline-read.ts variants. Closes #3007 Co-authored-by: Claude Opus 4.6 --- .../src/core/tools/hashline-read.ts | 14 ++- .../pi-coding-agent/src/core/tools/read.ts | 18 ++- src/tests/read-tool-offset-clamp.test.ts | 106 ++++++++++++++++++ 3 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 src/tests/read-tool-offset-clamp.test.ts diff --git a/packages/pi-coding-agent/src/core/tools/hashline-read.ts b/packages/pi-coding-agent/src/core/tools/hashline-read.ts index fc2da81eb..f7d944d14 100644 --- a/packages/pi-coding-agent/src/core/tools/hashline-read.ts +++ b/packages/pi-coding-agent/src/core/tools/hashline-read.ts @@ -123,12 +123,15 @@ export function createHashlineReadTool(cwd: string, options?: HashlineReadToolOp const allLines = textContent.split("\n"); const totalFileLines = allLines.length; - const startLine = offset ? Math.max(0, offset - 1) : 0; - const startLineDisplay = startLine + 1; + let startLine = offset ? Math.max(0, offset - 1) : 0; + // Clamp offset to file bounds instead of throwing (#3007) + let offsetClamped = false; if (startLine >= allLines.length) { - throw new Error(`Offset ${offset} is beyond end of file (${allLines.length} lines total)`); + startLine = Math.max(0, allLines.length - 1); + offsetClamped = true; } + const startLineDisplay = startLine + 1; let selectedContent: string; let userLimitedLines: number | undefined; @@ -172,6 +175,11 @@ export function createHashlineReadTool(cwd: string, options?: HashlineReadToolOp outputText = formatHashLines(truncation.content, startLineDisplay); } + // Prepend clamp notice so the agent knows offset was adjusted + if (offsetClamped) { + outputText = `[Offset ${offset} beyond end of file (${totalFileLines} lines). Clamped to line ${startLineDisplay}.]\n\n${outputText}`; + } + content = [{ type: "text", text: outputText }]; } diff --git a/packages/pi-coding-agent/src/core/tools/read.ts b/packages/pi-coding-agent/src/core/tools/read.ts index c2f23e60a..309e43b57 100644 --- a/packages/pi-coding-agent/src/core/tools/read.ts +++ b/packages/pi-coding-agent/src/core/tools/read.ts @@ -133,13 +133,18 @@ export function createReadTool(cwd: string, options?: ReadToolOptions): AgentToo const totalFileLines = allLines.length; // Apply offset if specified (1-indexed to 0-indexed) - const startLine = offset ? Math.max(0, offset - 1) : 0; - const startLineDisplay = startLine + 1; // For display (1-indexed) + let startLine = offset ? Math.max(0, offset - 1) : 0; - // Check if offset is out of bounds + // Clamp offset to file bounds instead of throwing (#3007). + // When an agent requests offset:30 on a 13-line file, return + // the last line with a notice rather than an error that + // propagates as invalid JSON downstream. + let offsetClamped = false; if (startLine >= allLines.length) { - throw new Error(`Offset ${offset} is beyond end of file (${allLines.length} lines total)`); + startLine = Math.max(0, allLines.length - 1); + offsetClamped = true; } + const startLineDisplay = startLine + 1; // For display (1-indexed) // If limit is specified by user, use it; otherwise we'll let truncateHead decide let selectedContent: string; @@ -187,6 +192,11 @@ export function createReadTool(cwd: string, options?: ReadToolOptions): AgentToo outputText = truncation.content; } + // Prepend clamp notice so the agent knows offset was adjusted + if (offsetClamped) { + outputText = `[Offset ${offset} beyond end of file (${totalFileLines} lines). Clamped to line ${startLineDisplay}.]\n\n${outputText}`; + } + content = [{ type: "text", text: outputText }]; } diff --git a/src/tests/read-tool-offset-clamp.test.ts b/src/tests/read-tool-offset-clamp.test.ts new file mode 100644 index 000000000..4dc4c5e78 --- /dev/null +++ b/src/tests/read-tool-offset-clamp.test.ts @@ -0,0 +1,106 @@ +/** + * Tests for read tool offset clamping (#3007). + * + * When offset exceeds file length, the read tool should clamp to the + * last line instead of throwing, preventing downstream JSON parse errors + * in auto-mode milestone completion. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { createReadTool } from "../../packages/pi-coding-agent/src/core/tools/read.ts"; + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function makeTmpDir(): { dir: string; cleanup: () => void } { + const dir = mkdtempSync(join(tmpdir(), "read-tool-test-")); + return { dir, cleanup: () => rmSync(dir, { recursive: true, force: true }) }; +} + +function writeLines(dir: string, name: string, lineCount: number): string { + const lines = Array.from({ length: lineCount }, (_, i) => `Line ${i + 1}: content`); + const filePath = join(dir, name); + writeFileSync(filePath, lines.join("\n")); + return filePath; +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Offset beyond file bounds — should clamp, not throw (#3007) +// ═══════════════════════════════════════════════════════════════════════════ + +test("read tool: offset exceeding file length should NOT throw (#3007)", async (t) => { + const { dir, cleanup } = makeTmpDir(); + t.after(cleanup); + writeLines(dir, "small-artifact.md", 13); + + const readTool = createReadTool(dir); + + // offset 30 on a 13-line file — exact reproduction of #3007 + const result = await readTool.execute("test-call", { + path: "small-artifact.md", + offset: 30, + }); + + assert.ok(result, "should return a result, not throw"); + assert.ok(result.content, "should have content"); + assert.ok(result.content.length > 0, "should have at least one content block"); + + const text = (result.content[0] as any).text as string; + assert.ok(typeof text === "string", "first content block should be text"); + // Should include the last line of the file (clamped) + assert.ok(text.includes("Line 13"), "should include last line of file after clamping"); +}); + +test("read tool: offset 100 on a 5-line file clamps to last line", async (t) => { + const { dir, cleanup } = makeTmpDir(); + t.after(cleanup); + writeLines(dir, "tiny-file.txt", 5); + + const readTool = createReadTool(dir); + const result = await readTool.execute("test-call", { + path: "tiny-file.txt", + offset: 100, + }); + + const text = (result.content[0] as any).text as string; + assert.ok(text.includes("Line 5"), "should include the last line of the file"); +}); + +test("read tool: offset at exact last line works normally", async (t) => { + const { dir, cleanup } = makeTmpDir(); + t.after(cleanup); + writeLines(dir, "exact-offset.txt", 5); + + const readTool = createReadTool(dir); + // offset 5 on a 5-line file — should return line 5 (valid, no clamping needed) + const result = await readTool.execute("test-call", { + path: "exact-offset.txt", + offset: 5, + }); + + const text = (result.content[0] as any).text as string; + assert.ok(text.includes("Line 5"), "should include line 5"); +}); + +test("read tool: clamped offset includes notice about adjustment", async (t) => { + const { dir, cleanup } = makeTmpDir(); + t.after(cleanup); + writeLines(dir, "notice-test.md", 10); + + const readTool = createReadTool(dir); + const result = await readTool.execute("test-call", { + path: "notice-test.md", + offset: 50, + }); + + const text = (result.content[0] as any).text as string; + // Should contain some notice that the offset was adjusted + assert.ok( + text.includes("clamped") || text.includes("adjusted") || text.includes("beyond"), + `should indicate offset was clamped, got: ${text.slice(0, 200)}`, + ); +});