Skip to content

Commit d11df13

Browse files
committed
computer: Stream ranged tool reads
1 parent 2bfce96 commit d11df13

4 files changed

Lines changed: 125 additions & 63 deletions

File tree

packages/computer/src/stub.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,16 @@ describe("WorkspaceStub", () => {
205205
});
206206
});
207207

208+
it("fs.readFile forwards ranged stream options", async () => {
209+
await withStub(async (ws) => {
210+
const stub = ws.stub();
211+
await stub.fs.writeFile("/bin", new Uint8Array([1, 2, 3, 4, 5]));
212+
const stream = await stub.fs.readFile("/bin", { byteOffset: 1, byteLength: 3 });
213+
const bytes = new Uint8Array(await new Response(stream).arrayBuffer());
214+
expect(Array.from(bytes)).toEqual([2, 3, 4]);
215+
});
216+
});
217+
208218
it("fs.readRange forwards bounded byte reads", async () => {
209219
await withStub(async (ws) => {
210220
const stub = ws.stub();

packages/computer/src/stub.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ export class WorkspaceFilesystemStub extends RpcTarget {
104104

105105
readFile(path: string): Promise<ReadableStream<Uint8Array>>;
106106
readFile(path: string, encoding: "utf8"): Promise<string>;
107+
readFile(
108+
path: string,
109+
options: ReadFileOptions & { encoding?: undefined },
110+
): Promise<ReadableStream<Uint8Array>>;
111+
readFile(path: string, options: ReadFileOptions & { encoding: "utf8" }): Promise<string>;
107112
readFile(path: string, options: ReadFileOptions): Promise<string | ReadableStream<Uint8Array>>;
108113
readFile(
109114
path: string,

packages/computer/src/tools/ai.test.ts

Lines changed: 85 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -179,82 +179,124 @@ function memoryStore(options: {
179179
}
180180

181181
describe("WorkspaceFileStore", () => {
182-
it("uses readRange without streaming skipped bytes from Workspace.fs", async () => {
183-
const calls: Array<{ offset: number; length: number }> = [];
182+
it("opens one ranged stream instead of issuing repeated range calls", async () => {
183+
const calls: Array<{ byteOffset?: number; byteLength?: number }> = [];
184184
const content = bytes("abcdefghij");
185185
const workspace = {
186186
fs: {
187187
async stat() {
188-
return {
189-
size: content.length,
190-
mtime: 1,
191-
mode: 0o100644,
192-
isFile: true,
193-
isDirectory: false,
194-
};
188+
throw new Error("stat must not be called by readChunks");
195189
},
196-
async readRange(_path: string, offset: number, length: number) {
197-
calls.push({ offset, length });
198-
return content.slice(offset, offset + length);
190+
async readRange() {
191+
throw new Error("readRange must not be called by readChunks");
199192
},
200-
async readFile(): Promise<ReadableStream<Uint8Array>> {
201-
throw new Error("readFile must not be called for ranged reads");
193+
async readFile(
194+
_path: string,
195+
options: { byteOffset?: number; byteLength?: number } = {},
196+
): Promise<ReadableStream<Uint8Array>> {
197+
calls.push(options);
198+
const start = options.byteOffset ?? 0;
199+
const end = options.byteLength === undefined ? undefined : start + options.byteLength;
200+
return new ReadableStream({
201+
start(controller) {
202+
controller.enqueue(content.slice(start, end));
203+
controller.close();
204+
},
205+
});
202206
},
203207
async writeFile() {},
204208
async mkdir() {},
205209
async rm() {},
206-
async readdir() {
207-
return [];
208-
},
209210
},
210211
};
211212
const store = new WorkspaceFileStore(workspace);
212213

213214
await expect(
214215
drainChunks(store.readChunks("/workspace/range.txt", 2, 5)).then(decode),
215216
).resolves.toBe("cdefg");
216-
expect(calls).toEqual([{ offset: 2, length: 5 }]);
217+
expect(calls).toEqual([{ byteOffset: 2, byteLength: 5 }]);
218+
});
219+
220+
it("still validates the path for a zero-length read", async () => {
221+
const store = new WorkspaceFileStore(makeWorkspace());
222+
223+
await expect(drainChunks(store.readChunks("/missing", 0, 0))).rejects.toMatchObject({
224+
code: "ENOENT",
225+
});
217226
});
218227

219-
it("splits unbounded reads into fixed-size ranges", async () => {
220-
const calls: Array<{ offset: number; length: number }> = [];
221-
const content = new Uint8Array(150_000).fill(7);
228+
it("rejects directories instead of treating them as empty files", async () => {
229+
const workspace = makeWorkspace();
230+
await workspace.fs.mkdir("/directory");
231+
const store = new WorkspaceFileStore(workspace);
232+
233+
await expect(drainChunks(store.readChunks("/directory"))).rejects.toMatchObject({
234+
code: "EISDIR",
235+
});
236+
});
237+
238+
it("keeps a real multi-chunk workspace read on one snapshot", async () => {
239+
const workspace = makeWorkspace();
240+
await workspace.fs.mkdir("/workspace", { recursive: true });
241+
const original = new Uint8Array(600_000);
242+
original.fill(0x41, 0, 500_000);
243+
original.fill(0x42, 500_000);
244+
await workspace.fs.writeFile("/workspace/large.bin", original);
245+
const store = new WorkspaceFileStore(workspace);
246+
247+
const chunks = store.readChunks("/workspace/large.bin")[Symbol.asyncIterator]();
248+
const first = await chunks.next();
249+
expect(first.done).toBe(false);
250+
await workspace.fs.writeFile(
251+
"/workspace/large.bin",
252+
new Uint8Array(original.length).fill(0x43),
253+
);
254+
255+
const parts = [first.value];
256+
while (true) {
257+
const next = await chunks.next();
258+
if (next.done) break;
259+
parts.push(next.value);
260+
}
261+
const result = await drainChunks(
262+
(async function* () {
263+
yield* parts;
264+
})(),
265+
);
266+
expect(result.byteLength).toBe(original.byteLength);
267+
expect(result.every((value, index) => value === original[index])).toBe(true);
268+
});
269+
270+
it("cancels a ranged stream when its consumer stops early", async () => {
271+
let cancelled = false;
222272
const workspace = {
223273
fs: {
224274
async stat() {
225-
return {
226-
size: content.length,
227-
mtime: 1,
228-
mode: 0o100644,
229-
isFile: true,
230-
isDirectory: false,
231-
};
275+
throw new Error("stat must not be called by readChunks");
232276
},
233-
async readRange(_path: string, offset: number, length: number) {
234-
calls.push({ offset, length });
235-
return content.slice(offset, offset + length);
277+
async readRange() {
278+
throw new Error("readRange must not be called by readChunks");
236279
},
237280
async readFile(): Promise<ReadableStream<Uint8Array>> {
238-
throw new Error("readFile must not be called for ranged reads");
281+
return new ReadableStream({
282+
start(controller) {
283+
controller.enqueue(bytes("first"));
284+
controller.enqueue(bytes("second"));
285+
},
286+
cancel() {
287+
cancelled = true;
288+
},
289+
});
239290
},
240291
async writeFile() {},
241292
async mkdir() {},
242293
async rm() {},
243-
async readdir() {
244-
return [];
245-
},
246294
},
247295
};
248296
const store = new WorkspaceFileStore(workspace);
249297

250-
await expect(drainChunks(store.readChunks("/workspace/large.bin"))).resolves.toHaveLength(
251-
content.length,
252-
);
253-
expect(calls).toEqual([
254-
{ offset: 0, length: 65_536 },
255-
{ offset: 65_536, length: 65_536 },
256-
{ offset: 131_072, length: 18_928 },
257-
]);
298+
for await (const _chunk of store.readChunks("/workspace/range.txt")) break;
299+
expect(cancelled).toBe(true);
258300
});
259301
});
260302

packages/computer/src/tools/fs/store.ts

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@
66
* class. This adapter is the bridge from that contract to the public
77
* `workspace.fs` surface.
88
*
9-
* Bounded reads go through `fs.readRange`; whole-file reads used by edit
10-
* and multimodal output still drain `fs.readFile(path)`.
9+
* Chunked and ranged reads use one `fs.readFile` stream so remote workspaces
10+
* keep one snapshot and one RPC invocation. Whole-file reads used by edit and
11+
* multimodal output drain the same stream interface.
1112
*/
1213

1314
import type { FileStat, FileStore } from "./types.js";
1415

15-
const RANGE_CHUNK_BYTES = 64 * 1024;
16-
1716
/**
1817
* Structural subset of `@cloudflare/computer.Workspace` the tools
1918
* depend on.
@@ -27,8 +26,10 @@ export interface WorkspaceLike {
2726
isFile: boolean;
2827
isDirectory: boolean;
2928
}>;
30-
readFile(path: string): Promise<ReadableStream<Uint8Array>>;
31-
readRange(path: string, offset: number, length: number): Promise<Uint8Array>;
29+
readFile(
30+
path: string,
31+
options?: { byteOffset?: number; byteLength?: number },
32+
): Promise<ReadableStream<Uint8Array>>;
3233
writeFile(path: string, content: Uint8Array, options?: { mode?: number }): Promise<void>;
3334
mkdir(path: string, options?: { recursive?: boolean }): Promise<void>;
3435
rm(path: string, options?: { recursive?: boolean; force?: boolean }): Promise<void>;
@@ -84,20 +85,24 @@ export class WorkspaceFileStore implements FileStore {
8485
if (byteLength !== undefined && (!Number.isSafeInteger(byteLength) || byteLength < 0)) {
8586
throw new Error("readChunks: byteLength must be a non-negative safe integer");
8687
}
87-
if (byteLength === 0) return;
88-
89-
const stat = await this.ws.fs.stat(path);
90-
let remaining = Math.max(0, stat.size - byteOffset);
91-
if (byteLength !== undefined) remaining = Math.min(remaining, byteLength);
92-
let offset = byteOffset;
93-
while (remaining > 0) {
94-
const requested = Math.min(remaining, RANGE_CHUNK_BYTES);
95-
const chunk = await this.ws.fs.readRange(path, offset, requested);
96-
if (chunk.byteLength === 0) return;
97-
yield chunk;
98-
offset += chunk.byteLength;
99-
remaining -= chunk.byteLength;
100-
if (chunk.byteLength < requested) return;
88+
const stream = await this.ws.fs.readFile(path, { byteOffset, byteLength });
89+
const reader = stream.getReader();
90+
let completed = false;
91+
try {
92+
while (true) {
93+
const { value, done } = await reader.read();
94+
if (done) {
95+
completed = true;
96+
return;
97+
}
98+
if (value !== undefined && value.byteLength > 0) yield value;
99+
}
100+
} finally {
101+
try {
102+
if (!completed) await reader.cancel();
103+
} finally {
104+
reader.releaseLock();
105+
}
101106
}
102107
}
103108
}

0 commit comments

Comments
 (0)