From 8c082f84e21d897d7f5f3ebf5f4cb8744c536d08 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 11 Jul 2026 12:35:48 +0000 Subject: [PATCH] feat(ui): workspace-validation diagnosis card from run evidence (PAP-13578) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PAP-13568 Phase 4b. Render a workspace-validation diagnosis card on the run page from the run's own `resultJson.workspaceValidation` evidence even when the source issue has no live recovery action (blocked / pending-interaction issues, per L3) — a failed workspace-validation run never renders nothing. - Enrich the shared IssueRecoveryActionCard: lead with the plain-language reason, expected-vs-found branches with ahead/behind counts (read defensively — optional until Phase 2 lands them), and parked-by provenance (which issue/run left the worktree on the live branch). - New lossless "Restore recorded branch" CTA (Contract A) for a clean divergence, wired to a new `restore` reconcile mode; the parked branch keeps its commits. Hidden when the recorded branch is contended (steer to isolated re-issue). The dangerous `override` stays demoted behind its reason-prompt popover. - Run-page fallback reuses the same card via a presentational synthetic action built from run evidence; no resolve menu (no persisted action to resolve). - Thread the restore handler through IssueDetail → IssueChatThread so the issue thread card gains the same enrichment. - Component tests (50) + a Storybook panel for the card states; screenshots on the issue. Coordinates with Phase 2 (PAP-13575): calls POST /execution-workspaces/:id/reconcile-branch { mode: "restore" } and reads optional provenance.aheadCount/behindCount. UI-only; no server files touched. Co-Authored-By: Claude Opus 4.8 --- ui/src/api/execution-workspaces.ts | 11 +- ui/src/components/IssueChatThread.tsx | 6 + .../IssueRecoveryActionCard.test.tsx | 112 +++++++++ ui/src/components/IssueRecoveryActionCard.tsx | 235 +++++++++++++++++- .../RunWorkspaceRecoverySurface.test.tsx | 94 +++++++ .../RunWorkspaceRecoverySurface.tsx | 89 +++++-- ui/src/lib/workspace-validation-evidence.ts | 97 ++++++++ ui/src/pages/IssueDetail.tsx | 59 +++-- .../stories/source-issue-recovery.stories.tsx | 162 ++++++++++++ 9 files changed, 819 insertions(+), 46 deletions(-) create mode 100644 ui/src/lib/workspace-validation-evidence.ts diff --git a/ui/src/api/execution-workspaces.ts b/ui/src/api/execution-workspaces.ts index e6c075a86d1..6cc518d88c4 100644 --- a/ui/src/api/execution-workspaces.ts +++ b/ui/src/api/execution-workspaces.ts @@ -131,9 +131,18 @@ export const executionWorkspacesApi = { * `runtime:manage` permission, and requires a non-empty operator `reason`. * - `mode: "quarantine_restore"` — lossless dirty-worktree repair; the server quarantines the * dirty changes onto a rescue branch and restores the recorded branch. No `reason` needed. + * - `mode: "restore"` — lossless *clean*-divergence repair (PAP-13568 Contract A). Restores the + * recorded branch in a clean, anchored, uncontended worktree parked on a foreign branch that is + * itself a real local ref; the parked branch keeps its commits (nothing is discarded). No + * `reason` needed. The server side lands in PAP-13568 Phase 2 (PAP-13575); this client is + * additive and forward-compatible with that route. */ reconcile: ( id: string, - body: { mode: "forward" } | { mode: "override"; reason: string } | { mode: "quarantine_restore" }, + body: + | { mode: "forward" } + | { mode: "override"; reason: string } + | { mode: "quarantine_restore" } + | { mode: "restore" }, ) => api.post(`/execution-workspaces/${id}/reconcile-branch`, body), }; diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index c7a954d41c6..61dd389eeb0 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -443,6 +443,8 @@ interface IssueChatThreadProps { onBreakGlassOverrideRecoveryAction?: (reason: string) => void; onQuarantineRestoreRecoveryAction?: () => void; quarantineRestoreRecoveryActionPending?: boolean; + onRestoreRecordedBranchRecoveryAction?: () => void; + restoreRecoveryActionPending?: boolean; canBreakGlassRecoveryAction?: boolean; reconcileRecoveryActionPending?: boolean; canFalsePositiveRecoveryAction?: boolean; @@ -4198,6 +4200,8 @@ export function IssueChatThread({ onBreakGlassOverrideRecoveryAction, onQuarantineRestoreRecoveryAction, quarantineRestoreRecoveryActionPending = false, + onRestoreRecordedBranchRecoveryAction, + restoreRecoveryActionPending = false, canBreakGlassRecoveryAction = false, reconcileRecoveryActionPending = false, canFalsePositiveRecoveryAction = false, @@ -4938,6 +4942,8 @@ export function IssueChatThread({ onBreakGlassOverride={onBreakGlassOverrideRecoveryAction} onQuarantineRestore={onQuarantineRestoreRecoveryAction} quarantineRestorePending={quarantineRestoreRecoveryActionPending} + onRestoreRecordedBranch={onRestoreRecordedBranchRecoveryAction} + restorePending={restoreRecoveryActionPending} canBreakGlass={canBreakGlassRecoveryAction} reconcilePending={reconcileRecoveryActionPending} canFalsePositive={canFalsePositiveRecoveryAction} diff --git a/ui/src/components/IssueRecoveryActionCard.test.tsx b/ui/src/components/IssueRecoveryActionCard.test.tsx index 5613da8638b..5ad6dc43319 100644 --- a/ui/src/components/IssueRecoveryActionCard.test.tsx +++ b/ui/src/components/IssueRecoveryActionCard.test.tsx @@ -704,3 +704,115 @@ describe("IssueRecoveryActionCard repair workspace (quarantine_restore)", () => expect(node.querySelector("[data-testid='recovery-action-repair-trigger']")).not.toBeNull(); }); }); + +describe("IssueRecoveryActionCard restore recorded branch (PAP-13568 Phase 4b)", () => { + it("offers the restore action only for a clean divergence, not a dirty one", () => { + const cleanNode = render( + {}} + />, + ); + expect(cleanNode.querySelector("[data-testid='recovery-action-restore-trigger']")).not.toBeNull(); + + const dirtyNode = render( + {}} />, + ); + expect(dirtyNode.querySelector("[data-testid='recovery-action-restore-trigger']")).toBeNull(); + }); + + it("does not offer the restore action without a handler or for non-workspace kinds", () => { + const noHandler = render( + , + ); + expect(noHandler.querySelector("[data-testid='recovery-action-restore-trigger']")).toBeNull(); + + const nonWorkspace = render( + {}} />, + ); + expect(nonWorkspace.querySelector("[data-testid='recovery-action-restore-trigger']")).toBeNull(); + }); + + it("hides restore when the recorded branch is contended (steer to isolated re-issue instead)", () => { + const node = render( + {}} + onReissueIsolated={() => {}} + />, + ); + expect(node.querySelector("[data-testid='recovery-action-restore-trigger']")).toBeNull(); + expect(node.querySelector("[data-testid='recovery-action-reissue-trigger']")).not.toBeNull(); + }); + + it("confirm popover restates the kept parked branch + recorded branch, then fires the handler (no reason field)", () => { + const onRestoreRecordedBranch = vi.fn(); + const node = render( + , + ); + click(node.querySelector("[data-testid='recovery-action-restore-trigger']")); + const restated = document.body.querySelector("[data-testid='recovery-restore-restated']"); + const text = restated?.textContent ?? ""; + expect(text).toContain("nleach/PAP-1405-live"); + expect(text).toContain("kept"); + expect(text).toContain("PAP-522-recorded"); + // Lossless — no reason textarea. + expect(document.body.querySelector("textarea")).toBeNull(); + click(document.body.querySelector("[data-testid='recovery-action-restore-confirm']")); + expect(onRestoreRecordedBranch).toHaveBeenCalledTimes(1); + }); + + it("disables the restore trigger while a restore is pending", () => { + const node = render( + {}} + restorePending + />, + ); + expect( + node.querySelector("[data-testid='recovery-action-restore-trigger']")?.disabled, + ).toBe(true); + }); + + it("renders ahead/behind counts and the parked-by provenance when present", () => { + const node = render( + , + ); + expect(node.querySelector("[data-testid='recovery-ahead-count']")?.textContent).toContain("6 ahead"); + expect(node.querySelector("[data-testid='recovery-behind-count']")?.textContent).toContain("59 behind"); + const parkedBy = node.querySelector("[data-testid='recovery-parked-by']")?.textContent ?? ""; + expect(parkedBy).toContain("PAP-13359"); + expect(parkedBy).toContain("2ef32c67"); + }); + + it("omits the ahead/behind row when counts are absent (pre-Phase-2 evidence)", () => { + const node = render(); + expect(node.querySelector("[data-testid='recovery-ahead-behind']")).toBeNull(); + }); +}); diff --git a/ui/src/components/IssueRecoveryActionCard.tsx b/ui/src/components/IssueRecoveryActionCard.tsx index da9197221cd..55eba466471 100644 --- a/ui/src/components/IssueRecoveryActionCard.tsx +++ b/ui/src/components/IssueRecoveryActionCard.tsx @@ -15,6 +15,7 @@ import { Lock, OctagonAlert, RefreshCw, + RotateCcw, Sparkles, TriangleAlert, Wrench, @@ -101,7 +102,16 @@ export interface IssueRecoveryActionCardProps { onQuarantineRestore?: () => void; /** Whether a quarantine-restore repair is currently in flight (shares the reconcile spinner). */ quarantineRestorePending?: boolean; - /** Whether a reconcile (forward, override, or quarantine-restore) is currently in flight. */ + /** + * Handler for the clean-divergence lossless repair — "Restore recorded branch" (PAP-13568 + * Contract A). Rendered only for a *clean* divergence; the caller invokes the S4 reconcile op in + * `restore` mode, which checks out the recorded branch (the parked branch keeps its commits). If + * omitted, the restore action is not shown. + */ + onRestoreRecordedBranch?: () => void; + /** Whether a restore-recorded-branch repair is currently in flight (shares the reconcile spinner). */ + restorePending?: boolean; + /** Whether a reconcile (forward, override, quarantine-restore, or restore) is currently in flight. */ reconcilePending?: boolean; /** Whether the viewer can run destructive board-only actions (e.g. false-positive dismissal). */ canFalsePositive?: boolean; @@ -265,14 +275,14 @@ function formatShortSha(sha: string | null): string | null { * live ("actual"/checked-out) branch, both HEAD shas, and a server-computed ancestry verdict + * plain-language explanation of why the run was declined. */ -interface WorkspaceContention { +export interface WorkspaceContention { claimedByIssueId: string | null; claimedByIssueIdentifier: string | null; /** True when the claiming workspace has a queued/running run (not just a stale claim). */ hasActiveRun: boolean; } -interface WorkspaceDivergence { +export interface WorkspaceDivergence { expectedBranch: string | null; liveBranch: string | null; expectedHeadSha: string | null; @@ -280,6 +290,22 @@ interface WorkspaceDivergence { ancestryVerdict: GitWorktreeBranchAncestryVerdict | null; plainLanguageReason: string | null; cleanliness: "clean" | "dirty" | "unknown" | null; + /** + * How many commits the live (checked-out) branch is ahead of / behind the recorded branch, when + * the server computed them. Optional — older evidence payloads and the pre-PAP-13568-Phase-2 + * server omit these, so both stay null and the UI falls back to the ancestry verdict alone. + */ + aheadCount: number | null; + behindCount: number | null; + /** + * Provenance of the parked branch: which issue/run left the worktree on the live branch. The + * source issue that owns the failing run always comes through as `sourceIdentifier`; a distinct + * "parked by" attribution (a *different* run than the one that failed) is optional and only + * present once the server records it. + */ + sourceIdentifier: string | null; + parkedByIdentifier: string | null; + parkedByRunId: string | null; /** Number of dirty (uncommitted) status entries in the live worktree, when known. */ dirtyFileCount: number | null; /** Sample of dirty paths (already truncated server-side) for the confirm step. */ @@ -334,9 +360,15 @@ function readContention(value: unknown): WorkspaceContention | null { }; } -function readWorkspaceDivergence(action: IssueRecoveryAction): WorkspaceDivergence | null { - if (action.kind !== "workspace_validation") return null; - const workspaceValidation = asRecord(action.evidence?.workspaceValidation); +/** + * Reads a git-worktree branch-incoherence divergence from a raw `workspaceValidation` evidence + * payload. This is the same `GitWorktreeBranchIncoherenceEvidence` shape whether it is nested under + * a recovery action's `evidence.workspaceValidation` (issue thread / recovery card) or stamped + * directly on a failed run's `resultJson.workspaceValidation` (run-page fallback card, PAP-13568 + * Phase 4b). Returns null when the payload is missing or is not a branch-incoherence failure. + */ +export function readWorkspaceDivergenceFromEvidence(evidence: unknown): WorkspaceDivergence | null { + const workspaceValidation = asRecord(evidence); if (!workspaceValidation) return null; if (workspaceValidation.reason !== "git_worktree_branch_incoherence") return null; const provenance = asRecord(workspaceValidation.provenance) ?? {}; @@ -358,6 +390,19 @@ function readWorkspaceDivergence(action: IssueRecoveryAction): WorkspaceDivergen ancestryVerdict: asAncestryVerdict(provenance.ancestryVerdict), plainLanguageReason: asNonEmptyString(provenance.plainLanguageReason), cleanliness, + // Optional ahead/behind — read from provenance first, then the evidence root, so we tolerate + // either placement once Phase 2 lands them (and stay null before then). + aheadCount: + asNonNegativeInt(provenance.aheadCount) ?? asNonNegativeInt(workspaceValidation.aheadCount), + behindCount: + asNonNegativeInt(provenance.behindCount) ?? asNonNegativeInt(workspaceValidation.behindCount), + sourceIdentifier, + parkedByIdentifier: + asNonEmptyString(workspaceValidation.parkedByIdentifier) ?? + asNonEmptyString(provenance.parkedByIdentifier), + parkedByRunId: + asNonEmptyString(workspaceValidation.parkedByRunId) ?? + asNonEmptyString(provenance.parkedByRunId), dirtyFileCount: asNonNegativeInt(workspaceValidation.statusEntryCount), dirtyPathSample: asStringArray(workspaceValidation.dirtyPathSample), contention: readContention(workspaceValidation.contention), @@ -366,6 +411,11 @@ function readWorkspaceDivergence(action: IssueRecoveryAction): WorkspaceDivergen }; } +function readWorkspaceDivergence(action: IssueRecoveryAction): WorkspaceDivergence | null { + if (action.kind !== "workspace_validation") return null; + return readWorkspaceDivergenceFromEvidence(action.evidence?.workspaceValidation); +} + const ANCESTRY_BADGE: Record< GitWorktreeBranchAncestryVerdict, { label: string; className: string } @@ -414,7 +464,41 @@ function BranchFacet({ ); } -function DivergenceDiagnosis({ +/** + * Renders the ahead/behind commit spread between the live and recorded branches when the server + * computed it. Falls back to nothing (the ancestry badge already summarises the relationship) when + * the counts are absent — e.g. pre-PAP-13568-Phase-2 evidence. + */ +function AheadBehindSummary({ divergence }: { divergence: WorkspaceDivergence }) { + const { aheadCount, behindCount } = divergence; + if (aheadCount === null && behindCount === null) return null; + return ( +
+ Live vs. recorded + {aheadCount !== null ? ( + + ↑ {aheadCount} ahead + + ) : null} + {behindCount !== null ? ( + + ↓ {behindCount} behind + + ) : null} +
+ ); +} + +export function DivergenceDiagnosis({ divergence, dividerClass, }: { @@ -422,6 +506,7 @@ function DivergenceDiagnosis({ dividerClass: string; }) { const badge = ANCESTRY_BADGE[divergence.ancestryVerdict ?? "unknown"]; + const parkedByLabel = divergence.parkedByIdentifier ?? divergence.sourceIdentifier; return (
+ {divergence.plainLanguageReason ? ( +

{divergence.plainLanguageReason}

+ ) : null}
- {divergence.plainLanguageReason ? ( -

{divergence.plainLanguageReason}

+ + {parkedByLabel ? ( +

+ Parked on the live branch by{" "} + {parkedByLabel} + {divergence.parkedByRunId ? ( + <> + {" "} + (run {divergence.parkedByRunId.slice(0, 8)}) + + ) : null} + . +

) : null} {divergence.contention ? (

` restores it without discarding anything: the parked branch keeps + * its commits on its own ref. Like the quarantine repair this is non-destructive (no reason + * required); the confirm popover simply restates that the parked branch is left untouched and the + * recorded branch will be restored. Invokes the S4 reconcile op in `restore` mode. + */ +function RestoreRecordedBranch({ + divergence, + onConfirm, + pending, +}: { + divergence: WorkspaceDivergence; + onConfirm: () => void; + pending: boolean; +}) { + return ( + + + + + +

+
+ + Restore recorded branch +
+

+ This is lossless — no reason required. The worktree is clean, so Paperclip simply checks + out the recorded branch and the task resumes. The parked branch keeps all of its commits + on its own ref; nothing is discarded. +

+
+
+
+
Parked branch
+
+ {divergence.liveBranch ?? "detached"} + (kept) +
+
+
+
Restore to
+
+ {divergence.expectedBranch ?? "recorded branch"} +
+
+
+ + + + ); +} + function readWakePolicySummary(action: IssueRecoveryAction): string | null { const policy = action.wakePolicy; if (!policy) return null; @@ -905,6 +1098,8 @@ export function IssueRecoveryActionCard({ onBreakGlassOverride, onQuarantineRestore, quarantineRestorePending = false, + onRestoreRecordedBranch, + restorePending = false, canBreakGlass = false, reconcilePending = false, canFalsePositive = false, @@ -985,6 +1180,16 @@ export function IssueRecoveryActionCard({ cardState !== "resolved" && divergence !== null && divergence.cleanliness === "dirty"; + // The clean-divergence lossless repair — "Restore recorded branch" (PAP-13568 Contract A). Offered + // only for a *clean* divergence (a dirty one quarantines instead). Disabled server-side when the + // recorded branch is contended by another active workspace claim, so we hide it in that case and + // steer to the isolated re-issue like the quarantine repair does. + const showRestoreAction = + onRestoreRecordedBranch !== undefined && + cardState !== "resolved" && + divergence !== null && + divergence.cleanliness === "clean" && + repairContention === null; const repairDisabledReason = repairContention ? `Held by ${contentionLabel(repairContention)} — re-issue on an isolated workspace instead.` : null; @@ -996,7 +1201,8 @@ export function IssueRecoveryActionCard({ showReissueAction || showReconcileForward || showBreakGlass || - showRepairAction; + showRepairAction || + showRestoreAction; return (
) : null} + {showRestoreAction && divergence ? ( + onRestoreRecordedBranch?.()} + /> + ) : null} {showReconcileForward ? (