computer: Fix issues found by a deployed test of the stack

.changeset/container-sync-report.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/computer": patch
+---
+
+`ws:container`'s `exec` also returns `sync: { status, skipped, skippedCount, error? }`. `status` is `pending` when the container's file changes have not reached the Workspace, and `skipped` lists up to 100 paths the Workspace refused, such as files in a read-only mount, with `skippedCount` giving the full count. The docs now state that the sync is last-writer-wins: the container's changes replace files written in the Workspace while the command runs.

.changeset/isolate-values.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/computer": patch
+---
+
+A `WorkerJavaScriptBackend` run that returns an object with `undefined` fields now completes with those fields dropped, as `JSON.stringify` does, instead of failing with "must be JSON-compatible values". A cyclic argument to a `node:fs` or host module call now fails with a clear "must be acyclic" error instead of a stack overflow.

.changeset/remote-client-assets.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/computer": patch
+---
+
+A remote `WorkspaceClient` from `getWorkspace(stub)` reports `assets` as `undefined` when the Workspace has no assets publisher, as a local client does. `createAITools` built from a remote client no longer offers a `publish` tool that fails when called.

docs/17_isolate_javascript.md

@@ -287,6 +287,8 @@ export default async function () {
 
 `exec(command, { cwd, env, stdin, timeoutMs })` runs through `workspace.runtime.exec` on the container backend: `ContainerBackend`, registered as `"container-shell"` unless you pass `backend`. If that backend is missing, or runs module source rather than shell commands, the JavaScript backend fails to connect. The container shares the Workspace's files: writes the module made before the call are pushed to the container, and the container's changes are pulled back before `exec` returns. A non-zero exit code comes back as a value, not as an error.
 
+The result also carries `sync`: `{ status, skipped, skippedCount, error? }`. `status` is `pending` when the container's file changes have not reached the Workspace yet, and `error` says why. `skipped` lists up to 100 paths the container wrote that the Workspace refused, such as files in a read-only mount, and `skippedCount` gives the full count. The sync is last-writer-wins: the container's changes replace files written in the Workspace while the command runs, without reporting them as skipped. Don't write files from the isolate that the running command also writes.
+
 A few limits follow from `exec` being a host call:
 
 - Output comes back when the command finishes, not while it runs. Each stream is cut at `maxOutputBytes` (64 KiB by default), which must stay well under the backend's `maxCapabilityBytes`.

packages/computer/src/backends/worker-javascript/module-graph.ts

@@ -359,14 +359,16 @@ function capabilitiesModule(maxCapabilityBytes: number) {
       }
       return payload.result;
     }
