Fix small bugs with exec

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):

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:
 
@@ -419,6 +420,36 @@ stub's `exec` rejects a raw tagged-template call so the unescaped path
 fails loudly. `shellQuote` is exported too, for the rare case where
 you need to quote a single argument outside a template.
 
+### Reattaching to a run
+
+A command outlives the request that started it. Every handle carries
+the run's `id`, and `shell.get(id)` reattaches to it from a later
+request:
+
+```ts
+using ws = await getWorkspace(env.ContainerExample.get(id));
+
+// First request: start a long install and remember its id.
+using started = await ws.shell.exec("npm install", { id: "install-1" });
+
+// Later request, new handle, same run. `resume` picks where the
+// replayed event stream starts: "tail" for live events only, "full"
+// (the default) for everything the runner still holds, or a sequence
+// number to resume after.
+using again = await ws.shell.get("install-1", { encoding: "utf8", resume: "tail" });
+const { exitCode } = await again.result();
+```
+
+The client mints an id when the caller doesn't pass one, so
+`handle.id` is available either way. Reattach doesn't run the
+push/pull bracket — it joins a run already in flight — so its sync
+counts cover only what lands after the reattach.
+
+A run streams to one reader at a time. Dropping a handle — the `using`
+above, or an explicit `cancel()` — hands the stream back, which is what
+lets the next request reattach. Reattaching while an earlier handle is
+still reading is refused.
+
 ## Observability
 
 The package emits one span per documented operation through an optional

packages/computer/src/client.test.ts

@@ -10,24 +10,34 @@
 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;
   options: Record<string, unknown> | undefined;
 }
 
-// A fake shell that records exec calls. Stands in for both
+interface GetCall {
+  id: string;
+  options: Record<string, unknown> | undefined;
+}
+
+// A fake shell that records exec and get calls. Stands in for both
 // Workspace.shell (local) and the shell stub (remote) — the client
-// only needs `exec(command, options?)`.
+// only needs `exec(command, options?)` and `get(id, options?)`.
 function fakeShell(): {
-  shell: { exec: (c: string, o?: Record<string, unknown>) => Promise<unknown> };
+  shell: {
+    exec: (c: string, o?: Record<string, unknown>) => Promise<unknown>;
+    get: (id: string, o?: Record<string, unknown>) => Promise<unknown>;
+  };
   calls: ExecCall[];
+  gets: GetCall[];
   disposedHandles: () => number;
 } {
   const calls: ExecCall[] = [];
+  const gets: GetCall[] = [];
   let disposedHandles = 0;
   // A minimal handle satisfying both the local identity rehydrate
   // (which passes it through) and the remote rebuild (which calls
@@ -53,12 +63,17 @@ function fakeShell(): {
   });
   return {
     calls,
+    gets,
     disposedHandles: () => disposedHandles,
     shell: {
       exec(command: string, options?: Record<string, unknown>) {
         calls.push({ command, options });
         return Promise.resolve(makeHandle());
       },
+      get(id: string, options?: Record<string, unknown>) {
+        gets.push({ id, options });
+        return Promise.resolve(makeHandle());
+      },
     },
   };
 }
