Skip to content

Commit cc83c65

Browse files
committed
refactor(read-file): resume large reads by same-path offset only
Drop the dual footer and live-cursor reuse so continuation is a single stateless path+offset notice, matching the single-way resume direction. Pre-existing cursor tests become same-URI+offset chains; the truncation 10k exemption keys on the offset footer only. Paging mechanics untouched.
1 parent d93ea64 commit cc83c65

3 files changed

Lines changed: 79 additions & 305 deletions

File tree

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

Lines changed: 62 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -389,41 +389,34 @@ describe("CL-8979 large-file pagination", () => {
389389
);
390390
}, 120_000);
391391

392-
test("chains cursor resumption on a blob past the scan ceiling without re-scanning", async () => {
392+
test("chains same-URI+offset continuation on a blob past the scan ceiling without re-scanning", async () => {
393393
const rows = Array.from({ length: BIG_LINES }, (_, i) => bigRow(i));
394394
const bytes = new TextEncoder().encode(`${rows.join("\n")}\n`);
395395
const run = blobChainRunner(async (key) => {
396396
if (key === "cl8979-blob") return bytes;
397397
throw new Error(`missing ${key}`);
398398
});
399399
const collected: string[] = [];
400-
let path = "tool-output:///cl8979-blob";
400+
const path = "tool-output:///cl8979-blob";
401+
let offset = 0;
401402
let hops = 0;
402403
let sawOffsetFooter = false;
403-
let sawCursorAlias = false;
404404
for (;;) {
405-
const result = await run(`blob-${hops}`, { path, limit: 200 });
405+
const result = await run(`blob-${hops}`, { path, limit: 200, offset });
406406
hops += 1;
407407
const content = String(result.content);
408408
expect(result.isError).toBeFalsy();
409409
expect(content).not.toContain("scan limit");
410410
collected.push(...bodyRows(content));
411-
const cursor = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(content);
412-
const offset = continueOffset(content);
413-
if (offset !== null) {
414-
sawOffsetFooter = true;
415-
path = "tool-output:///cl8979-blob";
416-
}
417-
if (cursor !== null) {
418-
sawCursorAlias = true;
419-
path = cursor[1] as string;
420-
}
421-
if (offset === null && cursor === null) break;
411+
const next = continueOffset(content);
412+
if (next === null) break;
413+
sawOffsetFooter = true;
414+
expect(next).toBeGreaterThan(offset);
415+
offset = next;
422416
expect(hops).toBeLessThan(2000);
423417
}
424418
expect(hops).toBeGreaterThan(1);
425419
expect(sawOffsetFooter).toBe(true);
426-
expect(sawCursorAlias).toBe(true);
427420
expect(collected.length).toBe(BIG_LINES);
428421
expect(collected[BIG_LINES - 1]).toBe(bigRow(BIG_LINES - 1));
429422
}, 120_000);
@@ -550,8 +543,8 @@ describe("readFileGuardPlugin", () => {
550543
expect(result.content).toContain(" 2\tl2");
551544
expect(result.content).toContain(" 3\tl3");
552545
expect(result.content).not.toContain(" 4\tl4");
553-
expect(result.content).toContain('Use path="tool-output:///');
554-
expect(result.content).not.toContain("Use offset=");
546+
expect(result.content).toContain("Use offset=");
547+
expect(result.content).not.toContain('Use path="tool-output:///');
555548
});
556549

557550
test("rejects tool-output URIs when no blob reader is configured", async () => {
@@ -589,7 +582,7 @@ describe("readFileGuardPlugin", () => {
589582
expect(result.content).not.toBe("FALLBACK");
590583
});
591584

592-
test("pages a giant one-line tool-output blob across byte windows and resumes via the minted cursor", async () => {
585+
test("pages a giant one-line tool-output blob across byte windows on the same URI with rising offsets", async () => {
593586
const encoder = new TextEncoder();
594587
const payload = `HEAD-${"x".repeat(READ_FILE_MAX_BYTES)}-TAIL`;
595588
const blobReader = createBlobReader({
@@ -617,13 +610,19 @@ describe("readFileGuardPlugin", () => {
617610
expect(Buffer.byteLength(firstContent, "utf8")).toBeLessThanOrEqual(
618611
READ_FILE_MAX_BYTES,
619612
);
620-
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(firstContent);
613+
const match = /Use offset=(\d+) to continue/.exec(firstContent);
621614
expect(match).not.toBeNull();
622-
const nextPath = (match as RegExpExecArray)[1] as string;
623-
expect(nextPath).toMatch(/^tool-output:\/\/\//);
615+
const nextOffset = Number((match as RegExpExecArray)[1]);
624616

625617
const second = await middleware(
626-
{ id: "g2", name: "read_file", arguments: { path: nextPath } },
618+
{
619+
id: "g2",
620+
name: "read_file",
621+
arguments: {
622+
path: "tool-output:///giant-line",
623+
offset: nextOffset,
624+
},
625+
},
627626
neverAbort(),
628627
);
629628
expect(second.isError).toBeFalsy();
@@ -711,7 +710,7 @@ describe("readFileGuardPlugin", () => {
711710
expect(result.content).toBe("FALLBACK");
712711
});
713712

714-
test("a truncated read never asks the model to re-read the same path (CL-6961)", async () => {
713+
test("a truncated read names the same path with an explicit offset (CL-8980)", async () => {
715714
await fixture(
716715
"many-lines.txt",
717716
Array.from({ length: 10 }, (_, i) => `line-${i}`).join("\n"),
@@ -726,19 +725,17 @@ describe("readFileGuardPlugin", () => {
726725
},
727726
neverAbort(),
728727
);
729-
expect(result.content).not.toContain("Use offset=");
730-
expect(String(result.content)).toContain('Use path="tool-output:///');
731-
// The literal source path never reappears as the thing to read next.
732-
expect(String(result.content)).not.toContain("many-lines.txt");
728+
expect(String(result.content)).toMatch(/Use offset=(\d+) to continue/);
729+
expect(String(result.content)).not.toContain('Use path="tool-output:///');
730+
expect(String(result.content)).not.toContain("single-use");
733731
});
734732

735-
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 () => {
733+
test("following same-path offsets reads a large file to completion; every hop re-issues the original path with a rising offset (CL-8980)", async () => {
736734
const lines = Array.from({ length: 9_000 }, (_, i) => `line-${i} payload`);
737735
await fixture("huge.txt", lines.join("\n"));
738736
const plugin = readFileGuardPlugin(dir, {});
739737
const middleware = defined(plugin.middleware)(fallback);
740738

741-
const pathsRead: string[] = ["huge.txt"];
742739
let result = await middleware(
743740
{ id: "c1", name: "read_file", arguments: { path: "huge.txt" } },
744741
neverAbort(),
@@ -747,35 +744,32 @@ describe("readFileGuardPlugin", () => {
747744
let guard = 0;
748745
for (;;) {
749746
guard++;
750-
expect(guard).toBeLessThan(50); // fails loudly instead of hanging on a broken cursor chain
747+
expect(guard).toBeLessThan(50); // fails loudly instead of hanging on a broken offset chain
751748
const content = String(result.content);
752749
const numbered = content.split("\n\n")[0] ?? "";
753750
seen += numbered.trimEnd().split("\n").length;
754751

755-
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(content);
752+
const match = /Use offset=(\d+) to continue/.exec(content);
756753
if (match === undefined || match === null) break;
757-
const nextPath = match[1] as string;
758-
expect(pathsRead).not.toContain(nextPath); // every hop targets a fresh, distinct path
759-
pathsRead.push(nextPath);
754+
const offset = Number(match[1] as string);
760755

761756
result = await middleware(
762757
{
763-
id: `c${pathsRead.length}`,
758+
id: `c${guard + 1}`,
764759
name: "read_file",
765-
arguments: { path: nextPath },
760+
arguments: { path: "huge.txt", offset },
766761
},
767762
neverAbort(),
768763
);
764+
expect(result.isError).toBeFalsy();
769765
}
770766

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

777-
test("a stale (already-consumed) cursor names the original path and offset instead of a dead end", async () => {
778-
const absolutePath = await fixture(
771+
test("reusing a continuation offset after first use still yields the window — reads never expire", async () => {
772+
await fixture(
779773
"stale.txt",
780774
Array.from({ length: 10 }, (_, i) => `line-${i}`).join("\n"),
781775
);
@@ -789,31 +783,29 @@ describe("readFileGuardPlugin", () => {
789783
},
790784
neverAbort(),
791785
);
792-
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(
793-
String(first.content),
794-
);
786+
const match = /Use offset=(\d+) to continue/.exec(String(first.content));
795787
expect(match).not.toBeNull();
796-
const cursorPath = (match as RegExpExecArray)[1] as string;
788+
const offset = Number((match as RegExpExecArray)[1] as string);
797789

798-
await middleware(
799-
{ id: "s2", name: "read_file", arguments: { path: cursorPath } },
790+
const second = await middleware(
791+
{ id: "s2", name: "read_file", arguments: { path: "stale.txt", offset } },
800792
neverAbort(),
801793
);
802-
// Second use of the same, already-consumed cursor: distinct from a
803-
// generic missing-blob error, this must name a followable next step —
804-
// the original source and the offset to resume from — rather than
805-
// leaving the model to re-read the whole file from scratch.
794+
expect(second.isError).toBeFalsy();
795+
expect(String(second.content)).toContain("line-4");
796+
// Second use of the same offset: reads are idempotent, so the replay is
797+
// byte-identical instead of a spent-handle error.
806798
const replay = await middleware(
807-
{ id: "s3", name: "read_file", arguments: { path: cursorPath } },
799+
{ id: "s3", name: "read_file", arguments: { path: "stale.txt", offset } },
808800
neverAbort(),
809801
);
810-
expect(replay.isError).toBe(true);
811-
expect(String(replay.content)).toContain("already used");
812-
expect(String(replay.content)).toContain(absolutePath);
813-
expect(String(replay.content)).toMatch(/offset=4\b/);
802+
expect(replay.isError).toBeFalsy();
803+
expect(String(replay.content)).toBe(String(second.content));
804+
expect(String(replay.content)).not.toContain("already used");
805+
expect(String(replay.content)).not.toContain("single-use");
814806
});
815807

816-
test("an unknown tool-output URI against a real blobReader gets the production 'blob not found' error, not a stale-cursor message", async () => {
808+
test("an unknown tool-output URI against a real blobReader surfaces the blob store error", async () => {
817809
const blobReader = {
818810
async read(uri: string): Promise<Uint8Array> {
819811
throw new Error(`Blob not found for key: ${uri}`);
@@ -829,16 +821,14 @@ describe("readFileGuardPlugin", () => {
829821
);
830822
expect(result.isError).toBe(true);
831823
expect(String(result.content)).toContain("Blob not found for key");
832-
// Never a cursor's own wording, since this ID was never one of ours.
824+
// No handle machinery remains: there is no spent/cursor wording anywhere.
833825
expect(String(result.content)).not.toContain("already used");
826+
expect(String(result.content)).not.toContain("single-use");
834827
});
835828

836-
test("a stale cursor short-circuits before reaching a real blobReader's production 'blob not found' error", async () => {
837-
const encoder = new TextEncoder();
838-
const body = Array.from({ length: 8_000 }, (_, i) => `row-${i}`).join("\n");
829+
test("a replayed unknown tool-output URI surfaces the same blob error twice — no spent-handle state", async () => {
839830
const blobReader = {
840831
async read(uri: string): Promise<Uint8Array> {
841-
if (uri === "tool-output:///spill-1") return encoder.encode(body);
842832
throw new Error(`Blob not found for key: ${uri}`);
843833
},
844834
};
@@ -849,31 +839,21 @@ describe("readFileGuardPlugin", () => {
849839
{
850840
id: "b1",
851841
name: "read_file",
852-
arguments: { path: "tool-output:///spill-1", limit: 5 },
842+
arguments: { path: "tool-output:///gone", limit: 5 },
853843
},
854844
neverAbort(),
855845
);
856-
const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(
857-
String(first.content),
858-
);
859-
expect(match).not.toBeNull();
860-
const cursorPath = (match as RegExpExecArray)[1] as string;
861-
862-
await middleware(
863-
{ id: "b2", name: "read_file", arguments: { path: cursorPath } },
864-
neverAbort(),
865-
);
866-
// Replaying the consumed cursor must not fall through to blobReader.read()
867-
// (which would throw the opaque "Blob not found" error naming only the
868-
// random cursor UUID) -- it must short-circuit to the actionable message
869-
// naming the real spill URI and the offset to resume from.
846+
expect(first.isError).toBe(true);
847+
expect(String(first.content)).toContain("Blob not found for key");
870848
const replay = await middleware(
871-
{ id: "b3", name: "read_file", arguments: { path: cursorPath } },
849+
{
850+
id: "b2",
851+
name: "read_file",
852+
arguments: { path: "tool-output:///gone", limit: 5 },
853+
},
872854
neverAbort(),
873855
);
874856
expect(replay.isError).toBe(true);
875-
expect(String(replay.content)).toContain("already used");
876-
expect(String(replay.content)).toContain("tool-output:///spill-1");
877-
expect(String(replay.content)).not.toContain("Blob not found");
857+
expect(String(replay.content)).toBe(String(first.content));
878858
});
879859
});

0 commit comments

Comments
 (0)