mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
Require blockers for blocked recovery resolution
This commit is contained in:
1 parent
d604d323fd
commit
a59fff963d
5 files changed
+142
-11
No files matched your search
@@ -10,6 +10,7 @@ import {
|
||||
createDb,
|
||||
issueComments,
|
||||
issueRecoveryActions,
|
||||
issueRelations,
|
||||
issues,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
@@ -457,6 +458,106 @@ describeEmbeddedPostgres("issue recovery actions", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects blocked recovery resolution when the source issue has no first-class blockers", async () => {
|
||||
const { companyId, managerId, sourceIssueId } = await seedCompany();
|
||||
const recoveryActionSvc = issueRecoveryActionService(db);
|
||||
const action = await recoveryActionSvc.upsertSourceScoped({
|
||||
companyId,
|
||||
sourceIssueId,
|
||||
kind: "issue_graph_liveness",
|
||||
ownerType: "agent",
|
||||
ownerAgentId: managerId,
|
||||
cause: "issue_graph_liveness",
|
||||
fingerprint: "graph-liveness:blocked-without-blocker",
|
||||
evidence: { latestIssueStatus: "in_progress" },
|
||||
nextAction: "Choose a disposition with a live continuation path.",
|
||||
wakePolicy: { type: "manual" },
|
||||
});
|
||||
const app = createApp();
|
||||
|
||||
const rejected = await request(app)
|
||||
.post(`/api/issues/${sourceIssueId}/recovery-actions/resolve`)
|
||||
.send({
|
||||
actionId: action.id,
|
||||
outcome: "blocked",
|
||||
sourceIssueStatus: "blocked",
|
||||
})
|
||||
.expect(422);
|
||||
|
||||
expect(rejected.body.error).toContain("requires an unresolved first-class blocker");
|
||||
|
||||
const [sourceIssue] = await db.select().from(issues).where(eq(issues.id, sourceIssueId));
|
||||
expect(sourceIssue?.status).toBe("in_progress");
|
||||
|
||||
const [actionRow] = await db
|
||||
.select()
|
||||
.from(issueRecoveryActions)
|
||||
.where(eq(issueRecoveryActions.id, action.id));
|
||||
expect(actionRow).toMatchObject({
|
||||
status: "active",
|
||||
outcome: null,
|
||||
resolvedAt: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("allows blocked recovery resolution when the source issue has an unresolved first-class blocker", async () => {
|
||||
const { companyId, managerId, sourceIssueId, prefix } = await seedCompany();
|
||||
const blockerIssueId = randomUUID();
|
||||
await db.insert(issues).values({
|
||||
id: blockerIssueId,
|
||||
companyId,
|
||||
title: "Unblock recovery disposition",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
assigneeAgentId: managerId,
|
||||
issueNumber: 2,
|
||||
identifier: `${prefix}-2`,
|
||||
});
|
||||
await db.insert(issueRelations).values({
|
||||
companyId,
|
||||
issueId: blockerIssueId,
|
||||
relatedIssueId: sourceIssueId,
|
||||
type: "blocks",
|
||||
});
|
||||
const recoveryActionSvc = issueRecoveryActionService(db);
|
||||
const action = await recoveryActionSvc.upsertSourceScoped({
|
||||
companyId,
|
||||
sourceIssueId,
|
||||
kind: "issue_graph_liveness",
|
||||
ownerType: "agent",
|
||||
ownerAgentId: managerId,
|
||||
cause: "issue_graph_liveness",
|
||||
fingerprint: "graph-liveness:blocked-with-blocker",
|
||||
evidence: { latestIssueStatus: "in_progress" },
|
||||
nextAction: "Wait for the blocker before continuing.",
|
||||
wakePolicy: { type: "manual" },
|
||||
});
|
||||
const app = createApp();
|
||||
|
||||
const resolved = await request(app)
|
||||
.post(`/api/issues/${sourceIssueId}/recovery-actions/resolve`)
|
||||
.send({
|
||||
actionId: action.id,
|
||||
outcome: "blocked",
|
||||
sourceIssueStatus: "blocked",
|
||||
resolutionNote: "The source issue is explicitly blocked by a follow-up.",
|
||||
})
|
||||
.expect(200);
|
||||
|
||||
expect(resolved.body.issue).toMatchObject({
|
||||
id: sourceIssueId,
|
||||
status: "blocked",
|
||||
activeRecoveryAction: null,
|
||||
});
|
||||
expect(resolved.body.recoveryAction).toMatchObject({
|
||||
id: action.id,
|
||||
status: "resolved",
|
||||
outcome: "blocked",
|
||||
resolutionNote: "The source issue is explicitly blocked by a follow-up.",
|
||||
});
|
||||
expect(await recoveryActionSvc.getActiveForIssue(companyId, sourceIssueId)).toBeNull();
|
||||
});
|
||||
|
||||
it("records false positives without cancelling the source issue", async () => {
|
||||
const { companyId, managerId, sourceIssueId } = await seedCompany();
|
||||
const recoveryActionSvc = issueRecoveryActionService(db);
|
||||
|
||||
@@ -2,9 +2,16 @@ import { randomUUID } from "node:crypto";
|
||||
import { Router, type Request, type Response } from "express";
|
||||
import multer from "multer";
|
||||
import { z } from "zod";
|
||||
import { and, desc, eq, inArray } from "drizzle-orm";
|
||||
import { and, desc, eq, inArray, notInArray } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { activityLog, executionWorkspaces, issueExecutionDecisions, projectWorkspaces } from "@paperclipai/db";
|
||||
import {
|
||||
activityLog,
|
||||
executionWorkspaces,
|
||||
issueExecutionDecisions,
|
||||
issueRelations,
|
||||
issues as issueRows,
|
||||
projectWorkspaces,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
addIssueCommentSchema,
|
||||
acceptIssueThreadInteractionSchema,
|
||||
@@ -1781,6 +1788,25 @@ export function issueRoutes(
|
||||
const actionStatus = outcome === "cancelled" ? "cancelled" : "resolved";
|
||||
const result = await db.transaction(async (tx) => {
|
||||
let issue = existing;
|
||||
if (outcome === "blocked") {
|
||||
const unresolvedBlockers = await tx
|
||||
.select({ id: issueRows.id })
|
||||
.from(issueRelations)
|
||||
.innerJoin(issueRows, eq(issueRelations.issueId, issueRows.id))
|
||||
.where(
|
||||
and(
|
||||
eq(issueRelations.companyId, existing.companyId),
|
||||
eq(issueRelations.relatedIssueId, existing.id),
|
||||
eq(issueRelations.type, "blocks"),
|
||||
notInArray(issueRows.status, ["done", "cancelled"]),
|
||||
),
|
||||
)
|
||||
.limit(1);
|
||||
if (unresolvedBlockers.length === 0) {
|
||||
throw unprocessable("Blocked recovery resolution requires an unresolved first-class blocker on the source issue");
|
||||
}
|
||||
}
|
||||
|
||||
if (sourceIssueStatus) {
|
||||
const updatedIssue = await svc.update(
|
||||
id,
|
||||
|
||||
@@ -172,12 +172,25 @@ describe("IssueRecoveryActionCard", () => {
|
||||
click(node.querySelector("[data-testid='recovery-action-resolve-trigger']"));
|
||||
|
||||
expect(document.body.textContent).toContain("Mark issue done");
|
||||
expect(document.body.textContent).not.toContain("Mark blocked");
|
||||
expect(document.body.textContent).not.toContain("Delegate follow-up issue");
|
||||
click([...document.body.querySelectorAll("button")].find((button) => button.textContent?.includes("Mark issue done")) ?? null);
|
||||
|
||||
expect(onResolve).toHaveBeenCalledWith("done");
|
||||
});
|
||||
|
||||
it("does not offer blocked recovery resolution without a blocker selection flow", () => {
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard action={buildAction()} onResolve={() => {}} canCancelRecovery />,
|
||||
);
|
||||
click(node.querySelector("[data-testid='recovery-action-resolve-trigger']"));
|
||||
|
||||
expect(document.body.textContent).toContain("Mark issue done");
|
||||
expect(document.body.textContent).toContain("Send for review");
|
||||
expect(document.body.textContent).toContain("Mark false positive");
|
||||
expect(document.body.textContent).not.toContain("Mark blocked");
|
||||
});
|
||||
|
||||
it("hides false_positive option unless canCancelRecovery is set", () => {
|
||||
const first = render(
|
||||
<IssueRecoveryActionCard action={buildAction()} onResolve={() => {}} />,
|
||||
|
||||
@@ -27,7 +27,6 @@ export type RecoveryCardCardState =
|
||||
export type RecoveryResolveOutcome =
|
||||
| "done"
|
||||
| "in_review"
|
||||
| "blocked"
|
||||
| "false_positive";
|
||||
|
||||
export interface IssueRecoveryActionCardProps {
|
||||
@@ -313,11 +312,6 @@ const RESOLVE_OPTIONS: Array<{
|
||||
label: "Send for review",
|
||||
description: "Hand off to a reviewer with a real review path.",
|
||||
},
|
||||
{
|
||||
outcome: "blocked",
|
||||
label: "Mark blocked…",
|
||||
description: "Resolve recovery by recording that the source issue is blocked.",
|
||||
},
|
||||
{
|
||||
outcome: "false_positive",
|
||||
label: "Mark false positive",
|
||||
|
||||
@@ -2965,9 +2965,6 @@ export function IssueDetail() {
|
||||
case "in_review":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
case "blocked":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "blocked", sourceIssueStatus: "blocked" });
|
||||
return;
|
||||
case "false_positive":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive" });
|
||||
return;
|
||||
|
||||
Reference in new issue
Block a user