diff --git a/.changeset/proxy-wakes-by-name.md b/.changeset/proxy-wakes-by-name.md new file mode 100644 index 00000000..fca7baeb --- /dev/null +++ b/.changeset/proxy-wakes-by-name.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": patch +--- + +`WorkspaceProxy` and `WorkspaceServiceProxy` wake their host Durable Object by name, so `ctx.id.name` survives a wake caused by a container or Dynamic Worker callback. `WorkspaceRef`, `WorkspaceProxyProps` and `WorkspaceServiceProxyProps` accept an optional `name`, filled in from `ctx.id.name` when the backend runs in the host object itself. diff --git a/packages/computer/src/backends/container-legacy/container-host.ts b/packages/computer/src/backends/container-legacy/container-host.ts index bd51127b..ddf58869 100644 --- a/packages/computer/src/backends/container-legacy/container-host.ts +++ b/packages/computer/src/backends/container-legacy/container-host.ts @@ -19,6 +19,7 @@ // an RpcTarget so it works the same in-isolate and across RPC. import { RpcTarget } from "cloudflare:workers"; +import { withHostName } from "../../durable-object-name.js"; import { WorkspaceTransportError } from "../../transport-failure.js"; import { ContainerClientSecret } from "./container-client-secret.js"; import { @@ -46,6 +47,11 @@ export interface WorkspaceRef { binding: string; // Stringified DurableObjectId of the specific Workspace owner. id: string; + // Name the Workspace owner is addressed by, typically + // ctx.id.name. The proxy wakes the owner by name when this yields + // `id`, so an owner the container wakes keeps ctx.id.name. Filled + // in automatically when the container host is the owner itself. + name?: string; } // Driver surface LegacyContainerBackend talks to. Implemented @@ -259,7 +265,9 @@ export class LegacyWorkspaceContainerAPI extends RpcTarget implements ILegacyWor const exports = (this.#ctx as unknown as { exports: Record }).exports as { WorkspaceProxy: (opts: { props: WorkspaceRef & { egressToken: string } }) => Fetcher; }; - const proxy = exports.WorkspaceProxy({ props: { ...ref, egressToken: token } }); + const proxy = exports.WorkspaceProxy({ + props: { ...withHostName(ref, this.#ctx), egressToken: token }, + }); await Promise.all([ this.#container.interceptAllOutboundHttp(proxy), this.#container.interceptOutboundHttps("*", proxy), @@ -276,7 +284,10 @@ export class LegacyWorkspaceContainerAPI extends RpcTarget implements ILegacyWor const exports = (this.#ctx as unknown as { exports: Record }).exports as { WorkspaceProxy: (opts: { props: WorkspaceRef }) => Fetcher; }; - await this.#container.interceptOutboundHttp(host, exports.WorkspaceProxy({ props: ref })); + await this.#container.interceptOutboundHttp( + host, + exports.WorkspaceProxy({ props: withHostName(ref, this.#ctx) }), + ); } fetchPort(port: number, input: RequestInfo | URL, init?: RequestInit): Promise { diff --git a/packages/computer/src/backends/container/container-host.ts b/packages/computer/src/backends/container/container-host.ts index 81f3d6c3..d133b856 100644 --- a/packages/computer/src/backends/container/container-host.ts +++ b/packages/computer/src/backends/container/container-host.ts @@ -19,6 +19,7 @@ // an RpcTarget so it works the same in-isolate and across RPC. import { RpcTarget } from "cloudflare:workers"; +import { withHostName } from "../../durable-object-name.js"; import { WorkspaceTransportError } from "../../transport-failure.js"; import { ContainerClientSecret } from "./container-client-secret.js"; import { @@ -50,6 +51,11 @@ export interface WorkspaceRef { binding: string; // Stringified DurableObjectId of the specific Workspace owner. id: string; + // Name the Workspace owner is addressed by, typically + // ctx.id.name. The proxy wakes the owner by name when this yields + // `id`, so an owner the container wakes keeps ctx.id.name. Filled + // in automatically when the container host is the owner itself. + name?: string; } // Driver surface ContainerBackend talks to. Implemented @@ -323,7 +329,9 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai const exports = (this.#ctx as unknown as { exports: Record }).exports as { WorkspaceProxy: (opts: { props: WorkspaceRef & { egressToken: string } }) => Fetcher; }; - const proxy = exports.WorkspaceProxy({ props: { ...ref, egressToken: token } }); + const proxy = exports.WorkspaceProxy({ + props: { ...withHostName(ref, this.#ctx), egressToken: token }, + }); await Promise.all([ this.#container.interceptAllOutboundHttp(proxy), this.#container.interceptOutboundHttps("*", proxy), @@ -340,7 +348,10 @@ export class WorkspaceContainerAPI extends RpcTarget implements IWorkspaceContai const exports = (this.#ctx as unknown as { exports: Record }).exports as { WorkspaceProxy: (opts: { props: WorkspaceRef }) => Fetcher; }; - await this.#container.interceptOutboundHttp(host, exports.WorkspaceProxy({ props: ref })); + await this.#container.interceptOutboundHttp( + host, + exports.WorkspaceProxy({ props: withHostName(ref, this.#ctx) }), + ); } fetchPort(port: number, input: RequestInfo | URL, init?: RequestInit): Promise { diff --git a/packages/computer/src/backends/worker-shell/worker-shell.ts b/packages/computer/src/backends/worker-shell/worker-shell.ts index 3a035c50..282d76d6 100644 --- a/packages/computer/src/backends/worker-shell/worker-shell.ts +++ b/packages/computer/src/backends/worker-shell/worker-shell.ts @@ -20,6 +20,7 @@ import type { ExecEvent, ShellRPC, SyncRPC, WorkspaceRPC } from "@cloudflare/computer-rpc"; import type { BackendHandle, WorkspaceBackend } from "../../backend.js"; +import { withHostName } from "../../durable-object-name.js"; import type { WorkspaceServiceProxyProps } from "../../proxy.js"; import { dynamicWorkerEgress, type WorkspaceEgressPolicy } from "../../runtime/egress.js"; import { SHELL_RUNTIME_MODULES } from "./runtime-modules.js"; @@ -254,7 +255,7 @@ export class WorkerShellBackend implements WorkspaceBackend { // The shell calls env.HOST.getWorkspace() on every exec; // the proxy resolves env[binding].get(id).getWorkspace() // on the host side. - HOST: ctx.exports.WorkspaceServiceProxy({ props: workspace }), + HOST: ctx.exports.WorkspaceServiceProxy({ props: withHostName(workspace, ctx) }), }, ...dynamicWorkerEgress(this.#egress), })); diff --git a/packages/computer/src/durable-object-name.test.ts b/packages/computer/src/durable-object-name.test.ts new file mode 100644 index 00000000..2ab1baf5 --- /dev/null +++ b/packages/computer/src/durable-object-name.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; + +import { hostDurableObjectId, withHostName } from "./durable-object-name.js"; + +// A namespace whose ids remember how they were made, standing in for +// DurableObjectNamespace: a named id knows its name, as ctx.id.name +// does on the instance it wakes. +class FakeId { + constructor( + readonly hex: string, + readonly name?: string, + ) {} + toString(): string { + return this.hex; + } +} + +const ns = { + idFromName: (name: string) => new FakeId(`hex:${name}`, name), + idFromString: (id: string) => new FakeId(id), +}; + +describe("hostDurableObjectId", () => { + it("addresses by name when the name yields the id", () => { + const id = hostDurableObjectId(ns, "hex:main", "main"); + expect(id.toString()).toBe("hex:main"); + expect(id.name).toBe("main"); + }); + + it("falls back to the id when the name maps elsewhere", () => { + const id = hostDurableObjectId(ns, "hex:main", "other"); + expect(id.toString()).toBe("hex:main"); + expect(id.name).toBeUndefined(); + }); + + it("uses the id when there is no name", () => { + expect(hostDurableObjectId(ns, "hex:main", undefined).name).toBeUndefined(); + expect(hostDurableObjectId(ns, "hex:main", "").name).toBeUndefined(); + }); +}); + +describe("withHostName", () => { + const ctx = { id: new FakeId("hex:main", "main") }; + + it("adds the name of the object that owns ctx", () => { + expect(withHostName({ binding: "Agent", id: "hex:main" }, ctx)).toEqual({ + binding: "Agent", + id: "hex:main", + name: "main", + }); + }); + + it("leaves a reference to another object alone", () => { + const ref = { binding: "Agent", id: "hex:other" }; + expect(withHostName(ref, ctx)).toBe(ref); + }); + + it("keeps a name the caller supplied", () => { + const ref = { binding: "Agent", id: "hex:main", name: "explicit" }; + expect(withHostName(ref, ctx)).toBe(ref); + }); + + it("leaves the reference alone when the runtime has no name", () => { + const ref = { binding: "Agent", id: "hex:main" }; + expect(withHostName(ref, { id: new FakeId("hex:main") })).toBe(ref); + expect(withHostName(ref, undefined)).toBe(ref); + expect(withHostName(ref, {})).toBe(ref); + }); +}); diff --git a/packages/computer/src/durable-object-name.ts b/packages/computer/src/durable-object-name.ts new file mode 100644 index 00000000..43749db5 --- /dev/null +++ b/packages/computer/src/durable-object-name.ts @@ -0,0 +1,58 @@ +// Keeping a Durable Object's name across proxy wakes. +// +// The workspace proxies reach their host Durable Object from props +// that a container or a Dynamic Worker holds on to. Both outlive the +// object: after an eviction or a deploy, the proxy is often the first +// caller, and the instance it wakes is the one every later caller +// shares. An instance woken through idFromString() has no +// ctx.id.name for its whole lifetime, and anything that needs the +// name (agents' Lifecycle, for one) then refuses to start. So the +// props may carry the name too, and the proxy addresses the object +// by name whenever that name produces the same id. + +// The part of DurableObjectNamespace the proxies use. Structural, so +// this module needs no runtime import. +interface DurableObjectIds { + idFromName(name: string): Id; + idFromString(id: string): Id; +} + +/** + * Resolve the id a proxy should wake its host Durable Object with. + * + * Prefers `idFromName(name)` when it yields `id`, so the woken + * instance keeps `ctx.id.name`. A missing name, or one that maps to a + * different object, falls back to `idFromString(id)`: `id` stays the + * authority on which object is meant. + */ +export function hostDurableObjectId( + ns: DurableObjectIds, + id: string, + name: string | undefined, +): Id { + if (name !== undefined && name !== "") { + const named = ns.idFromName(name); + if (named.toString() === id) return named; + } + return ns.idFromString(id); +} + +/** + * Fill in `name` on a reference to the Durable Object that owns `ctx`. + * + * Callers usually build the reference from `ctx.id.toString()` alone. + * When the reference points at this very object and the runtime knows + * its name, the name is added so the proxy can wake it by name later. + * A reference to another object, or one that already names itself, is + * returned unchanged. + */ +export function withHostName( + ref: Ref, + ctx: unknown, +): Ref { + if (ref.name !== undefined) return ref; + const id = (ctx as { id?: { name?: string; toString(): string } } | undefined)?.id; + const name = id?.name; + if (name === undefined || name === "" || id?.toString() !== ref.id) return ref; + return { ...ref, name }; +} diff --git a/packages/computer/src/proxy-stub.ts b/packages/computer/src/proxy-stub.ts index 59c03c9e..14c5f7c5 100644 --- a/packages/computer/src/proxy-stub.ts +++ b/packages/computer/src/proxy-stub.ts @@ -15,6 +15,7 @@ import type { ArtifactsCLIInput, ArtifactsCLIResult } from "./artifacts/index.js export interface WorkspaceProxyProps { binding: string; id: string; + name?: string; } export class WorkspaceProxy { @@ -33,6 +34,7 @@ export class WorkspaceProxy { export interface WorkspaceServiceProxyProps { binding: string; id: string; + name?: string; } export class ArtifactsCLITarget { diff --git a/packages/computer/src/proxy.ts b/packages/computer/src/proxy.ts index 2ee86cc1..29bf7bec 100644 --- a/packages/computer/src/proxy.ts +++ b/packages/computer/src/proxy.ts @@ -26,6 +26,9 @@ // binding: "COMPUTERD", // // The DO instance the upgrade should route to. // id: ctx.id.toString(), +// // Lets the proxy wake the DO by name, so +// // ctx.id.name survives a wake the proxy causes. +// name: ctx.id.name, // }, // }), // }); @@ -51,6 +54,7 @@ import { RpcTarget, WorkerEntrypoint } from "cloudflare:workers"; import type { ArtifactsCLIInput, ArtifactsCLIResult } from "./artifacts/index.js"; +import { hostDurableObjectId } from "./durable-object-name.js"; import { WORKSPACE_EGRESS_TOKEN_HEADER, WORKSPACE_EGRESS_URL_HEADER } from "./runtime/egress.js"; export interface WorkspaceProxyProps { @@ -60,6 +64,11 @@ export interface WorkspaceProxyProps { // Stringified DurableObjectId — typically `ctx.id.toString()` // from inside the owning DO's constructor. id: string; + // The name the DO is addressed by — typically `ctx.id.name`. When + // `idFromName(name)` yields `id`, the proxy wakes the DO by name, + // so an instance the proxy wakes keeps `ctx.id.name`. Woken by id + // alone, it never has one. + name?: string; egressToken?: string; } @@ -108,9 +117,9 @@ export class WorkspaceProxy extends WorkerEntrypoint)[binding] as DurableObjectNamespace | undefined; - return ns?.get(ns.idFromString(id)); + return ns?.get(hostDurableObjectId(ns, id, name)); } #missingBindingResponse(): Response { @@ -155,7 +164,7 @@ export class ArtifactsCLITarget extends RpcTarget { // // // Inside the DO that owns the Workspace: // const host = ctx.exports.WorkspaceServiceProxy({ -// props: { binding: "ContainerExample", id: ctx.id.toString() }, +// props: { binding: "ContainerExample", id: ctx.id.toString(), name: ctx.id.name }, // }); // env.LOADER.get(`shell:${ctx.id}`, () => ({ // env: { HOST: host }, @@ -177,6 +186,10 @@ export interface WorkspaceServiceProxyProps { // Stringified DurableObjectId — typically ctx.id.toString() // from inside the owning DO's constructor. id: string; + // The name the DO is addressed by — typically ctx.id.name. See + // WorkspaceProxyProps.name: without it, a DO this proxy wakes has + // no ctx.id.name. + name?: string; } export class WorkspaceServiceProxy extends WorkerEntrypoint { @@ -217,13 +230,13 @@ export class WorkspaceServiceProxy extends WorkerEntrypoint(): Rpc.DurableObjectBranded & T { - const { binding, id } = this.ctx.props; + const { binding, id, name } = this.ctx.props; const ns = (this.env as Record)[binding] as | DurableObjectNamespace | undefined; if (!ns) { throw new Error(`WorkspaceServiceProxy: env.${binding} is not a DurableObjectNamespace`); } - return ns.get(ns.idFromString(id)) as unknown as Rpc.DurableObjectBranded & T; + return ns.get(hostDurableObjectId(ns, id, name)) as unknown as Rpc.DurableObjectBranded & T; } } diff --git a/packages/computer/tests/proxy-worker.ts b/packages/computer/tests/proxy-worker.ts index 0a9ac83c..428dd5c3 100644 --- a/packages/computer/tests/proxy-worker.ts +++ b/packages/computer/tests/proxy-worker.ts @@ -16,15 +16,24 @@ import { DurableObject, WorkerEntrypoint } from "cloudflare:workers"; -export { WorkspaceProxy } from "../src/proxy.js"; +export { WorkspaceProxy, WorkspaceServiceProxy } from "../src/proxy.js"; export interface Env { COMPUTERD: DurableObjectNamespace; } export class TestStorageDO extends DurableObject { + // What the runtime reports as this instance's name. An instance + // first woken through idFromString() never learns it. + async __getWorkspaceStub(): Promise { + return { name: this.ctx.id.name ?? null }; + } + override async fetch(request: Request): Promise { const url = new URL(request.url); + if (url.pathname === "/api" && request.headers.has("x-test-report-name")) { + return Response.json({ name: this.ctx.id.name ?? null }); + } const egressToken = request.headers.get("x-workspace-egress-token"); if (url.pathname === "/api" && egressToken !== null) { return Response.json({ @@ -59,9 +68,15 @@ export default class TestDriver extends WorkerEntrypoint { const binding = request.headers.get("x-test-binding") ?? "COMPUTERD"; const id = request.headers.get("x-test-id") ?? ""; const egressToken = request.headers.get("x-test-egress-token") ?? undefined; + const name = request.headers.get("x-test-name") ?? undefined; // oxlint-disable-next-line typescript/no-explicit-any -- ctx.exports isn't in @cloudflare/workers-types yet - const proxy = (this.ctx as any).exports.WorkspaceProxy({ - props: { binding, id, egressToken }, + const exports = (this.ctx as any).exports; + if (new URL(request.url).pathname === "/__service/getWorkspace") { + const service = exports.WorkspaceServiceProxy({ props: { binding, id, name } }); + return Response.json(await service.getWorkspace()); + } + const proxy = exports.WorkspaceProxy({ + props: { binding, id, name, egressToken }, }); return proxy.fetch(request); } diff --git a/packages/computer/tests/proxy.test.ts b/packages/computer/tests/proxy.test.ts index 95f42e0f..69fb261e 100644 --- a/packages/computer/tests/proxy.test.ts +++ b/packages/computer/tests/proxy.test.ts @@ -82,6 +82,58 @@ describe("WorkspaceProxy", () => { expect(await res.text()).toMatch(/NOT_A_BINDING is not a DurableObjectNamespace/); }); + // A container outlives its durable object, so computerd reconnecting + // through the proxy is often what wakes the object. An instance + // woken by idFromString() never gets ctx.id.name, which breaks + // anything (agents' Lifecycle) that needs the name. Issue #243. + it("/api wakes the DO by name when props carry one", async () => { + const name = `named-${crypto.randomUUID()}`; + const res = await SELF.fetch("http://proxy.test/api", { + headers: { + "x-test-id": env.COMPUTERD.idFromName(name).toString(), + "x-test-name": name, + "x-test-report-name": "1", + }, + }); + expect(await res.json()).toEqual({ name }); + }); + + it("egress callbacks wake the DO by name when props carry one", async () => { + const name = `named-${crypto.randomUUID()}`; + const res = await SELF.fetch("https://api.example.test/anything", { + headers: { + "x-test-id": env.COMPUTERD.idFromName(name).toString(), + "x-test-name": name, + "x-test-egress-token": "secret-token", + "x-test-report-name": "1", + }, + }); + expect(await res.json()).toEqual({ name }); + }); + + it("/api falls back to the id when the name does not match it", async () => { + const name = `named-${crypto.randomUUID()}`; + const res = await SELF.fetch("http://proxy.test/api", { + headers: { + "x-test-id": env.COMPUTERD.idFromName(name).toString(), + "x-test-name": `other-${name}`, + "x-test-report-name": "1", + }, + }); + expect(await res.json()).toEqual({ name: null }); + }); + + it("/api without a name still reaches the DO by id", async () => { + const name = `named-${crypto.randomUUID()}`; + const res = await SELF.fetch("http://proxy.test/api", { + headers: { + "x-test-id": env.COMPUTERD.idFromName(name).toString(), + "x-test-report-name": "1", + }, + }); + expect(await res.json()).toEqual({ name: null }); + }); + it("returns 404 for paths other than /health and /api", async () => { const res = await SELF.fetch("http://proxy.test/anything-else", { headers: { "x-test-id": freshId() }, @@ -89,3 +141,24 @@ describe("WorkspaceProxy", () => { expect(res.status).toBe(404); }); }); + +describe("WorkspaceServiceProxy", () => { + it("wakes the DO by name when props carry one", async () => { + const name = `named-${crypto.randomUUID()}`; + const res = await SELF.fetch("http://proxy.test/__service/getWorkspace", { + headers: { "x-test-id": env.COMPUTERD.idFromName(name).toString(), "x-test-name": name }, + }); + expect(await res.json()).toEqual({ name }); + }); + + it("falls back to the id when the name does not match it", async () => { + const name = `named-${crypto.randomUUID()}`; + const res = await SELF.fetch("http://proxy.test/__service/getWorkspace", { + headers: { + "x-test-id": env.COMPUTERD.idFromName(name).toString(), + "x-test-name": `other-${name}`, + }, + }); + expect(await res.json()).toEqual({ name: null }); + }); +});