@@ -68,13 +83,15 @@ function fakeShell(): {
 function fakeRemote(): {
   host: WorkspaceStubHost;
   calls: ExecCall[];
+  gets: GetCall[];
   disposed: () => boolean;
   disposedHandles: () => number;
 } {
-  const { shell, calls, disposedHandles } = fakeShell();
+  const { shell, calls, gets, disposedHandles } = fakeShell();
   let disposed = false;
   const stub = {
     fs: { marker: "fs" },
+    useThink: false,
     git: { marker: "git" },
     assets: undefined,
     artifacts: { marker: "artifacts" },
@@ -85,20 +102,44 @@ function fakeRemote(): {
   };
   return {
     calls,
+    gets,
     disposed: () => disposed,
     disposedHandles,
     host: { __getWorkspaceStub: () => Promise.resolve(stub as never) },
   };
 }
 
+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.
-function fakeLocal(): { host: { [WORKSPACE]: Workspace }; calls: ExecCall[] } {
+function fakeLocal(): {
+  host: { [WORKSPACE]: Workspace };
+  calls: ExecCall[];
+  gets: GetCall[];
+} {
   const ws = new Workspace({ storage: new SQLiteTestStorage() });
-  const { shell, calls } = fakeShell();
+  const { shell, calls, gets } = fakeShell();
   Object.defineProperty(ws, "shell", { get: () => shell });
-  return { calls, host: { [WORKSPACE]: ws } };
+  return { calls, gets, host: { [WORKSPACE]: ws } };
 }
 
 describe("getWorkspace — remote dispatch", () => {
@@ -108,6 +149,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 +158,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,13 +190,28 @@ 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 () => {
     const { host } = fakeLocal();
     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", () => {
@@ -145,7 +226,7 @@ describe("client shell.exec — tagged template form", () => {
     const { host, calls } = fakeRemote();
     const ws = await getWorkspace(host);
     await ws.shell.exec`ls`;
-    expect(calls[0].options).toEqual({ encoding: "utf8" });
+    expect(calls[0].options).toMatchObject({ encoding: "utf8" });
   });
 
   it("quotes each element of an interpolated array", async () => {
@@ -157,21 +238,19 @@ describe("client shell.exec — tagged template form", () => {
 });
 
 describe("client shell.exec — plain string form", () => {
-  it("forwards a bare command with no options", async () => {
+  it("forwards a bare command", async () => {
     const { host, calls } = fakeRemote();
     const ws = await getWorkspace(host);
     await ws.shell.exec("npm test");
-    expect(calls[0]).toEqual({ command: "npm test", options: undefined });
+    expect(calls[0].command).toBe("npm test");
   });
 
   it("forwards options unchanged", async () => {
     const { host, calls } = fakeRemote();
     const ws = await getWorkspace(host);
     await ws.shell.exec("npm test", { cwd: "/workspace", backend: "sandbox" });
-    expect(calls[0]).toEqual({
-      command: "npm test",
-      options: { cwd: "/workspace", backend: "sandbox" },
-    });
+    expect(calls[0].command).toBe("npm test");
+    expect(calls[0].options).toMatchObject({ cwd: "/workspace", backend: "sandbox" });
   });
 
   it("does not escape a plain string command", async () => {
@@ -214,6 +293,25 @@ describe("client shell.exec — remote handle rebuild", () => {
     expect(() => (handle as { result(): unknown }).result()).toThrow(/already streaming/);
   });
 
+  it("carries the id the caller asked for onto the rebuilt handle", async () => {
+    const { host, calls } = fakeRemote();
+    const ws = await getWorkspace(host);
+    const handle = await ws.shell.exec("npm install", { id: "install-1" });
+    expect(handle.id).toBe("install-1");
+    expect(calls[0].options).toMatchObject({ id: "install-1" });
+  });
+
+  it("mints an id when the caller omits one so the handle can be reattached", async () => {
+    const { host, calls } = fakeRemote();
+    const ws = await getWorkspace(host);
+    const handle = await ws.shell.exec("npm install");
+    // The minted id is what went over the wire, so a later
+    // shell.get(handle.id) addresses this run.
+    expect(typeof handle.id).toBe("string");
+    expect(handle.id).not.toBe("");
+    expect(calls[0].options?.id).toBe(handle.id);
+  });
+
   it("disposes the remote handle when the rebuilt handle is disposed", async () => {
     const { host, disposedHandles } = fakeRemote();
     const ws = await getWorkspace(host);
@@ -222,3 +320,25 @@ describe("client shell.exec — remote handle rebuild", () => {
     expect(disposedHandles()).toBe(1);
   });
 });
+
+describe("client shell.get — reattach", () => {
+  it("reattaches over RPC and rebuilds a handle carrying the run's id", async () => {
+    const { host, gets } = fakeRemote();
+    const ws = await getWorkspace(host);
+    const handle = await ws.shell.get("install-1", { encoding: "utf8", resume: "tail" });
+    expect(gets[0]).toEqual({
+      id: "install-1",
+      options: { encoding: "utf8", resume: "tail" },
+    });
+    expect(handle.id).toBe("install-1");
+    const result = await handle.result();
+    expect(result).toEqual({ exitCode: 0, stdout: "", stderr: "" });
+  });
+
+  it("delegates to the in-isolate Workspace on the local path", async () => {
+    const { host, gets } = fakeLocal();
+    const ws = await getWorkspace(host);
+    await ws.shell.get("install-1", { resume: 12 });
+    expect(gets[0]).toEqual({ id: "install-1", options: { resume: 12 } });
+  });
+});