From c2e6b1152e8cedbabdc26ededa8adc882ebe2e8f Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 12 Jul 2026 16:08:02 +0000 Subject: [PATCH] fix: enforce blocker-edge hygiene Co-Authored-By: Paperclip --- packages/shared/src/types/issue.ts | 2 + .../__tests__/issue-blocker-attention.test.ts | 18 +++ .../issue-dependency-wakeups-routes.test.ts | 153 ++++++++++++++++++ server/src/__tests__/issues-service.test.ts | 70 ++++++++ server/src/routes/issues.ts | 21 +++ server/src/services/issues.ts | 55 ++++++- 6 files changed, 314 insertions(+), 5 deletions(-) diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index d9c3324a47c..3e1075bc1f5 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -388,6 +388,7 @@ export type IssueBlockerAttentionState = "none" | "covered" | "stalled" | "needs export type IssueBlockerAttentionReason = | "active_child" | "active_dependency" + | "missing_blocker_path" | "stalled_review" | "attention_required" | null; @@ -419,6 +420,7 @@ export type IssueBlockedInboxReason = | "blocked_by_assigned_backlog_issue" | "blocked_by_uninvokable_assignee" | "blocked_by_cancelled_issue" + | "blocked_without_blocker_path" | "blocked_chain_stalled" | "invalid_review_participant" | "in_review_without_action_path" diff --git a/server/src/__tests__/issue-blocker-attention.test.ts b/server/src/__tests__/issue-blocker-attention.test.ts index 13480b66491..30d0f1f7fd6 100644 --- a/server/src/__tests__/issue-blocker-attention.test.ts +++ b/server/src/__tests__/issue-blocker-attention.test.ts @@ -146,6 +146,24 @@ describeEmbeddedPostgres("issue blocker attention", () => { return runId; } + it("surfaces a blocked issue with zero blocker edges as a missing blocker path", async () => { + const { companyId } = await createCompany("PBZ"); + const issueId = await insertIssue({ + companyId, + identifier: "PBZ-1", + title: "Stranded blocked issue", + status: "blocked", + }); + + const issue = (await svc.list(companyId, { status: "blocked" })).find((row) => row.id === issueId); + + expect(issue?.blockerAttention).toMatchObject({ + state: "needs_attention", + reason: "missing_blocker_path", + unresolvedBlockerCount: 0, + }); + }); + it("classifies a blocked parent as covered when its child has a running execution path", async () => { const { companyId, agentId } = await createCompany("PBC"); const parentId = await insertIssue({ companyId, identifier: "PBC-1", title: "Parent", status: "blocked" }); diff --git a/server/src/__tests__/issue-dependency-wakeups-routes.test.ts b/server/src/__tests__/issue-dependency-wakeups-routes.test.ts index da27484dbf7..7c9ae241644 100644 --- a/server/src/__tests__/issue-dependency-wakeups-routes.test.ts +++ b/server/src/__tests__/issue-dependency-wakeups-routes.test.ts @@ -67,6 +67,11 @@ vi.mock("../services/index.js", () => ({ syncDocument: async () => undefined, syncIssue: async () => undefined, }), + issueReportService: () => ({ + create: vi.fn(), + listForIssue: vi.fn(async () => []), + resolveOrigin: vi.fn(), + }), issueRecoveryActionService: () => ({ getActiveForIssue: vi.fn(async () => null), listActiveForIssues: vi.fn(async () => new Map()), @@ -280,6 +285,154 @@ describe("issue dependency wakeups in issue routes", () => { }); }); + it("wakes an assigned issue when its final unresolved blocker edge is deleted", async () => { + const dependentIssueId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; + const blockerIssueId = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; + mockIssueService.getById.mockResolvedValue({ + id: dependentIssueId, + companyId: "company-1", + identifier: "PAP-201", + title: "Deleted blocker edge", + description: null, + status: "blocked", + priority: "medium", + parentId: null, + assigneeAgentId: "agent-2", + assigneeUserId: null, + createdByAgentId: null, + createdByUserId: null, + executionWorkspaceId: null, + labels: [], + labelIds: [], + }); + mockIssueService.update.mockResolvedValue({ + id: dependentIssueId, + companyId: "company-1", + identifier: "PAP-201", + title: "Deleted blocker edge", + description: null, + status: "todo", + priority: "medium", + parentId: null, + assigneeAgentId: "agent-2", + assigneeUserId: null, + createdByAgentId: null, + createdByUserId: null, + executionWorkspaceId: null, + labels: [], + labelIds: [], + }); + mockIssueService.getRelationSummaries + .mockResolvedValueOnce({ + blockedBy: [{ id: blockerIssueId, status: "todo", title: "Unresolved blocker" }], + blocks: [], + }) + .mockResolvedValueOnce({ blockedBy: [], blocks: [] }); + mockIssueService.getDependencyReadiness.mockResolvedValue({ + issueId: dependentIssueId, + blockerIssueIds: [], + unresolvedBlockerIssueIds: [], + unresolvedBlockerCount: 0, + pendingFinalizeBlockerIssueIds: [], + allBlockersDone: true, + isDependencyReady: true, + }); + + const res = await request(await createApp()) + .patch(`/api/issues/${dependentIssueId}`) + .send({ blockedByIssueIds: [] }); + + expect(res.status).toBe(200); + await vi.waitFor(() => { + expect(mockWakeup).toHaveBeenCalledWith( + "agent-2", + expect.objectContaining({ + reason: "issue_blockers_resolved", + payload: expect.objectContaining({ + issueId: dependentIssueId, + resolvedBlockerIssueId: blockerIssueId, + mutation: "blocker_edge_deleted", + }), + contextSnapshot: expect.objectContaining({ source: "issue.blocker_edge_deleted" }), + }), + ); + }); + }); + + it("wakes an assigned issue when its final resolved blocker edge is stale-cleared", async () => { + const dependentIssueId = "cccccccc-cccc-4ccc-8ccc-cccccccccccc"; + const blockerIssueId = "dddddddd-dddd-4ddd-8ddd-dddddddddddd"; + mockIssueService.getById.mockResolvedValue({ + id: dependentIssueId, + companyId: "company-1", + identifier: "PAP-202", + title: "Stale blocker edge", + description: null, + status: "blocked", + priority: "medium", + parentId: null, + assigneeAgentId: "agent-2", + assigneeUserId: null, + createdByAgentId: null, + createdByUserId: null, + executionWorkspaceId: null, + labels: [], + labelIds: [], + }); + mockIssueService.update.mockResolvedValue({ + id: dependentIssueId, + companyId: "company-1", + identifier: "PAP-202", + title: "Stale blocker edge", + description: null, + status: "todo", + priority: "medium", + parentId: null, + assigneeAgentId: "agent-2", + assigneeUserId: null, + createdByAgentId: null, + createdByUserId: null, + executionWorkspaceId: null, + labels: [], + labelIds: [], + }); + mockIssueService.getRelationSummaries + .mockResolvedValueOnce({ + blockedBy: [{ id: blockerIssueId, status: "done", title: "Resolved blocker" }], + blocks: [], + }) + .mockResolvedValueOnce({ blockedBy: [], blocks: [] }); + mockIssueService.getDependencyReadiness.mockResolvedValue({ + issueId: dependentIssueId, + blockerIssueIds: [], + unresolvedBlockerIssueIds: [], + unresolvedBlockerCount: 0, + pendingFinalizeBlockerIssueIds: [], + allBlockersDone: true, + isDependencyReady: true, + }); + + const res = await request(await createApp()) + .patch(`/api/issues/${dependentIssueId}`) + .send({ blockedByIssueIds: [] }); + + expect(res.status).toBe(200); + await vi.waitFor(() => { + expect(mockWakeup).toHaveBeenCalledWith( + "agent-2", + expect.objectContaining({ + reason: "issue_blockers_resolved", + payload: expect.objectContaining({ + issueId: dependentIssueId, + resolvedBlockerIssueId: blockerIssueId, + mutation: "blocker_stale_cleared", + }), + contextSnapshot: expect.objectContaining({ source: "issue.blocker_stale_cleared" }), + }), + ); + }); + }); + it("wakes the parent when all direct children become terminal", async () => { mockIssueService.getById.mockResolvedValue({ id: "child-1", diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 0d6675133ee..661f9cc410d 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -3410,6 +3410,76 @@ describeEmbeddedPostgres("issueService blockers and dependency wake readiness", expect(created.blocks).toEqual([]); }); + it("rejects blocked issue writes without an unresolved blocker or external owner/action", async () => { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + await expect(svc.create(companyId, { + title: "Invalid blocked issue", + status: "blocked", + priority: "medium", + })).rejects.toMatchObject({ status: 422 }); + + const issueId = randomUUID(); + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Todo issue", + status: "todo", + priority: "medium", + }); + + await expect(svc.update(issueId, { status: "blocked" })).rejects.toMatchObject({ status: 422 }); + }); + + it("accepts a blocked issue with an explicit external owner/action", async () => { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + const created = await svc.create(companyId, { + title: "External wait", + description: "external owner: Vendor support\nexternal action: Restore the service account", + status: "blocked", + priority: "medium", + }); + + expect(created.status).toBe("blocked"); + }); + + it("auto-routes a blocked issue to todo when its final blocker edge is deleted", async () => { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + const blockerId = randomUUID(); + const blockedId = randomUUID(); + await db.insert(issues).values([ + { id: blockerId, companyId, title: "Blocker", status: "todo", priority: "medium" }, + { id: blockedId, companyId, title: "Blocked", status: "blocked", priority: "medium" }, + ]); + await svc.update(blockedId, { blockedByIssueIds: [blockerId] }); + + const updated = await svc.update(blockedId, { blockedByIssueIds: [] }); + + expect(updated?.status).toBe("todo"); + await expect(svc.getDependencyReadiness(blockedId)).resolves.toMatchObject({ + unresolvedBlockerCount: 0, + }); + }); + it("returns blocked-by summaries on newly created child issues", async () => { const companyId = randomUUID(); await db.insert(companies).values({ diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 391c5a25b33..d4fd823ead1 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -8602,6 +8602,27 @@ export function issueRoutes( } } + const removedBlockerRelations = Array.isArray(req.body.blockedByIssueIds) + ? (existingRelations?.blockedBy ?? []).filter( + (relation) => !(req.body.blockedByIssueIds as string[]).includes(relation.id), + ) + : []; + if (removedBlockerRelations.length > 0 && issue.assigneeAgentId) { + const readiness = await dependencyReadinessSvc.getDependencyReadiness?.(issue.id); + if (readiness && readiness.unresolvedBlockerCount === 0) { + const clearedBlocker = [...removedBlockerRelations].sort((left, right) => left.id.localeCompare(right.id))[0]!; + const staleClear = clearedBlocker.status === "done" || clearedBlocker.status === "cancelled"; + await addDependencyResolvedWakeup({ + agentId: issue.assigneeAgentId, + dependentIssueId: issue.id, + resolvedBlockerIssueId: clearedBlocker.id, + blockerIssueIds: readiness.blockerIssueIds, + source: staleClear ? "issue.blocker_stale_cleared" : "issue.blocker_edge_deleted", + mutation: staleClear ? "blocker_stale_cleared" : "blocker_edge_deleted", + }); + } + } + const becameTerminal = !["done", "cancelled"].includes(existing.status) && ["done", "cancelled"].includes(issue.status); if (becameTerminal) { diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 2f75a672bf8..c5a853a7381 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -2361,7 +2361,7 @@ async function listIssueBlockerAttentionMap( if (topLevelEdges.length === 0) { attentionMap.set(root.id, createIssueBlockerAttention({ state: "needs_attention", - reason: "attention_required", + reason: "missing_blocker_path", })); continue; } @@ -3352,6 +3352,21 @@ async function listIssueBlockedInboxAttentionMap( const blockerAttention = await listIssueBlockerAttentionMap(dbOrTx, companyId, [row]); const blockerState = blockerAttention.get(row.id); + if (row.status === "blocked" && blockerState?.reason === "missing_blocker_path") { + result.set(row.id, attentionBase({ + state: "needs_attention", + reason: "blocked_without_blocker_path", + severity: "high", + stoppedSinceAt: row.updatedAt, + owner: { type: "unknown", agentId: null, userId: null, label: null }, + action: { + label: "Add blocker or external owner", + detail: "Add an unresolved blocker edge or record external owner/action, then resume the issue.", + }, + sourceIssue: source, + })); + continue; + } if (row.status === "blocked" && (blockerState?.state === "needs_attention" || blockerState?.state === "stalled")) { result.set(row.id, attentionBase({ state: "needs_attention", @@ -6006,6 +6021,18 @@ export function issueService(db: Db) { if (data.status === "in_progress" && !data.assigneeAgentId && !data.assigneeUserId) { throw unprocessable("in_progress issues require an assignee"); } + if (data.status === "blocked") { + const unresolvedBlockerIssueIds = await listUnresolvedBlockerIssueIds( + db, + companyId, + blockedByIssueIds ?? [], + ); + if (unresolvedBlockerIssueIds.length === 0 && !externalWaitFromDescription(issueData.description ?? null)) { + throw unprocessable( + "blocked issues require an unresolved blocker or external owner/action", + ); + } + } return db.transaction(async (tx) => { const defaultCompanyGoal = await getDefaultCompanyGoal(tx, companyId); let projectWorkspaceId = issueData.projectWorkspaceId ?? null; @@ -6288,6 +6315,24 @@ export function issueService(db: Db) { throw unprocessable("Issue is blocked by unresolved blockers", { unresolvedBlockerIssueIds }); } } + const nextStatus = patch.status ?? existing.status; + if (nextStatus === "blocked") { + const unresolvedBlockerIssueIds = blockedByIssueIds !== undefined + ? await listUnresolvedBlockerIssueIds(dbOrTx, existing.companyId, blockedByIssueIds) + : ( + await listIssueDependencyReadinessMap(dbOrTx, existing.companyId, [id]) + ).get(id)?.unresolvedBlockerIssueIds ?? []; + const nextDescription = issueData.description !== undefined ? issueData.description : existing.description; + if (unresolvedBlockerIssueIds.length === 0 && !externalWaitFromDescription(nextDescription ?? null)) { + if (existing.status === "blocked" && blockedByIssueIds !== undefined && issueData.status === undefined) { + patch.status = "todo"; + } else { + throw unprocessable( + "blocked issues require an unresolved blocker or external owner/action", + ); + } + } + } const shouldValidateNextAssignee = Boolean(nextAssigneeAgentId) && (issueData.assigneeAgentId !== undefined || patch.status === "in_progress"); @@ -6349,14 +6394,14 @@ export function issueService(db: Db) { }); } - applyStatusSideEffects(issueData.status, patch); - if (issueData.status && issueData.status !== "done") { + applyStatusSideEffects(patch.status, patch); + if (patch.status && patch.status !== "done") { patch.completedAt = null; } - if (issueData.status && issueData.status !== "cancelled") { + if (patch.status && patch.status !== "cancelled") { patch.cancelledAt = null; } - if (issueData.status && issueData.status !== "in_progress") { + if (patch.status && patch.status !== "in_progress") { patch.checkoutRunId = null; patch.executionRunId = null; patch.executionAgentNameKey = null;