Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 141 additions & 1 deletion src/plugins/read-file-guard-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ describe("readFileGuardPlugin", () => {
expect(result.content).toContain(" 2\tl2");
expect(result.content).toContain(" 3\tl3");
expect(result.content).not.toContain(" 4\tl4");
expect(result.content).toContain("Use offset=");
expect(result.content).toContain('Use path="tool-output:///');
expect(result.content).not.toContain("Use offset=");
});

test("rejects tool-output URIs when no blob reader is configured", async () => {
Expand Down Expand Up @@ -277,4 +278,143 @@ describe("readFileGuardPlugin", () => {
const result = await run({ id: "r4", name: "grep", arguments: { pattern: "x" } });
expect(result.content).toBe("FALLBACK");
});

test("a truncated read never asks the model to re-read the same path (CL-6961)", async () => {
await fixture("many-lines.txt", Array.from({ length: 10 }, (_, i) => `line-${i}`).join("\n"));
const plugin = readFileGuardPlugin(dir, {});
const middleware = plugin.middleware!(fallback);
const result = await middleware(
{ id: "c1", name: "read_file", arguments: { path: "many-lines.txt", limit: 4 } },
neverAbort(),
);
expect(result.content).not.toContain("Use offset=");
expect(String(result.content)).toContain('Use path="tool-output:///');
// The literal source path never reappears as the thing to read next.
expect(String(result.content)).not.toContain("many-lines.txt");
});

test("following the minted cursor resumes and eventually reads a large file to completion without any repeat call on the original path (CL-6961)", async () => {
const lines = Array.from({ length: 9_000 }, (_, i) => `line-${i} payload`);
await fixture("huge.txt", lines.join("\n"));
const plugin = readFileGuardPlugin(dir, {});
const middleware = plugin.middleware!(fallback);

const pathsRead: string[] = ["huge.txt"];
let result = await middleware(
{ id: "c1", name: "read_file", arguments: { path: "huge.txt" } },
neverAbort(),
);
let seen = 0;
let guard = 0;
for (;;) {
guard++;
expect(guard).toBeLessThan(50); // fails loudly instead of hanging on a broken cursor chain
const content = String(result.content);
const numbered = content.split("\n\n")[0] ?? "";
seen += numbered.trimEnd().split("\n").length;

const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(content);
if (match === undefined || match === null) break;
const nextPath = match[1] as string;
expect(pathsRead).not.toContain(nextPath); // every hop targets a fresh, distinct path
pathsRead.push(nextPath);

result = await middleware(
{ id: `c${pathsRead.length}`, name: "read_file", arguments: { path: nextPath } },
neverAbort(),
);
}

expect(seen).toBe(lines.length);
expect(pathsRead.length).toBeGreaterThan(1); // it actually paginated
// Never told to re-issue a call against the literal original path.
expect(pathsRead.filter((p) => p === "huge.txt").length).toBe(1);
});

test("a stale (already-consumed) cursor names the original path and offset instead of a dead end", async () => {
const absolutePath = await fixture(
"stale.txt",
Array.from({ length: 10 }, (_, i) => `line-${i}`).join("\n"),
);
const plugin = readFileGuardPlugin(dir, {});
const middleware = plugin.middleware!(fallback);
const first = await middleware(
{ id: "s1", name: "read_file", arguments: { path: "stale.txt", limit: 4 } },
neverAbort(),
);
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(String(first.content));
expect(match).not.toBeNull();
const cursorPath = (match as RegExpExecArray)[1] as string;

await middleware(
{ id: "s2", name: "read_file", arguments: { path: cursorPath } },
neverAbort(),
);
// Second use of the same, already-consumed cursor: distinct from a
// generic missing-blob error, this must name a followable next step —
// the original source and the offset to resume from — rather than
// leaving the model to re-read the whole file from scratch.
const replay = await middleware(
{ id: "s3", name: "read_file", arguments: { path: cursorPath } },
neverAbort(),
);
expect(replay.isError).toBe(true);
expect(String(replay.content)).toContain("already used");
expect(String(replay.content)).toContain(absolutePath);
expect(String(replay.content)).toMatch(/offset=4\b/);
});

test("an unknown tool-output URI against a real blobReader gets the production 'blob not found' error, not a stale-cursor message", async () => {
const blobReader = {
async read(uri: string): Promise<Uint8Array> {
throw new Error(`Blob not found for key: ${uri}`);
},
};
const result = await run(
{ id: "u1", name: "read_file", arguments: { path: "tool-output:///never-minted" } },
blobReader,
);
expect(result.isError).toBe(true);
expect(String(result.content)).toContain("Blob not found for key");
// Never a cursor's own wording, since this ID was never one of ours.
expect(String(result.content)).not.toContain("already used");
});

test("a stale cursor short-circuits before reaching a real blobReader's production 'blob not found' error", async () => {
const encoder = new TextEncoder();
const body = Array.from({ length: 8_000 }, (_, i) => `row-${i}`).join("\n");
const blobReader = {
async read(uri: string): Promise<Uint8Array> {
if (uri === "tool-output:///spill-1") return encoder.encode(body);
throw new Error(`Blob not found for key: ${uri}`);
},
};
const plugin = readFileGuardPlugin(dir, { blobReader });
const middleware = plugin.middleware!(fallback);

const first = await middleware(
{ id: "b1", name: "read_file", arguments: { path: "tool-output:///spill-1", limit: 5 } },
neverAbort(),
);
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(String(first.content));
expect(match).not.toBeNull();
const cursorPath = (match as RegExpExecArray)[1] as string;

await middleware(
{ id: "b2", name: "read_file", arguments: { path: cursorPath } },
neverAbort(),
);
// Replaying the consumed cursor must not fall through to blobReader.read()
// (which would throw the opaque "Blob not found" error naming only the
// random cursor UUID) -- it must short-circuit to the actionable message
// naming the real spill URI and the offset to resume from.
const replay = await middleware(
{ id: "b3", name: "read_file", arguments: { path: cursorPath } },
neverAbort(),
);
expect(replay.isError).toBe(true);
expect(String(replay.content)).toContain("already used");
expect(String(replay.content)).toContain("tool-output:///spill-1");
expect(String(replay.content)).not.toContain("Blob not found");
});
});
154 changes: 150 additions & 4 deletions src/plugins/read-file-guard-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { randomUUID } from "node:crypto";
import { createReadStream } from "node:fs";
import { stat } from "node:fs/promises";
import { resolve } from "node:path";
import { Readable } from "node:stream";
import { StringDecoder } from "node:string_decoder";
import type { ToolPlugin } from "@intx/tools-posix";
import type { BlobReader } from "@intx/types/runtime";
import { canonicalToolOutputUri, isToolOutputLike } from "../util/tool-output-uri.js";
import {
canonicalToolOutputUri,
isToolOutputLike,
TOOL_OUTPUT_URI_PREFIX,
} from "../util/tool-output-uri.js";
import { formatReadFileTimeoutMessage } from "./tool-time-budget.js";

