fix(authz): refuse delegate-recovery unpark when blockers are unresolved (BLO-20385)

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<string, unknown>) => ({
+        ...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<string, unknown>];
+      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),

server/src/routes/issues.ts

@@ -4506,6 +4506,30 @@ export function issueRoutes(
       creatorOrManagerChainDecision &&
       isCreatorOrManagerChainRecoveryPatch(issue, req.body as Record<string, unknown>)
     ) {
+      // 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) {