Files
PaperClipAI/server/src/__tests__/execution-workspaces-routes.test.ts
T
1477d1ecea test: remove the no-op sequential describe modifier (#15286)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Paperclip runs the server and runner test suites with Vitest.
> - Vitest 5 removes the deprecated `describe.sequential` property, so
the pending Vitest 5 upgrade fails the type-check and test jobs.
> - `describe.sequential` only changes behaviour inside a
`describe.concurrent` suite, or when `sequence.concurrent` is on.
> - This repository has neither, so the modifier changed nothing at run
time.
> - The benefit is that the Vitest 5 upgrade can land, and the test
files lose a modifier that did no work.

## Linked Issues or Issue Description

Refs: #12969

## What Changed

- Replace every `describe.sequential` use with a plain `describe` call.
- Drop the `{ concurrent: false }` suite option from the two runner test
files.
- Add a comment to `server/vitest.config.ts` that records why these
suites must run one test at a time.
- Leave the package manifests and the lockfile unchanged.

## Why the modifier did nothing

The Vitest documentation states that `describe.sequential` is useful to
run tests in sequence inside a `describe.concurrent` suite, or with the
`--sequence.concurrent` option. `sequence.concurrent` defaults to
`false`.

This repository satisfies neither condition:

- No test file uses `describe.concurrent`, `it.concurrent`, or
`test.concurrent`.
- `server/vitest.config.ts` sets `sequence.concurrent: false`, with
`maxWorkers: 1`, `maxConcurrency: 1`, and `isolate: true`.
- `packages/paperclip-runner/vitest.config.ts` sets no `sequence` block,
so the `false` default applies.

`packages/db` and `cli` already run the same embedded-Postgres suites
with a plain `describe`, and those jobs are green. The server package
was the only outlier.

The modifier did carry one real piece of knowledge: these suites need
their tests to run one at a time. The new comment in
`server/vitest.config.ts` records that reason next to the setting that
enforces it.

## Verification

- `git grep` for `describe.sequential` returns nothing outside
`node_modules`.
- The author ran the changed server test files under the installed
Vitest 4, and the results match the results without this change.
- Two very large embedded-Postgres test files exceeded the author's
local memory limit, so the CI test jobs cover those two.
- The two changed runner test files have pre-existing local failures
caused by a missing Rust toolchain and a missing global `pnpm` binary.
The failures are identical with and without this change.
- The author type-checked the changed files and found no new error.
- CI must pass the typecheck, build, server test, and runner verify
jobs.

## Risks

- Low risk. Suite execution stays serial, because the Vitest config
enforces it.
- The change adds no dependency and changes no package manifest or
lockfile.
- A future change that turns `sequence.concurrent` on would break these
suites. The new config comment warns against it.

## Model Used

- Claude Sonnet 5 — code edits and local verification.
- OpenAI Codex, GPT-5 — the earlier revision of this branch.

## Test plan

- [x] Every CI check reaches a terminal green state. A pending or queued
check is not a pass.
- [x] The `Typecheck + Release Registry` job passes. This change must
not introduce a type error.
- [x] The `Build` job passes.
- [x] The server test jobs and the runner verify jobs pass.
- [x] Greptile re-reviews this commit set and posts a passing verdict.
The dependabot waiver does not apply to this pull request.
- [x] `mergeable` reads `MERGEABLE` as a terminal value.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have linked the related public issue with `Refs: #12969`
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-Authored-By: Priya Raman <priya.raman@paperclip.ing>

---------

Co-authored-by: Priya Raman <priya.raman@paperclip.ing>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: nickyleach <331803+nickyleach@users.noreply.github.com>
2026-10-06 07:19:15 -07:00

549 lines
19 KiB
TypeScript

import express from "express";
import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { errorHandler } from "../middleware/index.js";
import { executionWorkspaceRoutes } from "../routes/execution-workspaces.js";
const mockExecutionWorkspaceService = vi.hoisted(() => ({
list: vi.fn(),
listOverview: vi.fn(),
listSummaries: vi.fn(),
getById: vi.fn(),
getCloseReadiness: vi.fn(),
archiveWorkspaceUnderLifecycleLock: vi.fn(),
fenceClosedWorkspaceDestruction: vi.fn(),
reconcileExecutionWorkspaceBranch: vi.fn(),
update: vi.fn(),
}));
const mockWorkspaceOperationService = vi.hoisted(() => ({
listForExecutionWorkspace: vi.fn(),
createRecorder: vi.fn(),
assertRuntimeControlAvailable: vi.fn(async () => undefined),
}));
const mockWorkspaceRuntimeLeaseService = vi.hoisted(() => ({
claim: vi.fn(async () => ({ outcome: "created", ownerKey: "issue:issue-1", lease: null, reclaimedFrom: null })),
release: vi.fn(async () => ({ released: false, ownerKey: null })),
get: vi.fn(async () => null),
}));
const mockHeartbeatService = vi.hoisted(() => ({
wakeup: vi.fn(),
}));
const mockAccessService = vi.hoisted(() => ({
decide: vi.fn(),
}));
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
const mockEnvironmentRuntimeService = vi.hoisted(() => ({
destroyReusableSandboxLeases: vi.fn(async () => undefined),
}));
vi.mock("../services/index.js", () => ({
accessService: () => mockAccessService,
executionWorkspaceService: () => mockExecutionWorkspaceService,
heartbeatService: () => mockHeartbeatService,
logActivity: mockLogActivity,
workspaceOperationService: () => mockWorkspaceOperationService,
workspaceRuntimeLeaseService: () => mockWorkspaceRuntimeLeaseService,
LEASED_WORKSPACE_RUNTIME_ACTIONS: ["start", "stop", "restart", "repair"],
}));
vi.mock("../services/environment-runtime.js", () => ({
environmentRuntimeService: () => mockEnvironmentRuntimeService,
}));
const mockWorkspaceRuntimeTeardown = vi.hoisted(() => ({
stopRuntimeServicesForExecutionWorkspace: vi.fn(async () => undefined),
cleanupExecutionWorkspaceArtifacts: vi.fn(async () => ({ cleaned: true, warnings: [] as string[] })),
}));
vi.mock("../services/workspace-runtime.js", async (importActual) => {
const actual = await importActual<typeof import("../services/workspace-runtime.js")>();
return {
...actual,
stopRuntimeServicesForExecutionWorkspace:
mockWorkspaceRuntimeTeardown.stopRuntimeServicesForExecutionWorkspace,
cleanupExecutionWorkspaceArtifacts: mockWorkspaceRuntimeTeardown.cleanupExecutionWorkspaceArtifacts,
};
});
function createApp(actor: Record<string, unknown> = {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "session",
isInstanceAdmin: false,
}) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = actor;
next();
});
app.use("/api", executionWorkspaceRoutes({} as any));
app.use(errorHandler);
return app;
}
describe("execution workspace routes", () => {
beforeEach(() => {
vi.clearAllMocks();
mockAccessService.decide.mockResolvedValue({
allowed: true,
action: "company_scope:read",
reason: "allow_test",
explanation: "Allowed by test mock.",
});
mockExecutionWorkspaceService.list.mockResolvedValue([]);
mockExecutionWorkspaceService.listOverview.mockResolvedValue({
items: [],
total: 0,
limit: 50,
offset: 0,
hasMore: false,
nextOffset: null,
});
mockExecutionWorkspaceService.listSummaries.mockResolvedValue([
{
id: "workspace-1",
name: "Alpha",
mode: "isolated_workspace",
projectWorkspaceId: null,
},
]);
mockExecutionWorkspaceService.getById.mockResolvedValue(null);
mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch.mockResolvedValue(null);
mockHeartbeatService.wakeup.mockResolvedValue(null);
});
it("uses summary mode for lightweight workspace lookups", async () => {
const res = await request(createApp())
.get("/api/companies/company-1/execution-workspaces?summary=true&reuseEligible=true");
expect(res.status).toBe(200);
expect(res.body).toEqual([
{
id: "workspace-1",
name: "Alpha",
mode: "isolated_workspace",
projectWorkspaceId: null,
},
]);
expect(mockExecutionWorkspaceService.listSummaries).toHaveBeenCalledWith("company-1", {
projectId: undefined,
projectWorkspaceId: undefined,
issueId: undefined,
status: undefined,
reuseEligible: true,
});
expect(mockExecutionWorkspaceService.list).not.toHaveBeenCalled();
});
it("delegates bounded workspace overview queries", async () => {
const res = await request(createApp())
.get("/api/companies/company-1/workspace-overview?status=active,idle&limit=25&offset=10");
expect(res.status).toBe(200);
expect(res.body).toEqual({
items: [],
total: 0,
limit: 50,
offset: 0,
hasMore: false,
nextOffset: null,
});
expect(mockExecutionWorkspaceService.listOverview).toHaveBeenCalledWith("company-1", {
status: ["active", "idle"],
limit: 25,
offset: 10,
});
});
it("rejects invalid workspace overview pagination", async () => {
const res = await request(createApp())
.get("/api/companies/company-1/workspace-overview?limit=1000");
expect(res.status).toBe(422);
expect(mockExecutionWorkspaceService.listOverview).not.toHaveBeenCalled();
});
it.each([
["forward", { mode: "forward" }],
["override", { mode: "override", reason: "operator break-glass" }],
["quarantine_restore", { mode: "quarantine_restore", reason: "rescue dirty branch" }],
])("rejects agent actors for %s branch reconciliation", async (_mode, body) => {
mockExecutionWorkspaceService.getById.mockResolvedValue({
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
});
const res = await request(createApp({
type: "agent",
agentId: "agent-1",
companyId: "company-1",
source: "agent_jwt",
runId: "run-1",
}))
.post("/api/execution-workspaces/workspace-1/reconcile-branch")
.send(body);
expect(res.status).toBe(403);
expect(mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch).not.toHaveBeenCalled();
expect(mockLogActivity).not.toHaveBeenCalled();
});
it("logs branch reconciliation activity after the service operation succeeds", async () => {
mockExecutionWorkspaceService.getById.mockResolvedValue({
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
});
mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch.mockResolvedValue({
workspace: {
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
branchName: "feature/current",
},
inspection: {
fingerprint: "workspace_incoherence:v1:sha256:test",
worktreePath: "/tmp/worktree",
repoRoot: "/tmp/repo",
fromBranch: "feature/recorded",
toBranch: "feature/current",
fromSha: "1111111",
toSha: "2222222",
ancestryVerdict: "ancestor",
cleanliness: "clean",
statusEntryCount: 0,
plainLanguageReason: "forward",
},
recoveryAction: {
id: "recovery-1",
},
auditCommentId: "comment-1",
});
const res = await request(createApp())
.post("/api/execution-workspaces/workspace-1/reconcile-branch")
.send({ mode: "forward" });
expect(res.status).toBe(200);
expect(mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch).toHaveBeenCalledWith("workspace-1", {
mode: "forward",
reason: null,
actor: {
actorType: "user",
actorId: "local-board",
agentId: null,
runId: null,
},
});
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
action: "execution_workspace.branch_reconciled",
entityType: "execution_workspace",
entityId: "workspace-1",
details: expect.objectContaining({
mode: "forward",
fromBranch: "feature/recorded",
toBranch: "feature/current",
fromSha: "1111111",
toSha: "2222222",
ancestryVerdict: "ancestor",
fingerprint: "workspace_incoherence:v1:sha256:test",
sourceIssueId: "issue-1",
auditCommentId: "comment-1",
recoveryActionId: "recovery-1",
}),
}));
});
it("accepts quarantine_restore, logs the rescue ref, and wakes the restored source issue", async () => {
mockExecutionWorkspaceService.getById.mockResolvedValue({
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
});
mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch.mockResolvedValue({
workspace: {
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
branchName: "feature/recorded",
},
inspection: {
fingerprint: "workspace_incoherence:v1:sha256:dirty",
worktreePath: "/tmp/worktree",
repoRoot: "/tmp/repo",
fromBranch: "feature/recorded",
toBranch: "feature/live",
fromSha: "1111111",
toSha: "2222222",
ancestryVerdict: "diverged",
cleanliness: "dirty",
statusEntryCount: 2,
plainLanguageReason: "dirty live branch",
},
recoveryAction: {
id: "recovery-1",
},
auditCommentId: "comment-1",
rescueRef: {
branchName: "paperclip/rescue/PAP-123/20260709T120000Z",
commitSha: "3333333",
fileCount: 2,
sourceAuditCommentId: "comment-0",
claimantAuditCommentId: null,
},
restoredSourceIssue: {
id: "issue-1",
companyId: "company-1",
status: "todo",
assigneeAgentId: "agent-1",
},
sourceIssueStatusChanged: true,
});
const res = await request(createApp())
.post("/api/execution-workspaces/workspace-1/reconcile-branch")
.send({ mode: "quarantine_restore" });
expect(res.status).toBe(200);
expect(mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch).toHaveBeenCalledWith("workspace-1", {
mode: "quarantine_restore",
reason: null,
actor: {
actorType: "user",
actorId: "local-board",
agentId: null,
runId: null,
},
});
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
action: "execution_workspace.branch_reconciled",
entityType: "execution_workspace",
entityId: "workspace-1",
details: expect.objectContaining({
mode: "quarantine_restore",
fingerprint: "workspace_incoherence:v1:sha256:dirty",
recoveryActionId: "recovery-1",
rescueRef: expect.objectContaining({
branchName: "paperclip/rescue/PAP-123/20260709T120000Z",
commitSha: "3333333",
}),
sourceIssueStatus: "todo",
}),
}));
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith("agent-1", expect.objectContaining({
source: "automation",
reason: "issue_recovery_action_restored",
payload: expect.objectContaining({
issueId: "issue-1",
recoveryActionId: "recovery-1",
executionWorkspaceId: "workspace-1",
rescueRef: "paperclip/rescue/PAP-123/20260709T120000Z",
mutation: "execution_workspace_quarantine_restore",
}),
contextSnapshot: expect.objectContaining({
issueId: "issue-1",
taskId: "issue-1",
wakeReason: "issue_recovery_action_restored",
source: "execution_workspace.quarantine_restore",
recoveryActionId: "recovery-1",
executionWorkspaceId: "workspace-1",
rescueRef: "paperclip/rescue/PAP-123/20260709T120000Z",
}),
}));
});
it("wakes a restored in_review agent participant after quarantine_restore", async () => {
mockExecutionWorkspaceService.getById.mockResolvedValue({
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
});
mockExecutionWorkspaceService.reconcileExecutionWorkspaceBranch.mockResolvedValue({
workspace: {
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
branchName: "feature/recorded",
},
inspection: {
fingerprint: "workspace_incoherence:v1:sha256:dirty",
worktreePath: "/tmp/worktree",
repoRoot: "/tmp/repo",
fromBranch: "feature/recorded",
toBranch: "feature/live",
fromSha: "1111111",
toSha: "2222222",
ancestryVerdict: "diverged",
cleanliness: "dirty",
statusEntryCount: 2,
plainLanguageReason: "dirty live branch",
},
recoveryAction: {
id: "recovery-1",
},
auditCommentId: "comment-1",
rescueRef: null,
restoredSourceIssue: {
id: "issue-1",
companyId: "company-1",
status: "in_review",
assigneeAgentId: "reviewer-agent-1",
},
sourceIssueStatusChanged: true,
});
const res = await request(createApp())
.post("/api/execution-workspaces/workspace-1/reconcile-branch")
.send({ mode: "quarantine_restore" });
expect(res.status).toBe(200);
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
details: expect.objectContaining({
sourceIssueStatus: "in_review",
}),
}));
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith("reviewer-agent-1", expect.objectContaining({
reason: "issue_recovery_action_restored",
payload: expect.objectContaining({
issueId: "issue-1",
mutation: "execution_workspace_quarantine_restore",
}),
contextSnapshot: expect.objectContaining({
issueId: "issue-1",
wakeReason: "issue_recovery_action_restored",
source: "execution_workspace.quarantine_restore",
}),
}));
});
it("returns 409 and skips destructive cleanup when the archive hits a reopen-pending workspace", async () => {
// A reopen published the workspace active while its source issue is still
// terminal. The archive control must return 409 before any lease teardown,
// runtime-service stop, or artifact cleanup, so it never removes the rebuilt
// worktree.
mockExecutionWorkspaceService.getById.mockResolvedValue({
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
status: "active",
mode: "isolated_workspace",
});
mockExecutionWorkspaceService.getCloseReadiness.mockResolvedValue({
state: "ready",
blockingReasons: [],
});
mockExecutionWorkspaceService.archiveWorkspaceUnderLifecycleLock.mockResolvedValue({
outcome: "reopen_pending",
});
const res = await request(createApp())
.patch("/api/execution-workspaces/workspace-1")
.send({ status: "archived" });
expect(res.status).toBe(409);
expect(mockExecutionWorkspaceService.archiveWorkspaceUnderLifecycleLock).toHaveBeenCalledTimes(1);
// The destruction fence never runs, so no worktree is removed.
expect(mockExecutionWorkspaceService.fenceClosedWorkspaceDestruction).not.toHaveBeenCalled();
});
it("destroys the reusable sandbox leases inside the destruction fence when the archive wins", async () => {
// The archive wins the lifecycle race. The fence runs the destroy callback,
// so the reusable sandbox lease teardown runs with the worktree teardown.
const archivedWorkspace = {
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
status: "archived",
mode: "isolated_workspace",
projectWorkspaceId: null,
projectId: null,
cwd: "/tmp/worktree",
};
mockExecutionWorkspaceService.getById.mockResolvedValue({
...archivedWorkspace,
status: "active",
});
mockExecutionWorkspaceService.getCloseReadiness.mockResolvedValue({
state: "ready",
blockingReasons: [],
});
mockExecutionWorkspaceService.archiveWorkspaceUnderLifecycleLock.mockResolvedValue({
outcome: "archived",
workspace: archivedWorkspace,
capturedGeneration: 3,
});
mockExecutionWorkspaceService.fenceClosedWorkspaceDestruction.mockImplementation(
async ({ destroy }: { destroy: () => Promise<unknown> }) => ({
skippedReopened: false,
result: await destroy(),
}),
);
const res = await request(createApp())
.patch("/api/execution-workspaces/workspace-1")
.send({ status: "archived" });
expect(res.status).toBe(200);
// The lease teardown runs inside the fence, so it uses the closed-workspace
// failure reason and targets the archived row.
expect(mockEnvironmentRuntimeService.destroyReusableSandboxLeases).toHaveBeenCalledWith(
expect.objectContaining({
companyId: "company-1",
executionWorkspaceId: "workspace-1",
failureReason: "execution_workspace_closed",
}),
);
});
it("keeps the reusable sandbox leases when a reopen makes the fence skip the archive teardown", async () => {
// A reopen raised the lifecycle generation after the archive captured its
// own generation. The fence skips the destroy callback and keeps the
// reopened row, so the lease teardown must not run. Before the fix the lease
// teardown ran before the fence, so an overlapping reopen lost its leases.
const archivedWorkspace = {
id: "workspace-1",
companyId: "company-1",
sourceIssueId: "issue-1",
status: "archived",
mode: "isolated_workspace",
projectWorkspaceId: null,
projectId: null,
cwd: "/tmp/worktree",
};
mockExecutionWorkspaceService.getById.mockResolvedValue({
...archivedWorkspace,
status: "active",
});
mockExecutionWorkspaceService.getCloseReadiness.mockResolvedValue({
state: "ready",
blockingReasons: [],
});
mockExecutionWorkspaceService.archiveWorkspaceUnderLifecycleLock.mockResolvedValue({
outcome: "archived",
workspace: archivedWorkspace,
capturedGeneration: 3,
});
// The fence detects the reopen and never runs the destroy callback.
mockExecutionWorkspaceService.fenceClosedWorkspaceDestruction.mockResolvedValue({
skippedReopened: true,
});
const res = await request(createApp())
.patch("/api/execution-workspaces/workspace-1")
.send({ status: "archived" });
expect(res.status).toBe(200);
expect(mockExecutionWorkspaceService.fenceClosedWorkspaceDestruction).toHaveBeenCalledTimes(1);
// The reopen keeps its reusable leases because the fence skipped the destroy.
expect(mockEnvironmentRuntimeService.destroyReusableSandboxLeases).not.toHaveBeenCalled();
});
});