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 4e360db3ce7..b6244ae1d0d 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -2878,6 +2878,76 @@ describe("agent issue mutation checkout ownership", () => { expect(mockIssueService.update).not.toHaveBeenCalled(); }); + // BLO-20385: the patch shape mandates `blockedByIssueIds: []` and that empty + // array is applied, so admitting it on an issue with live blockers deletes + // dependency edges the actor could not otherwise remove. Probed in production + // on BLO-18946: a 200 silently dropped a live edge to a still-blocked issue. + it.each(commentGrantMutationDenialCases)( + "refuses the delegate recovery patch when blockers are unresolved, for a %s comment grant holder", + async (_kind, agentRows, issueOverrides) => { + useProductionIssueAuthorization(agentRows); + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId, ...issueOverrides }), + ); + const liveBlockerId = "11111111-1111-4111-8111-111111111111"; + mockIssueService.getDependencyReadiness.mockResolvedValue({ + issueId, + blockerIssueIds: [liveBlockerId], + unresolvedBlockerCount: 1, + unresolvedBlockerIssueIds: [liveBlockerId], + pendingFinalizeBlockerIssueIds: [], + allBlockersDone: false, + isDependencyReady: false, + }); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "todo", blockedByIssueIds: [] }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.details).toMatchObject({ + reason: "delegate_recovery_unresolved_blockers", + unresolvedBlockerCount: 1, + unresolvedBlockerIssueIds: [liveBlockerId], + }); + // The edge must survive: no write may reach the service at all. + expect(mockIssueService.update).not.toHaveBeenCalled(); + }, + ); + + it.each(commentGrantMutationDenialCases)( + "still unparks past stale terminal blocker edges for a %s comment grant holder", + async (_kind, agentRows, issueOverrides) => { + useProductionIssueAuthorization(agentRows); + const stored = makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId, ...issueOverrides }); + mockIssueService.getById.mockResolvedValue(stored); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...stored, + ...patch, + })); + // Edges exist but every one is terminal: clearing these is the whole point + // of the recovery patch and must keep working. + mockIssueService.getDependencyReadiness.mockResolvedValue({ + issueId, + blockerIssueIds: ["22222222-2222-4222-8222-222222222222"], + unresolvedBlockerCount: 0, + unresolvedBlockerIssueIds: [], + pendingFinalizeBlockerIssueIds: [], + allBlockersDone: true, + isDependencyReady: true, + }); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status: "todo", blockedByIssueIds: [] }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + const [, patch] = mockIssueService.update.mock.calls.at(-1) as [string, Record]; + expect(patch).toMatchObject({ status: "todo", expectedCurrentStatus: "blocked" }); + expect(patch.blockedByIssueIds).toEqual([]); + }, + ); + it("surfaces 409 when the issue stops being blocked before the delegate recovery write lands", async () => { useProductionIssueAuthorization([ makeAgent(peerAgentId), diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index b5e144e45dd..c20c97eb289 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -4506,6 +4506,30 @@ export function issueRoutes( creatorOrManagerChainDecision && isCreatorOrManagerChainRecoveryPatch(issue, req.body as Record) ) { + // BLO-20385: the patch shape *mandates* `blockedByIssueIds: []`, and that + // empty array is then applied — so admitting the bypass on an issue that + // still has live blockers does not merely unpark it, it silently deletes + // dependency edges the actor could not otherwise remove. The shape check + // above only inspects the request body; it never looked at the issue's + // actual blockers. Unparking a row whose blockers are all terminal is the + // intended use and stays allowed (clearing those stale edges is the + // point). Refuse when any blocker is unresolved, and say so explicitly + // rather than reusing the opaque boundary 403 — an unexplained deny on + // this path already cost a full diagnostic cycle once. + const readiness = await svc.getDependencyReadiness(issue.id); + if (readiness.unresolvedBlockerCount > 0) { + res.status(409).json({ + error: + "Cannot unpark an issue that still has unresolved blockers: this patch shape clears blockedByIssueIds and would delete live dependency edges", + details: { + issueId: issue.id, + reason: "delegate_recovery_unresolved_blockers", + unresolvedBlockerCount: readiness.unresolvedBlockerCount, + unresolvedBlockerIssueIds: readiness.unresolvedBlockerIssueIds, + }, + }); + return false; + } return true; } if (creatorOrManagerChainDecision && !options.allowCreatorOrManagerChainOwnership) {