// Corbits Code-side guard for read_file. Stock @intx/tools-posix read-file loads the
Expand Down Expand Up @@ -41,6 +46,86 @@ export interface ReadFileGuardPluginOptions {
blobReader?: BlobReader;
}

// A truncated read used to tell the model "Use offset=N to continue" against
// the identical path -- exactly the same-path pagination fan-out CL-6961
// measured (97% of 4+-reads-per-path clusters were legitimate chunked reads
// of one large file, penalized by detectors that only see "same path, many
// calls"). Each truncated result instead mints a single-use tool-output://
// cursor pointing at the exact resumption point (source + next offset) and
// tells the model to pass THAT as `path`. Every follow-up read therefore
// targets a distinct path, so pagination no longer looks like a same-path
// loop, and the cursor is a real, resolvable handle -- not the "see the blob"
// promise result-truncation-plugin.ts's comment forbids, since nothing here
// claims discarded bytes are retrievable; it just remembers where to resume
// a fresh bounded read.
type ReadCursor =
| { kind: "file"; absolutePath: string; offset: number; consumed: boolean }
| { kind: "blob"; uri: string; offset: number; consumed: boolean };

// A cursor is single-use, but the record survives consumption (bounded by
// MAX_CURSOR_HISTORY below) so a stale replay -- consumed already, or a
// second process/turn racing the first -- can be told exactly where to
// resume instead of hitting an opaque "blob not found" dead end that names
// neither the file nor an offset and leaves re-reading from scratch (the
// original path, no offset) as the model's only move.
const MAX_CURSOR_HISTORY = 200;

const CONTINUE_OFFSET_RE = /Use offset=(\d+) to continue\.\]$/;

function pruneCursorHistory(cursors: Map<string, ReadCursor>): void {
while (cursors.size > MAX_CURSOR_HISTORY) {
const oldest = cursors.keys().next().value;
if (oldest === undefined) break;
cursors.delete(oldest);
}
}

function mintCursor(
content: string,
cursors: Map<string, ReadCursor>,
source: { kind: "file"; absolutePath: string } | { kind: "blob"; uri: string },
): string {
const match = CONTINUE_OFFSET_RE.exec(content);
if (match === null) return content;
const offset = Number(match[1]);
const cursorId = randomUUID();
cursors.set(
cursorId,
source.kind === "file"
? { kind: "file", absolutePath: source.absolutePath, offset, consumed: false }
: { kind: "blob", uri: source.uri, offset, consumed: false },
);
pruneCursorHistory(cursors);
return content.replace(
CONTINUE_OFFSET_RE,
`Use path="${TOOL_OUTPUT_URI_PREFIX}///${cursorId}" (same tool, no offset needed) to continue reading the remainder — a fresh, working handle, not the original path.]`,
);
}

// Bound the source shown in a stale-cursor message: an adversarial or
// pathological path must not blow past a reasonable notice size.
const STALE_CURSOR_SOURCE_MAX = 300;

