diff --git a/packages/computer/README.md b/packages/computer/README.md index 0b86472..fd49151 100644 --- a/packages/computer/README.md +++ b/packages/computer/README.md @@ -420,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 diff --git a/packages/computer/src/client.test.ts b/packages/computer/src/client.test.ts index 4b47a29..76c211d 100644 --- a/packages/computer/src/client.test.ts +++ b/packages/computer/src/client.test.ts @@ -19,15 +19,25 @@ interface ExecCall { options: Record | undefined; } -// A fake shell that records exec calls. Stands in for both +interface GetCall { + id: string; + options: Record | 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) => Promise }; + shell: { + exec: (c: string, o?: Record) => Promise; + get: (id: string, o?: Record) => Promise; + }; 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) { calls.push({ command, options }); return Promise.resolve(makeHandle()); }, + get(id: string, options?: Record) { + gets.push({ id, options }); + return Promise.resolve(makeHandle()); + }, }, }; } @@ -68,10 +83,11 @@ 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" }, @@ -86,6 +102,7 @@ function fakeRemote(): { }; return { calls, + gets, disposed: () => disposed, disposedHandles, host: { __getWorkspaceStub: () => Promise.resolve(stub as never) }, @@ -114,11 +131,15 @@ function fakeBrokenRemote(): { // 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", () => { @@ -205,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 () => { @@ -217,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 () => { @@ -274,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); @@ -282,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 } }); + }); +}); diff --git a/packages/computer/src/client.ts b/packages/computer/src/client.ts index 6895076..dec68e7 100644 --- a/packages/computer/src/client.ts +++ b/packages/computer/src/client.ts @@ -52,7 +52,7 @@ interface RemoteExecHandle { } // Rebuild a host-shaped ExecHandle from a remote handle stub. The -// result is a ReadableStream of decoded events with result() and +// result is a ReadableStream of decoded events with id, result() and // kill() tacked on, matching what the local path returns from // Workspace.shell.exec. // @@ -61,7 +61,7 @@ interface RemoteExecHandle { // whichever is used first wins. result() goes through the stub's // run-and-wait path (which runs the post-exit pull); iterating goes // through the stub's byte stream (which doesn't). -function rebuildExecHandle(remote: RemoteExecHandle): unknown { +function rebuildExecHandle(remote: RemoteExecHandle, id: string): unknown { let started = false; let reader: ReadableStreamDefaultReader> | undefined; const stream = new ReadableStream>( @@ -101,11 +101,16 @@ function rebuildExecHandle(remote: RemoteExecHandle): un remote[Symbol.dispose]?.(); }; const handle = stream as ReadableStream> & { + readonly id: string; result(): Promise; kill(signal?: "SIGTERM" | "SIGKILL" | "SIGINT" | "SIGHUP"): Promise; [Symbol.dispose](): void; }; Object.defineProperties(handle, { + id: { + value: id, + enumerable: false, + }, result: { value: () => { if (started) { @@ -140,6 +145,15 @@ function rebuildExecHandle(remote: RemoteExecHandle): un // the underlying surface's default. Wrap an interpolated command // in `sh` to escape it: `exec(sh`cat ${file}`, { cwd })`. // +// `get` reattaches to a run by id, from any request and any handle: +// +// using ws = await getWorkspace(env.MyDO.get(id)); +// const handle = await ws.shell.get(runId, { resume: "tail" }); +// +// The id comes off a prior handle's `id` or from the caller's own +// `exec` options, so a long command outlives the request that started +// it. +// // `R` is the handle type the underlying surface returns (the host // `ExecHandle` locally, the handle stub remotely). export interface WorkspaceShellClient { @@ -147,6 +161,9 @@ export interface WorkspaceShellClient { exec(command: string): Promise; exec(command: string, options: ShellExecOptions & { encoding: "utf8" }): Promise; exec(command: string, options: ShellExecOptions): Promise; + get(id: string): Promise; + get(id: string, options: ShellGetOptions & { encoding: "utf8" }): Promise; + get(id: string, options: ShellGetOptions): Promise; } // Options accepted by the plain `exec` form, common to both paths. @@ -158,6 +175,16 @@ export interface ShellExecOptions { timeoutMs?: number; } +// Options accepted by `get`, common to both paths. `resume` picks +// where the replayed event stream starts: "tail" for live events only, +// "full" (the default) for everything buffered, or a sequence number +// to resume after. +export interface ShellGetOptions { + encoding?: "utf8"; + resume?: "tail" | "full" | number; + backend?: string; +} + function isTemplateStringsArray(value: unknown): value is TemplateStringsArray { // A tagged-template call hands the cooked strings as the first // argument: an array. A plain `exec(command)` call hands a string. @@ -165,19 +192,22 @@ function isTemplateStringsArray(value: unknown): value is TemplateStringsArray { } // The underlying shell surface both paths expose: an `exec` taking a -// command string and options. Locally this is `Workspace.shell`; -// remotely it's the shell stub. +// command string and options, and a `get` taking an exec id. Locally +// this is `Workspace.shell`; remotely it's the shell stub. interface UnderlyingShell { // biome-ignore lint/suspicious/noExplicitAny: bridges two concrete exec overload sets exec(command: string, options?: Record): Promise; + // biome-ignore lint/suspicious/noExplicitAny: bridges two concrete get overload sets + get(id: string, options?: Record): Promise; } function makeShellClient( shell: UnderlyingShell, // Adapts the handle the underlying `exec` resolves to: identity on - // the local path (already a host ExecHandle), rebuild on the remote - // path (a handle stub that needs inflating from its JSONL stream). - rehydrate: (handle: unknown) => unknown, + // the local path (already a host ExecHandle carrying its own id), + // rebuild on the remote path (a handle stub that needs inflating + // from its JSONL stream and carries no id of its own). + rehydrate: (handle: unknown, id: string) => unknown, // biome-ignore lint/suspicious/noExplicitAny: handle types differ per path ): WorkspaceShellClient { async function exec( @@ -189,17 +219,33 @@ function makeShellClient( if (isTemplateStringsArray(commandOrStrings)) { const values = optionsOrValue === undefined ? rest : [optionsOrValue as ShellValue, ...rest]; const command = sh(commandOrStrings, ...values); - return rehydrate(await shell.exec(command, { encoding: "utf8" })); + const id = crypto.randomUUID(); + return rehydrate(await shell.exec(command, { id, encoding: "utf8" }), id); } - const options = optionsOrValue as ShellExecOptions | undefined; + const options = (optionsOrValue as ShellExecOptions | undefined) ?? {}; + // Every run is addressed by an id the caller knows, so the handle + // exposes one on both paths and a later get() can reattach to a + // command that outlived the request that started it. A caller + // passing its own id keeps it; minting one here rather than + // reading the runner's back saves a round trip on the remote + // path, and the shell stub rejects a spawn that came back under a + // different id. + const id = options.id ?? crypto.randomUUID(); + return rehydrate(await shell.exec(commandOrStrings, { ...options, id }), id); + } + async function get( + id: string, + options?: ShellGetOptions, + // biome-ignore lint/suspicious/noExplicitAny: handle types differ per path + ): Promise { const handle = options === undefined - ? await shell.exec(commandOrStrings) - : await shell.exec(commandOrStrings, options as Record); - return rehydrate(handle); + ? await shell.get(id) + : await shell.get(id, options as Record); + return rehydrate(handle, id); } // biome-ignore lint/suspicious/noExplicitAny: handle types differ per path - return { exec } as WorkspaceShellClient; + return { exec, get } as WorkspaceShellClient; } // The canonical client surface. `shell.exec` is the adapted member; @@ -222,7 +268,7 @@ export interface WorkspaceClient extends Partial { function makeClient( // biome-ignore lint/suspicious/noExplicitAny: underlying surface differs per path surface: any, - rehydrate: (handle: unknown) => unknown, + rehydrate: (handle: unknown, id: string) => unknown, dispose: () => void, useThink: boolean, ): WorkspaceClient { @@ -280,7 +326,7 @@ export async function getWorkspace(handle: WorkspaceHandle): Promise rebuildExecHandle(h as RemoteExecHandle), + (h, id) => rebuildExecHandle(h as RemoteExecHandle, id), () => { (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.(); }, diff --git a/packages/computer/src/index.ts b/packages/computer/src/index.ts index dc925d4..9eeca77 100644 --- a/packages/computer/src/index.ts +++ b/packages/computer/src/index.ts @@ -27,6 +27,7 @@ export { TestBackend, type TestBackendOptions } from "./backends/test.js"; export { getWorkspace, type ShellExecOptions, + type ShellGetOptions, type WorkspaceClient, type WorkspaceHandle, type WorkspaceShellClient, @@ -73,6 +74,7 @@ export { type WorkspaceExecOptions, type WorkspaceExecResult, WorkspaceFilesystemStub, + type WorkspaceGetExecOptions, WorkspaceGitStub, WorkspaceShellStub, WorkspaceStub, diff --git a/packages/computer/src/observe-integration.test.ts b/packages/computer/src/observe-integration.test.ts index 1fe197e..564359e 100644 --- a/packages/computer/src/observe-integration.test.ts +++ b/packages/computer/src/observe-integration.test.ts @@ -301,6 +301,117 @@ describe("Workspace observer — shell stub", () => { expect(childNames).toContain("workspace.sync.pull"); }); + it("closes the exec span when the caller only kills the command", async () => { + // kill() is a terminal path for callers that never read the + // output: the signal goes out, the child exits, and nothing else + // touches the handle. The span has to close on that path, and the + // handle stays consumable so a caller can still collect the + // output of the killed run. + const observer = makeRecorder(); + let events!: ReadableStreamDefaultController; + const shellRpc: import("@cloudflare/computer-rpc").ShellRPC = { + async exec() { + return { + id: "exec-killed", + events: new ReadableStream({ + start(c) { + events = c; + }, + }), + }; + }, + async getExec() { + throw new Error("not used"); + }, + async killExec() { + events.enqueue({ id: "exec-killed", seq: 0, name: "exit", value: 137 }); + events.close(); + }, + async disposeExec() {}, + }; + const ws = new Workspace({ + storage: makeStorage(), + backends: [ + { + id: "shelled", + async connect() { + return { rpc: { sync: fakeSync(), shell: shellRpc }, close: async () => {} }; + }, + }, + ], + observer, + }); + await ws.ready(); + using handle = await new WorkspaceShellStub(ws).exec("sleep 100"); + await handle.kill("SIGKILL"); + + const execSpan = findSpan(observer.spans, "workspace.shell.exec"); + expect(execSpan.attributes["workspace.shell.sync.status"]).toBe("complete"); + expect(execSpan.attributes["workspace.shell.exit_code"]).toBe(-1); + + // The signal doesn't claim the handle: the output of the killed + // run is still there for the asking. + await expect(handle.result()).resolves.toMatchObject({ exitCode: 137 }); + }); + + it("leaves the span to a stream() the kill interrupts", async () => { + // Killing a command someone is streaming is not a terminal path + // for the span: the reader sees the exit event the signal + // produced, so it reports the real exit code and kill() keeps out + // of the way. + const observer = makeRecorder(); + let events!: ReadableStreamDefaultController; + const shellRpc: import("@cloudflare/computer-rpc").ShellRPC = { + async exec() { + return { + id: "exec-streamed", + events: new ReadableStream({ + start(c) { + events = c; + }, + }), + }; + }, + async getExec() { + throw new Error("not used"); + }, + async killExec() { + events.enqueue({ id: "exec-streamed", seq: 0, name: "exit", value: 137 }); + events.close(); + }, + async disposeExec() {}, + }; + const ws = new Workspace({ + storage: makeStorage(), + backends: [ + { + id: "shelled", + async connect() { + return { rpc: { sync: fakeSync(), shell: shellRpc }, close: async () => {} }; + }, + }, + ], + observer, + }); + await ws.ready(); + using handle = await new WorkspaceShellStub(ws).exec("sleep 100"); + const reader = handle.stream().getReader(); + const first = reader.read(); + + await handle.kill("SIGKILL"); + await first; + while (!(await reader.read()).done) { + // drain to close + } + reader.releaseLock(); + // The span closes once the reader reports the stream done; unlike + // result(), nothing hands the caller a promise to await for it. + await new Promise((resolve) => setTimeout(resolve, 0)); + + const execSpan = findSpan(observer.spans, "workspace.shell.exec"); + expect(execSpan.attributes["workspace.shell.exit_code"]).toBe(137); + }); + it("reports pending sync on the exec span without exposing secrets", async () => { const observer = makeRecorder(); const secret = "observer-secret"; diff --git a/packages/computer/src/shell.test.ts b/packages/computer/src/shell.test.ts index b3d1d50..7bce396 100644 --- a/packages/computer/src/shell.test.ts +++ b/packages/computer/src/shell.test.ts @@ -749,4 +749,66 @@ describe("WorkspaceShell.get — reattach", () => { expect(result.stdout).toBe("replay"); expect(result.exitCode).toBe(5); }); + + it("reattaches to a live run once the first handle is dropped", async () => { + // The runner allows one live subscriber per run, so dropping a + // handle has to give up its subscription. The handle keeps a + // second reader on the event stream to watch for the exit event + // (kill() awaits it), and that reader has to go too — otherwise + // the subscription outlives the handle and reattach is refused + // for the rest of the run. + const f = liveRpc(); + const shell = new WorkspaceShell(f.shell, makeSync()); + const started = await shell.exec("sleep 100", { id: "long-run" }); + await started.cancel(); + + const again = await shell.get("long-run", { encoding: "utf8", resume: "tail" }); + f.emit(stdout(1, "still here\n")); + f.emit(exit(2, 0)); + const result = await again.result(); + expect(result.stdout).toBe("still here\n"); + }); }); + +// A ShellRPC that models the runner's live subscriber bookkeeping: one +// subscriber per run, and the slot only frees when that subscriber +// cancels. Events are pushed by the test through emit(), so the run +// stays live for as long as the test wants it to. +function liveRpc(): { + shell: ShellRPC; + emit: (event: ExecEvent) => void; +} { + let subscriber: ReadableStreamDefaultController | undefined; + const subscribe = (id: string): ReadableStream => { + if (subscriber !== undefined) { + throw new Error(`EEXEC_BUSY: exec ${id} already has a live subscriber`); + } + return new ReadableStream({ + start(c) { + subscriber = c; + }, + cancel() { + subscriber = undefined; + }, + }); + }; + return { + // The runner closes the subscriber's stream after the exit + // event; result() drains until close. + emit: (event) => { + subscriber?.enqueue(event); + if (event.name === "exit") subscriber?.close(); + }, + shell: { + async exec(input) { + const id = input.id ?? "runner-minted-id"; + return { id, events: subscribe(id) }; + }, + async getExec(input) { + return { id: input.id, events: subscribe(input.id) }; + }, + async killExec() {}, + async disposeExec() {}, + }, + }; +} diff --git a/packages/computer/src/shell.ts b/packages/computer/src/shell.ts index fa3d5ad..e717c2f 100644 --- a/packages/computer/src/shell.ts +++ b/packages/computer/src/shell.ts @@ -236,8 +236,8 @@ function wrapHandle( pushed: number, ): ExecHandle { const [forUser, forWatcher] = wireEvents.tee(); - const exited = watchForExit(forWatcher); - const stream = pipeEvents(forUser, encoding); + const watcher = watchForExit(forWatcher); + const stream = pipeEvents(releaseWatcherOnCancel(forUser, watcher.release), encoding); const handle = stream as ExecHandle; // configurable: true on result/kill lets the Workspace-level // router redefine them to add cross-cutting concerns (transport @@ -254,7 +254,7 @@ function wrapHandle( kill: { value: async (signal?: KillSignal) => { await shell.killExec({ id, signal }); - await exited; + await watcher.exited; }, enumerable: false, writable: false, @@ -268,10 +268,14 @@ function wrapHandle( // first exit event is observed (or the stream closes / errors // without one). Errors are swallowed so kill() doesn't reject on a // torn-down wire — the caller's own branch will surface any real -// stream error. -function watchForExit(events: ReadableStream): Promise { - return (async () => { - const reader = events.getReader(); +// stream error. `release` abandons the watch, letting the branch +// cancel through to the wire stream. +function watchForExit(events: ReadableStream): { + exited: Promise; + release: () => void; +} { + const reader = events.getReader(); + const exited = (async () => { try { while (true) { const { value, done } = await reader.read(); @@ -284,6 +288,46 @@ function watchForExit(events: ReadableStream): Promise { reader.releaseLock(); } })(); + return { + exited, + release: () => { + reader.cancel().catch(() => {}); + }, + }; +} + +// A run allows one live subscriber in the container, so a caller +// that cancels its stream has to give the subscription up — until +// then a reattach through get() is refused for the rest of the run. +// Cancelling the caller's tee branch isn't enough: a tee holds its +// source until both branches cancel, and the watcher branch reads +// to the exit event. So the caller's cancel takes the watcher with +// it. +function releaseWatcherOnCancel( + source: ReadableStream, + release: () => void, +): ReadableStream { + const reader = source.getReader(); + return new ReadableStream( + { + pull: async (controller) => { + const { value, done } = await reader.read(); + if (done) { + controller.close(); + return; + } + controller.enqueue(value); + }, + cancel: async (reason) => { + // Order matters: the branch cancel doesn't settle until the + // watcher has cancelled too. + const cancelled = reader.cancel(reason); + release(); + await cancelled; + }, + }, + { highWaterMark: 0 }, + ); } function pipeEvents( diff --git a/packages/computer/src/stub.test.ts b/packages/computer/src/stub.test.ts index 45d493f..cf319c4 100644 --- a/packages/computer/src/stub.test.ts +++ b/packages/computer/src/stub.test.ts @@ -418,6 +418,87 @@ describe("WorkspaceStub", () => { ); }); + it("shell.exec forwards id and timeoutMs to the runner", async () => { + // The remote surface accepts the same options as the host shell, + // so a stable id and a per-call timeout have to reach the runner + // rather than being dropped at the boundary. + const requests: Array<{ id?: string; timeoutMs?: number }> = []; + const shellRpc: import("@cloudflare/computer-rpc").ShellRPC = { + async exec(request) { + requests.push({ id: request.id, timeoutMs: request.timeoutMs }); + return { + id: request.id ?? "e-1", + events: new ReadableStream({ + start(c) { + c.enqueue({ id: request.id ?? "e-1", seq: 1, name: "exit", value: 0 }); + c.close(); + }, + }), + }; + }, + getExec: () => Promise.reject(new Error("not used")), + killExec: () => Promise.reject(new Error("not used")), + disposeExec: () => Promise.reject(new Error("not used")), + }; + await withStub( + async (ws) => { + const stub = ws.stub(); + using handle = await stub.shell.exec("noop", { + encoding: "utf8", + id: "install-1", + timeoutMs: 250, + }); + await handle.result(); + expect(requests).toEqual([{ id: "install-1", timeoutMs: 250 }]); + }, + { backend: backend({ shell: shellRpc }) }, + ); + }); + + it("shell.get reattaches to a run and resumes where the caller asked", async () => { + // A command outlives the request that spawned it, so a later + // request reattaches by id. `resume` reaches the runner as the + // `after` cursor of the replay. + const requests: Array<{ id: string; after: number | "tail" | undefined }> = []; + const shellRpc: import("@cloudflare/computer-rpc").ShellRPC = { + exec: () => Promise.reject(new Error("not used")), + async getExec(request) { + requests.push({ id: request.id, after: request.after }); + return { + id: request.id, + events: new ReadableStream({ + start(c) { + c.enqueue({ + id: request.id, + seq: 8, + name: "stdout", + value: new TextEncoder().encode("tail\n"), + }); + c.enqueue({ id: request.id, seq: 9, name: "exit", value: 0 }); + c.close(); + }, + }), + }; + }, + killExec: () => Promise.reject(new Error("not used")), + disposeExec: () => Promise.reject(new Error("not used")), + }; + await withStub( + async (ws) => { + const stub = ws.stub(); + using handle = await stub.shell.get("install-1", { encoding: "utf8", resume: 7 }); + await expect(handle.result()).resolves.toMatchObject({ exitCode: 0, stdout: "tail\n" }); + using tail = await stub.shell.get("install-1", { resume: "tail" }); + await tail.result(); + expect(requests).toEqual([ + { id: "install-1", after: 7 }, + { id: "install-1", after: "tail" }, + ]); + }, + { backend: backend({ shell: shellRpc }) }, + ); + }); + it("shell.exec handle.stream() frames events as JSONL that decode back", async () => { // The handle's stream() projects the event stream as JSONL bytes // for the wire. Decoding it back yields the original events, @@ -536,4 +617,82 @@ describe("WorkspaceStub", () => { { backend: backend({ shell: shellRpc }) }, ); }); + + it("shell.exec rejects when the backend runs under a different id", async () => { + // The caller reattaches by the id it asked for, so a backend that + // substitutes its own has to fail loudly instead of handing back a + // handle shell.get can't find. + const shellRpc: import("@cloudflare/computer-rpc").ShellRPC = { + async exec() { + return { + id: "runner-picked", + events: new ReadableStream({ + start(c) { + c.enqueue({ id: "runner-picked", seq: 1, name: "exit", value: 0 }); + c.close(); + }, + }), + }; + }, + getExec: () => Promise.reject(new Error("not used")), + killExec: () => Promise.reject(new Error("not used")), + disposeExec: () => Promise.reject(new Error("not used")), + }; + await withStub( + async (ws) => { + const stub = ws.stub(); + using handle = await stub.shell.exec("noop", { id: "asked-for" }); + await expect(handle.result()).rejects.toThrow(/not the requested id asked-for/); + }, + { backend: backend({ shell: shellRpc }) }, + ); + }); + + it("disposing an exec handle frees the run for a later shell.get", async () => { + // The documented Worker flow: start a long command in one + // request, reattach in a later one. Only one live subscriber per + // run is allowed, so the handle stub's disposal has to hand the + // subscription back. + let subscriber: ReadableStreamDefaultController< + import("@cloudflare/computer-rpc").ExecEvent + > | null = null; + const subscribe = (id: string) => { + if (subscriber !== null) throw new Error(`exec ${id} already has a live subscriber`); + return new ReadableStream({ + start(c) { + subscriber = c; + }, + cancel() { + subscriber = null; + }, + }); + }; + const shellRpc: import("@cloudflare/computer-rpc").ShellRPC = { + async exec(request) { + const id = request.id ?? "e-1"; + return { id, events: subscribe(id) }; + }, + async getExec(request) { + return { id: request.id, events: subscribe(request.id) }; + }, + killExec: () => Promise.reject(new Error("not used")), + disposeExec: () => Promise.reject(new Error("not used")), + }; + + await withStub( + async (ws) => { + const stub = ws.stub(); + { + using handle = await stub.shell.exec("sleep 100", { id: "long-run" }); + expect(handle).toBeDefined(); + } + // Disposal cancels the handle's stream in the background. + await new Promise((resolve) => setTimeout(resolve, 0)); + + using again = await stub.shell.get("long-run", { resume: "tail" }); + expect(again).toBeDefined(); + }, + { backend: backend({ shell: shellRpc }) }, + ); + }); }); diff --git a/packages/computer/src/stub.ts b/packages/computer/src/stub.ts index 8e11744..68c99ef 100644 --- a/packages/computer/src/stub.ts +++ b/packages/computer/src/stub.ts @@ -75,17 +75,37 @@ import type { ExecEncoding, ExecHandle, ExecSyncResult, WorkspaceExecEvent } fro import type { Workspace } from "./workspace.js"; export interface WorkspaceExecOptions { + // Stable id for the run. Omit to let the runner mint a UUID. + // Reusing an id while a previous run is still active throws + // EEXEC_BUSY. + id?: string; cwd?: string; // "utf8" decodes stdout/stderr chunks through a streaming // TextDecoder so multi-byte boundaries survive. Default leaves // bytes as Uint8Array. encoding?: "utf8"; + // Per-call timeout in milliseconds. Past this duration the + // container signals the child. Omit for the runner's default; + // pass 0 to disable the timeout for this call. + timeoutMs?: number; // Backend selector. Omit to use the default backend (the first // one configured on the Workspace); pass the id of another // configured backend to route this call there. backend?: string; } +export interface WorkspaceGetExecOptions { + // See WorkspaceExecOptions.encoding. + encoding?: "utf8"; + // 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. + resume?: "tail" | "full" | number; + // See WorkspaceExecOptions.backend. Reattach is per-backend: the id + // is only known to the backend that ran the command. + backend?: string; +} + export interface WorkspaceExecResult { exitCode: number; stdout: E extends "utf8" ? string : Uint8Array; @@ -301,7 +321,9 @@ function emptyConsumeOutcome(exitCode = -1): ConsumeOutcome { // result() and stream() are mutually exclusive: each drains the // single underlying handle, so the first one called wins and the // other throws (the host ExecHandle is a one-shot ReadableStream). -// This mirrors the host contract exactly. +// This mirrors the host contract exactly. kill() is not one of the +// two: it signals the child without draining anything, so the output +// of a killed run remains available. export class WorkspaceExecHandleStub extends RpcTarget { readonly #handle: Promise>; readonly #consumer: Consumer; @@ -310,6 +332,7 @@ export class WorkspaceExecHandleStub e // — observers see a complete span synchronously after the await. readonly #span: Promise; #consumed = false; + #settled = false; constructor(handle: Promise>, consumer: Consumer, span: Promise) { super(); @@ -321,11 +344,12 @@ export class WorkspaceExecHandleStub e [Symbol.dispose](): void { // If neither result() nor stream() ran, release the exec span so - // it doesn't hang open, and cancel the handle's stream so computerd - // stops the command rather than buffering forever. + // it doesn't hang open, and cancel the handle's stream so the run + // stops streaming to nobody — and stays reattachable through + // shell.get, which needs the subscription slot free. if (!this.#consumed) { this.#consumed = true; - this.#consumer.resolve(emptyConsumeOutcome()); + this.#settle(emptyConsumeOutcome()); this.#handle .then((handle) => handle.cancel?.()) .catch(() => { @@ -342,12 +366,23 @@ export class WorkspaceExecHandleStub e this.#consumed = true; } + // Close the exec span, once. Every terminal path settles: result(), + // stream(), kill(), and dispose. The first one to arrive describes + // the outcome; later ones are no-ops, so a result() after a kill() + // still returns the real result while the span keeps the outcome the + // kill recorded. + #settle(outcome: ConsumeOutcome): void { + if (this.#settled) return; + this.#settled = true; + this.#consumer.resolve(outcome); + } + async result(): Promise> { this.#claim(); try { const handle = await this.#handle; const result = await handle.result(); - this.#consumer.resolve({ + this.#settle({ exitCode: result.exitCode, pushed: result.pushed, pulled: result.pulled, @@ -366,7 +401,7 @@ export class WorkspaceExecHandleStub e sync: result.sync, }; } catch (error) { - this.#consumer.resolve(emptyConsumeOutcome()); + this.#settle(emptyConsumeOutcome()); throw error; } } @@ -377,7 +412,7 @@ export class WorkspaceExecHandleStub e // counts; the exit code is captured off the wire as it passes. stream(): ReadableStream { this.#claim(); - const consumer = this.#consumer; + const settle = (outcome: ConsumeOutcome) => this.#settle(outcome); let reader: ReadableStreamDefaultReader> | undefined; let exitCode = -1; return new ReadableStream( @@ -393,18 +428,18 @@ export class WorkspaceExecHandleStub e reader.releaseLock(); reader = undefined; controller.close(); - consumer.resolve(emptyConsumeOutcome(exitCode)); + settle(emptyConsumeOutcome(exitCode)); return; } if (value.name === "exit") exitCode = value.value; controller.enqueue(encodeExecEvent(value as WorkspaceExecEvent)); } catch (error) { - consumer.resolve(emptyConsumeOutcome()); + settle(emptyConsumeOutcome()); controller.error(error); } }, cancel: async (reason) => { - consumer.resolve(emptyConsumeOutcome()); + settle(emptyConsumeOutcome()); await reader?.cancel(reason); reader?.releaseLock(); reader = undefined; @@ -417,6 +452,20 @@ export class WorkspaceExecHandleStub e async kill(signal?: "SIGTERM" | "SIGKILL" | "SIGINT" | "SIGHUP"): Promise { const handle = await this.#handle; await handle.kill(signal); + // A result() or stream() already reading the handle describes the + // outcome itself — it sees the exit event this signal produced, + // exit code and all. Leave the span to it, and don't wait on it + // either: it closes when the caller finishes reading, which may be + // long after kill() returns. + if (this.#consumed) return; + // Otherwise this is a terminal path: the child has exited and + // nothing will read the output, so close the span rather than + // leave it open for the life of the session. With nothing drained + // the outcome carries no exit code or sync counts. + this.#settle(emptyConsumeOutcome()); + // Same reason result() waits: the span's attributes are set by the + // time kill() resolves. + await this.#span.catch(() => {}); } } @@ -547,10 +596,9 @@ export class WorkspaceGitStub extends RpcTarget { } } -// Shell half. exec() returns an RpcTarget handle whose only -// method today is result(). Streaming exec lands as a separate -// method when a concrete caller needs it; see the note at the -// top of this file. +// Shell half. exec() and get() both return an RpcTarget handle the +// caller drains through result() or stream(); see the note at the top +// of this file. export class WorkspaceShellStub extends RpcTarget { readonly #ws: Workspace; @@ -626,14 +674,24 @@ export class WorkspaceShellStub extends RpcTarget { const spawned = options.encoding === "utf8" ? await this.#ws.shell.exec(command, { + id: options.id, cwd: options.cwd, encoding: "utf8", + timeoutMs: options.timeoutMs, backend: options.backend, }) : await this.#ws.shell.exec(command, { + id: options.id, cwd: options.cwd, + timeoutMs: options.timeoutMs, backend: options.backend, }); + // The caller addresses the run by the id it asked for, so a + // backend that mints its own instead has to say so rather + // than hand back a handle nothing can reattach to. + if (options.id !== undefined && spawned.id !== options.id) { + throw new Error(`backend ran exec as ${spawned.id}, not the requested id ${options.id}`); + } resolveHandle(spawned as ExecHandle<"utf8" | undefined>); return consumer.promise; }, @@ -655,6 +713,50 @@ export class WorkspaceShellStub extends RpcTarget { span.catch((error) => rejectHandle(error)); return new WorkspaceExecHandleStub<"utf8" | undefined>(handle, consumer, span); } + + get(id: string): Promise>; + get( + id: string, + options: WorkspaceGetExecOptions & { encoding: "utf8" }, + ): Promise>; + get(id: string, options: WorkspaceGetExecOptions): Promise>; + // Reattach to a run by id. The command outlives the request that + // started it, so a later request — or a caller that dropped its + // handle stub — picks the events back up here. + async get( + id: string, + options: WorkspaceGetExecOptions = {}, + ): Promise> { + // Same reason exec() calls ready(): heal a torn-down session + // before reaching for the shell. + await this.#ws.ready(); + const reattached = + options.encoding === "utf8" + ? await this.#ws.shell.get(id, { + encoding: "utf8", + resume: options.resume, + backend: options.backend, + }) + : await this.#ws.shell.get(id, { + resume: options.resume, + backend: options.backend, + }); + // Reattach runs no bracket: no pre-exec push, no post-exit pull, + // and so no span to hold open until the handle is consumed (see + // WorkspaceShell.get). The handle stub still reports how it was + // consumed, so give it a consumer nobody reads and a span that has + // already closed. + // + // Disposing this stub cancels the event stream, which gives the + // subscription back so the run can be reattached again. It doesn't + // stop the command — this handle didn't start it. Use kill() for + // that. + return new WorkspaceExecHandleStub<"utf8" | undefined>( + Promise.resolve(reattached as ExecHandle<"utf8" | undefined>), + makeConsumer(), + Promise.resolve(), + ); + } } // Top-level wrapper. Two sub-RpcTargets let callers use promise