Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 87 additions & 7 deletions src/node/services/workflows/workspaceHostActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
Expand Down Expand Up @@ -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({
Expand All @@ -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", () => {
Expand Down Expand Up @@ -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/);
});
});
115 changes: 96 additions & 19 deletions src/node/services/workflows/workspaceHostActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<HistoryService, "getHistoryFromLatestBoundary">;
config: Pick<Config, "loadConfigOrDefault" | "findWorkspace" | "getAllWorkspaceMetadata">;
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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<boolean> {
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
Expand Down Expand Up @@ -318,10 +346,32 @@ const WORKSPACE_HOST_ACTION_DEFINITIONS: Record<string, WorkspaceHostActionDefin
new Set([requestedProjectPath, owningProjectPath])
);
if (existing) {
const archived = isWorkspaceArchived(existing.archivedAt, existing.unarchivedAt);
if (!archived) {
return {
action: "reused",
created: false,
workspaceId: existing.id,
archived: false,
unarchived: false,
};
}

// `ensure` means downstream workflow steps can use the workspace. An
// archived match satisfies identity, but not that active-state postcondition.
throwIfAborted(ctx, "workspace.ensure");
const unarchiveResult = await services.workspaceService.unarchive(existing.id);
if (!unarchiveResult.success) {
throw new Error(
`workspace.ensure failed to unarchive workspace: ${unarchiveResult.error}`
);
}
return {
action: "unarchived",
created: false,
workspaceId: existing.id,
archived: isWorkspaceArchived(existing.archivedAt, existing.unarchivedAt),
archived: false,
unarchived: true,
};
}

Expand All @@ -346,9 +396,11 @@ const WORKSPACE_HOST_ACTION_DEFINITIONS: Record<string, WorkspaceHostActionDefin
throw new Error(`workspace.ensure failed to create workspace: ${createResult.error}`);
}
return {
action: "created",
created: true,
workspaceId: createResult.data.metadata.id,
archived: false,
unarchived: false,
};
});
};
Expand Down Expand Up @@ -530,20 +582,45 @@ const WORKSPACE_HOST_ACTION_DEFINITIONS: Record<string, WorkspaceHostActionDefin
hasReconcile: true,
createExecute: (services) => 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 };
},
},
};
Expand Down
Loading