function displaySource(source: string): string {
return source.length > STALE_CURSOR_SOURCE_MAX
? `${source.slice(0, STALE_CURSOR_SOURCE_MAX)}…`
: source;
}

/**
* Message for a cursor that is known but already used (or is being replayed
* from a stale/compacted turn). Distinct from "blob not found": it names the
* original source and the exact offset to resume from, so recovery is a
* single new call rather than a re-read from scratch of the whole file.
*/
function staleCursorMessage(cursor: ReadCursor): string {
const source = cursor.kind === "file" ? cursor.absolutePath : cursor.uri;
return (
`this read_file continuation handle was already used (each cursor is single-use). ` +
`Resume with read_file, path="${displaySource(source)}", offset=${cursor.offset}.`
);
}

function numArg(value: unknown): number | undefined {
return typeof value === "number" && Number.isFinite(value) ? value : undefined;
}
Expand Down Expand Up @@ -297,6 +382,11 @@ export function readFileGuardPlugin(
options: ReadFileGuardPluginOptions = {},
): ToolPlugin {
const { blobReader } = options;
// Single-use resumption pointers minted by mintCursor(); scoped to this
// plugin instance (one per session/agent, per buildCorePosixToolPlugins), so
// it never outlives the session and never crosses sessions.
const cursors = new Map<string, ReadCursor>();
const cursorUriPrefix = `${TOOL_OUTPUT_URI_PREFIX}///`;
return {
middleware: (next) => async (call, signal) => {
if (call.name !== "read_file") return next(call, signal);
Expand All @@ -306,13 +396,64 @@ export function readFileGuardPlugin(
return next(call, signal);
}

const { offset, limit } = resolveReadFilePaging(call);
const { limit } = resolveReadFilePaging(call);

if (isToolOutputLike(rawPath)) {
const uri = canonicalToolOutputUri(rawPath);
if (uri === undefined) {
return next(call, signal);
}

const cursorId = uri.startsWith(cursorUriPrefix) ? uri.slice(cursorUriPrefix.length) : "";
const cursor = cursorId.length > 0 ? cursors.get(cursorId) : undefined;
if (cursor !== undefined && cursor.consumed) {
// Known cursor, already used -- distinct from a genuine missing
// blob: name the original source and offset so recovery is one
// targeted call, not a from-scratch re-read of the whole file.
return { callId: call.id, content: staleCursorMessage(cursor), isError: true };
}
if (cursor !== undefined) {
// A cursor is authoritative on position: the model passes only the
// handle (and optionally a limit), never an offset back into it.
cursor.consumed = true;
try {
signal.throwIfAborted();
if (cursor.kind === "file") {
const res = await readFileBounded(cursor.absolutePath, cursor.offset, limit, signal);
return res.isError
? { callId: call.id, content: res.content, isError: true }
: {
callId: call.id,
content: mintCursor(res.content, cursors, {
kind: "file",
absolutePath: cursor.absolutePath,
}),
};
}
if (blobReader === undefined) {
return {
callId: call.id,
content: `cannot read ${rawPath}: no blob reader is configured for tool-output spills`,
isError: true,
};
}
const bytes = await blobReader.read(cursor.uri);
const res = await readBytesBounded(bytes, cursor.offset, limit, signal, cursor.uri);
return res.isError
? { callId: call.id, content: res.content, isError: true }
: {
callId: call.id,
content: mintCursor(res.content, cursors, { kind: "blob", uri: cursor.uri }),
};
} catch (err) {
return {
callId: call.id,
content: err instanceof Error ? err.message : String(err),
isError: true,
};
}
}

if (blobReader === undefined) {
return {
callId: call.id,
Expand All @@ -322,11 +463,12 @@ export function readFileGuardPlugin(
}
try {
signal.throwIfAborted();
const { offset } = resolveReadFilePaging(call);
const bytes = await blobReader.read(uri);
const res = await readBytesBounded(bytes, offset, limit, signal, uri);
return res.isError
? { callId: call.id, content: res.content, isError: true }
: { callId: call.id, content: res.content };
: { callId: call.id, content: mintCursor(res.content, cursors, { kind: "blob", uri }) };
} catch (err) {
return {
callId: call.id,
Expand All @@ -346,10 +488,14 @@ export function readFileGuardPlugin(
}

try {
const { offset } = resolveReadFilePaging(call);
const res = await readFileBounded(absolutePath, offset, limit, signal);
return res.isError
? { callId: call.id, content: res.content, isError: true }
: { callId: call.id, content: res.content };
: {
callId: call.id,
content: mintCursor(res.content, cursors, { kind: "file", absolutePath }),
};
} catch (err) {
return {
callId: call.id,
Expand Down
2 changes: 1 addition & 1 deletion src/util/tool-output-uri.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const TOOL_OUTPUT_URI_PREFIX = "tool-output:";
export const TOOL_OUTPUT_URI_PREFIX = "tool-output:";

export function isToolOutputLike(path: string): boolean {
return path.startsWith(TOOL_OUTPUT_URI_PREFIX);
Expand Down
Loading