From d77c3debb479dc0f0a936fdb7c5c0cb310261a35 Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Mon, 28 Sep 2026 12:30:21 +0800 Subject: [PATCH 1/7] fix(api): coordinate workspace deletion with queued and running turns --- apps/docs/docs/guides/operate-story.md | 5 +- services/api/src/stories/service.ts | 97 +++++- services/api/src/turns/dispatcher.ts | 6 +- .../test/deleted-workspace-dispatch.test.ts | 57 ++++ .../test/story-workspaces.integration.test.ts | 172 ++++++++++- .../test/turn-dispatcher.integration.test.ts | 43 +++ ...kspace-deletion-routes.integration.test.ts | 286 ++++++++++++++++++ 7 files changed, 656 insertions(+), 10 deletions(-) create mode 100644 services/api/test/deleted-workspace-dispatch.test.ts create mode 100644 services/api/test/workspace-deletion-routes.integration.test.ts diff --git a/apps/docs/docs/guides/operate-story.md b/apps/docs/docs/guides/operate-story.md index 2486cc9c..d68fedfc 100644 --- a/apps/docs/docs/guides/operate-story.md +++ b/apps/docs/docs/guides/operate-story.md @@ -150,7 +150,10 @@ story done and suspends compute. Branch protection and required review remain th - **Archive** retains the same data but removes the story from the active workflow. - **Restore** makes an archived story active again and wakes compute when work requires it. - **Delete workspace** permanently removes the durable workspace. It requires explicit - confirmation and a matching idempotency key. + confirmation and a matching idempotency key. A running turn blocks deletion with a 409 + response; wait for it to finish, then retry with a new idempotency key. Deletion cancels queued turns and prevents + pending messages or new requests from starting more work. If provider cleanup fails, retry + deletion; the workspace remains closed to new work until cleanup succeeds. Archive a completed story when it may still be useful. Delete only after the pull request, commits, artifacts, and any uncommitted files have been preserved elsewhere. See the [lifecycle diff --git a/services/api/src/stories/service.ts b/services/api/src/stories/service.ts index 7eb69eba..0c79f1e5 100644 --- a/services/api/src/stories/service.ts +++ b/services/api/src/stories/service.ts @@ -167,6 +167,8 @@ export class StoryWorkspaceService { "this story's workspace was explicitly deleted; start a new story identity", ); } + await lockStory(tx, input.orgId, input.projectId, story.id); + await this.assertWorkspaceNotDeleting(tx, input.orgId, input.projectId, story.id); await recordParticipation(tx, story, input.actor); let conversation = ( @@ -333,6 +335,7 @@ export class StoryWorkspaceService { if (story.deletedAt) { throw new StoryServiceError("story_workspace_deleted", "story workspace was deleted"); } + await this.assertWorkspaceNotDeleting(tx, input.orgId, input.projectId, input.storyId); const conversation = ( await tx .select() @@ -717,6 +720,16 @@ export class StoryWorkspaceService { const tx = rawTx as unknown as FacilityDb; await lockStory(tx, input.orgId, input.projectId, input.storyId); const story = await scopedStory(tx, input.orgId, input.projectId, input.storyId); + // A late queue callback must not create a retry notice for deleted work. + const workspace = await this.activeWorkspace( + input.orgId, + input.projectId, + input.storyId, + true, + tx, + ); + if (story.deletedAt || workspace?.state === "deleting" || workspace?.state === "destroyed") + return undefined; await tx .update(stories) .set({ @@ -959,6 +972,14 @@ export class StoryWorkspaceService { const turn = await this.db.transaction(async (rawTx) => { const tx = rawTx as unknown as FacilityDb; await lockStory(tx, input.orgId, input.projectId, input.storyId); + const workspace = await this.activeWorkspace( + input.orgId, + input.projectId, + input.storyId, + true, + tx, + ); + if (!workspace || ["deleting", "destroyed"].includes(workspace.state)) return undefined; const active = ( await tx .select() @@ -1765,27 +1786,73 @@ export class StoryWorkspaceService { 400, ); } - const workspace = await this.activeWorkspace(input.orgId, input.projectId, input.storyId, true); - if (!workspace || workspace.state === "destroyed") { - return this.get(input.orgId, input.projectId, input.storyId); - } - await this.db.transaction(async (rawTx) => { + const workspace = await this.db.transaction(async (rawTx) => { const tx = rawTx as unknown as FacilityDb; await lockStory(tx, input.orgId, input.projectId, input.storyId); + const workspace = await this.activeWorkspace( + input.orgId, + input.projectId, + input.storyId, + true, + tx, + ); + if (!workspace || workspace.state === "destroyed") return undefined; + // Lock the same rows dispatch claims with UPDATE. Whichever operation wins + // determines whether deletion cancels a queued turn or refuses a running one. + const active = await tx + .select() + .from(turns) + .where( + and( + eq(turns.orgId, input.orgId), + eq(turns.projectId, input.projectId), + eq(turns.storyId, input.storyId), + inArray(turns.state, ["queued", "running"]), + ), + ) + .for("update"); + if (active.some((turn) => turn.state === "running")) { + throw new StoryServiceError( + "workspace_busy", + "wait for the running turn to finish before deleting the workspace", + 409, + ); + } + const now = new Date(); + for (const turn of active) { + await tx + .update(turns) + .set({ state: "canceled", endedAt: now, retryAfter: null, error: null, updatedAt: now }) + .where(eq(turns.id, turn.id)); + await appendTurnEvent(tx, { + orgId: input.orgId, + projectId: input.projectId, + storyId: input.storyId, + turnId: turn.id, + type: "turn.canceled", + data: { actor: input.actor, reason: "workspace_deleted" }, + }); + } + await tx + .update(stories) + .set({ activeAgentName: null, updatedAt: now }) + .where(eq(stories.id, input.storyId)); await tx .update(workspaces) - .set({ state: "deleting", updatedAt: new Date() }) + .set({ state: "deleting", updatedAt: now }) .where(and(eq(workspaces.orgId, input.orgId), eq(workspaces.id, workspace.id))); await tx .update(previewSessions) - .set({ revokedAt: new Date(), updatedAt: new Date() }) + .set({ revokedAt: now, updatedAt: now }) .where( and( eq(previewSessions.orgId, input.orgId), eq(previewSessions.workspaceId, workspace.id), ), ); + return workspace; }); + if (!workspace) return this.get(input.orgId, input.projectId, input.storyId); // This is intentionally the only lifecycle path that destroys provider // storage. Merge, archive, suspend, reconciliation, and schedule handling @@ -1810,6 +1877,22 @@ export class StoryWorkspaceService { return this.get(input.orgId, input.projectId, input.storyId); } + private async assertWorkspaceNotDeleting( + db: FacilityDb, + orgId: string, + projectId: string, + storyId: string, + ) { + const workspace = await this.activeWorkspace(orgId, projectId, storyId, true, db); + if (workspace && ["deleting", "destroyed"].includes(workspace.state)) { + throw new StoryServiceError( + "story_workspace_deleted", + "story workspace is being deleted or was deleted", + 409, + ); + } + } + private async activeWorkspace( orgId: string, projectId: string, diff --git a/services/api/src/turns/dispatcher.ts b/services/api/src/turns/dispatcher.ts index fa452fad..0f1a2cf4 100644 --- a/services/api/src/turns/dispatcher.ts +++ b/services/api/src/turns/dispatcher.ts @@ -560,6 +560,7 @@ export class TurnDispatcher { storyId: string; turnId: string; }) { + if (!(await this.activeWorkspace(input.orgId, input.projectId, input.storyId))) return; const next = await this.nextQueuedMessage(input.orgId, input.projectId, input.storyId); if (!next?.requestedAgentName) return; try { @@ -579,8 +580,10 @@ export class TurnDispatcher { }, }); } catch (error) { + // Deletion may have committed while catalog resolution was in flight. + if (!(await this.activeWorkspace(input.orgId, input.projectId, input.storyId))) return; const detail = error instanceof Error ? error.message : String(error); - await this.storiesService.flagAttention({ + const attention = await this.storiesService.flagAttention({ orgId: input.orgId, projectId: input.projectId, storyId: input.storyId, @@ -589,6 +592,7 @@ export class TurnDispatcher { title: `Could not dispatch queued ${next.requestedAgentName} turn`, detail, }); + if (!attention) return; await appendTurnEvent(this.db, { orgId: input.orgId, projectId: input.projectId, diff --git a/services/api/test/deleted-workspace-dispatch.test.ts b/services/api/test/deleted-workspace-dispatch.test.ts new file mode 100644 index 00000000..10cef0fb --- /dev/null +++ b/services/api/test/deleted-workspace-dispatch.test.ts @@ -0,0 +1,57 @@ +import type { FacilityDb } from "@facility/db"; +import { expect, it, vi } from "vitest"; +import type { AgentCatalogService } from "../src/agents/catalog.js"; +import type { StoryWorkspaceService } from "../src/stories/service.js"; +import { TurnDispatcher } from "../src/turns/dispatcher.js"; + +const input = { + orgId: "org_test", + projectId: "proj_test", + storyId: "story_test", + turnId: "turn_test", +}; + +function fixture(rows: unknown[][]) { + const limit = vi.fn(); + for (const result of rows) limit.mockResolvedValueOnce(result); + const query = { + from: vi.fn().mockReturnThis(), + where: vi.fn().mockReturnThis(), + orderBy: vi.fn().mockReturnThis(), + limit, + }; + const db = { select: vi.fn(() => query) }; + const service = { activateNextMessage: vi.fn(), flagAttention: vi.fn() }; + const catalog = { get: vi.fn().mockRejectedValue(new Error("catalog unavailable")) }; + const dispatcher = new TurnDispatcher( + db as unknown as FacilityDb, + service as unknown as StoryWorkspaceService, + catalog as unknown as AgentCatalogService, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + ); + return { dispatcher, service, catalog }; +} + +it("does not resolve agents or activate retained messages without an active workspace", async () => { + const { dispatcher, service, catalog } = fixture([[]]); + await dispatcher.activateQueuedSuccessor(input); + expect(catalog.get).not.toHaveBeenCalled(); + expect(service.activateNextMessage).not.toHaveBeenCalled(); + expect(service.flagAttention).not.toHaveBeenCalled(); +}); + +it("does not create a retry notice when deletion wins during catalog lookup", async () => { + const { dispatcher, service, catalog } = fixture([ + [{ id: "ws_test" }], + [{ requestedAgentName: "builder" }], + [], + ]); + await expect(dispatcher.activateQueuedSuccessor(input)).resolves.toBeUndefined(); + expect(catalog.get).toHaveBeenCalledOnce(); + expect(service.activateNextMessage).not.toHaveBeenCalled(); + expect(service.flagAttention).not.toHaveBeenCalled(); +}); diff --git a/services/api/test/story-workspaces.integration.test.ts b/services/api/test/story-workspaces.integration.test.ts index 18d0da73..e07fa87e 100644 --- a/services/api/test/story-workspaces.integration.test.ts +++ b/services/api/test/story-workspaces.integration.test.ts @@ -19,7 +19,7 @@ import { turns, workspaces, } from "@facility/db"; -import { eq } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import postgres from "postgres"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { StoryServiceError, StoryWorkspaceService } from "../src/stories/service.js"; @@ -795,6 +795,175 @@ environment: ).resolves.toMatchObject({ story: { id: result.story.id } }); }); + it("cancels queued turns and never activates retained messages after deletion", async () => { + const result = await service.start(startInput(`delete-queued-${randomUUID()}`)); + if (!result.queued.turn) throw new Error("expected queued turn"); + const input = { + orgId, + projectId, + storyId: result.story.id, + actor: { type: "user" as const, id: "user_test" }, + }; + const message = { + ...input, + body: "Follow up", + dedupeKey: randomUUID(), + agent: builder, + trigger: { type: "manual" as const }, + }; + await service.queueMessage(message); + await service.deleteWorkspace({ ...input, confirm: true }); + const bundle = await service.get(orgId, projectId, result.story.id); + expect(bundle.turns).toHaveLength(1); + expect(bundle.turns[0]).toMatchObject({ + state: "canceled", + error: null, + endedAt: expect.any(Date), + }); + expect(bundle.story.activeAgentName).toBeNull(); + expect(bundle.attention).toEqual([]); + await expect( + service.flagAttention({ + ...input, + kind: "queued_turn_dispatch_error", + title: "Late callback", + detail: "Catalog failed during deletion", + }), + ).resolves.toBeUndefined(); + expect((await service.get(orgId, projectId, result.story.id)).attention).toEqual([]); + expect(bundle.events).toContainEqual(expect.objectContaining({ type: "turn.canceled" })); + await expect( + service.activateNextMessage({ ...input, agent: builder }), + ).resolves.toBeUndefined(); + await expect( + service.queueMessage({ ...message, dedupeKey: randomUUID() }), + ).rejects.toMatchObject({ code: "story_workspace_deleted" }); + // A stale queue delivery cannot claim the canceled turn. + expect( + await db + .update(turns) + .set({ state: "running" }) + .where(and(eq(turns.id, result.queued.turn.id), eq(turns.state, "queued"))) + .returning(), + ).toEqual([]); + }); + + it("refuses running turns without destroying state, including cross-tenant requests", async () => { + const result = await service.start(startInput(`delete-running-${randomUUID()}`)); + if (!result.queued.turn) throw new Error("expected queued turn"); + const input = { + orgId, + projectId, + storyId: result.story.id, + actor: { type: "user" as const, id: "user_test" }, + confirm: true, + }; + await db + .update(turns) + .set({ state: "running", startedAt: new Date() }) + .where(eq(turns.id, result.queued.turn.id)); + const destroy = vi.spyOn(runtime, "destroy"); + try { + await expect( + service.deleteWorkspace({ ...input, orgId: otherOrgId, projectId: otherProjectId }), + ).rejects.toMatchObject({ code: "story_not_found", statusCode: 404 }); + await expect(service.deleteWorkspace(input)).rejects.toMatchObject({ + code: "workspace_busy", + statusCode: 409, + }); + expect(destroy).not.toHaveBeenCalled(); + expect(await service.get(orgId, projectId, result.story.id)).toMatchObject({ + workspace: { state: "running" }, + turns: [expect.objectContaining({ state: "running" })], + }); + } finally { + destroy.mockRestore(); + } + }); + + it("blocks admission while destruction is pending and permits retry after provider failure", async () => { + const start = startInput(`delete-failure-${randomUUID()}`); + const result = await service.start(start); + const input = { + orgId, + projectId, + storyId: result.story.id, + actor: { type: "user" as const, id: "user_test" }, + confirm: true, + }; + const destroy = vi + .spyOn(runtime, "destroy") + .mockRejectedValueOnce(new Error("provider unavailable")); + try { + await expect(service.deleteWorkspace(input)).rejects.toThrow("provider unavailable"); + expect((await service.get(orgId, projectId, result.story.id)).workspace?.state).toBe( + "deleting", + ); + await expect(service.start(start)).rejects.toMatchObject({ code: "story_workspace_deleted" }); + await expect( + service.queueMessage({ + ...input, + body: "Must not run", + dedupeKey: randomUUID(), + agent: builder, + trigger: { type: "manual" }, + }), + ).rejects.toMatchObject({ code: "story_workspace_deleted" }); + await expect( + service.activateNextMessage({ ...input, agent: builder }), + ).resolves.toBeUndefined(); + await expect(service.deleteWorkspace(input)).resolves.toMatchObject({ + workspace: { state: "destroyed" }, + }); + } finally { + destroy.mockRestore(); + } + }); + + it("observes a concurrent worker claim before deciding whether deletion is safe", async () => { + const result = await service.start(startInput(`delete-race-${randomUUID()}`)); + if (!result.queued.turn) throw new Error("expected queued turn"); + const turnId = result.queued.turn.id; + let release!: () => void; + let claimed!: () => void; + const held = new Promise((resolve) => { + release = resolve; + }); + const ready = new Promise((resolve) => { + claimed = resolve; + }); + const claiming = db.transaction(async (tx) => { + await tx + .update(turns) + .set({ state: "running", startedAt: new Date() }) + .where(eq(turns.id, turnId)); + claimed(); + await held; + }); + await ready; + const deleting = service.deleteWorkspace({ + orgId, + projectId, + storyId: result.story.id, + actor: { type: "user", id: "user_test" }, + confirm: true, + }); + const rejected = expect(deleting).rejects.toMatchObject({ code: "workspace_busy" }); + try { + await vi.waitFor(async () => { + const waiting = await db.execute(sql`select 1 from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock' + and query like '%"turns"%' and query like '%for update%'`); + expect(waiting.length).toBeGreaterThan(0); + }); + } finally { + release(); + } + await claiming; + await rejected; + expect((await service.get(orgId, projectId, result.story.id)).workspace?.state).toBe("running"); + }); + it("records runtime failures as visible attention instead of losing the story", async () => { class FailingRuntime extends FakeWorkspaceRuntime { override async create(): Promise { @@ -856,6 +1025,7 @@ environment: title: "Builder needs a reply", detail: "Which migration policy should be used?", }); + if (!waiting) throw new Error("expected attention"); await service.dismissAttention({ orgId, projectId, diff --git a/services/api/test/turn-dispatcher.integration.test.ts b/services/api/test/turn-dispatcher.integration.test.ts index 13d9a7de..9a9dc7b6 100644 --- a/services/api/test/turn-dispatcher.integration.test.ts +++ b/services/api/test/turn-dispatcher.integration.test.ts @@ -863,6 +863,49 @@ environment: expect(engine.requests.at(-1)?.nativeSessionId).toBe("native-before-failure"); }, 15_000); + it("ignores stale delivery and retained follow-ups after workspace deletion", async () => { + const started = await storiesService.start({ + orgId, + projectId, + provider: "manual", + externalId: `deleted-${suffix}`, + title: "Delete queued work", + agent: builder, + message: "Never execute", + messageDedupeKey: `delete-start-${suffix}`, + actor: { type: "user", id: "user_test" }, + workspace: { image: "facility-runner:test", ports: [] }, + }); + if (!started.queued.turn) throw new Error("expected queued turn"); + await storiesService.queueMessage({ + orgId, + projectId, + storyId: started.story.id, + body: "Do not activate this follow-up", + dedupeKey: `delete-followup-${suffix}`, + agent: builder, + actor: { type: "user", id: "user_test" }, + trigger: { type: "manual" }, + }); + await storiesService.deleteWorkspace({ + orgId, + projectId, + storyId: started.story.id, + actor: { type: "user", id: "user_test" }, + confirm: true, + }); + const requests = engine.requests.length; + await expect( + dispatcher.dispatch({ orgId, projectId, turnId: started.queued.turn.id }), + ).resolves.toEqual({ claimed: false }); + expect(engine.requests).toHaveLength(requests); + const bundle = await storiesService.get(orgId, projectId, started.story.id); + expect(bundle.turns).toHaveLength(1); + expect(bundle.turns[0]?.state).toBe("canceled"); + expect(bundle.attention).toEqual([]); + expect(bundle.workspace?.state).toBe("destroyed"); + }); + it("cancels a running agent process while preserving the workspace and future turns", async () => { engine.blockUntilCanceled = true; engine.blockingStarted = false; diff --git a/services/api/test/workspace-deletion-routes.integration.test.ts b/services/api/test/workspace-deletion-routes.integration.test.ts new file mode 100644 index 00000000..c45e84cc --- /dev/null +++ b/services/api/test/workspace-deletion-routes.integration.test.ts @@ -0,0 +1,286 @@ +import { randomUUID } from "node:crypto"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { parseAgentManifest } from "@facility/agents"; +import { generateApiKey, newId, open, seal } from "@facility/core"; +import { apiKeys, createDb, migrate, orgs, projects, seed, turns } from "@facility/db"; +import { eq } from "drizzle-orm"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { buildApp } from "../src/app.js"; +import { createStoryDomain } from "../src/story-domain.js"; +import type { AppConfig } from "../src/types.js"; +import { FakeWorkspaceRuntime } from "../src/workspaces/fake.js"; + +const databaseUrl = + process.env.DATABASE_URL ?? "postgres://facility:facility@localhost:5461/facility_test"; +const builder = parseAgentManifest( + `--- +name: builder +description: Deletion fixture. +engine: codex +model: gpt-5.5 +enabled: true +triggers: + - type: manual +--- +Complete the requested work. +`, + "builder.md", +); + +describe("workspace deletion HTTP contract", () => { + const { db, client } = createDb(databaseUrl); + const projectId = newId("proj"); + const otherProjectId = newId("proj"); + const otherOrgId = newId("org"); + const foreignProjectId = newId("proj"); + const config: AppConfig = { + databaseUrl, + secretMasterKey: Buffer.alloc(32, 27).toString("base64"), + port: 4400, + publicUrl: "http://localhost:4400", + webUrl: "http://localhost:3400", + workspaceImage: "facility-runner:test", + workspaceDriver: "docker", + facilityInsecureDev: true, + logLevel: "silent", + }; + let root: string; + let runtime: FakeWorkspaceRuntime; + let domain: ReturnType; + let app: Awaited>; + let owner = ""; + let viewer = ""; + let revoked = ""; + let expiredCookie = ""; + + beforeAll(async () => { + await migrate(databaseUrl); + await seed(databaseUrl, { includeDemoData: true }); + await db + .insert(orgs) + .values({ id: otherOrgId, name: "Foreign deletion tenant", slug: randomUUID() }); + await db.insert(projects).values([ + { id: projectId, orgId: "org_local", name: "Deletion", slug: randomUUID() }, + { id: otherProjectId, orgId: "org_local", name: "Other project", slug: randomUUID() }, + { id: foreignProjectId, orgId: otherOrgId, name: "Foreign project", slug: randomUUID() }, + ]); + for (const kind of ["owner", "viewer", "revoked"] as const) { + const key = await generateApiKey("fak"); + await db.insert(apiKeys).values({ + id: key.id, + orgId: "org_local", + name: kind, + prefix: key.lookup, + last4: key.last4, + hash: key.hash, + scopeType: "project", + projectId, + roleId: kind === "viewer" ? "role_bundled_viewer" : "role_bundled_owner", + ...(kind === "revoked" ? { revokedAt: new Date() } : {}), + }); + if (kind === "owner") owner = key.secret; + if (kind === "viewer") viewer = key.secret; + if (kind === "revoked") revoked = key.secret; + } + root = await mkdtemp(join(tmpdir(), "facility-delete-http-")); + runtime = new FakeWorkspaceRuntime(root); + domain = createStoryDomain({ db, config, runtime, enqueue: async () => undefined }); + app = await buildApp(config, { storyDomain: domain, rateLimitMax: 10000 }); + await app.ready(); + const login = await app.inject({ method: "GET", url: "/auth/dev-login" }); + const cookie = login.cookies.find((item) => item.name === "facility_session"); + if (!cookie) throw new Error("session fixture missing"); + const session = JSON.parse(await open(cookie.value, config.secretMasterKey)); + expiredCookie = `facility_session=${await seal(JSON.stringify({ ...session, exp: 1 }), config.secretMasterKey)}`; + }); + + afterAll(async () => { + await app?.close(); + await client.end(); + if (root) await rm(root, { recursive: true, force: true }); + }); + + async function start(orgId = "org_local", project = projectId) { + return domain.stories.start({ + orgId, + projectId: project, + provider: "manual", + externalId: randomUUID(), + title: "Delete through HTTP", + agent: builder, + message: "Queued work", + messageDedupeKey: randomUUID(), + actor: { type: "user", id: "user_local" }, + workspace: { image: "facility-runner:test", ports: [] }, + }); + } + + function request(storyId: string, key = randomUUID(), project = projectId) { + return { + method: "DELETE" as const, + url: `/v1/projects/${project}/workspace-stories/${storyId}/workspace`, + headers: { authorization: `Bearer ${owner}`, "idempotency-key": key }, + payload: { confirm: true, idempotency_key: key }, + }; + } + + it("cancels queued work and replays successful deletion without destroying twice", async () => { + const started = await start(); + const destroy = vi.spyOn(runtime, "destroy"); + try { + const req = request(started.story.id); + const first = await app.inject(req); + expect(first.statusCode, first.body).toBe(200); + const replay = await app.inject(req); + expect(replay.statusCode, replay.body).toBe(200); + expect(replay.headers["idempotency-status"]).toBe("replayed"); + expect(replay.json()).toEqual(first.json()); + expect(destroy).toHaveBeenCalledOnce(); + const bundle = await domain.stories.get("org_local", projectId, started.story.id); + expect(bundle.workspace?.state).toBe("destroyed"); + expect(bundle.turns[0]?.state).toBe("canceled"); + expect(bundle.attention).toEqual([]); + const fresh = await app.inject(request(started.story.id)); + expect(fresh.statusCode, fresh.body).toBe(200); + expect(destroy).toHaveBeenCalledOnce(); + } finally { + destroy.mockRestore(); + } + }); + + it("returns 409 for running work and accepts a fresh retry after completion", async () => { + const started = await start(); + const turn = started.queued.turn; + if (!turn) throw new Error("queued turn fixture missing"); + await db + .update(turns) + .set({ state: "running", startedAt: new Date() }) + .where(eq(turns.id, turn.id)); + const destroy = vi.spyOn(runtime, "destroy"); + try { + const req = request(started.story.id); + const blocked = await app.inject(req); + expect(blocked.statusCode, blocked.body).toBe(409); + expect(blocked.json().error.code).toBe("workspace_busy"); + expect(destroy).not.toHaveBeenCalled(); + await domain.stories.completeTurn({ + orgId: "org_local", + projectId, + turnId: turn.id, + output: "Done", + actor: { type: "system", id: "fixture" }, + }); + // 4xx results are replayed by the shared idempotency contract. A new + // operator attempt must use a new key once the running turn has settled. + const replay = await app.inject(req); + expect(replay.statusCode).toBe(409); + expect(replay.headers["idempotency-status"]).toBe("replayed"); + const retried = await app.inject(request(started.story.id)); + expect(retried.statusCode, retried.body).toBe(200); + expect(destroy).toHaveBeenCalledOnce(); + } finally { + destroy.mockRestore(); + } + }); + + it("permits the same idempotency key to retry provider cleanup after a 500", async () => { + const started = await start(); + const destroy = vi + .spyOn(runtime, "destroy") + .mockRejectedValueOnce(new Error("provider unavailable")); + try { + const req = request(started.story.id); + const failed = await app.inject(req); + expect(failed.statusCode, failed.body).toBe(500); + expect( + (await domain.stories.get("org_local", projectId, started.story.id)).workspace?.state, + ).toBe("deleting"); + const retry = await app.inject(req); + expect(retry.statusCode, retry.body).toBe(200); + expect(destroy).toHaveBeenCalledTimes(2); + } finally { + destroy.mockRestore(); + } + }); + + it("requires confirmation and a matching, valid idempotency key", async () => { + const started = await start(); + const destroy = vi.spyOn(runtime, "destroy"); + try { + for (const confirm of [false, "true", null]) { + const req = request(started.story.id); + const response = await app.inject({ ...req, payload: { ...req.payload, confirm } }); + expect(response.statusCode, response.body).toBe(400); + } + const req = request(started.story.id); + for (const headers of [ + { authorization: `Bearer ${owner}` }, + { authorization: `Bearer ${owner}`, "idempotency-key": "short" }, + { authorization: `Bearer ${owner}`, "idempotency-key": randomUUID() }, + ]) { + const response = await app.inject({ ...req, headers }); + expect(response.statusCode, response.body).toBe(400); + } + expect(destroy).not.toHaveBeenCalled(); + expect( + (await domain.stories.get("org_local", projectId, started.story.id)).workspace?.state, + ).toBe("running"); + } finally { + destroy.mockRestore(); + } + }); + + it("rejects unauthenticated, malformed, expired, revoked and read-only credentials", async () => { + const started = await start(); + const destroy = vi.spyOn(runtime, "destroy"); + try { + for (const [auth, expected] of [ + [{}, 401], + [{ authorization: "Bearer invalid" }, 401], + [{ authorization: `Bearer ${revoked}` }, 401], + [{ cookie: expiredCookie }, 401], + [{ authorization: `Bearer ${viewer}` }, 403], + ] as const) { + const req = request(started.story.id); + const response = await app.inject({ + ...req, + headers: { ...auth, "idempotency-key": req.payload.idempotency_key }, + }); + expect(response.statusCode, response.body).toBe(expected); + } + expect(destroy).not.toHaveBeenCalled(); + expect( + (await domain.stories.get("org_local", projectId, started.story.id)).turns[0]?.state, + ).toBe("queued"); + } finally { + destroy.mockRestore(); + } + }); + + it("denies both cross-project and cross-tenant deletion without touching their workspaces", async () => { + const other = await start("org_local", otherProjectId); + const foreign = await start(otherOrgId, foreignProjectId); + const destroy = vi.spyOn(runtime, "destroy"); + try { + for (const [storyId, project] of [ + [other.story.id, otherProjectId], + [foreign.story.id, foreignProjectId], + [foreign.story.id, projectId], + ] as const) { + const response = await app.inject(request(storyId, randomUUID(), project)); + expect(response.statusCode, response.body).toBe(404); + } + expect(destroy).not.toHaveBeenCalled(); + expect( + (await domain.stories.get("org_local", otherProjectId, other.story.id)).workspace?.state, + ).toBe("running"); + expect( + (await domain.stories.get(otherOrgId, foreignProjectId, foreign.story.id)).workspace?.state, + ).toBe("running"); + } finally { + destroy.mockRestore(); + } + }); +}); From 6a339dd0ebedb4b29fe0ef5de4416ed8d44a824a Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Mon, 28 Sep 2026 17:48:52 +0800 Subject: [PATCH 2/7] test(api): verify workspace deletion with real Docker resources --- .../workspace-runtime.integration.test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/services/api/test/workspace-runtime.integration.test.ts b/services/api/test/workspace-runtime.integration.test.ts index 969af72a..92d2e280 100644 --- a/services/api/test/workspace-runtime.integration.test.ts +++ b/services/api/test/workspace-runtime.integration.test.ts @@ -1,7 +1,12 @@ import { randomBytes } from "node:crypto"; import { readFile } from "node:fs/promises"; +import { parseAgentManifest } from "@facility/agents"; +import { newId } from "@facility/core"; +import { createDb, migrate, orgs, projects, turns } from "@facility/db"; import Docker from "dockerode"; +import { eq } from "drizzle-orm"; import { describe, expect, it } from "vitest"; +import { StoryWorkspaceService } from "../src/stories/service.js"; import { stopInterruptedEngineProcess } from "../src/turns/engines.js"; import { DockerWorkspaceRuntime } from "../src/workspaces/docker.js"; import { @@ -13,6 +18,98 @@ import { const enabled = process.env.FACILITY_E2E_DOCKER === "1"; describe.skipIf(!enabled)("DockerWorkspaceRuntime integration", () => { + it.each([ + "queued", + "running", + ] as const)("coordinates %s turns with real container and volume deletion", async (state) => { + const databaseUrl = + process.env.DATABASE_URL ?? "postgres://facility:facility@127.0.0.1:5461/facility_ws"; + const { db, client } = createDb(databaseUrl); + const docker = new Docker(); + const runtime = new DockerWorkspaceRuntime(docker); + const service = new StoryWorkspaceService(db, runtime, async () => undefined); + const orgId = newId("org"); + const projectId = newId("proj"); + const actor = { type: "system" as const, id: "docker-deletion-regression" }; + const image = process.env.FACILITY_WORKSPACE_TEST_IMAGE ?? "facility-runner:serialized"; + let workspace: WorkspaceLocator | undefined; + try { + await migrate(databaseUrl); + await db.insert(orgs).values({ id: orgId, name: "Docker deletion", slug: orgId }); + await db.insert(projects).values({ + id: projectId, + orgId, + name: "Docker deletion", + slug: projectId, + }); + const started = await service.start({ + orgId, + projectId, + provider: "manual", + externalId: newId("story"), + title: "Docker deletion regression", + agent: parseAgentManifest( + "---\nname: builder\ndescription: Docker fixture\nengine: codex\nmodel: gpt-5.5\nenabled: true\ntriggers:\n - type: manual\n---\nTest deletion.\n", + "builder.md", + ), + message: "Queued work", + messageDedupeKey: newId("msg"), + actor, + workspace: { image, ports: [] }, + }); + const row = started.workspace; + const turn = started.queued.turn; + if (!row?.externalRef || !turn) throw new Error("workspace fixture missing"); + workspace = { + id: row.id, + image, + ports: [], + externalRef: row.externalRef, + volumeRef: row.volumeRef, + }; + const filters = { label: [`facility.workspace.id=${row.id}`] }; + const request = { orgId, projectId, storyId: started.story.id, actor, confirm: true }; + expect(await docker.listContainers({ all: true, filters })).toHaveLength(1); + expect((await docker.listVolumes({ filters })).Volumes).toHaveLength(1); + if (state === "running") { + await db + .update(turns) + .set({ state: "running", startedAt: new Date() }) + .where(eq(turns.id, turn.id)); + await expect(service.deleteWorkspace(request)).rejects.toMatchObject({ + code: "workspace_busy", + statusCode: 409, + }); + await expect(runtime.inspect(workspace)).resolves.toMatchObject({ state: "running" }); + expect((await docker.listVolumes({ filters })).Volumes).toHaveLength(1); + const alive = await runtime.exec(workspace, { + command: "sh", + args: ["-lc", "printf still-running"], + }); + expect(alive).toMatchObject({ exitCode: 0, stdout: "still-running" }); + await service.completeTurn({ orgId, projectId, turnId: turn.id, output: "Done", actor }); + } + const deleted = await service.deleteWorkspace(request); + expect(deleted.workspace?.state).toBe("destroyed"); + expect(deleted.turns.find((item) => item.id === turn.id)?.state).toBe( + state === "queued" ? "canceled" : "succeeded", + ); + expect(deleted.attention).toEqual([]); + expect(await docker.listContainers({ all: true, filters })).toEqual([]); + expect((await docker.listVolumes({ filters })).Volumes ?? []).toEqual([]); + expect(await docker.listNetworks({ filters })).toEqual([]); + await expect(runtime.inspect(workspace)).resolves.toMatchObject({ state: "destroyed" }); + await service.deleteWorkspace(request); + expect(await docker.listContainers({ all: true, filters })).toEqual([]); + } finally { + try { + if (workspace) await runtime.destroy(workspace); + } finally { + await client.end(); + } + } + }, 180_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"); From 6d138c0ad2988a9c304623e6cb81e2643099ea03 Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Tue, 29 Sep 2026 00:34:26 +0800 Subject: [PATCH 3/7] style(api): format Docker deletion assertion --- services/api/test/workspace-runtime.integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api/test/workspace-runtime.integration.test.ts b/services/api/test/workspace-runtime.integration.test.ts index 92d2e280..3905c1da 100644 --- a/services/api/test/workspace-runtime.integration.test.ts +++ b/services/api/test/workspace-runtime.integration.test.ts @@ -92,7 +92,7 @@ describe.skipIf(!enabled)("DockerWorkspaceRuntime integration", () => { const deleted = await service.deleteWorkspace(request); expect(deleted.workspace?.state).toBe("destroyed"); expect(deleted.turns.find((item) => item.id === turn.id)?.state).toBe( - state === "queued" ? "canceled" : "succeeded", + state === "queued" ? "canceled" : "succeeded", ); expect(deleted.attention).toEqual([]); expect(await docker.listContainers({ all: true, filters })).toEqual([]); From de28bb78d5a5f2d3e0ba59017126ea2630a599b2 Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Tue, 29 Sep 2026 10:55:03 +0800 Subject: [PATCH 4/7] fix(api): serialize workspace startup with deletion --- services/api/src/stories/service.ts | 145 ++++++++++-------- .../test/story-workspaces.integration.test.ts | 91 +++++++++++ 2 files changed, 171 insertions(+), 65 deletions(-) diff --git a/services/api/src/stories/service.ts b/services/api/src/stories/service.ts index 0c79f1e5..ae7fb5ef 100644 --- a/services/api/src/stories/service.ts +++ b/services/api/src/stories/service.ts @@ -243,52 +243,69 @@ export class StoryWorkspaceService { return { story, conversation, workspace }; }); - let handle: WorkspaceHandle; - const runtimeOperation = - aggregate.workspace.state === "creating" || !aggregate.workspace.externalRef - ? "create" - : "wake"; - const runtimeStartedAt = performance.now(); - try { - handle = - runtimeOperation === "create" - ? await this.runtime.create({ - id: aggregate.workspace.id, - ...workspaceInput(aggregate.workspace), - }) - : await this.runtime.wake(locatorFromRow(aggregate.workspace)); - await this.db - .update(workspaces) - .set({ - externalRef: handle.externalRef, - volumeRef: handle.volumeRef, - state: "running", - error: null, - lastActivityAt: new Date(), - updatedAt: new Date(), - }) - .where( - and( - eq(workspaces.orgId, input.orgId), - eq(workspaces.projectId, input.projectId), - eq(workspaces.id, aggregate.workspace.id), - ), - ); - await appendWorkspaceEvent(this.db, aggregate.workspace.id, input.orgId, "workspace.ready", { - provider: this.runtime.provider, - computeRef: handle.computeRef, - operation: runtimeOperation, - durationMs: Math.round(performance.now() - runtimeStartedAt), - }); - } catch (error) { - await this.markRuntimeFailure(aggregate.story.id, aggregate.workspace.id, input, error); - throw new StoryServiceError( - "workspace_start_failed", - error instanceof Error ? error.message : "workspace failed to start", - 503, - { cause: error }, + const failure = await this.db.transaction(async (rawTx) => { + const tx = rawTx as unknown as FacilityDb; + // Admission commits before provider I/O so a failed first start remains visible. + // Reacquire deletion's lock and re-read state before touching external resources. + await lockStory(tx, input.orgId, input.projectId, aggregate.story.id); + await this.assertWorkspaceNotDeleting(tx, input.orgId, input.projectId, aggregate.story.id); + const workspace = await this.activeWorkspace( + input.orgId, + input.projectId, + aggregate.story.id, + false, + tx, ); - } + if (!workspace) + throw new StoryServiceError("story_workspace_deleted", "workspace is unavailable", 409); + try { + const runtimeOperation = + workspace.state === "creating" || !workspace.externalRef ? "create" : "wake"; + const runtimeStartedAt = performance.now(); + const handle: WorkspaceHandle = + runtimeOperation === "create" + ? await this.runtime.create({ + id: workspace.id, + ...workspaceInput(workspace), + }) + : await this.runtime.wake(locatorFromRow(workspace)); + await tx + .update(workspaces) + .set({ + externalRef: handle.externalRef, + volumeRef: handle.volumeRef, + state: "running", + error: null, + lastActivityAt: new Date(), + updatedAt: new Date(), + }) + .where( + and( + eq(workspaces.orgId, input.orgId), + eq(workspaces.projectId, input.projectId), + eq(workspaces.id, workspace.id), + ), + ); + await appendWorkspaceEvent(tx, workspace.id, input.orgId, "workspace.ready", { + provider: this.runtime.provider, + computeRef: handle.computeRef, + operation: runtimeOperation, + durationMs: Math.round(performance.now() - runtimeStartedAt), + }); + + return undefined; + } catch (error) { + // Commit failure state before releasing the lifecycle lock as well. + await this.markRuntimeFailure(tx, aggregate.story.id, workspace.id, input, error); + return new StoryServiceError( + "workspace_start_failed", + error instanceof Error ? error.message : "workspace failed to start", + 503, + { cause: error }, + ); + } + }); + if (failure) throw failure; const queued = await this.queueMessage({ orgId: input.orgId, @@ -1922,33 +1939,31 @@ export class StoryWorkspaceService { } private async markRuntimeFailure( + tx: FacilityDb, storyId: string, workspaceId: string, input: StartStoryInput, error: unknown, ) { const detail = error instanceof Error ? error.message : String(error); - await this.db.transaction(async (rawTx) => { - const tx = rawTx as unknown as FacilityDb; - await tx - .update(workspaces) - .set({ state: "error", error: detail, updatedAt: new Date() }) - .where(and(eq(workspaces.orgId, input.orgId), eq(workspaces.id, workspaceId))); - await tx - .update(stories) - .set({ status: "attention", updatedAt: new Date() }) - .where(and(eq(stories.orgId, input.orgId), eq(stories.id, storyId))); - await tx.insert(attentionItems).values({ - id: newId("attn"), - orgId: input.orgId, - projectId: input.projectId, - storyId, - kind: "runtime_error", - title: "Workspace could not start", - detail, - }); + await tx + .update(workspaces) + .set({ state: "error", error: detail, updatedAt: new Date() }) + .where(and(eq(workspaces.orgId, input.orgId), eq(workspaces.id, workspaceId))); + await tx + .update(stories) + .set({ status: "attention", updatedAt: new Date() }) + .where(and(eq(stories.orgId, input.orgId), eq(stories.id, storyId))); + await tx.insert(attentionItems).values({ + id: newId("attn"), + orgId: input.orgId, + projectId: input.projectId, + storyId, + kind: "runtime_error", + title: "Workspace could not start", + detail, }); - await appendWorkspaceEvent(this.db, workspaceId, input.orgId, "workspace.provider_error", { + await appendWorkspaceEvent(tx, workspaceId, input.orgId, "workspace.provider_error", { operation: "create", error: detail.slice(0, 8_000), }); diff --git a/services/api/test/story-workspaces.integration.test.ts b/services/api/test/story-workspaces.integration.test.ts index e07fa87e..d599831c 100644 --- a/services/api/test/story-workspaces.integration.test.ts +++ b/services/api/test/story-workspaces.integration.test.ts @@ -920,6 +920,97 @@ environment: } }); + it("does not wake a workspace deleted after start admission commits", async () => { + const input = startInput(`start-delete-gap-${randomUUID()}`); + const result = await service.start(input); + let first = true; + const delayedDb = new Proxy(db, { + get(target, property, receiver) { + if (property !== "transaction") return Reflect.get(target, property, receiver); + return async (...args: Parameters) => { + const value = await target.transaction(...args); + if (first) { + first = false; + await service.deleteWorkspace({ + orgId, + projectId, + storyId: result.story.id, + actor: input.actor, + confirm: true, + }); + } + return value; + }; + }, + }); + const wake = vi.spyOn(runtime, "wake"); + try { + await expect( + new StoryWorkspaceService(delayedDb, runtime).start(input), + ).rejects.toMatchObject({ + code: "story_workspace_deleted", + }); + expect(wake).not.toHaveBeenCalled(); + expect((await service.get(orgId, projectId, result.story.id)).workspace?.state).toBe( + "destroyed", + ); + } finally { + wake.mockRestore(); + } + }); + + it.each([ + false, + true, + ])("serializes deletion with an in-flight wake (failure=%s)", async (fails) => { + const input = startInput(`start-delete-held-${randomUUID()}`); + const result = await service.start(input); + let release!: () => void; + let entered!: () => void; + const held = new Promise((resolve) => { + release = resolve; + }); + const ready = new Promise((resolve) => { + entered = resolve; + }); + const originalWake = runtime.wake.bind(runtime); + const wake = vi.spyOn(runtime, "wake").mockImplementationOnce(async (locator) => { + entered(); + await held; + if (fails) throw new Error("wake unavailable"); + return originalWake(locator); + }); + const destroy = vi.spyOn(runtime, "destroy"); + const starting = service.start(input).catch((error: unknown) => error); + await ready; + const deleting = service.deleteWorkspace({ + orgId, + projectId, + storyId: result.story.id, + actor: input.actor, + confirm: true, + }); + try { + await vi.waitFor(async () => { + const waiting = await db.execute(sql`select 1 from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock' + and query like '%pg_advisory_xact_lock%'`); + expect(waiting.length).toBeGreaterThan(0); + }); + expect(destroy).not.toHaveBeenCalled(); + } finally { + release(); + await starting; + await deleting; + wake.mockRestore(); + destroy.mockRestore(); + } + const final = await service.get(orgId, projectId, result.story.id); + expect(final.workspace?.state).toBe("destroyed"); + expect(final.story.status).toBe("archived"); + expect(final.story.deletedAt).not.toBeNull(); + }); + it("observes a concurrent worker claim before deciding whether deletion is safe", async () => { const result = await service.start(startInput(`delete-race-${randomUUID()}`)); if (!result.queued.turn) throw new Error("expected queued turn"); From ad663886369784dc852dda3a56657d424f1e6ab6 Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Wed, 30 Sep 2026 14:50:24 +0800 Subject: [PATCH 5/7] fix(api): serialize workspace suspension with deletion --- services/api/src/stories/service.ts | 22 ++-- .../test/story-workspaces.integration.test.ts | 106 ++++++++++++++++++ 2 files changed, 120 insertions(+), 8 deletions(-) diff --git a/services/api/src/stories/service.ts b/services/api/src/stories/service.ts index ae7fb5ef..903f1e82 100644 --- a/services/api/src/stories/service.ts +++ b/services/api/src/stories/service.ts @@ -1631,14 +1631,20 @@ export class StoryWorkspaceService { } async suspend(orgId: string, projectId: string, storyId: string) { - const workspace = await this.activeWorkspace(orgId, projectId, storyId); - if (!workspace || workspace.state === "destroyed") return this.get(orgId, projectId, storyId); - await this.runtime.suspend(locatorFromRow(workspace)); - await this.db - .update(workspaces) - .set({ state: "sleeping", updatedAt: new Date() }) - .where(and(eq(workspaces.orgId, orgId), eq(workspaces.id, workspace.id))); - await appendWorkspaceEvent(this.db, workspace.id, orgId, "workspace.suspended", {}); + await this.db.transaction(async (rawTx) => { + const tx = rawTx as unknown as FacilityDb; + // Keep provider suspension and its state writeback under deletion's lock. + // Re-read after locking so deleting/destroyed workspaces remain untouched. + await lockStory(tx, orgId, projectId, storyId); + const workspace = await this.activeWorkspace(orgId, projectId, storyId, false, tx); + if (!workspace) return; + await this.runtime.suspend(locatorFromRow(workspace)); + await tx + .update(workspaces) + .set({ state: "sleeping", updatedAt: new Date() }) + .where(and(eq(workspaces.orgId, orgId), eq(workspaces.id, workspace.id))); + await appendWorkspaceEvent(tx, workspace.id, orgId, "workspace.suspended", {}); + }); return this.get(orgId, projectId, storyId); } diff --git a/services/api/test/story-workspaces.integration.test.ts b/services/api/test/story-workspaces.integration.test.ts index d599831c..3fe3d38f 100644 --- a/services/api/test/story-workspaces.integration.test.ts +++ b/services/api/test/story-workspaces.integration.test.ts @@ -920,6 +920,112 @@ environment: } }); + it("does not let an in-flight suspension reopen admission during deletion", async () => { + const input = startInput(`suspend-delete-${randomUUID()}`); + const result = await service.start(input); + let releaseSuspend!: () => void; + let enteredSuspend!: () => void; + let releaseDestroy!: () => void; + let enteredDestroy!: () => void; + const suspendHeld = new Promise((resolve) => { + releaseSuspend = resolve; + }); + const suspendReady = new Promise((resolve) => { + enteredSuspend = resolve; + }); + const destroyHeld = new Promise((resolve) => { + releaseDestroy = resolve; + }); + const destroyReady = new Promise((resolve) => { + enteredDestroy = resolve; + }); + const originalSuspend = runtime.suspend.bind(runtime); + const originalDestroy = runtime.destroy.bind(runtime); + const suspend = vi.spyOn(runtime, "suspend").mockImplementationOnce(async (workspace) => { + enteredSuspend(); + await suspendHeld; + return originalSuspend(workspace); + }); + const destroy = vi.spyOn(runtime, "destroy").mockImplementationOnce(async (workspace) => { + enteredDestroy(); + await destroyHeld; + return originalDestroy(workspace); + }); + const suspending = service.suspend(orgId, projectId, result.story.id); + await suspendReady; + const deleting = service.deleteWorkspace({ + orgId, + projectId, + storyId: result.story.id, + actor: input.actor, + confirm: true, + }); + try { + // The old implementation reaches destroy while suspension is paused. + // The fixed implementation waits on the shared story lock instead. + await vi.waitFor(async () => { + const waiting = await db.execute(sql`select 1 from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock' + and query like '%pg_advisory_xact_lock%'`); + expect(destroy.mock.calls.length > 0 || waiting.length > 0).toBe(true); + }); + releaseSuspend(); + await suspending; + await destroyReady; + expect((await service.get(orgId, projectId, result.story.id)).workspace?.state).toBe( + "deleting", + ); + await expect( + service.queueMessage({ + orgId, + projectId, + storyId: result.story.id, + actor: input.actor, + body: "Must not run during deletion", + dedupeKey: randomUUID(), + agent: builder, + trigger: { type: "manual" }, + }), + ).rejects.toMatchObject({ code: "story_workspace_deleted" }); + } finally { + releaseSuspend(); + releaseDestroy(); + await Promise.allSettled([suspending, deleting]); + suspend.mockRestore(); + destroy.mockRestore(); + } + await expect(deleting).resolves.toMatchObject({ workspace: { state: "destroyed" } }); + }); + + it("does not suspend resources once deletion has started or completed", async () => { + const input = startInput(`delete-suspend-${randomUUID()}`); + const result = await service.start(input); + const originalDestroy = runtime.destroy.bind(runtime); + const suspend = vi.spyOn(runtime, "suspend"); + const destroy = vi.spyOn(runtime, "destroy").mockImplementationOnce(async (workspace) => { + await expect(service.suspend(orgId, projectId, result.story.id)).resolves.toMatchObject({ + workspace: { state: "deleting" }, + }); + return originalDestroy(workspace); + }); + try { + await service.deleteWorkspace({ + orgId, + projectId, + storyId: result.story.id, + actor: input.actor, + confirm: true, + }); + await expect(service.suspend(orgId, projectId, result.story.id)).resolves.toMatchObject({ + workspace: { state: "destroyed" }, + }); + expect(suspend).not.toHaveBeenCalled(); + } finally { + suspend.mockRestore(); + destroy.mockRestore(); + } + }); + it("does not wake a workspace deleted after start admission commits", async () => { const input = startInput(`start-delete-gap-${randomUUID()}`); const result = await service.start(input); From e47d9167bec74c7584e1ac5ec1640c1f9ea095cb Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Mon, 5 Oct 2026 14:03:04 +0800 Subject: [PATCH 6/7] test(api): cover issue closure racing workspace deletion --- .../test/story-workspaces.integration.test.ts | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/services/api/test/story-workspaces.integration.test.ts b/services/api/test/story-workspaces.integration.test.ts index 4a3f048a..44367d0d 100644 --- a/services/api/test/story-workspaces.integration.test.ts +++ b/services/api/test/story-workspaces.integration.test.ts @@ -1087,9 +1087,14 @@ environment: } }); - it("does not let an in-flight suspension reopen admission during deletion", async () => { + it.each([ + "manual", + "issue closure", + ] as const)("does not let an in-flight %s suspension reopen admission during deletion", async (source) => { const input = startInput(`suspend-delete-${randomUUID()}`); - const result = await service.start(input); + const issue = source === "issue closure" ? await issueFixture(7004) : undefined; + const result = issue ? issue.result : await service.start(input); + if (issue) await issue.finish(); let releaseSuspend!: () => void; let enteredSuspend!: () => void; let releaseDestroy!: () => void; @@ -1118,7 +1123,9 @@ environment: await destroyHeld; return originalDestroy(workspace); }); - const suspending = service.suspend(orgId, projectId, result.story.id); + const suspending = issue + ? issue.observe("closed", "2026-10-05T10:00:00Z") + : service.suspend(orgId, projectId, result.story.id); await suspendReady; const deleting = service.deleteWorkspace({ orgId, @@ -1164,13 +1171,25 @@ environment: await expect(deleting).resolves.toMatchObject({ workspace: { state: "destroyed" } }); }); - it("does not suspend resources once deletion has started or completed", async () => { + it.each([ + "manual", + "issue closure", + ] as const)("does not suspend resources via %s once deletion has started or completed", async (source) => { const input = startInput(`delete-suspend-${randomUUID()}`); - const result = await service.start(input); + const issue = source === "issue closure" ? await issueFixture(7005) : undefined; + const result = issue ? issue.result : await service.start(input); + if (issue) await issue.finish(); + const suspendWorkspace = async () => { + if (issue) { + await issue.observe("closed", "2026-10-05T11:00:00Z"); + return service.get(orgId, projectId, result.story.id); + } + return service.suspend(orgId, projectId, result.story.id); + }; const originalDestroy = runtime.destroy.bind(runtime); const suspend = vi.spyOn(runtime, "suspend"); const destroy = vi.spyOn(runtime, "destroy").mockImplementationOnce(async (workspace) => { - await expect(service.suspend(orgId, projectId, result.story.id)).resolves.toMatchObject({ + await expect(suspendWorkspace()).resolves.toMatchObject({ workspace: { state: "deleting" }, }); return originalDestroy(workspace); @@ -1183,7 +1202,7 @@ environment: actor: input.actor, confirm: true, }); - await expect(service.suspend(orgId, projectId, result.story.id)).resolves.toMatchObject({ + await expect(suspendWorkspace()).resolves.toMatchObject({ workspace: { state: "destroyed" }, }); expect(suspend).not.toHaveBeenCalled(); From c003d772c3255ca1337a52fb919a9f6113067113 Mon Sep 17 00:00:00 2001 From: shuntianyifang <2016860013@qq.com> Date: Fri, 16 Oct 2026 21:30:21 +0800 Subject: [PATCH 7/7] test(api): verify deletion after development branch restore --- ...pment-branch-workspace.integration.test.ts | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/services/api/test/development-branch-workspace.integration.test.ts b/services/api/test/development-branch-workspace.integration.test.ts index ee0d6002..eb77f8ed 100644 --- a/services/api/test/development-branch-workspace.integration.test.ts +++ b/services/api/test/development-branch-workspace.integration.test.ts @@ -4,8 +4,10 @@ import { join } from "node:path"; import { newId } from "@facility/core"; import { createDb, migrate, orgs, projects, stories, workspaces } from "@facility/db"; import Docker from "dockerode"; +import { eq } from "drizzle-orm"; import { expect, it } from "vitest"; import type { GithubWorkspaceCredentials } from "../src/github/workspace-credentials.js"; +import { StoryWorkspaceService } from "../src/stories/service.js"; import { DockerWorkspaceRuntime } from "../src/workspaces/docker.js"; import { FakeWorkspaceRuntime } from "../src/workspaces/fake.js"; import { @@ -54,7 +56,8 @@ it("creates work from development and retains commits and dirty files after the orgId, projectId, storyId, - provider: "fake", + provider: runtime.provider, + environment: { image: workspace.image }, externalRef: workspace.externalRef, volumeRef: workspace.volumeRef, state: "running", @@ -106,6 +109,10 @@ it("creates work from development and retains commits and dirty files after the await runtime.replaceCompute(workspace); const resumed = await runtime.wake(workspace); Object.assign(workspace, resumed); + await db + .update(workspaces) + .set({ externalRef: resumed.externalRef, volumeRef: resumed.volumeRef }) + .where(eq(workspaces.id, workspaceId)); await service.prepare(input); expect(await command("git rev-parse HEAD", "repos/acme/app")).toBe(head); expect(await command("git branch --show-current", "repos/acme/app")).toBe("feature/story"); @@ -113,6 +120,20 @@ it("creates work from development and retains commits and dirty files after the expect(await command("test ! -f advanced.txt && echo preserved", "repos/acme/app")).toBe( "preserved", ); + const lifecycle = new StoryWorkspaceService(db, runtime); + const deleted = await lifecycle.deleteWorkspace({ + orgId, + projectId, + storyId, + actor: { type: "system", id: "test" }, + confirm: true, + }); + expect(deleted.story.deletedAt).toBeInstanceOf(Date); + expect(deleted.workspace?.state).toBe("destroyed"); + expect((await runtime.inspect(workspace)).state).toBe("destroyed"); + await expect(lifecycle.restore(orgId, projectId, storyId)).rejects.toMatchObject({ + code: "story_workspace_deleted", + }); } finally { await runtime.destroy(workspace); await client.end();