diff --git a/.changeset/container-sync-report.md b/.changeset/container-sync-report.md new file mode 100644 index 00000000..4b39440f --- /dev/null +++ b/.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. diff --git a/.changeset/isolate-values.md b/.changeset/isolate-values.md new file mode 100644 index 00000000..bf76fc94 --- /dev/null +++ b/.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. diff --git a/.changeset/remote-client-assets.md b/.changeset/remote-client-assets.md new file mode 100644 index 00000000..64ef299d --- /dev/null +++ b/.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. diff --git a/docs/17_isolate_javascript.md b/docs/17_isolate_javascript.md index 03f74cd1..3b8d1db6 100644 --- a/docs/17_isolate_javascript.md +++ b/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`. diff --git a/packages/computer/src/backends/worker-javascript/module-graph.ts b/packages/computer/src/backends/worker-javascript/module-graph.ts index 08de6ac8..34e9026c 100644 --- a/packages/computer/src/backends/worker-javascript/module-graph.ts +++ b/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; } `; } diff --git a/packages/computer/src/client.test.ts b/packages/computer/src/client.test.ts index 7111edab..46855bc1 100644 --- a/packages/computer/src/client.test.ts +++ b/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()] }), diff --git a/packages/computer/src/client.ts b/packages/computer/src/client.ts index 5247b105..949432fc 100644 --- a/packages/computer/src/client.ts +++ b/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 {}, 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 void })[Symbol.dispose]?.(); diff --git a/packages/computer/src/modules/container.test.ts b/packages/computer/src/modules/container.test.ts index 6a1ff849..f7e151d8 100644 --- a/packages/computer/src/modules/container.test.ts +++ b/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"); }); diff --git a/packages/computer/src/modules/container.ts b/packages/computer/src/modules/container.ts index 0002a082..cf8068b8 100644 --- a/packages/computer/src/modules/container.ts +++ b/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; diff --git a/packages/computer/src/modules/git.ts b/packages/computer/src/modules/git.ts index cce42eed..33b3380b 100644 --- a/packages/computer/src/modules/git.ts +++ b/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 \`.${allowNetwork ? "" : " Network commands such as clone, fetch, and push are not allowed."}`, }); } diff --git a/packages/computer/src/runtime/capability.ts b/packages/computer/src/runtime/capability.ts index 96e8a500..af78ec62 100644 --- a/packages/computer/src/runtime/capability.ts +++ b/packages/computer/src/runtime/capability.ts @@ -274,6 +274,10 @@ function assertValue(value: unknown, seen: WeakSet): 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); } diff --git a/packages/computer/src/stub.ts b/packages/computer/src/stub.ts index 2759b141..9e7d058f 100644 --- a/packages/computer/src/stub.ts +++ b/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; } diff --git a/packages/computer/tests/script-runner.test.ts b/packages/computer/tests/script-runner.test.ts index cdda1a21..f01debf8 100644 --- a/packages/computer/tests/script-runner.test.ts +++ b/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: `