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 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-30 16:48:01 -04:00 committed by GitHub
parent 70e3d9d6c2
commit 893c525578
3 changed files with 131 additions and 7 deletions

View file

@ -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 }];
}

View file

@ -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 }];
}

View file

@ -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)}`,
);
});