Skip to content

Commit 8e72c5e

Browse files
committed
Give stale cursors an actionable message instead of a dead end
A consumed or unknown tool-output cursor previously fell through to the generic blob-URI branch, so a live blobReader's "Blob not found for key: tool-output:///<uuid>" error named neither the original file nor an offset -- the model's only recovery was to recall the path itself and re-read from scratch, reproducing exactly one instance of the same-path repeat this mechanism exists to eliminate. Consumed cursor records now survive (bounded, oldest evicted past 200 entries) instead of being deleted on use. A replay of an already-used cursor is now distinguished from a genuinely unknown tool-output URI: it gets a message naming the original source and the exact offset to resume from, so recovery is one targeted call instead of a blind re-read of the whole file.
1 parent f774ba6 commit 8e72c5e

2 files changed

Lines changed: 115 additions & 10 deletions

File tree

src/plugins/read-file-guard-plugin.test.ts

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,11 @@ describe("readFileGuardPlugin", () => {
331331
expect(pathsRead.filter((p) => p === "huge.txt").length).toBe(1);
332332
});
333333

334-
test("a stale (already-consumed) cursor is rejected rather than silently re-served", async () => {
335-
await fixture("stale.txt", Array.from({ length: 10 }, (_, i) => `line-${i}`).join("\n"));
334+
test("a stale (already-consumed) cursor names the original path and offset instead of a dead end", async () => {
335+
const absolutePath = await fixture(
336+
"stale.txt",
337+
Array.from({ length: 10 }, (_, i) => `line-${i}`).join("\n"),
338+
);
336339
const plugin = readFileGuardPlugin(dir, {});
337340
const middleware = plugin.middleware!(fallback);
338341
const first = await middleware(
@@ -344,13 +347,68 @@ describe("readFileGuardPlugin", () => {
344347
const cursorPath = (match as RegExpExecArray)[1] as string;
345348

346349
await middleware({ id: "s2", name: "read_file", arguments: { path: cursorPath } }, neverAbort());
347-
// Second use of the same, already-consumed cursor: no blob reader is
348-
// configured, so it falls through to the direct tool-output-URI path
349-
// and reports the honest error rather than fabricating stale content.
350+
// Second use of the same, already-consumed cursor: distinct from a
351+
// generic missing-blob error, this must name a followable next step —
352+
// the original source and the offset to resume from — rather than
353+
// leaving the model to re-read the whole file from scratch.
350354
const replay = await middleware(
351355
{ id: "s3", name: "read_file", arguments: { path: cursorPath } },
352356
neverAbort(),
353357
);
354358
expect(replay.isError).toBe(true);
359+
expect(String(replay.content)).toContain("already used");
360+
expect(String(replay.content)).toContain(absolutePath);
361+
expect(String(replay.content)).toMatch(/offset=4\b/);
362+
});
363+
364+
test("an unknown tool-output URI against a real blobReader gets the production 'blob not found' error, not a stale-cursor message", async () => {
365+
const blobReader = {
366+
async read(uri: string): Promise<Uint8Array> {
367+
throw new Error(`Blob not found for key: ${uri}`);
368+
},
369+
};
370+
const result = await run(
371+
{ id: "u1", name: "read_file", arguments: { path: "tool-output:///never-minted" } },
372+
blobReader,
373+
);
374+
expect(result.isError).toBe(true);
375+
expect(String(result.content)).toContain("Blob not found for key");
376+
// Never a cursor's own wording, since this ID was never one of ours.
377+
expect(String(result.content)).not.toContain("already used");
378+
});
379+
380+
test("a stale cursor short-circuits before reaching a real blobReader's production 'blob not found' error", async () => {
381+
const encoder = new TextEncoder();
382+
const body = Array.from({ length: 8_000 }, (_, i) => `row-${i}`).join("\n");
383+
const blobReader = {
384+
async read(uri: string): Promise<Uint8Array> {
385+
if (uri === "tool-output:///spill-1") return encoder.encode(body);
386+
throw new Error(`Blob not found for key: ${uri}`);
387+
},
388+
};
389+
const plugin = readFileGuardPlugin(dir, { blobReader });
390+
const middleware = plugin.middleware!(fallback);
391+
392+
const first = await middleware(
393+
{ id: "b1", name: "read_file", arguments: { path: "tool-output:///spill-1", limit: 5 } },
394+
neverAbort(),
395+
);
396+
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(String(first.content));
397+
expect(match).not.toBeNull();
398+
const cursorPath = (match as RegExpExecArray)[1] as string;
399+
400+
await middleware({ id: "b2", name: "read_file", arguments: { path: cursorPath } }, neverAbort());
401+
// Replaying the consumed cursor must not fall through to blobReader.read()
402+
// (which would throw the opaque "Blob not found" error naming only the
403+
// random cursor UUID) -- it must short-circuit to the actionable message
404+
// naming the real spill URI and the offset to resume from.
405+
const replay = await middleware(
406+
{ id: "b3", name: "read_file", arguments: { path: cursorPath } },
407+
neverAbort(),
408+
);
409+
expect(replay.isError).toBe(true);
410+
expect(String(replay.content)).toContain("already used");
411+
expect(String(replay.content)).toContain("tool-output:///spill-1");
412+
expect(String(replay.content)).not.toContain("Blob not found");
355413
});
356414
});

src/plugins/read-file-guard-plugin.ts

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,27 @@ export interface ReadFileGuardPluginOptions {
5959
// claims discarded bytes are retrievable; it just remembers where to resume
6060
// a fresh bounded read.
6161
type ReadCursor =
62-
| { kind: "file"; absolutePath: string; offset: number }
63-
| { kind: "blob"; uri: string; offset: number };
62+
| { kind: "file"; absolutePath: string; offset: number; consumed: boolean }
63+
| { kind: "blob"; uri: string; offset: number; consumed: boolean };
64+
65+
// A cursor is single-use, but the record survives consumption (bounded by
66+
// MAX_CURSOR_HISTORY below) so a stale replay -- consumed already, or a
67+
// second process/turn racing the first -- can be told exactly where to
68+
// resume instead of hitting an opaque "blob not found" dead end that names
69+
// neither the file nor an offset and leaves re-reading from scratch (the
70+
// original path, no offset) as the model's only move.
71+
const MAX_CURSOR_HISTORY = 200;
6472

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

75+
function pruneCursorHistory(cursors: Map<string, ReadCursor>): void {
76+
while (cursors.size > MAX_CURSOR_HISTORY) {
77+
const oldest = cursors.keys().next().value;
78+
if (oldest === undefined) break;
79+
cursors.delete(oldest);
80+
}
81+
}
82+
6783
function mintCursor(
6884
content: string,
6985
cursors: Map<string, ReadCursor>,
@@ -76,15 +92,40 @@ function mintCursor(
7692
cursors.set(
7793
cursorId,
7894
source.kind === "file"
79-
? { kind: "file", absolutePath: source.absolutePath, offset }
80-
: { kind: "blob", uri: source.uri, offset },
95+
? { kind: "file", absolutePath: source.absolutePath, offset, consumed: false }
96+
: { kind: "blob", uri: source.uri, offset, consumed: false },
8197
);
98+
pruneCursorHistory(cursors);
8299
return content.replace(
83100
CONTINUE_OFFSET_RE,
84101
`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.]`,
85102
);
86103
}
87104

105+
// Bound the source shown in a stale-cursor message: an adversarial or
106+
// pathological path must not blow past a reasonable notice size.
107+
const STALE_CURSOR_SOURCE_MAX = 300;
108+
109+
function displaySource(source: string): string {
110+
return source.length > STALE_CURSOR_SOURCE_MAX
111+
? `${source.slice(0, STALE_CURSOR_SOURCE_MAX)}…`
112+
: source;
113+
}
114+
115+
/**
116+
* Message for a cursor that is known but already used (or is being replayed
117+
* from a stale/compacted turn). Distinct from "blob not found": it names the
118+
* original source and the exact offset to resume from, so recovery is a
119+
* single new call rather than a re-read from scratch of the whole file.
120+
*/
121+
function staleCursorMessage(cursor: ReadCursor): string {
122+
const source = cursor.kind === "file" ? cursor.absolutePath : cursor.uri;
123+
return (
124+
`this read_file continuation handle was already used (each cursor is single-use). ` +
125+
`Resume with read_file, path="${displaySource(source)}", offset=${cursor.offset}.`
126+
);
127+
}
128+
88129
function numArg(value: unknown): number | undefined {
89130
return typeof value === "number" && Number.isFinite(value) ? value : undefined;
90131
}
@@ -365,10 +406,16 @@ export function readFileGuardPlugin(
365406

366407
const cursorId = uri.startsWith(cursorUriPrefix) ? uri.slice(cursorUriPrefix.length) : "";
367408
const cursor = cursorId.length > 0 ? cursors.get(cursorId) : undefined;
409+
if (cursor !== undefined && cursor.consumed) {
410+
// Known cursor, already used -- distinct from a genuine missing
411+
// blob: name the original source and offset so recovery is one
412+
// targeted call, not a from-scratch re-read of the whole file.
413+
return { callId: call.id, content: staleCursorMessage(cursor), isError: true };
414+
}
368415
if (cursor !== undefined) {
369416
// A cursor is authoritative on position: the model passes only the
370417
// handle (and optionally a limit), never an offset back into it.
371-
cursors.delete(cursorId);
418+
cursor.consumed = true;
372419
try {
373420
signal.throwIfAborted();
374421
if (cursor.kind === "file") {

0 commit comments

Comments
 (0)