-    function approximateBytes(value) {
+    function approximateBytes(value, seen = new Set()) {
       if (typeof value === "string") return value.length;
       if (value instanceof Uint8Array) return value.byteLength;
-      if (Array.isArray(value)) return value.reduce((total, item) => total + approximateBytes(item), 8);
-      if (value && typeof value === "object") {
-        return Object.entries(value).reduce((total, [key, item]) => total + key.length + approximateBytes(item), 8);
-      }
-      return 8;
+      if (!value || typeof value !== "object") return 8;
+      if (seen.has(value)) throw new Error("Workspace capability request values must be acyclic.");
+      seen.add(value);
+      const entries = Array.isArray(value) ? value.map((item) => ["", item]) : Object.entries(value);
+      const total = entries.reduce((sum, [key, item]) => sum + key.length + approximateBytes(item, seen), 8);
+      seen.delete(value);
+      return total;
     }
   `;
 }

packages/computer/src/client.test.ts

@@ -426,6 +426,26 @@ describe("getWorkspace — backend information", () => {
     });
   }
 
+  for (const [path, connect] of [
+    ["local", (ws: Workspace) => getWorkspace({ [WORKSPACE]: ws })],
+    [
+      "remote",
+      (ws: Workspace) => getWorkspace({ __getWorkspaceStub: () => Promise.resolve(ws.stub()) }),
+    ],
+  ] as const) {
+    it(`leaves publish out on a ${path} client without assets`, async () => {
+      const workspace = new Workspace({
+        storage: new SQLiteTestStorage(),
+        backends: [echoBackend()],
+      });
+      const client = await connect(workspace);
+
+      expect(client.assets).toBeUndefined();
+      expect(createAITools({ workspace: client }).publish).toBeUndefined();
+      await workspace.close();
+    });
+  }
+
   it("keeps its backend snapshot from being edited", async () => {
     const client = await getWorkspace({
       [WORKSPACE]: new Workspace({ storage: new SQLiteTestStorage(), backends: [echoBackend()] }),

packages/computer/src/client.ts

@@ -351,6 +351,7 @@ function makeClient(
   dispose: () => void,
   useThink: boolean,
   backends: readonly WorkspaceBackendInfo[],
+  hasAssets: boolean,
 ): WorkspaceClient {
   const runtime = makeRuntimeClient(
     surface.runtime as UnderlyingRuntime,
@@ -365,8 +366,10 @@ function makeClient(
     get git() {
       return surface.git;
     },
+    // Undefined when the Workspace has no assets publisher, so tools
+    // built from the client leave `publish` out, as they do locally.
     get assets() {
-      return surface.assets;
+      return hasAssets ? surface.assets : undefined;
     },
     get artifacts() {
       return surface.artifacts;
@@ -404,6 +407,7 @@ export async function getWorkspace(handle: WorkspaceHandle): Promise<WorkspaceCl
       () => {},
       local.useThink,
       local.runtime.backends(),
+      local.assets !== undefined,
     );
   }
   // Remote path: fetch the stub over RPC and delegate to it. Handle
@@ -419,6 +423,7 @@ export async function getWorkspace(handle: WorkspaceHandle): Promise<WorkspaceCl
       },
       await stub.useThink,
       await stub.runtime.backends(),
+      await stub.hasAssets,
     );
   } catch (error) {
     (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.();

packages/computer/src/modules/container.test.ts

@@ -5,6 +5,7 @@ import type {
   WorkspaceModuleFunction,
   WorkspaceModuleHost,
 } from "../runtime/types.js";
+import type { ExecSyncResult } from "../shell.js";
 import { createContainerModule } from "./container.js";
 
 interface ExecOptions {
@@ -29,6 +30,7 @@ function fakeRuntime(output: {
   stdout?: string;
   stderr?: string;
   hang?: boolean;
+  sync?: ExecSyncResult;
 }) {
   const runs: Run[] = [];
   const runtime = {
@@ -51,6 +53,7 @@ function fakeRuntime(output: {
             exitCode: run.killed ? 130 : (output.exitCode ?? 0),
             stdout: output.stdout ?? "",
             stderr: output.stderr ?? "",
+            sync: output.sync ?? { status: "complete" as const, applied: 0, skipped: [] },
           };
         },
         async kill() {
@@ -98,7 +101,12 @@ describe("createContainerModule", () => {
         ["npm test", { cwd: "/workspace/app", env: { CI: "1" }, stdin: "y\n" }],
         callContext(),
       ),
-    ).resolves.toEqual({ exitCode: 3, stdout: "out", stderr: "err" });
+    ).resolves.toEqual({
+      exitCode: 3,
+      stdout: "out",
+      stderr: "err",
+      sync: { status: "complete", skipped: [], skippedCount: 0 },
+    });
     expect(runs).toHaveLength(1);
     expect(runs[0]).toMatchObject({
       command: "npm test",
@@ -181,6 +189,7 @@ describe("createContainerModule", () => {
       exitCode: 0,
       stdout: "a🙂\n\n[truncated, 1 more bytes]",
       stderr: "🙂\n\n[truncated, 4 more bytes]",
+      sync: { status: "complete", skipped: [], skippedCount: 0 },
     });
   });
 
@@ -224,6 +233,54 @@ describe("createContainerModule", () => {
     );
   });
 
+  it("reports a sync that has not reached the Workspace, and skipped paths", async () => {
+    const { runtime } = fakeRuntime({
+      sync: {
+        status: "pending",
+        applied: 1,
+        error: "pull failed",
+        skipped: [
+          {
+            path: "/workspace/ro/x.txt",
+            mountRoot: "/workspace/ro",
+            op: "write",
+            reason: "read-only",
+          },
+        ],
+      },
+    });
+    const container = build(runtime);
+
+    await expect(container.exec(["touch ro/x.txt"], callContext())).resolves.toMatchObject({
+      sync: {
+        status: "pending",
+        error: "pull failed",
+        skipped: ["/workspace/ro/x.txt"],
+        skippedCount: 1,
+      },
+    });
+  });
+
+  it("caps a large skipped list so the result fits the bridge limits", async () => {
+    const skipped = Array.from({ length: 5000 }, (_, index) => ({
+      path: `/workspace/ro/${index}.txt`,
+      mountRoot: "/workspace/ro",
+      op: "write" as const,
+      reason: "read-only" as const,
+    }));
+    const { runtime } = fakeRuntime({
+      sync: { status: "pending", applied: 0, error: "e".repeat(10_000), skipped },
+    });
+    const container = build(runtime);
+
+    const result = (await container.exec(["touch ro/*"], callContext())) as {
+      sync: { skipped: string[]; skippedCount: number; error: string };
+    };
+    expect(result.sync.skipped).toHaveLength(100);
+    expect(result.sync.skippedCount).toBe(5000);
+    expect(new TextEncoder().encode(result.sync.error).byteLength).toBeLessThan(1200);
+  });
+
   it("describes itself for a model", () => {
     expect(createContainerModule().description).toContain("full Linux container");
   });

packages/computer/src/modules/container.ts

@@ -21,11 +21,14 @@ import type {
   WorkspaceModuleHost,
   WorkspaceRuntimeValue,
 } from "../runtime/types.js";
+import type { ExecSyncResult } from "../shell.js";
 import { truncateText } from "../text-truncation.js";
 
 const DEFAULT_BACKEND = "container-shell";
 const DEFAULT_MAX_OUTPUT_BYTES = 64 * 1024;
 const EXEC_OPTION_KEYS = new Set(["cwd", "env", "stdin", "timeoutMs"]);
+const MAX_SKIPPED_PATHS = 100;
+const MAX_SYNC_ERROR_BYTES = 1024;
 
 /** Options for {@link createContainerModule}. */
 export interface ContainerModuleOptions {
@@ -45,7 +48,9 @@ export interface ContainerModuleOptions {
  * backend.
  *
  * It exports `exec(command, { cwd, env, stdin, timeoutMs })`, which
- * returns `{ exitCode, stdout, stderr }` once the command finishes. A
+ * returns `{ exitCode, stdout, stderr, sync }` once the command
+ * finishes. `sync` reports whether the container's file changes reached
+ * the Workspace, the first 100 paths it skipped, and `skippedCount`. A
  * non-zero exit code is a normal result, not an error. Cancelling the
  * execution kills the command.
  *
@@ -118,6 +123,7 @@ export function createContainerModule(
           exitCode: result.exitCode,
           stdout: truncateText(result.stdout, maxOutputBytes),
           stderr: truncateText(result.stderr, maxOutputBytes),
+          sync: syncSummary(result.sync),
         };
       } finally {
         context.signal.removeEventListener("abort", kill);
@@ -131,8 +137,28 @@ const DESCRIPTION = [
   "Use it for npm, node, python, package managers, and native binaries. The container can take a while to start on first use.",
   'Call `const { exitCode, stdout, stderr } = await exec("npm test", { cwd: "/workspace" })`. Options are `cwd`, `env`, `stdin`, and `timeoutMs`.',
   "Output comes back when the command finishes, and long output is truncated. A non-zero `exitCode` is returned, not thrown.",
+  "`sync.status` is `pending` if the container's file changes have not reached the workspace yet. The container's changes win over files written meanwhile, so do not write files the command also writes while it runs.",
 ].join(" ");
 
+// How the container's file changes came back to the Workspace. A
+// "pending" status means they did not, yet; `skipped` lists paths the
+// container wrote that the Workspace refused, such as read-only mounts.
+//
+// The list is capped, and the error cut short, so a command that skips
+// thousands of paths still fits within the bridge's response limits;
+// otherwise the call would fail after the command had already run.
+// `skippedCount` is the full count.
+function syncSummary(sync: ExecSyncResult) {
+  return {
+    status: sync.status,
+    skipped: sync.skipped.slice(0, MAX_SKIPPED_PATHS).map((entry) => entry.path),
+    skippedCount: sync.skipped.length,
+    ...(sync.status === "pending" && sync.error !== undefined
+      ? { error: truncateText(sync.error, MAX_SYNC_ERROR_BYTES) }
+      : {}),
+  };
+}
+
 interface ExecRequest {
   readonly command: string;
   readonly cwd: string | undefined;

packages/computer/src/modules/git.ts

@@ -85,7 +85,7 @@ export function createGitModule(options: GitModuleOptions = {}): WorkspaceModule
     },
   });
   return Object.assign(create, {
-    description: `The workspace's Git repository tools: \`status({ dir })\`, \`diff({ dir })\`, \`log({ dir, depth })\`, \`clone({ url, dir })\`, and \`cli({ argv, cwd })\` for any other git subcommand.${allowNetwork ? "" : " Network commands such as clone, fetch, and push are not allowed."}`,
+    description: `The workspace's Git repository tools: \`status({ dir })\`, \`diff({ dir })\`, \`log({ dir, depth })\`, \`clone({ url, dir })\`, and \`cli({ argv, cwd })\` for any other git subcommand, including a leading \`-C <path>\`.${allowNetwork ? "" : " Network commands such as clone, fetch, and push are not allowed."}`,
   });
 }
 

