feat(recovery-card): W7 reconcile-forward + break-glass actions
Adds the task-page recovery affordance for workspace branch divergence: reconcile-forward when ancestry is safe, and a break-glass override flow that requires explicit confirmation and a reason. Includes client wiring for the existing reconcile-branch endpoint, issue-detail refresh after successful reconciliation, runtime-management gating for break-glass, and tests for action visibility, payloads, permission gating, and workspace-target selection. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
GitHub
parent
83f5f59842
commit
4f5abf6007
|
|
@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
|
|||
|
||||
const mockApi = vi.hoisted(() => ({
|
||||
get: vi.fn(),
|
||||
post: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./client", () => ({
|
||||
|
|
@ -93,3 +94,30 @@ describe("executionWorkspacesApi.listSummaries", () => {
|
|||
expect(overview.items[0]!.linkedIssues[0]!.updatedAt).toBeInstanceOf(Date);
|
||||
});
|
||||
});
|
||||
|
||||
describe("executionWorkspacesApi.reconcile", () => {
|
||||
beforeEach(() => {
|
||||
mockApi.post.mockReset();
|
||||
mockApi.post.mockResolvedValue({});
|
||||
});
|
||||
|
||||
// Regression pin (PAP-1705): the frontend path must match the reviewed, OpenAPI-documented
|
||||
// backend contract `POST /execution-workspaces/:id/reconcile-branch` (S4 / PAP-1586). A bare
|
||||
// `/reconcile` 404s both recovery-card actions. If the two sides drift, this test fails.
|
||||
it("posts forward reconcile to the /reconcile-branch route", async () => {
|
||||
await executionWorkspacesApi.reconcile("workspace-1", { mode: "forward" });
|
||||
|
||||
expect(mockApi.post).toHaveBeenCalledWith("/execution-workspaces/workspace-1/reconcile-branch", {
|
||||
mode: "forward",
|
||||
});
|
||||
});
|
||||
|
||||
it("posts break-glass override reconcile to the /reconcile-branch route", async () => {
|
||||
await executionWorkspacesApi.reconcile("workspace-1", { mode: "override", reason: "operator note" });
|
||||
|
||||
expect(mockApi.post).toHaveBeenCalledWith("/execution-workspaces/workspace-1/reconcile-branch", {
|
||||
mode: "override",
|
||||
reason: "operator note",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -115,4 +115,21 @@ export const executionWorkspacesApi = {
|
|||
sanitizeWorkspaceRuntimeControlTarget(target),
|
||||
),
|
||||
update: (id: string, data: Record<string, unknown>) => api.patch<ExecutionWorkspace>(`/execution-workspaces/${id}`, data),
|
||||
/**
|
||||
* Reconcile a git-worktree branch divergence via the S4 (`PAP-1586`) op.
|
||||
*
|
||||
* Hits `POST /execution-workspaces/:id/reconcile-branch`. That route is the reviewed,
|
||||
* OpenAPI-documented backend contract and already ships on `master`: it was merged ahead of this
|
||||
* client change in `server/src/routes/execution-workspaces.ts` (route registration:
|
||||
* `router.post("/execution-workspaces/:id/reconcile-branch", ...)`, landed in PR #9170, with the
|
||||
* `forward` auto-reconcile path in PR #9172). This client is therefore additive against an
|
||||
* existing endpoint, not a call to a missing one. Keep this path byte-identical to the backend
|
||||
* route; the drift is pinned by a regression test in `execution-workspaces.test.ts`.
|
||||
* - `mode: "forward"` — server re-verifies `ancestryVerdict === "ancestor"` (client hint is
|
||||
* never trusted); no `reason` needed.
|
||||
* - `mode: "override"` — audited break-glass; the server rejects agent actors, re-checks
|
||||
* `runtime:manage` permission, and requires a non-empty operator `reason`.
|
||||
*/
|
||||
reconcile: (id: string, body: { mode: "forward" } | { mode: "override"; reason: string }) =>
|
||||
api.post<ExecutionWorkspace>(`/execution-workspaces/${id}/reconcile-branch`, body),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -428,6 +428,10 @@ interface IssueChatThreadProps {
|
|||
onResolveRecoveryAction?: (outcome: RecoveryResolveOutcome) => void;
|
||||
onReissueIsolatedRecoveryAction?: (request: RecoveryReissueRequest) => void;
|
||||
reissueIsolatedRecoveryActionPending?: boolean;
|
||||
onReconcileForwardRecoveryAction?: () => void;
|
||||
onBreakGlassOverrideRecoveryAction?: (reason: string) => void;
|
||||
canBreakGlassRecoveryAction?: boolean;
|
||||
reconcileRecoveryActionPending?: boolean;
|
||||
canFalsePositiveRecoveryAction?: boolean;
|
||||
legacyRecoverySourceIssue?: {
|
||||
identifier: string | null;
|
||||
|
|
@ -4166,6 +4170,10 @@ export function IssueChatThread({
|
|||
onResolveRecoveryAction,
|
||||
onReissueIsolatedRecoveryAction,
|
||||
reissueIsolatedRecoveryActionPending = false,
|
||||
onReconcileForwardRecoveryAction,
|
||||
onBreakGlassOverrideRecoveryAction,
|
||||
canBreakGlassRecoveryAction = false,
|
||||
reconcileRecoveryActionPending = false,
|
||||
canFalsePositiveRecoveryAction = false,
|
||||
legacyRecoverySourceIssue = null,
|
||||
companyId,
|
||||
|
|
@ -4881,6 +4889,10 @@ export function IssueChatThread({
|
|||
onResolve={onResolveRecoveryAction}
|
||||
onReissueIsolated={onReissueIsolatedRecoveryAction}
|
||||
reissuePending={reissueIsolatedRecoveryActionPending}
|
||||
onReconcileForward={onReconcileForwardRecoveryAction}
|
||||
onBreakGlassOverride={onBreakGlassOverrideRecoveryAction}
|
||||
canBreakGlass={canBreakGlassRecoveryAction}
|
||||
reconcilePending={reconcileRecoveryActionPending}
|
||||
canFalsePositive={canFalsePositiveRecoveryAction}
|
||||
/>
|
||||
) : null}
|
||||
|
|
|
|||
|
|
@ -378,3 +378,127 @@ describe("IssueRecoveryActionCard workspace_validation divergence", () => {
|
|||
expect(trigger?.disabled).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
function setTextareaValue(element: HTMLTextAreaElement | null, value: string) {
|
||||
if (!element) throw new Error("Expected a textarea to exist");
|
||||
const setter = Object.getOwnPropertyDescriptor(
|
||||
window.HTMLTextAreaElement.prototype,
|
||||
"value",
|
||||
)?.set;
|
||||
act(() => {
|
||||
setter?.call(element, value);
|
||||
element.dispatchEvent(new Event("input", { bubbles: true }));
|
||||
});
|
||||
}
|
||||
|
||||
describe("IssueRecoveryActionCard W7 reconcile actions", () => {
|
||||
it("offers 'Reconcile forward & continue' only for an ancestor verdict and calls the handler", () => {
|
||||
const onReconcileForward = vi.fn();
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard
|
||||
action={buildWorkspaceValidationAction({ provenance: { ancestryVerdict: "ancestor" } })}
|
||||
onReconcileForward={onReconcileForward}
|
||||
/>,
|
||||
);
|
||||
const button = node.querySelector("[data-testid='recovery-action-reconcile-forward']");
|
||||
expect(button).not.toBeNull();
|
||||
click(button);
|
||||
expect(onReconcileForward).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("hides 'Reconcile forward & continue' when the verdict is not an ancestor", () => {
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard
|
||||
action={buildWorkspaceValidationAction({ provenance: { ancestryVerdict: "diverged" } })}
|
||||
onReconcileForward={() => {}}
|
||||
/>,
|
||||
);
|
||||
expect(node.querySelector("[data-testid='recovery-action-reconcile-forward']")).toBeNull();
|
||||
});
|
||||
|
||||
it("disables reconcile-forward while a reconcile is pending", () => {
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard
|
||||
action={buildWorkspaceValidationAction({ provenance: { ancestryVerdict: "ancestor" } })}
|
||||
onReconcileForward={() => {}}
|
||||
reconcilePending
|
||||
/>,
|
||||
);
|
||||
const button = node.querySelector<HTMLButtonElement>("[data-testid='recovery-action-reconcile-forward']");
|
||||
expect(button?.disabled).toBe(true);
|
||||
});
|
||||
|
||||
it("never renders the break-glass action for a non-permitted operator", () => {
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard
|
||||
action={buildWorkspaceValidationAction()}
|
||||
onBreakGlassOverride={() => {}}
|
||||
canBreakGlass={false}
|
||||
/>,
|
||||
);
|
||||
expect(node.querySelector("[data-testid='recovery-action-breakglass-trigger']")).toBeNull();
|
||||
});
|
||||
|
||||
it("break-glass restates the divergence and gates the override behind a required reason", () => {
|
||||
const onBreakGlassOverride = vi.fn();
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard
|
||||
action={buildWorkspaceValidationAction()}
|
||||
onBreakGlassOverride={onBreakGlassOverride}
|
||||
canBreakGlass
|
||||
/>,
|
||||
);
|
||||
click(node.querySelector("[data-testid='recovery-action-breakglass-trigger']"));
|
||||
|
||||
// The confirm step restates the divergence: both branches, both short SHAs, and the verdict.
|
||||
const restated = document.body.querySelector("[data-testid='recovery-breakglass-restated-divergence']");
|
||||
const restatedText = restated?.textContent ?? "";
|
||||
expect(restatedText).toContain("PAP-522-recorded");
|
||||
expect(restatedText).toContain("nleach/PAP-1405-live");
|
||||
expect(restatedText).toContain("aaaaaaaaaa");
|
||||
expect(restatedText).toContain("bbbbbbbbbb");
|
||||
expect(restatedText).toContain("Diverged");
|
||||
|
||||
// The override is disabled until a non-empty reason is recorded.
|
||||
const confirm = document.body.querySelector<HTMLButtonElement>(
|
||||
"[data-testid='recovery-action-breakglass-confirm']",
|
||||
);
|
||||
expect(confirm?.disabled).toBe(true);
|
||||
click(confirm);
|
||||
expect(onBreakGlassOverride).not.toHaveBeenCalled();
|
||||
|
||||
// Whitespace-only reason does not enable it.
|
||||
setTextareaValue(
|
||||
document.body.querySelector<HTMLTextAreaElement>("[data-testid='recovery-breakglass-reason']"),
|
||||
" ",
|
||||
);
|
||||
expect(
|
||||
document.body.querySelector<HTMLButtonElement>("[data-testid='recovery-action-breakglass-confirm']")?.disabled,
|
||||
).toBe(true);
|
||||
|
||||
// A real reason enables the override and is passed (trimmed) to the handler.
|
||||
setTextareaValue(
|
||||
document.body.querySelector<HTMLTextAreaElement>("[data-testid='recovery-breakglass-reason']"),
|
||||
" Verified live branch is safe to adopt. ",
|
||||
);
|
||||
const enabledConfirm = document.body.querySelector<HTMLButtonElement>(
|
||||
"[data-testid='recovery-action-breakglass-confirm']",
|
||||
);
|
||||
expect(enabledConfirm?.disabled).toBe(false);
|
||||
click(enabledConfirm);
|
||||
expect(onBreakGlassOverride).toHaveBeenCalledWith("Verified live branch is safe to adopt.");
|
||||
});
|
||||
|
||||
it("does not offer reconcile actions for non-workspace recovery kinds", () => {
|
||||
const node = render(
|
||||
<IssueRecoveryActionCard
|
||||
action={buildAction()}
|
||||
onReconcileForward={() => {}}
|
||||
onBreakGlassOverride={() => {}}
|
||||
canBreakGlass
|
||||
/>,
|
||||
);
|
||||
expect(node.querySelector("[data-testid='recovery-action-reconcile-forward']")).toBeNull();
|
||||
expect(node.querySelector("[data-testid='recovery-action-breakglass-trigger']")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { useMemo } from "react";
|
||||
import { useMemo, useState } from "react";
|
||||
import type {
|
||||
Agent,
|
||||
GitWorktreeBranchAncestryVerdict,
|
||||
|
|
@ -19,11 +19,13 @@ import {
|
|||
} from "lucide-react";
|
||||
import { Link } from "@/lib/router";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import {
|
||||
Popover,
|
||||
PopoverContent,
|
||||
PopoverTrigger,
|
||||
} from "@/components/ui/popover";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
import { agentUrl } from "@/lib/utils";
|
||||
import { cn } from "@/lib/utils";
|
||||
import {
|
||||
|
|
@ -68,6 +70,27 @@ export interface IssueRecoveryActionCardProps {
|
|||
onReissueIsolated?: (request: RecoveryReissueRequest) => void;
|
||||
/** Whether an isolated re-issue is currently in flight (disables the action + shows a spinner). */
|
||||
reissuePending?: boolean;
|
||||
/**
|
||||
* Handler for action 1 — "Reconcile forward & continue" (workspace_validation only). Rendered
|
||||
* only for an ancestry-proven (`ancestor`) git-worktree divergence; the caller invokes the S4
|
||||
* reconcile op in `forward` mode, which re-verifies ancestry server-side (the client hint is
|
||||
* never trusted). If omitted, the button is not shown.
|
||||
*/
|
||||
onReconcileForward?: () => void;
|
||||
/**
|
||||
* Handler for action 2 — the audited break-glass override (workspace_validation only). Receives
|
||||
* the operator's required, non-empty reason and invokes the S4 reconcile op in `override` mode.
|
||||
* Rendered only when `canBreakGlass` is true AND this handler is provided; the server independently
|
||||
* rejects agent actors and re-checks runtime-manage permission, so UI hiding is defense-in-depth.
|
||||
*/
|
||||
onBreakGlassOverride?: (reason: string) => void;
|
||||
/**
|
||||
* Whether the viewer may run the permission-gated break-glass override. When false, action 2 is
|
||||
* not rendered at all — a non-permitted user never sees the "reconcile anyway" affordance.
|
||||
*/
|
||||
canBreakGlass?: boolean;
|
||||
/** Whether a reconcile (forward or override) is currently in flight (disables both actions). */
|
||||
reconcilePending?: boolean;
|
||||
/** Whether the viewer can run destructive board-only actions (e.g. false-positive dismissal). */
|
||||
canFalsePositive?: boolean;
|
||||
className?: string;
|
||||
|
|
@ -360,6 +383,120 @@ function DivergenceDiagnosis({
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Action 2 — the audited break-glass override. Gated by an explicit confirm step that *restates the
|
||||
* divergence* (both branches + short SHAs + ancestry verdict) and a required, non-empty reason: the
|
||||
* confirm button stays disabled until the operator records why. The server re-checks the actor and
|
||||
* permission and appends the reason to the audit log — this UI gate is the operator-facing guardrail,
|
||||
* not the security boundary.
|
||||
*/
|
||||
function BreakGlassOverride({
|
||||
divergence,
|
||||
onConfirm,
|
||||
pending,
|
||||
}: {
|
||||
divergence: WorkspaceDivergence;
|
||||
onConfirm: (reason: string) => void;
|
||||
pending: boolean;
|
||||
}) {
|
||||
const [reason, setReason] = useState("");
|
||||
const trimmedReason = reason.trim();
|
||||
const canSubmit = trimmedReason.length > 0 && !pending;
|
||||
const verdictBadge = ANCESTRY_BADGE[divergence.ancestryVerdict ?? "unknown"];
|
||||
const expectedSha = formatShortSha(divergence.expectedHeadSha);
|
||||
const liveSha = formatShortSha(divergence.liveHeadSha);
|
||||
return (
|
||||
<Popover>
|
||||
<PopoverTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant="outline"
|
||||
disabled={pending}
|
||||
data-testid="recovery-action-breakglass-trigger"
|
||||
className="border-red-400/60 text-red-700 hover:bg-red-500/10 dark:border-red-500/40 dark:text-red-300"
|
||||
>
|
||||
<OctagonAlert className="h-3.5 w-3.5" aria-hidden />
|
||||
I've verified this — reconcile anyway
|
||||
</Button>
|
||||
</PopoverTrigger>
|
||||
<PopoverContent
|
||||
align="start"
|
||||
sideOffset={6}
|
||||
aria-labelledby="recovery-breakglass-title"
|
||||
className="w-96 max-w-[calc(100vw-2rem)] space-y-3 p-3"
|
||||
>
|
||||
<div className="space-y-1">
|
||||
<div
|
||||
id="recovery-breakglass-title"
|
||||
className="flex items-center gap-1.5 text-[11px] font-semibold uppercase tracking-[0.12em] text-red-700 dark:text-red-300"
|
||||
>
|
||||
<OctagonAlert className="h-3.5 w-3.5" aria-hidden />
|
||||
Break-glass reconciliation
|
||||
</div>
|
||||
<p className="text-[12px] leading-5 text-muted-foreground">
|
||||
This overrides Paperclip's safety check and points the recorded workspace at the live
|
||||
branch{" "}
|
||||
<span className="font-medium text-foreground/80">without an ancestry proof</span>. Confirm
|
||||
the divergence below and record why before continuing.
|
||||
</p>
|
||||
</div>
|
||||
<dl
|
||||
data-testid="recovery-breakglass-restated-divergence"
|
||||
className="space-y-1.5 rounded-md border border-red-400/40 bg-red-500/5 px-2.5 py-2 text-[11px]"
|
||||
>
|
||||
<div className="flex items-center justify-between gap-2">
|
||||
<dt className="shrink-0 text-muted-foreground">Recorded · expected</dt>
|
||||
<dd className="min-w-0 truncate font-mono text-foreground/90">
|
||||
{divergence.expectedBranch ?? "detached"}
|
||||
{expectedSha ? ` @ ${expectedSha}` : ""}
|
||||
</dd>
|
||||
</div>
|
||||
<div className="flex items-center justify-between gap-2">
|
||||
<dt className="shrink-0 text-muted-foreground">Live · checked out</dt>
|
||||
<dd className="min-w-0 truncate font-mono text-foreground/90">
|
||||
{divergence.liveBranch ?? "detached"}
|
||||
{liveSha ? ` @ ${liveSha}` : ""}
|
||||
</dd>
|
||||
</div>
|
||||
<div className="flex items-center justify-between gap-2">
|
||||
<dt className="shrink-0 text-muted-foreground">Ancestry verdict</dt>
|
||||
<dd className="font-medium">{verdictBadge.label}</dd>
|
||||
</div>
|
||||
</dl>
|
||||
<div className="space-y-1">
|
||||
<Label htmlFor="recovery-breakglass-reason" className="text-[11px] text-muted-foreground">
|
||||
Reason <span className="text-red-600 dark:text-red-400">(required — recorded in the audit log)</span>
|
||||
</Label>
|
||||
<Textarea
|
||||
id="recovery-breakglass-reason"
|
||||
value={reason}
|
||||
onChange={(event) => setReason(event.target.value)}
|
||||
placeholder="e.g. Verified the live branch carries only the intended follow-up commits; safe to adopt."
|
||||
className="min-h-20 text-[12px]"
|
||||
data-testid="recovery-breakglass-reason"
|
||||
aria-required="true"
|
||||
/>
|
||||
</div>
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant="destructive"
|
||||
className="w-full"
|
||||
disabled={!canSubmit}
|
||||
data-testid="recovery-action-breakglass-confirm"
|
||||
onClick={() => {
|
||||
if (!canSubmit) return;
|
||||
onConfirm(trimmedReason);
|
||||
}}
|
||||
>
|
||||
{pending ? "Reconciling…" : "Reconcile anyway (break-glass)"}
|
||||
</Button>
|
||||
</PopoverContent>
|
||||
</Popover>
|
||||
);
|
||||
}
|
||||
|
||||
function readWakePolicySummary(action: IssueRecoveryAction): string | null {
|
||||
const policy = action.wakePolicy;
|
||||
if (!policy) return null;
|
||||
|
|
@ -531,6 +668,10 @@ export function IssueRecoveryActionCard({
|
|||
onResolve,
|
||||
onReissueIsolated,
|
||||
reissuePending = false,
|
||||
onReconcileForward,
|
||||
onBreakGlassOverride,
|
||||
canBreakGlass = false,
|
||||
reconcilePending = false,
|
||||
canFalsePositive = false,
|
||||
className,
|
||||
}: IssueRecoveryActionCardProps) {
|
||||
|
|
@ -585,7 +726,22 @@ export function IssueRecoveryActionCard({
|
|||
const reissueVerdictBadge = divergence
|
||||
? ANCESTRY_BADGE[divergence.ancestryVerdict ?? "unknown"]
|
||||
: null;
|
||||
const showFooter = showResolveActions || showReissueAction;
|
||||
// Action 1 — the ancestry-proven safe path. Only offered when the server-computed verdict is
|
||||
// "ancestor"; the server re-verifies before mutating, so this gate mirrors (not replaces) it.
|
||||
const showReconcileForward =
|
||||
onReconcileForward !== undefined &&
|
||||
cardState !== "resolved" &&
|
||||
divergence !== null &&
|
||||
divergence.ancestryVerdict === "ancestor";
|
||||
// Action 2 — the break-glass override. Permission-hidden: absent entirely unless the viewer is a
|
||||
// permitted operator. The confirm step (restated divergence + required reason) lives in the popover.
|
||||
const showBreakGlass =
|
||||
onBreakGlassOverride !== undefined &&
|
||||
cardState !== "resolved" &&
|
||||
divergence !== null &&
|
||||
canBreakGlass;
|
||||
const showFooter =
|
||||
showResolveActions || showReissueAction || showReconcileForward || showBreakGlass;
|
||||
|
||||
return (
|
||||
<section
|
||||
|
|
@ -739,6 +895,23 @@ export function IssueRecoveryActionCard({
|
|||
</PopoverContent>
|
||||
</Popover>
|
||||
) : null}
|
||||
{showReconcileForward ? (
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant="default"
|
||||
disabled={reconcilePending}
|
||||
data-testid="recovery-action-reconcile-forward"
|
||||
onClick={() => onReconcileForward?.()}
|
||||
>
|
||||
{reconcilePending ? (
|
||||
<Loader2 className="h-3.5 w-3.5 animate-spin" aria-hidden />
|
||||
) : (
|
||||
<RefreshCw className="h-3.5 w-3.5" aria-hidden />
|
||||
)}
|
||||
Reconcile forward & continue
|
||||
</Button>
|
||||
) : null}
|
||||
{showReissueAction && divergence && reissueBaseRef ? (
|
||||
<Popover>
|
||||
<PopoverTrigger asChild>
|
||||
|
|
@ -805,6 +978,13 @@ export function IssueRecoveryActionCard({
|
|||
</PopoverContent>
|
||||
</Popover>
|
||||
) : null}
|
||||
{showBreakGlass && divergence ? (
|
||||
<BreakGlassOverride
|
||||
divergence={divergence}
|
||||
pending={reconcilePending}
|
||||
onConfirm={(reason) => onBreakGlassOverride?.(reason)}
|
||||
/>
|
||||
) : null}
|
||||
{showResolveActions ? (
|
||||
cardState === "observe_only" ? (
|
||||
<span className="text-(length:--text-micro) text-muted-foreground">
|
||||
|
|
|
|||
|
|
@ -8,8 +8,10 @@ import { flushSync } from "react-dom";
|
|||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
canBoardManageRuntime,
|
||||
canBoardResolveRecoveryAction,
|
||||
IssueDetail,
|
||||
readRecoveryReconcileWorkspaceId,
|
||||
shouldScrollIssueDetailToTopOnNavigation,
|
||||
} from "./IssueDetail";
|
||||
import { queryKeys } from "../lib/queryKeys";
|
||||
|
|
@ -2510,6 +2512,132 @@ describe("canBoardResolveRecoveryAction", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("canBoardManageRuntime", () => {
|
||||
it("falls back to companyIds when memberships are not populated", () => {
|
||||
expect(
|
||||
canBoardManageRuntime("company-1", {
|
||||
companyIds: ["company-1"],
|
||||
memberships: [],
|
||||
isInstanceAdmin: false,
|
||||
source: "session",
|
||||
keyId: null,
|
||||
user: null,
|
||||
userId: "user-1",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("denies viewers the runtime-manage-gated break-glass affordance", () => {
|
||||
expect(
|
||||
canBoardManageRuntime("company-1", {
|
||||
companyIds: ["company-1"],
|
||||
memberships: [
|
||||
{
|
||||
companyId: "company-1",
|
||||
membershipRole: "viewer",
|
||||
status: "active",
|
||||
},
|
||||
],
|
||||
isInstanceAdmin: false,
|
||||
source: "session",
|
||||
keyId: null,
|
||||
user: null,
|
||||
userId: "user-1",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("allows non-viewer active members (mirrors the backend runtime:manage member gate)", () => {
|
||||
expect(
|
||||
canBoardManageRuntime("company-1", {
|
||||
companyIds: ["company-1"],
|
||||
memberships: [
|
||||
{
|
||||
companyId: "company-1",
|
||||
membershipRole: "operator",
|
||||
status: "active",
|
||||
},
|
||||
],
|
||||
isInstanceAdmin: false,
|
||||
source: "session",
|
||||
keyId: null,
|
||||
user: null,
|
||||
userId: "user-1",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("readRecoveryReconcileWorkspaceId", () => {
|
||||
const makeAction = (evidence: Record<string, unknown>, kind = "workspace_validation") =>
|
||||
({ kind, evidence } as unknown as Parameters<typeof readRecoveryReconcileWorkspaceId>[0]);
|
||||
|
||||
it("returns null when the action is missing", () => {
|
||||
expect(readRecoveryReconcileWorkspaceId(null)).toBeNull();
|
||||
expect(readRecoveryReconcileWorkspaceId(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for non-workspace_validation actions even with a workspace id in evidence", () => {
|
||||
expect(
|
||||
readRecoveryReconcileWorkspaceId(
|
||||
makeAction(
|
||||
{ workspaceValidation: { persistedExecutionWorkspaceId: "ws-1" } },
|
||||
"stranded_assigned_issue",
|
||||
),
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("prefers persistedExecutionWorkspaceId (git_worktree_branch_incoherence shape)", () => {
|
||||
expect(
|
||||
readRecoveryReconcileWorkspaceId(
|
||||
makeAction({
|
||||
workspaceValidation: {
|
||||
reason: "git_worktree_branch_incoherence",
|
||||
persistedExecutionWorkspaceId: "ws-diverged",
|
||||
executionWorkspaceId: "ws-other",
|
||||
},
|
||||
}),
|
||||
),
|
||||
).toBe("ws-diverged");
|
||||
});
|
||||
|
||||
it("falls back to executionWorkspaceId (git_worktree_not_reusable shape)", () => {
|
||||
expect(
|
||||
readRecoveryReconcileWorkspaceId(
|
||||
makeAction({
|
||||
workspaceValidation: {
|
||||
reason: "git_worktree_not_reusable",
|
||||
executionWorkspaceId: "ws-not-reusable",
|
||||
},
|
||||
}),
|
||||
),
|
||||
).toBe("ws-not-reusable");
|
||||
});
|
||||
|
||||
it("returns null when the evidence carries no workspace reference (so the caller falls back to the page-level id)", () => {
|
||||
expect(readRecoveryReconcileWorkspaceId(makeAction({}))).toBeNull();
|
||||
expect(
|
||||
readRecoveryReconcileWorkspaceId(
|
||||
makeAction({ workspaceValidation: { reason: "git_worktree_branch_incoherence" } }),
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores non-string / empty workspace ids", () => {
|
||||
expect(
|
||||
readRecoveryReconcileWorkspaceId(
|
||||
makeAction({ workspaceValidation: { persistedExecutionWorkspaceId: "" } }),
|
||||
),
|
||||
).toBeNull();
|
||||
expect(
|
||||
readRecoveryReconcileWorkspaceId(
|
||||
makeAction({ workspaceValidation: { persistedExecutionWorkspaceId: 42 } }),
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldScrollIssueDetailToTopOnNavigation", () => {
|
||||
it("does not scroll when only URL search params changed for the same issue", () => {
|
||||
expect(shouldScrollIssueDetailToTopOnNavigation({
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import { accessApi, type CurrentBoardAccess } from "../api/access";
|
|||
import { agentsApi } from "../api/agents";
|
||||
import { authApi } from "../api/auth";
|
||||
import { projectsApi } from "../api/projects";
|
||||
import { executionWorkspacesApi } from "../api/execution-workspaces";
|
||||
import { useCompany } from "../context/CompanyContext";
|
||||
import { useDialogActions } from "../context/DialogContext";
|
||||
import { usePanel } from "../context/PanelContext";
|
||||
|
|
@ -182,6 +183,7 @@ import {
|
|||
type Agent,
|
||||
type FeedbackVote,
|
||||
type Issue,
|
||||
type IssueRecoveryAction,
|
||||
type IssueAttachment,
|
||||
type IssueComment,
|
||||
type IssueWorkProduct,
|
||||
|
|
@ -290,6 +292,60 @@ export function canBoardResolveRecoveryAction(
|
|||
return membership.membershipRole !== "viewer" && membership.membershipRole !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort client mirror of the backend `runtime:manage` gate that the break-glass override
|
||||
* reconcile (`POST /execution-workspaces/:id/reconcile-branch` in `override` mode) actually
|
||||
* enforces. The server re-checks `runtime:manage` for every reconcile and is authoritative, so
|
||||
* this is defense-in-depth: it hides the "reconcile anyway" affordance from viewers rather than
|
||||
* showing a button that always 403s. For human board members `runtime:manage` grants on the
|
||||
* same non-viewer, active-membership condition as recovery resolution (see
|
||||
* `server/src/services/authorization.ts`), so the shape matches; per-permission-key overrides
|
||||
* are not surfaced to the client and remain the server's call.
|
||||
*/
|
||||
export function canBoardManageRuntime(
|
||||
companyId: string | null | undefined,
|
||||
boardAccess: CurrentBoardAccess | undefined,
|
||||
) {
|
||||
if (!companyId || !boardAccess) return false;
|
||||
if (boardAccess.source === "local_implicit" || boardAccess.isInstanceAdmin) return true;
|
||||
if (!boardAccess.memberships || boardAccess.memberships.length === 0) {
|
||||
return boardAccess.companyIds.includes(companyId);
|
||||
}
|
||||
|
||||
const membership = boardAccess.memberships.find(
|
||||
(item) => item.companyId === companyId && item.status === "active",
|
||||
);
|
||||
if (!membership) return false;
|
||||
return membership.membershipRole !== "viewer" && membership.membershipRole !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The execution workspace a reconcile action should target. The recovery card is rendered from a
|
||||
* specific `workspace_validation` recovery action whose evidence pins the workspace that diverged;
|
||||
* that workspace — not the page-level `issue.executionWorkspaceId` — is the authoritative target.
|
||||
* The page-level id can drift (e.g. a re-issue rebinds the issue to a new workspace) while the card
|
||||
* still shows the older action, so we prefer the action's evidence and only fall back to the
|
||||
* page-level id when the evidence carries no workspace reference.
|
||||
*
|
||||
* The branch-incoherence failure (the one that renders the reconcile-forward / break-glass actions)
|
||||
* records the workspace under `persistedExecutionWorkspaceId`; the not-reusable failure records it
|
||||
* under `executionWorkspaceId`. We accept either key so both divergence shapes pin correctly.
|
||||
*/
|
||||
export function readRecoveryReconcileWorkspaceId(
|
||||
action: IssueRecoveryAction | null | undefined,
|
||||
): string | null {
|
||||
if (!action || action.kind !== "workspace_validation") return null;
|
||||
const workspaceValidation = asRecord(action.evidence?.workspaceValidation);
|
||||
if (!workspaceValidation) return null;
|
||||
const persisted = workspaceValidation.persistedExecutionWorkspaceId;
|
||||
if (typeof persisted === "string" && persisted.length > 0) return persisted;
|
||||
const executionWorkspaceId = workspaceValidation.executionWorkspaceId;
|
||||
if (typeof executionWorkspaceId === "string" && executionWorkspaceId.length > 0) {
|
||||
return executionWorkspaceId;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function shouldScrollIssueDetailToTopOnNavigation(input: {
|
||||
previousIssueId: string | undefined;
|
||||
nextIssueId: string | undefined;
|
||||
|
|
@ -837,6 +893,10 @@ type IssueDetailChatTabProps = {
|
|||
onResolveRecoveryAction?: (outcome: import("../components/IssueRecoveryActionCard").RecoveryResolveOutcome) => void;
|
||||
onReissueIsolatedRecoveryAction?: (request: import("../components/IssueRecoveryActionCard").RecoveryReissueRequest) => void;
|
||||
reissueIsolatedRecoveryActionPending?: boolean;
|
||||
onReconcileForwardRecoveryAction?: () => void;
|
||||
onBreakGlassOverrideRecoveryAction?: (reason: string) => void;
|
||||
canBreakGlassRecoveryAction?: boolean;
|
||||
reconcileRecoveryActionPending?: boolean;
|
||||
canFalsePositiveRecoveryAction?: boolean;
|
||||
legacyRecoverySourceIssue?: {
|
||||
identifier: string | null;
|
||||
|
|
@ -916,6 +976,10 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({
|
|||
onResolveRecoveryAction,
|
||||
onReissueIsolatedRecoveryAction,
|
||||
reissueIsolatedRecoveryActionPending,
|
||||
onReconcileForwardRecoveryAction,
|
||||
onBreakGlassOverrideRecoveryAction,
|
||||
canBreakGlassRecoveryAction,
|
||||
reconcileRecoveryActionPending,
|
||||
canFalsePositiveRecoveryAction,
|
||||
legacyRecoverySourceIssue,
|
||||
comments,
|
||||
|
|
@ -1135,6 +1199,10 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({
|
|||
onResolveRecoveryAction={onResolveRecoveryAction}
|
||||
onReissueIsolatedRecoveryAction={onReissueIsolatedRecoveryAction}
|
||||
reissueIsolatedRecoveryActionPending={reissueIsolatedRecoveryActionPending}
|
||||
onReconcileForwardRecoveryAction={onReconcileForwardRecoveryAction}
|
||||
onBreakGlassOverrideRecoveryAction={onBreakGlassOverrideRecoveryAction}
|
||||
canBreakGlassRecoveryAction={canBreakGlassRecoveryAction}
|
||||
reconcileRecoveryActionPending={reconcileRecoveryActionPending}
|
||||
canFalsePositiveRecoveryAction={canFalsePositiveRecoveryAction}
|
||||
legacyRecoverySourceIssue={legacyRecoverySourceIssue ?? null}
|
||||
companyId={companyId}
|
||||
|
|
@ -1686,6 +1754,9 @@ export function IssueDetail() {
|
|||
&& boardAccess?.companyIds?.includes(selectedCompanyId),
|
||||
);
|
||||
const canResolveBoardRecoveryAction = canBoardResolveRecoveryAction(selectedCompanyId, boardAccess);
|
||||
// The break-glass override reconcile is `runtime:manage`-gated server-side, not gated on the
|
||||
// recovery-resolution permission — so hide its affordance behind the matching client check.
|
||||
const canManageBoardRuntime = canBoardManageRuntime(selectedCompanyId, boardAccess);
|
||||
const { data: feedbackVotes } = useQuery({
|
||||
queryKey: queryKeys.issues.feedbackVotes(issueId!),
|
||||
queryFn: () => issuesApi.listFeedbackVotes(issueId!),
|
||||
|
|
@ -3629,6 +3700,83 @@ export function IssueDetail() {
|
|||
[reissueIsolatedRecoveryAction.mutateAsync],
|
||||
);
|
||||
|
||||
// Actions 1 & 2 (workspace_validation): reconcile the recorded workspace branch to the live one
|
||||
// via the S4 (PAP-1586) op. `forward` is the ancestry-proven safe path (server re-verifies);
|
||||
// `override` is the audited, permission-gated break-glass carrying the operator's reason. Both
|
||||
// resolve the matching recovery action server-side, so the task resumes via the existing flow.
|
||||
const reconcileRecoveryAction = useMutation({
|
||||
// The target workspace id is captured at click time (see the handlers below) and threaded
|
||||
// through as an explicit argument, so the in-flight mutation always reconciles the workspace
|
||||
// the operator saw on the card — never a value re-read from a `issue` snapshot that may have
|
||||
// been refetched to a different `executionWorkspaceId` while the request was pending.
|
||||
mutationFn: async (
|
||||
input:
|
||||
| { workspaceId: string; mode: "forward" }
|
||||
| { workspaceId: string; mode: "override"; reason: string },
|
||||
) => {
|
||||
const { workspaceId, ...body } = input;
|
||||
return executionWorkspacesApi.reconcile(workspaceId, body);
|
||||
},
|
||||
onSuccess: () => {
|
||||
// Refresh the detail card itself (not just the list collections): a successful reconcile
|
||||
// clears the active recovery action, so the card must re-fetch to stop showing stale actions.
|
||||
invalidateIssueDetail();
|
||||
invalidateIssueCollections();
|
||||
pushToast({
|
||||
title: "Workspace branch reconciled",
|
||||
body: "The recorded branch now matches the live branch; the task will resume.",
|
||||
tone: "success",
|
||||
});
|
||||
},
|
||||
onError: (err) => {
|
||||
pushToast({
|
||||
title: "Reconcile failed",
|
||||
body: err instanceof Error ? err.message : "Unable to reconcile the workspace branch.",
|
||||
tone: "error",
|
||||
});
|
||||
},
|
||||
});
|
||||
// Bind the workspace id at the moment the operator clicks, from the same render that produced the
|
||||
// visible recovery card, rather than re-reading it inside the async mutation body. The target is
|
||||
// the workspace pinned by the recovery action's evidence — the workspace that actually diverged —
|
||||
// not the page-level `issue.executionWorkspaceId`, which can drift (e.g. a re-issue rebinds the
|
||||
// issue to a new workspace) while the card still shows the older action. Fall back to the
|
||||
// page-level id only when the action carries no workspace reference.
|
||||
const reconcileExecutionWorkspaceId =
|
||||
readRecoveryReconcileWorkspaceId(issue?.activeRecoveryAction) ?? issue?.executionWorkspaceId ?? null;
|
||||
const handleReconcileForwardRecoveryAction = useCallback(() => {
|
||||
if (!reconcileExecutionWorkspaceId) {
|
||||
pushToast({
|
||||
title: "Reconcile failed",
|
||||
body: "This task has no execution workspace to reconcile.",
|
||||
tone: "error",
|
||||
});
|
||||
return;
|
||||
}
|
||||
void reconcileRecoveryAction.mutateAsync({
|
||||
workspaceId: reconcileExecutionWorkspaceId,
|
||||
mode: "forward",
|
||||
});
|
||||
}, [reconcileExecutionWorkspaceId, reconcileRecoveryAction.mutateAsync, pushToast]);
|
||||
const handleBreakGlassOverrideRecoveryAction = useCallback(
|
||||
(reason: string) => {
|
||||
if (!reconcileExecutionWorkspaceId) {
|
||||
pushToast({
|
||||
title: "Reconcile failed",
|
||||
body: "This task has no execution workspace to reconcile.",
|
||||
tone: "error",
|
||||
});
|
||||
return;
|
||||
}
|
||||
void reconcileRecoveryAction.mutateAsync({
|
||||
workspaceId: reconcileExecutionWorkspaceId,
|
||||
mode: "override",
|
||||
reason,
|
||||
});
|
||||
},
|
||||
[reconcileExecutionWorkspaceId, reconcileRecoveryAction.mutateAsync, pushToast],
|
||||
);
|
||||
|
||||
const treePreviewAffectedIssues = useMemo(
|
||||
() => (treeControlPreview?.issues ?? []).filter((candidate) => !candidate.skipped),
|
||||
[treeControlPreview],
|
||||
|
|
@ -4522,6 +4670,10 @@ export function IssueDetail() {
|
|||
onResolveRecoveryAction={handleResolveRecoveryAction}
|
||||
onReissueIsolatedRecoveryAction={handleReissueIsolatedRecoveryAction}
|
||||
reissueIsolatedRecoveryActionPending={reissueIsolatedRecoveryAction.isPending}
|
||||
onReconcileForwardRecoveryAction={handleReconcileForwardRecoveryAction}
|
||||
onBreakGlassOverrideRecoveryAction={handleBreakGlassOverrideRecoveryAction}
|
||||
canBreakGlassRecoveryAction={canManageBoardRuntime}
|
||||
reconcileRecoveryActionPending={reconcileRecoveryAction.isPending}
|
||||
canFalsePositiveRecoveryAction={canResolveBoardRecoveryAction}
|
||||
legacyRecoverySourceIssue={legacyRecoverySourceIssue}
|
||||
comments={threadComments}
|
||||
|
|
|
|||
Loading…
Reference in New Issue