mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
Polish recovery lifecycle review issues
This commit is contained in:
1 parent
114ef087e1
commit
b9a084587f
10 files changed
+116
-70
No files matched your search
@@ -66,6 +66,25 @@ describe("issue validators", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("allows cancelled recovery resolutions to atomically restore the source issue status", () => {
|
||||
expect(
|
||||
resolveIssueRecoveryActionSchema.parse({
|
||||
outcome: "cancelled",
|
||||
sourceIssueStatus: "in_review",
|
||||
}),
|
||||
).toMatchObject({
|
||||
outcome: "cancelled",
|
||||
sourceIssueStatus: "in_review",
|
||||
});
|
||||
|
||||
expect(
|
||||
resolveIssueRecoveryActionSchema.safeParse({
|
||||
outcome: "cancelled",
|
||||
sourceIssueStatus: "blocked",
|
||||
}).success,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("normalizes escaped line breaks in issue comment bodies", () => {
|
||||
const parsed = addIssueCommentSchema.parse({
|
||||
body: "Progress update\\r\\n\\r\\nNext action.",
|
||||
|
||||
@@ -239,7 +239,7 @@ export const resolveIssueRecoveryActionSchema = z.object({
|
||||
return;
|
||||
}
|
||||
|
||||
if (value.outcome === "false_positive") {
|
||||
if (value.outcome === "false_positive" || value.outcome === "cancelled") {
|
||||
if (
|
||||
value.sourceIssueStatus !== undefined &&
|
||||
value.sourceIssueStatus !== null &&
|
||||
@@ -248,7 +248,7 @@ export const resolveIssueRecoveryActionSchema = z.object({
|
||||
) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
message: "False-positive recovery actions may only move the source issue to done or in_review",
|
||||
message: "This recovery outcome may only move the source issue to done or in_review",
|
||||
path: ["sourceIssueStatus"],
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1630,7 +1630,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
reason: "no_invokable_recovery_owner",
|
||||
},
|
||||
monitorPolicy: null,
|
||||
maxAttempts: 1,
|
||||
maxAttempts: null,
|
||||
lastAttemptAt: now,
|
||||
});
|
||||
|
||||
@@ -1858,6 +1858,8 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
eq(issueComments.authorType, "system"),
|
||||
),
|
||||
)
|
||||
.orderBy(desc(issueComments.createdAt))
|
||||
.limit(50)
|
||||
.then((rows) => rows.some((row) => (row.body ?? "").includes(escalationCommentMarker)));
|
||||
|
||||
if (!hasEscalationComment) {
|
||||
@@ -1913,12 +1915,26 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
});
|
||||
|
||||
if (recoveryAction.ownerAgentId === input.issue.assigneeAgentId) {
|
||||
const reblocked = await issuesSvc.update(input.issue.id, {
|
||||
status: "blocked",
|
||||
blockedByIssueIds: blockerIds,
|
||||
assigneeAgentId: recoveryAction.ownerAgentId ?? input.issue.assigneeAgentId,
|
||||
});
|
||||
if (!reblocked) return null;
|
||||
const [currentIssue] = await db
|
||||
.select({
|
||||
status: issues.status,
|
||||
assigneeAgentId: issues.assigneeAgentId,
|
||||
})
|
||||
.from(issues)
|
||||
.where(eq(issues.id, input.issue.id))
|
||||
.limit(1);
|
||||
if (
|
||||
currentIssue &&
|
||||
(currentIssue.status !== "blocked" ||
|
||||
currentIssue.assigneeAgentId !== (recoveryAction.ownerAgentId ?? input.issue.assigneeAgentId))
|
||||
) {
|
||||
const reblocked = await issuesSvc.update(input.issue.id, {
|
||||
status: "blocked",
|
||||
blockedByIssueIds: blockerIds,
|
||||
assigneeAgentId: recoveryAction.ownerAgentId ?? input.issue.assigneeAgentId,
|
||||
});
|
||||
if (!reblocked) return null;
|
||||
}
|
||||
}
|
||||
|
||||
return updated;
|
||||
|
||||
@@ -7,22 +7,16 @@ import type { TranscriptEntry } from "../adapters";
|
||||
import { issuesApi } from "../api/issues";
|
||||
import { queryKeys } from "../lib/queryKeys";
|
||||
import { cn, relativeTime } from "../lib/utils";
|
||||
import {
|
||||
deriveActiveRecoveryDisplayState,
|
||||
type ActiveRecoveryDisplayState,
|
||||
} from "../lib/recovery-display";
|
||||
import { Eye, ExternalLink, OctagonAlert, RefreshCw, TriangleAlert } from "lucide-react";
|
||||
import { Identity } from "./Identity";
|
||||
import { RunChatSurface } from "./RunChatSurface";
|
||||
import { useLiveRunTranscripts } from "./transcript/useLiveRunTranscripts";
|
||||
|
||||
type RunCardRecoveryState = "needed" | "in_progress" | "observe_only" | "escalated";
|
||||
|
||||
function runCardRecoveryStateFor(action: IssueRecoveryAction): RunCardRecoveryState | null {
|
||||
if (action.status === "resolved" || action.status === "cancelled") return null;
|
||||
if (action.status === "escalated") return "escalated";
|
||||
if (action.kind === "active_run_watchdog") return "observe_only";
|
||||
if (action.outcome === "delegated") return "in_progress";
|
||||
return "needed";
|
||||
}
|
||||
|
||||
const RUN_CARD_RECOVERY_TONE: Record<RunCardRecoveryState, { icon: typeof TriangleAlert; label: string; className: string }> = {
|
||||
const RUN_CARD_RECOVERY_TONE: Record<ActiveRecoveryDisplayState, { icon: typeof TriangleAlert; label: string; className: string }> = {
|
||||
needed: {
|
||||
icon: TriangleAlert,
|
||||
label: "Recovery needed",
|
||||
@@ -46,7 +40,7 @@ const RUN_CARD_RECOVERY_TONE: Record<RunCardRecoveryState, { icon: typeof Triang
|
||||
};
|
||||
|
||||
function RunCardRecoveryChip({ action }: { action: IssueRecoveryAction }) {
|
||||
const state = runCardRecoveryStateFor(action);
|
||||
const state = deriveActiveRecoveryDisplayState(action);
|
||||
if (!state) return null;
|
||||
const tone = RUN_CARD_RECOVERY_TONE[state];
|
||||
const Icon = tone.icon;
|
||||
|
||||
@@ -9,18 +9,12 @@ import { Link } from "@/lib/router";
|
||||
import { createIssueDetailPath } from "../lib/issueDetailBreadcrumb";
|
||||
import { IssueLinkQuicklook } from "./IssueLinkQuicklook";
|
||||
import { isAssignedBacklogBlocker } from "../lib/issue-blockers";
|
||||
import {
|
||||
deriveActiveRecoveryDisplayState,
|
||||
type ActiveRecoveryDisplayState,
|
||||
} from "../lib/recovery-display";
|
||||
|
||||
type BlockerRecoveryState = "needed" | "in_progress" | "observe_only" | "escalated";
|
||||
|
||||
function blockerRecoveryStateFor(action: IssueRecoveryAction): BlockerRecoveryState | null {
|
||||
if (action.status === "resolved" || action.status === "cancelled") return null;
|
||||
if (action.status === "escalated") return "escalated";
|
||||
if (action.kind === "active_run_watchdog") return "observe_only";
|
||||
if (action.outcome === "delegated") return "in_progress";
|
||||
return "needed";
|
||||
}
|
||||
|
||||
const BLOCKER_RECOVERY_TONE: Record<BlockerRecoveryState, { icon: typeof TriangleAlert; label: string; className: string }> = {
|
||||
const BLOCKER_RECOVERY_TONE: Record<ActiveRecoveryDisplayState, { icon: typeof TriangleAlert; label: string; className: string }> = {
|
||||
needed: {
|
||||
icon: TriangleAlert,
|
||||
label: "Recovery needed",
|
||||
@@ -44,7 +38,7 @@ const BLOCKER_RECOVERY_TONE: Record<BlockerRecoveryState, { icon: typeof Triangl
|
||||
};
|
||||
|
||||
function BlockerRecoveryIndicator({ action }: { action: IssueRecoveryAction }) {
|
||||
const state = blockerRecoveryStateFor(action);
|
||||
const state = deriveActiveRecoveryDisplayState(action);
|
||||
if (!state) return null;
|
||||
const tone = BLOCKER_RECOVERY_TONE[state];
|
||||
const Icon = tone.icon;
|
||||
|
||||
@@ -188,16 +188,17 @@ describe("IssueRecoveryActionCard", () => {
|
||||
|
||||
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).toContain("False positive, done");
|
||||
expect(document.body.textContent).toContain("False positive, review");
|
||||
expect(document.body.textContent).not.toContain("Mark blocked");
|
||||
});
|
||||
|
||||
it("hides false_positive option unless canCancelRecovery is set", () => {
|
||||
it("hides false-positive options unless canCancelRecovery is set", () => {
|
||||
const first = render(
|
||||
<IssueRecoveryActionCard action={buildAction()} onResolve={() => {}} />,
|
||||
);
|
||||
click(first.querySelector("[data-testid='recovery-action-resolve-trigger']"));
|
||||
expect(document.body.textContent).not.toContain("Mark false positive");
|
||||
expect(document.body.textContent).not.toContain("False positive");
|
||||
|
||||
act(() => root?.unmount());
|
||||
root = null;
|
||||
@@ -209,8 +210,9 @@ describe("IssueRecoveryActionCard", () => {
|
||||
<IssueRecoveryActionCard action={buildAction()} onResolve={onResolve} canCancelRecovery />,
|
||||
);
|
||||
click(second.querySelector("[data-testid='recovery-action-resolve-trigger']"));
|
||||
expect(document.body.textContent).toContain("Mark false positive");
|
||||
click([...document.body.querySelectorAll("button")].find((button) => button.textContent?.includes("Mark false positive")) ?? null);
|
||||
expect(onResolve).toHaveBeenCalledWith("false_positive");
|
||||
expect(document.body.textContent).toContain("False positive, done");
|
||||
expect(document.body.textContent).toContain("False positive, review");
|
||||
click([...document.body.querySelectorAll("button")].find((button) => button.textContent?.includes("False positive, done")) ?? null);
|
||||
expect(onResolve).toHaveBeenCalledWith("false_positive_done");
|
||||
});
|
||||
});
|
||||
@@ -16,18 +16,19 @@ import {
|
||||
} from "@/components/ui/popover";
|
||||
import { agentUrl } from "@/lib/utils";
|
||||
import { cn } from "@/lib/utils";
|
||||
import {
|
||||
deriveRecoveryDisplayState,
|
||||
type RecoveryDisplayState,
|
||||
} from "@/lib/recovery-display";
|
||||
|
||||
export type RecoveryCardCardState =
|
||||
| "needed"
|
||||
| "in_progress"
|
||||
| "observe_only"
|
||||
| "escalated"
|
||||
| "resolved";
|
||||
export type RecoveryCardCardState = RecoveryDisplayState;
|
||||
export const deriveRecoveryCardState = deriveRecoveryDisplayState;
|
||||
|
||||
export type RecoveryResolveOutcome =
|
||||
| "done"
|
||||
| "in_review"
|
||||
| "false_positive";
|
||||
| "false_positive_done"
|
||||
| "false_positive_in_review";
|
||||
|
||||
export interface IssueRecoveryActionCardProps {
|
||||
action: IssueRecoveryAction;
|
||||
@@ -119,17 +120,6 @@ const STATE_TONE: Record<RecoveryCardCardState, {
|
||||
},
|
||||
};
|
||||
|
||||
export function deriveRecoveryCardState(
|
||||
action: Pick<IssueRecoveryAction, "status" | "kind" | "outcome">,
|
||||
): RecoveryCardCardState {
|
||||
if (action.status === "resolved") return "resolved";
|
||||
if (action.status === "escalated") return "escalated";
|
||||
if (action.status === "cancelled") return "resolved";
|
||||
if (action.kind === "active_run_watchdog") return "observe_only";
|
||||
if (action.outcome === "delegated") return "in_progress";
|
||||
return "needed";
|
||||
}
|
||||
|
||||
const OUTCOME_LABEL: Record<IssueRecoveryActionOutcome, string> = {
|
||||
restored: "restored",
|
||||
delegated: "delegated to follow-up",
|
||||
@@ -313,9 +303,16 @@ const RESOLVE_OPTIONS: Array<{
|
||||
description: "Hand off to a reviewer with a real review path.",
|
||||
},
|
||||
{
|
||||
outcome: "false_positive",
|
||||
label: "Mark false positive",
|
||||
description: "Cancel recovery — only the board may do this.",
|
||||
outcome: "false_positive_done",
|
||||
label: "False positive, done",
|
||||
description: "Dismiss recovery and mark the source issue complete.",
|
||||
destructive: true,
|
||||
boardOnly: true,
|
||||
},
|
||||
{
|
||||
outcome: "false_positive_in_review",
|
||||
label: "False positive, review",
|
||||
description: "Dismiss recovery and send the source issue for review.",
|
||||
destructive: true,
|
||||
boardOnly: true,
|
||||
},
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
withIssueDetailHeaderSeed,
|
||||
} from "../lib/issueDetailBreadcrumb";
|
||||
import { cn } from "../lib/utils";
|
||||
import { deriveActiveRecoveryDisplayState } from "../lib/recovery-display";
|
||||
import { StatusIcon } from "./StatusIcon";
|
||||
import { productivityReviewTriggerLabel } from "./ProductivityReviewBadge";
|
||||
import { hasAssignedBacklogBlocker } from "../lib/issue-blockers";
|
||||
@@ -263,16 +264,8 @@ const RECOVERY_CHIP_TONE: Record<
|
||||
},
|
||||
};
|
||||
|
||||
function rowRecoveryStateFor(action: IssueRecoveryAction): keyof typeof RECOVERY_CHIP_TONE | null {
|
||||
if (action.status === "resolved" || action.status === "cancelled") return null;
|
||||
if (action.status === "escalated") return "escalated";
|
||||
if (action.kind === "active_run_watchdog") return "observe_only";
|
||||
if (action.outcome === "delegated") return "in_progress";
|
||||
return "needed";
|
||||
}
|
||||
|
||||
function renderRecoveryChip(action: IssueRecoveryAction, selected: boolean): ReactNode {
|
||||
const state = rowRecoveryStateFor(action);
|
||||
const state = deriveActiveRecoveryDisplayState(action);
|
||||
if (!state) return null;
|
||||
const tone = RECOVERY_CHIP_TONE[state];
|
||||
const Icon = tone.icon;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
import type { IssueRecoveryAction } from "@paperclipai/shared";
|
||||
|
||||
export type RecoveryDisplayState =
|
||||
| "needed"
|
||||
| "in_progress"
|
||||
| "observe_only"
|
||||
| "escalated"
|
||||
| "resolved";
|
||||
|
||||
export type ActiveRecoveryDisplayState = Exclude<RecoveryDisplayState, "resolved">;
|
||||
|
||||
export function deriveRecoveryDisplayState(
|
||||
action: Pick<IssueRecoveryAction, "status" | "kind" | "outcome">,
|
||||
): RecoveryDisplayState {
|
||||
if (action.status === "resolved") return "resolved";
|
||||
if (action.status === "escalated") return "escalated";
|
||||
if (action.status === "cancelled") return "resolved";
|
||||
if (action.kind === "active_run_watchdog") return "observe_only";
|
||||
if (action.outcome === "delegated") return "in_progress";
|
||||
return "needed";
|
||||
}
|
||||
|
||||
export function deriveActiveRecoveryDisplayState(
|
||||
action: Pick<IssueRecoveryAction, "status" | "kind" | "outcome">,
|
||||
): ActiveRecoveryDisplayState | null {
|
||||
const state = deriveRecoveryDisplayState(action);
|
||||
return state === "resolved" ? null : state;
|
||||
}
|
||||
@@ -2965,7 +2965,10 @@ export function IssueDetail() {
|
||||
case "in_review":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
case "false_positive":
|
||||
case "false_positive_done":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive", sourceIssueStatus: "done" });
|
||||
return;
|
||||
case "false_positive_in_review":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user