From 6208b4372328d9fd3497179a7d84822b5881b984 Mon Sep 17 00:00:00 2001 From: Christopher Little-Savage Date: Thu, 30 Jul 2026 21:22:28 +0100 Subject: [PATCH] computer: Carry Think methods onto clients Workspace clients returned by getWorkspace() omit the Think compatibility surface even when the owner enables useThink. Remote Think consumers must otherwise duplicate Computer's filesystem translations or avoid the common client API. Preserve the owner option on Workspace and its remote stub, then install the shared compatibility methods while constructing local and remote clients. Type the common filesystem surface so consumers can use it without an unsafe filesystem cast. Clients whose owner leaves useThink disabled remain unchanged. Remote stubs are also disposed if client initialization fails. --- docs/01_vfs.md | 5 ++- packages/computer/README.md | 7 +-- packages/computer/src/client.test.ts | 64 +++++++++++++++++++++++++++- packages/computer/src/client.ts | 44 ++++++++++++------- packages/computer/src/stub.ts | 6 +++ packages/computer/src/workspace.ts | 35 ++++++++++----- 6 files changed, 128 insertions(+), 33 deletions(-) diff --git a/docs/01_vfs.md b/docs/01_vfs.md index b230d5d..f62e5f1 100644 --- a/docs/01_vfs.md +++ b/docs/01_vfs.md @@ -41,8 +41,9 @@ facade — sandbox wiring lives behind a `WorkspaceBackend`. Set `useThink: true` when assigning the Workspace to `Think.workspace`. This adds Think's string-oriented filesystem compatibility methods (`readFile`, `readFileBytes`, `writeFile`, -`readDir`, `rm`, `glob`, `mkdir`, and `stat`) directly to that -Workspace instance while leaving the primary API on `workspace.fs`. +`readDir`, `rm`, `glob`, `mkdir`, and `stat`) to that Workspace and to +clients returned by `getWorkspace()`, while leaving the primary API on +`workspace.fs`. Illustrative layout (nothing below `/` is auto-created beyond `ROOT_INODE` itself): diff --git a/packages/computer/README.md b/packages/computer/README.md index 5ee035f..0b86472 100644 --- a/packages/computer/README.md +++ b/packages/computer/README.md @@ -138,9 +138,10 @@ const body = await ws.fs.readFile("/notes.md", "utf8"); // ws.shell throws — there's no backend wired up. ``` -Pass `useThink: true` when assigning the same instance to -`Think.workspace`. That adds Think's compatibility methods directly -on the instance while keeping the primary file API on `workspace.fs`. +Pass `useThink: true` when assigning a workspace to `Think.workspace`. +That adds Think's compatibility methods directly to the local instance +and to clients returned by `getWorkspace()`, while keeping the primary +file API on `workspace.fs`. Git, also without a backend: diff --git a/packages/computer/src/client.test.ts b/packages/computer/src/client.test.ts index dffae8c..4b47a29 100644 --- a/packages/computer/src/client.test.ts +++ b/packages/computer/src/client.test.ts @@ -10,9 +10,9 @@ import { SQLiteTestStorage } from "@cloudflare/dofs/testing"; import { describe, expect, it } from "vitest"; -import { getWorkspace } from "./client.js"; +import { getWorkspace, type WorkspaceClient } from "./client.js"; import { WORKSPACE, type WorkspaceStubHost } from "./with-workspace.js"; -import { Workspace } from "./workspace.js"; +import { type ThinkWorkspaceCompatibility, Workspace } from "./workspace.js"; interface ExecCall { command: string; @@ -75,6 +75,7 @@ function fakeRemote(): { let disposed = false; const stub = { fs: { marker: "fs" }, + useThink: false, git: { marker: "git" }, assets: undefined, artifacts: { marker: "artifacts" }, @@ -91,6 +92,25 @@ function fakeRemote(): { }; } +function fakeBrokenRemote(): { + host: WorkspaceStubHost; + disposed: () => boolean; +} { + let disposed = false; + const stub = { + get useThink(): boolean { + throw new Error("compatibility lookup failed"); + }, + [Symbol.dispose]() { + disposed = true; + }, + }; + return { + disposed: () => disposed, + host: { __getWorkspaceStub: () => Promise.resolve(stub as never) }, + }; +} + // Local path fake: an object carrying a real Workspace under the // stash symbol, but with its shell swapped for a recording fake so we // can assert on exec without a backend. @@ -108,6 +128,7 @@ describe("getWorkspace — remote dispatch", () => { expect(ws.fs).toEqual({ marker: "fs" }); expect(ws.git).toEqual({ marker: "git" }); expect(ws.artifacts).toEqual({ marker: "artifacts" }); + expect(ws).not.toHaveProperty("readFile"); }); it("disposing the client disposes the remote stub", async () => { @@ -116,6 +137,30 @@ describe("getWorkspace — remote dispatch", () => { ws[Symbol.dispose](); expect(disposed()).toBe(true); }); + + it("disposes the remote stub when client initialization fails", async () => { + const { host, disposed } = fakeBrokenRemote(); + + await expect(getWorkspace(host)).rejects.toThrow("compatibility lookup failed"); + expect(disposed()).toBe(true); + }); + + it("adds Think compatibility when the remote Workspace enables it", async () => { + const workspace = new Workspace({ + storage: new SQLiteTestStorage(), + useThink: true, + }); + await workspace.fs.writeFile("/notes.txt", "hello"); + const host: WorkspaceStubHost = { + __getWorkspaceStub: () => Promise.resolve(workspace.stub()), + }; + + const client = (await getWorkspace(host)) as WorkspaceClient & ThinkWorkspaceCompatibility; + + expect(client).toHaveProperty("readFile"); + await expect(client.readFile("/notes.txt")).resolves.toBe("hello"); + await expect(client.readFile("/missing.txt")).resolves.toBeNull(); + }); }); describe("getWorkspace — local dispatch", () => { @@ -124,6 +169,7 @@ describe("getWorkspace — local dispatch", () => { const ws = await getWorkspace(host); await ws.shell.exec`cat ${"my file.txt"}`; expect(calls[0].command).toBe("cat 'my file.txt'"); + expect(ws).not.toHaveProperty("readFile"); }); it("does not throw on dispose (the durable object owns the lifecycle)", async () => { @@ -131,6 +177,20 @@ describe("getWorkspace — local dispatch", () => { const ws = await getWorkspace(host); expect(() => ws[Symbol.dispose]()).not.toThrow(); }); + + it("adds Think compatibility when the local Workspace enables it", async () => { + const workspace = new Workspace({ + storage: new SQLiteTestStorage(), + useThink: true, + }); + const host = { [WORKSPACE]: workspace }; + const { shell } = fakeShell(); + Object.defineProperty(workspace, "shell", { get: () => shell }); + + const client = (await getWorkspace(host)) as WorkspaceClient & ThinkWorkspaceCompatibility; + + expect(client).toHaveProperty("readFile"); + }); }); describe("client shell.exec — tagged template form", () => { diff --git a/packages/computer/src/client.ts b/packages/computer/src/client.ts index 03f9d29..6895076 100644 --- a/packages/computer/src/client.ts +++ b/packages/computer/src/client.ts @@ -30,11 +30,17 @@ // a `TemplateStringsArray`'s `.raw` does not survive structured clone // over RPC. +import type { WorkspaceFilesystem } from "@cloudflare/dofs"; + import { decodeExecEvents } from "./exec-wire.js"; import { type ShellValue, sh } from "./sh.js"; import type { ExecEncoding, WorkspaceExecEvent } from "./shell.js"; import { WORKSPACE, type WorkspaceStubHost } from "./with-workspace.js"; -import { Workspace } from "./workspace.js"; +import { + createThinkCompatibility, + type ThinkWorkspaceCompatibility, + Workspace, +} from "./workspace.js"; // The remote shell handle stub: a result / stream / kill surface // carried across Workers RPC. @@ -198,12 +204,10 @@ function makeShellClient( // The canonical client surface. `shell.exec` is the adapted member; // `fs`, `git`, `artifacts`, and `assets` are the underlying surface's -// members, passed through. Their concrete types differ between the -// local Workspace and the remote stub, so they're surfaced loosely -// here and narrowed by the caller when needed. -export interface WorkspaceClient { - // biome-ignore lint/suspicious/noExplicitAny: fs type differs local vs remote - readonly fs: any; +// members, passed through. The filesystem stub mirrors the local +// filesystem, so it also serves as the common client type. +export interface WorkspaceClient extends Partial { + readonly fs: WorkspaceFilesystem; // biome-ignore lint/suspicious/noExplicitAny: handle types differ per path readonly shell: WorkspaceShellClient; // biome-ignore lint/suspicious/noExplicitAny: git type differs local vs remote @@ -220,9 +224,10 @@ function makeClient( surface: any, rehydrate: (handle: unknown) => unknown, dispose: () => void, + useThink: boolean, ): WorkspaceClient { const shell = makeShellClient(surface.shell as UnderlyingShell, rehydrate); - return { + const client: WorkspaceClient = { get fs() { return surface.fs; }, @@ -238,6 +243,8 @@ function makeClient( }, [Symbol.dispose]: dispose, }; + if (useThink) Object.assign(client, createThinkCompatibility(client.fs)); + return client; } // What `getWorkspace` accepts: a local host carrying the symbol stash @@ -263,17 +270,24 @@ export async function getWorkspace(handle: WorkspaceHandle): Promise h, () => {}, + local.useThink, ); } // Remote path: fetch the stub over RPC and delegate to it. Handle // stubs need inflating from their JSONL stream into a host-shaped // ExecHandle. const stub = await (handle as WorkspaceStubHost).__getWorkspaceStub(); - return makeClient( - stub, - (h) => rebuildExecHandle(h as RemoteExecHandle), - () => { - (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.(); - }, - ); + try { + return makeClient( + stub, + (h) => rebuildExecHandle(h as RemoteExecHandle), + () => { + (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.(); + }, + await stub.useThink, + ); + } catch (error) { + (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.(); + throw error; + } } diff --git a/packages/computer/src/stub.ts b/packages/computer/src/stub.ts index 7d5893c..8e11744 100644 --- a/packages/computer/src/stub.ts +++ b/packages/computer/src/stub.ts @@ -678,6 +678,7 @@ export class WorkspaceStub extends RpcTarget { readonly #git: WorkspaceGitStub; readonly #assets: WorkspaceAssetsStub | undefined; readonly #artifacts: WorkspaceArtifactsStub; + readonly #useThink: boolean; constructor(ws: Workspace) { super(); @@ -686,6 +687,7 @@ export class WorkspaceStub extends RpcTarget { this.#git = new WorkspaceGitStub(ws); this.#assets = ws.assets === undefined ? undefined : new WorkspaceAssetsStub(ws); this.#artifacts = new WorkspaceArtifactsStub(ws.artifacts); + this.#useThink = ws.useThink; trackStub(this); } @@ -708,6 +710,10 @@ export class WorkspaceStub extends RpcTarget { return this.#fs; } + get useThink(): boolean { + return this.#useThink; + } + get shell(): WorkspaceShellStub { return this.#shell; } diff --git a/packages/computer/src/workspace.ts b/packages/computer/src/workspace.ts index 302e22f..c2b5091 100644 --- a/packages/computer/src/workspace.ts +++ b/packages/computer/src/workspace.ts @@ -165,6 +165,11 @@ export interface ThinkWorkspaceCompatibility { stat(path: string): Promise; } +export type ThinkWorkspaceFilesystem = Pick< + WorkspaceFilesystem, + "find" | "mkdir" | "readFile" | "readdir" | "rm" | "stat" | "writeFile" +>; + export class Workspace { readonly #db: Database; readonly #fs: WorkspaceFilesystem; @@ -184,6 +189,7 @@ export class Workspace { readonly #retryMaxAttempts: number; readonly #sessionId: string; readonly #defaultGitIdentity: GitIdentity | undefined; + readonly #useThink: boolean; readonly #assets: AssetsClient | undefined; readonly #artifacts: ArtifactClient; // Lazily-constructed git client, cached so the dynamic @@ -240,6 +246,7 @@ export class Workspace { ); this.#sessionId = options.sessionId ?? ""; this.#defaultGitIdentity = options.defaultGitIdentity; + this.#useThink = options.useThink ?? false; this.#artifacts = options.artifacts ? createArtifact( options.artifacts.binding, @@ -278,8 +285,8 @@ export class Workspace { mounts: this.#mounts, }); this.#assets = typeof options.assets === "function" ? options.assets(this) : options.assets; - if (options.useThink) { - const think = createThinkCompatibility(this); + if (this.#useThink) { + const think = createThinkCompatibility(this.fs); Object.assign(this, think); } } @@ -326,6 +333,10 @@ export class Workspace { return this.#fs; } + get useThink(): boolean { + return this.#useThink; + } + // Identifier for this workspace / session, as passed to the // constructor. Empty string when the caller did not supply one. // Forwarded to mount factories and used by the assets module to @@ -945,11 +956,13 @@ class WorkspaceShellRouter { } } -function createThinkCompatibility(ws: Workspace): ThinkWorkspaceCompatibility { +export function createThinkCompatibility( + fs: ThinkWorkspaceFilesystem, +): ThinkWorkspaceCompatibility { return { async readFile(path) { try { - return await ws.fs.readFile(path, "utf8"); + return await fs.readFile(path, "utf8"); } catch (err) { if (isEnoent(err)) return null; throw err; @@ -957,17 +970,17 @@ function createThinkCompatibility(ws: Workspace): ThinkWorkspaceCompatibility { }, async readFileBytes(path) { try { - return await drainBytes(await ws.fs.readFile(path)); + return await drainBytes(await fs.readFile(path)); } catch (err) { if (isEnoent(err)) return null; throw err; } }, async writeFile(path, content) { - await ws.fs.writeFile(path, content); + await fs.writeFile(path, content); }, async readDir(dir, opts) { - const entries = await ws.fs.readdir(dir); + const entries = await fs.readdir(dir); const offset = opts?.offset ?? 0; const limit = opts?.limit ?? entries.length; return entries.slice(offset, offset + limit).map((entry) => @@ -982,11 +995,11 @@ function createThinkCompatibility(ws: Workspace): ThinkWorkspaceCompatibility { ); }, async rm(path, opts) { - await ws.fs.rm(path, opts); + await fs.rm(path, opts); }, async glob(pattern) { const { directory, relativePattern } = splitGlobPattern(pattern); - const matches = await ws.fs.find(directory, relativePattern); + const matches = await fs.find(directory, relativePattern); return matches.map((match) => toThinkFileInfo({ path: match.path, @@ -999,11 +1012,11 @@ function createThinkCompatibility(ws: Workspace): ThinkWorkspaceCompatibility { ); }, async mkdir(path, opts) { - await ws.fs.mkdir(path, opts); + await fs.mkdir(path, opts); }, async stat(path) { try { - const stat = await ws.fs.stat(path); + const stat = await fs.stat(path); return toThinkFileInfo({ ...stat, path, name: basename(path) }); } catch (err) { if (isEnoent(err)) return null;