From c5f1ee5b0d2642f28f59c7668fc474c66435666d Mon Sep 17 00:00:00 2001 From: Pedro Lobato <69770518+Lob26@users.noreply.github.com> Date: Mon, 7 Sep 2026 10:01:31 -0500 Subject: [PATCH 1/2] fix(workspaces): wait for the Docker bootstrap before using a woken workspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `create` proved a container ready before handing it back; `wake` only started it. But the bootstrap deletes its readiness marker on every start and then brings up dockerd, fixes the socket permissions and launches the preview gateways — so `docker start` returning says nothing about whether the workspace can be used. Both callers of `wake` act on the handle immediately. `exec` runs the agent's command, which is where `docker`, `docker compose` and a half-chowned `/workspace` fail intermittently after a resume or a host restart; `preview.open` reads the published ports, which are bound before the gateway inside is listening. The Vercel runtime already re-initializes on `wake`, so the two providers did not honour the same contract. Prove readiness on `wake` as well. Readiness only moves forward within one container start, so each start is proven once and remembered by container name and `StartedAt` — a resumed or replaced container gets a new start and is proven again, and `exec` does not pay for a probe per command. --- services/api/src/workspaces/docker.ts | 32 +++++- services/api/test/workspace-docker.test.ts | 110 +++++++++++++++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 services/api/test/workspace-docker.test.ts diff --git a/services/api/src/workspaces/docker.ts b/services/api/src/workspaces/docker.ts index a49ef8f3..0b60d4e6 100644 --- a/services/api/src/workspaces/docker.ts +++ b/services/api/src/workspaces/docker.ts @@ -24,6 +24,9 @@ const KIND = "workspace-v2"; export class DockerWorkspaceRuntime implements WorkspaceRuntime { readonly provider = "docker" as const; + /** Workspace container name → the container start already proven ready. */ + private readonly readyStarts = new Map(); + constructor(private readonly docker = new Docker()) {} async create(input: CreateWorkspace): Promise { @@ -35,7 +38,7 @@ export class DockerWorkspaceRuntime implements WorkspaceRuntime { if (existing) { this.assertOwned(existing, input.id, names.volume); if (!existing.State?.Running) await this.docker.getContainer(existing.Id).start(); - await this.waitUntilReady(this.docker.getContainer(existing.Id)); + await this.ensureReady(names.container, this.docker.getContainer(existing.Id)); return this.handle(input, existing.Id, names); } @@ -79,7 +82,7 @@ export class DockerWorkspaceRuntime implements WorkspaceRuntime { }); try { await container.start(); - await this.waitUntilReady(container); + await this.ensureReady(names.container, container); } catch (error) { await container.remove({ force: true, v: false }).catch(() => undefined); throw error; @@ -92,7 +95,9 @@ export class DockerWorkspaceRuntime implements WorkspaceRuntime { const existing = await this.inspectContainer(names.container); if (!existing) return this.create(workspace); this.assertOwned(existing, workspace.id, names.volume); - if (!existing.State?.Running) await this.docker.getContainer(existing.Id).start(); + const container = this.docker.getContainer(existing.Id); + if (!existing.State?.Running) await container.start(); + await this.ensureReady(names.container, container); return this.handle(workspace, existing.Id, names); } @@ -241,6 +246,7 @@ export class DockerWorkspaceRuntime implements WorkspaceRuntime { async destroy(workspace: WorkspaceLocator): Promise { const names = this.assertLocator(workspace); + this.readyStarts.delete(names.container); const current = await this.inspectContainer(names.container); if (current) { this.assertOwned(current, workspace.id, names.volume); @@ -399,6 +405,26 @@ export class DockerWorkspaceRuntime implements WorkspaceRuntime { await killer.start({}).catch(() => undefined); } + /** + * A started container is not yet a usable workspace: the bootstrap removes its + * readiness marker on every start, then brings up dockerd, the socket + * permissions and the preview gateways. Readiness only ever moves forward + * within one start, so each start is proven once and then remembered. + */ + private async ensureReady(containerName: string, container: Docker.Container) { + const state = await container.inspect(); + if (!state.State.Running) { + throw new WorkspaceRuntimeError( + "workspace_initialize_failed", + `workspace bootstrap exited with status ${state.State.ExitCode}`, + ); + } + const start = `${state.Id}:${state.State.StartedAt}`; + if (this.readyStarts.get(containerName) === start) return; + await this.waitUntilReady(container); + this.readyStarts.set(containerName, start); + } + private async waitUntilReady(container: Docker.Container) { const deadline = Date.now() + 180_000; while (Date.now() < deadline) { diff --git a/services/api/test/workspace-docker.test.ts b/services/api/test/workspace-docker.test.ts new file mode 100644 index 00000000..a05004a5 --- /dev/null +++ b/services/api/test/workspace-docker.test.ts @@ -0,0 +1,110 @@ +import { createHash } from "node:crypto"; +import type Docker from "dockerode"; +import { describe, expect, it, vi } from "vitest"; +import { DockerWorkspaceRuntime } from "../src/workspaces/docker.js"; +import type { WorkspaceLocator } from "../src/workspaces/runtime.js"; + +const workspaceId = "ws_0123456789abcdef"; +const suffix = createHash("sha256").update(workspaceId).digest("hex").slice(0, 24); +const workspace: WorkspaceLocator = { + id: workspaceId, + image: "facility-runner:test", + externalRef: `facility-ws-${suffix}`, + volumeRef: `facility-ws-volume-${suffix}`, +}; + +/** + * A container that boots the way the workspace bootstrap does: `start` returns + * immediately, and the readiness marker only appears after `readyAfterProbes` + * probes. Every probe is recorded so a test can prove one was made — or wasn't. + */ +function fakeDocker(options: { + running: boolean; + readyAfterProbes: number; + bootstrapExits?: boolean; +}) { + const probes: string[][] = []; + const state = { + running: options.running, + startedAt: "2026-09-07T09:00:00.000000000Z", + remaining: options.readyAfterProbes, + }; + const start = vi.fn(async () => { + // A bootstrap that dies on start leaves the container stopped again. + state.running = options.bootstrapExits !== true; + state.startedAt = "2026-09-07T10:00:00.000000000Z"; + }); + const container = { + id: "container-1", + inspect: async () => ({ + Id: "container-1", + State: { Running: state.running, StartedAt: state.startedAt, ExitCode: 0 }, + Config: { + Labels: { "facility.workspace.id": workspaceId, "facility.workload.kind": "workspace-v2" }, + }, + Mounts: [{ Destination: "/workspace", Name: workspace.volumeRef }], + }), + start, + exec: async (options: { Cmd: string[] }) => { + probes.push(options.Cmd); + const ready = state.remaining <= 0; + state.remaining -= 1; + return { + start: async () => undefined, + inspect: async () => ({ Running: false, ExitCode: ready ? 0 : 1 }), + }; + }, + }; + const docker = { getContainer: vi.fn(() => container) } as unknown as Docker; + return { docker, container, start, probes }; +} + +describe("Docker workspace readiness", () => { + it("waits for the bootstrap before handing back a woken workspace", async () => { + const { docker, start, probes } = fakeDocker({ running: false, readyAfterProbes: 2 }); + const runtime = new DockerWorkspaceRuntime(docker); + + await expect(runtime.wake(workspace)).resolves.toMatchObject({ + computeRef: "container-1", + state: "running", + }); + + expect(start).toHaveBeenCalledOnce(); + // Two failing probes plus the one that finally observed the marker. + expect(probes).toHaveLength(3); + expect(probes.at(-1)?.at(-1)).toContain("/workspace/.facility/runtime-ready"); + }); + + it("waits for a container someone else started and is still booting", async () => { + const { docker, start, probes } = fakeDocker({ running: true, readyAfterProbes: 1 }); + const runtime = new DockerWorkspaceRuntime(docker); + + await runtime.wake(workspace); + + expect(start).not.toHaveBeenCalled(); + expect(probes).toHaveLength(2); + }); + + it("proves each container start once and reuses the result on later wakes", async () => { + const { docker, probes } = fakeDocker({ running: true, readyAfterProbes: 0 }); + const runtime = new DockerWorkspaceRuntime(docker); + + await runtime.wake(workspace); + await runtime.wake(workspace); + await runtime.wake(workspace); + + expect(probes).toHaveLength(1); + }); + + it("refuses a workspace whose bootstrap exited instead of reporting it ready", async () => { + const { docker } = fakeDocker({ + running: false, + readyAfterProbes: 0, + bootstrapExits: true, + }); + + await expect(new DockerWorkspaceRuntime(docker).wake(workspace)).rejects.toMatchObject({ + code: "workspace_initialize_failed", + }); + }); +}); From 14ef27325b45b71a8ff1a179b1d5c58920a15499 Mon Sep 17 00:00:00 2001 From: Pedro Lobato <69770518+Lob26@users.noreply.github.com> Date: Tue, 15 Sep 2026 08:39:25 -0500 Subject: [PATCH 2/2] fix(workspaces): clear the previous start's daemon state when a workspace wakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Waiting for the bootstrap was necessary but not sufficient: on a real suspend and wake the bootstrap never becomes ready at all. A stopped container keeps its writable layer, so dockerd finds the pidfile its own previous start wrote and refuses to boot. The readiness loop then spends its 120 attempts, `set -eu` aborts, and the container exits — which the readiness check now reports honestly instead of handing back a workspace, but the wake still fails. The bootstrap removes the daemon's stale runtime state before starting it: the pidfile, the socket, and the exec root holding containerd's own pidfile and socket. Nothing is running at that point, because the script is the container's entrypoint, so all of it is stale by construction. Verified on a real container rather than reasoned about: a value written to /var/run/docker.pid before `docker stop` is still there after `docker start`, and the new line removes it. Two tests, because one of them cannot run everywhere: - a unit case asserts the generated bootstrap clears that state *before* the dockerd line, since doing it afterwards would be useless; - an end-to-end case suspends and wakes the same container, asserts the compute reference is unchanged so it is a resume rather than a replacement, and asserts the nested daemon answers again. It runs the cycle twice: the first wake writes a pidfile of its own, so a fix that only cleaned up after the original create would pass once. The existing end-to-end case never met this. It exercises `replaceCompute`, which discards the writable layer; only stopping and starting the same container reaches the state that breaks. --- services/api/src/workspaces/docker.ts | 10 ++- services/api/test/workspace-docker.test.ts | 71 +++++++++++++++++++ .../workspace-runtime.integration.test.ts | 49 +++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/services/api/src/workspaces/docker.ts b/services/api/src/workspaces/docker.ts index 0b60d4e6..e2e6f078 100644 --- a/services/api/src/workspaces/docker.ts +++ b/services/api/src/workspaces/docker.ts @@ -484,7 +484,15 @@ function workspaceBootstrapCommand(input: CreateWorkspace) { "rm -f /workspace/.facility/runtime-ready", "mkdir -p /workspace/.facility/home /workspace/.facility/claude /workspace/.facility/codex /workspace/.facility/docker", "chown -R node:node /workspace", - "rm -f /var/run/docker.sock", + // A stopped container keeps its writable layer, so the previous start's + // runtime state is still on disk when it is woken. dockerd refuses to boot + // while /var/run/docker.pid exists, and its exec root holds containerd's + // stale socket and pidfile too — the daemon would wait for a peer that + // stopped with the container. Nothing is running yet, because this script + // is the container's own entrypoint, so all of it is stale by construction. + // Compute replacement never hit this: it discards the layer. Suspend and + // wake reuse it, which is the path this clears. + "rm -rf /var/run/docker /var/run/docker.pid /var/run/docker.sock", "dockerd --host=unix:///var/run/docker.sock --data-root=/workspace/.facility/docker --storage-driver=vfs >/workspace/.facility/dockerd.log 2>&1 &", "attempt=0; until docker info >/dev/null 2>&1; do attempt=$((attempt + 1)); test $attempt -lt 120; sleep 1; done", "chown root:node /var/run/docker.sock", diff --git a/services/api/test/workspace-docker.test.ts b/services/api/test/workspace-docker.test.ts index a05004a5..570f97e1 100644 --- a/services/api/test/workspace-docker.test.ts +++ b/services/api/test/workspace-docker.test.ts @@ -108,3 +108,74 @@ describe("Docker workspace readiness", () => { }); }); }); + +/** Records the container definition `create` builds, without a Docker daemon. */ +function recordingDocker() { + const created: Array> = []; + const notFound = Object.assign(new Error("no such container"), { statusCode: 404 }); + const container = { + id: "container-new", + start: async () => undefined, + remove: async () => undefined, + inspect: async () => ({ + Id: "container-new", + State: { Running: true, StartedAt: "2026-09-15T10:00:00.000000000Z", ExitCode: 0 }, + NetworkSettings: { Ports: {} }, + }), + exec: async () => ({ + start: async () => undefined, + inspect: async () => ({ Running: false, ExitCode: 0 }), + }), + }; + const docker = { + getContainer: () => ({ + ...container, + inspect: async () => { + throw notFound; + }, + }), + getVolume: () => ({ + inspect: async () => { + throw notFound; + }, + }), + getNetwork: () => ({ + inspect: async () => { + throw notFound; + }, + }), + getImage: () => ({ inspect: async () => ({}) }), + createNetwork: async () => ({}), + createVolume: async () => ({}), + createContainer: async (options: Record) => { + created.push(options); + return container; + }, + } as unknown as Docker; + return { docker, created }; +} + +describe("Docker workspace bootstrap", () => { + it("clears the previous start's daemon runtime state before starting dockerd", async () => { + const { docker, created } = recordingDocker(); + + await new DockerWorkspaceRuntime(docker).create({ + id: workspaceId, + image: "facility-runner:test", + }); + + const script = String((created[0]?.Cmd as string[])[0]); + const lines = script.split("\n"); + const cleanup = lines.findIndex((line) => line.includes("/var/run/docker.pid")); + const daemon = lines.findIndex((line) => line.startsWith("dockerd ")); + + // A stopped container keeps its writable layer, so dockerd finds the pidfile + // its previous start wrote and refuses to boot. Removing it after the daemon + // line would be useless, which is why the order is asserted and not just the + // presence of the command. + expect(cleanup).toBeGreaterThanOrEqual(0); + expect(daemon).toBeGreaterThan(cleanup); + expect(lines[cleanup]).toContain("/var/run/docker"); + expect(lines[cleanup]).toContain("/var/run/docker.sock"); + }); +}); diff --git a/services/api/test/workspace-runtime.integration.test.ts b/services/api/test/workspace-runtime.integration.test.ts index 969af72a..0c3c5f6d 100644 --- a/services/api/test/workspace-runtime.integration.test.ts +++ b/services/api/test/workspace-runtime.integration.test.ts @@ -13,6 +13,55 @@ import { const enabled = process.env.FACILITY_E2E_DOCKER === "1"; describe.skipIf(!enabled)("DockerWorkspaceRuntime integration", () => { + it("brings the nested daemon back after a suspend and wake of the same container", async () => { + // The suite's other case replaces compute, which discards the container's + // writable layer and so never meets the state a stop leaves behind. Suspend + // and wake reuse that layer: dockerd finds the pidfile its previous start + // wrote and refuses to boot, the readiness probe never passes, and the + // bootstrap exits. Nothing short of stopping and starting the real + // container observes it. + const id = `ws_${randomBytes(12).toString("hex")}`; + const runtime = new DockerWorkspaceRuntime(new Docker()); + const created = await runtime.create({ + id, + image: process.env.FACILITY_WORKSPACE_TEST_IMAGE ?? "facility-runner:serialized", + environment: { FACILITY_PREVIEW_GATEWAY_TOKEN: randomBytes(32).toString("base64url") }, + }); + const workspace = created as WorkspaceLocator; + try { + const before = await runtime.exec(workspace, { + command: "sh", + args: ["-lc", "printf marker > repo-state && docker info --format '{{.Driver}}'"], + }); + expect(before, before.stderr).toMatchObject({ exitCode: 0, stdout: "vfs\n" }); + + await runtime.suspend(workspace); + await expect(runtime.inspect(workspace)).resolves.toMatchObject({ state: "sleeping" }); + + const resumed = await runtime.wake(workspace); + // The same compute, not a replacement: that is the whole point of the case. + expect(resumed.computeRef).toBe(created.computeRef); + + const after = await runtime.exec(workspace, { + command: "sh", + args: ["-lc", "printf '%s|' \"$(cat repo-state)\"; docker info --format '{{.Driver}}'"], + }); + expect(after, after.stderr).toMatchObject({ exitCode: 0, stdout: "marker|vfs\n" }); + + // A second cycle, because the first wake writes a pidfile of its own and + // a fix that only cleans the original create's state would pass once. + await runtime.suspend(workspace); + await runtime.wake(workspace); + const twice = await runtime.exec(workspace, { + command: "docker", + args: ["info", "--format", "{{.Driver}}"], + }); + expect(twice, twice.stderr).toMatchObject({ exitCode: 0, stdout: "vfs\n" }); + } finally { + await runtime.destroy(workspace); + } + }, 300_000); + it("reattaches the same named volume after its compute is removed", async () => { const id = `ws_${randomBytes(12).toString("hex")}`; const gatewayToken = randomBytes(32).toString("base64url");