diff --git a/src/node/services/workflows/workspaceHostActions.test.ts b/src/node/services/workflows/workspaceHostActions.test.ts index 7a23f38804..97a5084b4b 100644 --- a/src/node/services/workflows/workspaceHostActions.test.ts +++ b/src/node/services/workflows/workspaceHostActions.test.ts @@ -78,6 +78,13 @@ function fakeServices(options: FakeServiceOptions = {}) { ), sendMessage: mock(() => Promise.resolve(Ok(undefined))), archive: mock(() => Promise.resolve(Ok({ archived: true }))), + unarchive: mock((workspaceId: string) => { + const metadata = knownWorkspaces.find((workspace) => workspace.id === workspaceId); + if (metadata) { + metadata.unarchivedAt = new Date().toISOString(); + } + return Promise.resolve(Ok(undefined)); + }), }; const services: WorkspaceHostActionServices = { workspaceService: { @@ -87,6 +94,8 @@ function fakeServices(options: FakeServiceOptions = {}) { calls.sendMessage as unknown as WorkspaceHostActionServices["workspaceService"]["sendMessage"], archive: calls.archive as unknown as WorkspaceHostActionServices["workspaceService"]["archive"], + unarchive: + calls.unarchive as unknown as WorkspaceHostActionServices["workspaceService"]["unarchive"], getGoalContinuationRuntimeState: () => ({ isInitializing: false, isRuntimeCompatible: true, @@ -371,15 +380,45 @@ describe("workspace.ensure", () => { const output = (await ensure.execute( { projectPath: "/proj", key: "issue-1-investigate", trunkBranch: "main" }, ctx - )) as { created: boolean; workspaceId: string }; - expect(output.created).toBe(true); - expect(output.workspaceId).toBe("created-ws"); + )) as { action: string; created: boolean; workspaceId: string; archived: boolean }; + expect(output).toMatchObject({ + action: "created", + created: true, + workspaceId: "created-ws", + archived: false, + unarchived: false, + }); expect(calls.create).toHaveBeenCalledTimes(1); const tags = calls.create.mock.calls[0][7]; expect(tags).toEqual({ [WORK_ITEM_TAG_KEY]: "issue-1-investigate" }); }); - test("is idempotent: an existing tagged workspace (even archived) blocks creation", async () => { + test("reuses an active tagged workspace without creating or unarchiving", async () => { + const { services, calls } = fakeServices({ + workspaces: [ + workspaceMeta({ + id: "existing", + tags: { [WORK_ITEM_TAG_KEY]: "issue-1-investigate" }, + }), + ], + }); + const ensure = getAction(createWorkspaceHostActions(services), "workspace.ensure"); + const output = await ensure.execute( + { projectPath: "/proj", key: "issue-1-investigate", trunkBranch: "main" }, + ctx + ); + expect(output).toEqual({ + action: "reused", + created: false, + workspaceId: "existing", + archived: false, + unarchived: false, + }); + expect(calls.create).not.toHaveBeenCalled(); + expect(calls.unarchive).not.toHaveBeenCalled(); + }); + + test("unarchives a tagged workspace instead of returning it archived", async () => { const { services, calls } = fakeServices({ workspaces: [ workspaceMeta({ @@ -390,12 +429,30 @@ describe("workspace.ensure", () => { ], }); const ensure = getAction(createWorkspaceHostActions(services), "workspace.ensure"); - const output = (await ensure.execute( + const output = await ensure.execute( { projectPath: "/proj", key: "issue-1-investigate", trunkBranch: "main" }, ctx - )) as { created: boolean; workspaceId: string; archived: boolean }; - expect(output).toEqual({ created: false, workspaceId: "existing", archived: true }); + ); + expect(output).toEqual({ + action: "unarchived", + created: false, + workspaceId: "existing", + archived: false, + unarchived: true, + }); + const second = await ensure.execute( + { projectPath: "/proj", key: "issue-1-investigate", trunkBranch: "main" }, + ctx + ); + expect(second).toEqual({ + action: "reused", + created: false, + workspaceId: "existing", + archived: false, + unarchived: false, + }); expect(calls.create).not.toHaveBeenCalled(); + expect(calls.unarchive).toHaveBeenCalledTimes(1); }); test("reconcile re-runs the idempotent ensure", () => { @@ -711,3 +768,26 @@ describe("workspace.archive", () => { await expectRejects(archive.execute({ workspaceId: "missing" }, ctx), /not found/); }); }); + +describe("workspace.unarchive", () => { + test("short-circuits when the workspace is already unarchived", async () => { + const { services, calls } = fakeServices({ workspaces: [workspaceMeta({ id: "ws-1" })] }); + const unarchive = getAction(createWorkspaceHostActions(services), "workspace.unarchive"); + const output = await unarchive.execute({ workspaceId: "ws-1" }, ctx); + expect(output).toEqual({ unarchived: true, alreadyUnarchived: true }); + expect(calls.unarchive).not.toHaveBeenCalled(); + }); + + test("unarchives archived workspaces and errors on unknown ids", async () => { + const { services, calls } = fakeServices({ + workspaces: [workspaceMeta({ id: "ws-1", archivedAt: new Date().toISOString() })], + }); + const unarchive = getAction(createWorkspaceHostActions(services), "workspace.unarchive"); + expect(await unarchive.execute({ workspaceId: "ws-1" }, ctx)).toEqual({ + unarchived: true, + alreadyUnarchived: false, + }); + expect(calls.unarchive).toHaveBeenCalledTimes(1); + await expectRejects(unarchive.execute({ workspaceId: "missing" }, ctx), /not found/); + }); +}); diff --git a/src/node/services/workflows/workspaceHostActions.ts b/src/node/services/workflows/workspaceHostActions.ts index 374faf81f1..917dde224f 100644 --- a/src/node/services/workflows/workspaceHostActions.ts +++ b/src/node/services/workflows/workspaceHostActions.ts @@ -20,11 +20,11 @@ * * Design notes (from the reconcile-loop dispatcher design): * - `ensure` is idempotent by work-item key (workspace tag `workItemKey`), so - * it is replay-safe and exports `reconcile = execute`. + * it is replay-safe, unarchives archived matches, and exports `reconcile = execute`. * - `sendMessage` deliberately has NO reconcile: re-sending a chat message is * not idempotent. A crashed workflow must restart the loop (which re-derives * the plan from observed state) rather than replay a half-finished send. - * - `archive` is a reconciliation outcome (source says done), idempotent. + * - `archive`/`unarchive` are reconciliation outcomes (source says done/active), idempotent. */ import { createHash } from "crypto"; @@ -56,7 +56,7 @@ export const WORK_ITEM_TAG_KEY = "workItemKey"; export interface WorkspaceHostActionServices { workspaceService: Pick< WorkspaceService, - "list" | "create" | "sendMessage" | "archive" | "getGoalContinuationRuntimeState" + "list" | "create" | "sendMessage" | "archive" | "unarchive" | "getGoalContinuationRuntimeState" >; historyService: Pick; config: Pick; @@ -164,7 +164,7 @@ async function findWorkspaceByWorkItemKey( /** * Look up a workspace by id from the live WorkspaceService.list(). Shared by the - * id-targeted host actions (sendMessage/archive), which receive an explicit + * id-targeted host actions (sendMessage/archive/unarchive), which receive an explicit * workspaceId. Distinct from findWorkspaceByWorkItemKey, which reads config * directly to avoid list()'s hidden-workspace filtering and error swallowing. */ @@ -173,6 +173,34 @@ async function findWorkspaceById(services: WorkspaceHostActionServices, workspac return all.find((metadata) => metadata.id === workspaceId); } +type WorkspaceArchivedStateTransition = ( + workspaceId: string +) => Promise<{ success: true } | { success: false; error: string }>; + +async function transitionWorkspaceArchivedState( + services: WorkspaceHostActionServices, + ctx: HostWorkflowActionContext, + actionName: "workspace.archive" | "workspace.unarchive", + workspaceId: string, + targetArchived: boolean, + transition: WorkspaceArchivedStateTransition +): Promise { + throwIfAborted(ctx, actionName); + const existing = await findWorkspaceById(services, workspaceId); + if (!existing) { + throw new Error(`${actionName}: workspace not found: ${workspaceId}`); + } + if (isWorkspaceArchived(existing.archivedAt, existing.unarchivedAt) === targetArchived) { + return true; + } + throwIfAborted(ctx, actionName); + const transitionResult = await transition(workspaceId); + if (!transitionResult.success) { + throw new Error(`${actionName} failed: ${transitionResult.error}`); + } + return false; +} + /** * Work-item keys (e.g. "PROJ-123", "release/v1.2") rarely satisfy * validateWorkspaceName ([a-z0-9_-], max 64 chars), which would make @@ -318,10 +346,32 @@ const WORKSPACE_HOST_ACTION_DEFINITIONS: Record async (rawInput, ctx) => { const input = WorkspaceIdInputSchema.parse(rawInput); - throwIfAborted(ctx, "workspace.archive"); - const existing = await findWorkspaceById(services, input.workspaceId); - if (!existing) { - throw new Error(`workspace.archive: workspace not found: ${input.workspaceId}`); - } - if (isWorkspaceArchived(existing.archivedAt, existing.unarchivedAt)) { - return { archived: true, alreadyArchived: true }; - } - throwIfAborted(ctx, "workspace.archive"); - const archiveResult = await services.workspaceService.archive(input.workspaceId); - if (!archiveResult.success) { - throw new Error(`workspace.archive failed: ${archiveResult.error}`); - } - return { archived: true, alreadyArchived: false }; + const alreadyArchived = await transitionWorkspaceArchivedState( + services, + ctx, + "workspace.archive", + input.workspaceId, + true, + (workspaceId) => services.workspaceService.archive(workspaceId) + ); + return { archived: true, alreadyArchived }; + }, + }, + + "workspace.unarchive": { + metadata: { + version: 1, + description: "Unarchive a workspace (idempotent; succeeds when already unarchived)", + effect: "external", + inputSchema: { + type: "object", + properties: { + workspaceId: { type: "string" }, + }, + required: ["workspaceId"], + }, + outputSchema: { type: "object" }, + timeoutMs: 120_000, + }, + hasReconcile: true, + createExecute: (services) => async (rawInput, ctx) => { + const input = WorkspaceIdInputSchema.parse(rawInput); + const alreadyUnarchived = await transitionWorkspaceArchivedState( + services, + ctx, + "workspace.unarchive", + input.workspaceId, + false, + (workspaceId) => services.workspaceService.unarchive(workspaceId) + ); + return { unarchived: true, alreadyUnarchived }; }, }, };