packages/computer/src/runtime/capability.ts

@@ -274,6 +274,10 @@ function assertValue(value: unknown, seen: WeakSet<object>): void {
   if (prototype !== Object.prototype && prototype !== null) {
     throw new Error("Workspace code inputs and results must use plain objects.");
   }
-  for (const item of Object.values(value)) assertValue(item, seen);
+  // An undefined field is absent, as with JSON.stringify, which drops
+  // it when the value is framed.
+  for (const item of Object.values(value)) {
+    if (item !== undefined) assertValue(item, seen);
+  }
   seen.delete(value);
 }

packages/computer/src/stub.ts

@@ -667,6 +667,13 @@ export class WorkspaceStub extends RpcTarget {
     return this.#assets;
   }
 
+  // Whether the Workspace has an assets publisher. Reading `assets`
+  // over RPC always yields a placeholder, so a client asks this plain
+  // boolean once instead.
+  get hasAssets(): boolean {
+    return this.#assets !== undefined;
+  }
+
   get artifacts(): WorkspaceArtifactsStub {
     return this.#artifacts;
   }

packages/computer/tests/script-runner.test.ts

@@ -398,6 +398,42 @@ describe("WorkspaceRuntime", () => {
     expect(JSON.parse(text).result.value).toContain('unknown option "shell"');
   });
 
+  it("rejects a cyclic argument with a clear error", async () => {
+    const response = await runtime({
+      source: `
+        import { echo } from "ws:test-host";
+        export default async () => {
+          const value = {};
+          value.self = value;
+          try {
+            await echo(value);
+            return "sent";
+          } catch (error) {
+            return error.message;
+          }
+        };
+      `,
+      cwd: "/workspace",
+    });
+    const text = await response.text();
+    expect(response.status, text).toBe(200);
+    expect(JSON.parse(text).result.value, text).toContain("acyclic");
+  });
+
+  it("drops undefined fields from a run result, as JSON does", async () => {
+    const response = await runtime({
+      source: `export default () => ({ kept: 1, dropped: undefined, nested: { also: undefined } });`,
+      cwd: "/workspace",
+    });
+    const text = await response.text();
+    expect(response.status, text).toBe(200);
+    expect(JSON.parse(text).result, text).toMatchObject({
+      status: "completed",
+      value: { kept: 1, nested: {} },
+    });
+    expect(JSON.parse(text).result.value).not.toHaveProperty("dropped");
+  });
+
   it("does not expose unrestricted host operations through the node:fs dispatcher", async () => {
     const response = await runtime({
       source: `