From b4dc67deada63c34646f09f0f74bc84072328a3d Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 11 Sep 2026 16:05:57 -0400 Subject: [PATCH] fix(issues): allow blocked ticket cancellation Co-Authored-By: Paperclip --- ...ue-agent-mutation-ownership-routes.test.ts | 191 ++++++++++++++++++ server/src/routes/issues.ts | 9 +- 2 files changed, 199 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 6dc3a4d461..4a1d6debab 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -1681,6 +1681,197 @@ describe("agent issue mutation checkout ownership", () => { }, ); + it("allows an authorized agent to cancel an unassigned blocked issue without changing ownership or dependencies", async () => { + const blockedBy = [ + { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + identifier: "PAP-1650", + title: "Upstream blocker", + status: "todo", + priority: "high", + assigneeAgentId: ownerAgentId, + assigneeUserId: null, + }, + ]; + const blockedIssue = makeIssue({ + status: "blocked", + assigneeAgentId: null, + assigneeUserId: null, + blockedBy, + }); + mockIssueService.getById.mockResolvedValue(blockedIssue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...blockedIssue, + ...patch, + })); + mockIssueService.getDependencyReadiness.mockResolvedValue({ + blockerIssueIds: blockedBy.map((blocker) => blocker.id), + isDependencyReady: false, + unresolvedBlockerCount: 1, + unresolvedBlockerIssueIds: blockedBy.map((blocker) => blocker.id), + pendingFinalizeBlockerIssueIds: [], + }); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ + status: "cancelled", + comment: "Cancel obsolete work without changing its routing metadata.", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body).toMatchObject({ + id: issueId, + status: "cancelled", + assigneeAgentId: null, + assigneeUserId: null, + blockedBy, + }); + expect(mockIssueService.update).toHaveBeenCalledWith( + issueId, + expect.objectContaining({ status: "cancelled" }), + expect.anything(), + undefined, + expect.any(Array), + ); + const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record; + expect(updatePatch).not.toHaveProperty("assigneeAgentId"); + expect(updatePatch).not.toHaveProperty("assigneeUserId"); + expect(updatePatch).not.toHaveProperty("blockedByIssueIds"); + expect(mockIssueService.getDependencyReadiness).not.toHaveBeenCalled(); + }); + + it("allows an authorized peer agent to cancel an assigned blocked issue without taking ownership", async () => { + const blockedIssue = makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }); + mockIssueService.getById.mockResolvedValue(blockedIssue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...blockedIssue, + ...patch, + })); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "cancelled" }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body).toMatchObject({ + id: issueId, + status: "cancelled", + assigneeAgentId: ownerAgentId, + }); + const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record; + expect(updatePatch).not.toHaveProperty("assigneeAgentId"); + expect(updatePatch).not.toHaveProperty("assigneeUserId"); + }); + + it.each(["todo", "in_progress"] as const)( + "keeps a blocked-to-%s transition behind unresolved dependency checks", + async (status) => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }), + ); + mockIssueService.getDependencyReadiness.mockResolvedValue({ + blockerIssueIds: ["aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"], + isDependencyReady: false, + unresolvedBlockerCount: 1, + unresolvedBlockerIssueIds: ["aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"], + pendingFinalizeBlockerIssueIds: [], + }); + + const res = await request(await createApp(ownerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.error).toBe("Issue follow-up blocked by unresolved blockers"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }, + ); + + it("keeps company boundaries enforced for blocked issue cancellation", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: null }), + ); + + const res = await request( + await createApp( + peerActor({ companyId: "99999999-9999-4999-8999-999999999999" }), + ), + ) + .patch(`/api/issues/${issueId}`) + .send({ status: "cancelled" }); + + expect(res.status, JSON.stringify(res.body)).toBe(404); + expect(res.body.error).toBe("Issue not found"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("keeps active checkout ownership enforced for issue cancellation", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "in_progress", assigneeAgentId: ownerAgentId }), + ); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "cancelled" }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.details.code).toBe("issue_write_assignee_run_lock"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("keeps human-only review authorization enforced for issue cancellation", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ + status: "in_review", + assigneeAgentId: ownerAgentId, + reviewPolicy: "human_only", + }), + ); + + const res = await request(await createApp(ownerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "cancelled" }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details.code).toBe("review_policy_denied"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("keeps low-trust agents from cancelling blocked issues", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: null }), + ); + mockAgentService.getById.mockImplementation(async (id: string) => { + if (id === peerAgentId) { + return makeAgent(peerAgentId, { + permissions: { + trustPreset: "low_trust_review", + authorizationPolicy: { + managedBy: "core-trust-preset", + trustBoundary: { + mode: "low_trust_review", + companyId, + issueIds: [issueId], + }, + }, + }, + }); + } + return id === ownerAgentId ? makeAgent(ownerAgentId) : null; + }); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "cancelled" }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe( + "Low-trust actors cannot use this control-plane surface", + ); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + it("allows same-company agent mutations on unassigned in-progress issues", async () => { mockIssueService.getById.mockResolvedValue(makeIssue({ assigneeAgentId: null })); mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index e587b51a2e..a20d864c69 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -12699,6 +12699,8 @@ export function issueRoutes( } const shouldCancelActiveRunForCancelledStatus = existing.status !== "cancelled" && updateFields.status === "cancelled"; + const agentCancellationRequested = + req.actor.type === "agent" && shouldCancelActiveRunForCancelledStatus; if (resumeRequested === true && !commentBody) { res.status(400).json({ error: "Follow-up intent requires a comment" }); return; @@ -12706,7 +12708,8 @@ export function issueRoutes( if ( (reopenRequested === true || resumeRequested === true || - Array.isArray(req.body.blockedByIssueIds)) && + Array.isArray(req.body.blockedByIssueIds) || + agentCancellationRequested) && (await assertLowTrustControlPlaneDenied( req, res, @@ -12727,6 +12730,10 @@ export function issueRoutes( req.actor.type === "agent" && typeof updateFields.status === "string" && updateFields.status !== existing.status && + // Cancellation stops work; it does not resume it. The ordinary + // mutation, checkout, recovery, review, and low-trust gates still + // authorize this terminal disposition. + updateFields.status !== "cancelled" && (isBlocked || (isClosed && !isClosedIssueStatus(updateFields.status))); if ( resumeRequested !== true &&