forked from farhoodlabs/paperclip
Guard closed isolated workspaces on issues
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -0,0 +1,178 @@
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
|
||||
const issueId = "11111111-1111-4111-8111-111111111111";
|
||||
const closedWorkspaceId = "33333333-3333-4333-8333-333333333333";
|
||||
const nextWorkspaceId = "44444444-4444-4444-8444-444444444444";
|
||||
const agentId = "22222222-2222-4222-8222-222222222222";
|
||||
|
||||
const mockIssueService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
update: vi.fn(),
|
||||
checkout: vi.fn(),
|
||||
addComment: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockExecutionWorkspaceService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockAccessService = vi.hoisted(() => ({
|
||||
canUser: vi.fn(),
|
||||
hasPermission: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockHeartbeatService = vi.hoisted(() => ({
|
||||
wakeup: vi.fn(async () => undefined),
|
||||
reportRunActivity: vi.fn(async () => undefined),
|
||||
getRun: vi.fn(async () => null),
|
||||
getActiveRunForAgent: vi.fn(async () => null),
|
||||
cancelRun: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
const mockProjectService = vi.hoisted(() => ({
|
||||
getById: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
|
||||
vi.mock("../services/index.js", () => ({
|
||||
accessService: () => mockAccessService,
|
||||
agentService: () => ({
|
||||
getById: vi.fn(async () => null),
|
||||
}),
|
||||
documentService: () => ({}),
|
||||
executionWorkspaceService: () => mockExecutionWorkspaceService,
|
||||
feedbackService: () => ({
|
||||
listIssueVotesForUser: vi.fn(async () => []),
|
||||
saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })),
|
||||
}),
|
||||
goalService: () => ({
|
||||
getDefaultCompanyGoal: vi.fn(async () => null),
|
||||
getById: vi.fn(async () => null),
|
||||
}),
|
||||
heartbeatService: () => mockHeartbeatService,
|
||||
instanceSettingsService: () => ({
|
||||
get: vi.fn(async () => ({
|
||||
id: "instance-settings-1",
|
||||
general: {
|
||||
censorUsernameInLogs: false,
|
||||
feedbackDataSharingPreference: "prompt",
|
||||
},
|
||||
})),
|
||||
listCompanyIds: vi.fn(async () => ["company-1"]),
|
||||
}),
|
||||
issueApprovalService: () => ({}),
|
||||
issueService: () => mockIssueService,
|
||||
logActivity: mockLogActivity,
|
||||
projectService: () => mockProjectService,
|
||||
routineService: () => ({
|
||||
syncRunStatusForIssue: vi.fn(async () => undefined),
|
||||
}),
|
||||
workProductService: () => ({}),
|
||||
}));
|
||||
|
||||
function createApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
(req as any).actor = {
|
||||
type: "board",
|
||||
userId: "local-board",
|
||||
companyIds: ["company-1"],
|
||||
source: "local_implicit",
|
||||
isInstanceAdmin: false,
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes({} as any, {} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
function makeIssue() {
|
||||
return {
|
||||
id: issueId,
|
||||
companyId: "company-1",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
assigneeAgentId: agentId,
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-1085",
|
||||
title: "Closed worktree issue",
|
||||
projectId: null,
|
||||
executionRunId: null,
|
||||
checkoutRunId: null,
|
||||
executionWorkspaceId: closedWorkspaceId,
|
||||
};
|
||||
}
|
||||
|
||||
function makeClosedWorkspace() {
|
||||
return {
|
||||
id: closedWorkspaceId,
|
||||
name: "PAP-1085-fix-worktree-guard",
|
||||
mode: "isolated_workspace",
|
||||
status: "archived",
|
||||
closedAt: new Date("2026-04-04T17:00:00.000Z"),
|
||||
};
|
||||
}
|
||||
|
||||
describe("closed isolated workspace issue routes", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue());
|
||||
mockExecutionWorkspaceService.getById.mockResolvedValue(makeClosedWorkspace());
|
||||
});
|
||||
|
||||
it("rejects new issue comments when the linked isolated workspace is closed", async () => {
|
||||
const res = await request(createApp())
|
||||
.post(`/api/issues/${issueId}/comments`)
|
||||
.send({ body: "hello" });
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.error).toContain("closed workspace");
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects comment updates when the linked isolated workspace is closed", async () => {
|
||||
const res = await request(createApp())
|
||||
.patch(`/api/issues/${issueId}`)
|
||||
.send({ comment: "hello" });
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.error).toContain("closed workspace");
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects checkout when the linked isolated workspace is closed", async () => {
|
||||
const res = await request(createApp())
|
||||
.post(`/api/issues/${issueId}/checkout`)
|
||||
.send({
|
||||
agentId,
|
||||
expectedStatuses: ["todo", "backlog", "blocked"],
|
||||
});
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.error).toContain("closed workspace");
|
||||
expect(mockIssueService.checkout).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still allows non-comment board updates so the issue can be moved to a new workspace", async () => {
|
||||
mockIssueService.update.mockResolvedValue({
|
||||
...makeIssue(),
|
||||
executionWorkspaceId: nextWorkspaceId,
|
||||
});
|
||||
|
||||
const res = await request(createApp())
|
||||
.patch(`/api/issues/${issueId}`)
|
||||
.send({ executionWorkspaceId: nextWorkspaceId });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockIssueService.update).toHaveBeenCalledWith(issueId, { executionWorkspaceId: nextWorkspaceId });
|
||||
});
|
||||
});
|
||||
@@ -19,6 +19,9 @@ import {
|
||||
updateIssueWorkProductSchema,
|
||||
upsertIssueDocumentSchema,
|
||||
updateIssueSchema,
|
||||
getClosedIsolatedExecutionWorkspaceMessage,
|
||||
isClosedIsolatedExecutionWorkspace,
|
||||
type ExecutionWorkspace,
|
||||
} from "@paperclipai/shared";
|
||||
import { trackAgentTaskCompleted } from "@paperclipai/shared/telemetry";
|
||||
import { getTelemetryClient } from "../telemetry.js";
|
||||
@@ -234,6 +237,23 @@ export function issueRoutes(
|
||||
return runToInterrupt?.status === "running" ? runToInterrupt : null;
|
||||
}
|
||||
|
||||
async function getClosedIssueExecutionWorkspace(issue: { executionWorkspaceId?: string | null }) {
|
||||
if (!issue.executionWorkspaceId) return null;
|
||||
const workspace = await executionWorkspacesSvc.getById(issue.executionWorkspaceId);
|
||||
if (!workspace || !isClosedIsolatedExecutionWorkspace(workspace)) return null;
|
||||
return workspace;
|
||||
}
|
||||
|
||||
function respondClosedIssueExecutionWorkspace(
|
||||
res: Response,
|
||||
workspace: Pick<ExecutionWorkspace, "closedAt" | "id" | "mode" | "name" | "status">,
|
||||
) {
|
||||
res.status(409).json({
|
||||
error: getClosedIsolatedExecutionWorkspaceMessage(workspace),
|
||||
executionWorkspace: workspace,
|
||||
});
|
||||
}
|
||||
|
||||
async function normalizeIssueIdentifier(rawId: string): Promise<string> {
|
||||
if (/^[A-Z]+-\d+$/i.test(rawId)) {
|
||||
const issue = await svc.getByIdentifier(rawId);
|
||||
@@ -1083,6 +1103,13 @@ export function issueRoutes(
|
||||
...updateFields
|
||||
} = req.body;
|
||||
let interruptedRunId: string | null = null;
|
||||
const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(existing);
|
||||
const isAgentWorkUpdate = req.actor.type === "agent" && Object.keys(updateFields).length > 0;
|
||||
|
||||
if (closedExecutionWorkspace && (commentBody || isAgentWorkUpdate)) {
|
||||
respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace);
|
||||
return;
|
||||
}
|
||||
|
||||
if (interruptRequested) {
|
||||
if (!commentBody) {
|
||||
@@ -1389,6 +1416,12 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
|
||||
const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue);
|
||||
if (closedExecutionWorkspace) {
|
||||
respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace);
|
||||
return;
|
||||
}
|
||||
|
||||
const checkoutRunId = requireAgentRunId(req, res);
|
||||
if (req.actor.type === "agent" && !checkoutRunId) return;
|
||||
const updated = await svc.checkout(id, req.body.agentId, req.body.expectedStatuses, checkoutRunId);
|
||||
@@ -1607,6 +1640,11 @@ export function issueRoutes(
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
if (!(await assertAgentRunCheckoutOwnership(req, res, issue))) return;
|
||||
const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue);
|
||||
if (closedExecutionWorkspace) {
|
||||
respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace);
|
||||
return;
|
||||
}
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
const reopenRequested = req.body.reopen === true;
|
||||
|
||||
Reference in New Issue
Block a user