mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix: enforce blocker-edge hygiene
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4a40c0cb13
commit
c2e6b1152e
6 files changed
+314
-5
No files matched your search
@@ -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"
|
||||
|
||||
@@ -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" });
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user