diff --git a/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs b/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs index ae526699f3..7574e4fdda 100644 --- a/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs +++ b/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs @@ -383,3 +383,129 @@ test("test_expiry_disables_buttons_after_clock_tick", async () => { "timed-out message shown after expiry", ); }); + +// ── Delivery-outcome recovery test (Carl's P1 regression) ───────────────────── +// +// Mutation target: `permission-request-card.tsx` line +// `if (outcome === "failed") setSubmitted(null);` +// Removing that line leaves the card permanently on "Decision sent" after a +// harness routing failure, and this test catches it while the orchestrator +// suite stays green. The test uses the `_deliveryFn` seam to control the +// outcome without a real relay. + +test("test_failed_delivery_re_enables_buttons_and_successful_retry_reaches_sent", async () => { + // Build a controllable delivery function whose outcome we resolve manually. + // Each call returns a fresh promise; `resolveDelivery` settles the most + // recently created one. + let resolveDelivery; + function makeDeliveryFn() { + return (..._args) => + new Promise((resolve) => { + resolveDelivery = resolve; + }); + } + + const { createElement, act } = await import("react"); + const { render, fireEvent } = await import("@testing-library/react"); + const { QueryClientProvider } = await import("@tanstack/react-query"); + const { PermissionRequestCardBlock } = await import( + "./PermissionRequestCardBlock.tsx" + ); + + const qc = await makeQueryClient(OWNER_PUBKEY); + + let container; + await act(async () => { + ({ container } = render( + createElement( + QueryClientProvider, + { client: qc }, + createElement(PermissionRequestCardBlock, { + message: makeMessage({ + content: makePendingContent(), + signerPubkey: AGENT_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }), + isKnownAgentPubkey: makeIsKnownAgentPubkey(AGENT_PUBKEY), + channelId: CHANNEL_ID, + _deliveryFn: makeDeliveryFn(), + }), + ), + )); + }); + + // ── Step 1: initial render shows action buttons ────────────────────────── + const allowBtnInitial = container.querySelector( + '[data-testid="permission-decision-opt-allow"]', + ); + assert.ok(allowBtnInitial !== null, "buttons present before any click"); + + // ── Step 2: click — card shows "Decision sent" ──────────────────────────── + await act(async () => { + fireEvent.click(allowBtnInitial); + }); + assert.ok( + container.textContent?.includes("Decision sent"), + "card shows Decision sent after click", + ); + assert.equal( + container.querySelector('[data-testid="permission-decision-opt-allow"]'), + null, + "buttons hidden while decision is in flight", + ); + + // ── Step 3: delivery resolves "failed" → buttons must return ───────────── + // This is Carl's regression: without `if (outcome === "failed") setSubmitted(null)` + // the card stays stuck on "Decision sent" and the owner cannot retry. + await act(async () => { + resolveDelivery("failed"); + // Drain microtasks so React processes the state update. + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + const allowBtnAfterFail = container.querySelector( + '[data-testid="permission-decision-opt-allow"]', + ); + assert.ok( + allowBtnAfterFail !== null, + "buttons re-enabled after failed delivery — owner can retry", + ); + assert.ok( + !container.textContent?.includes("Decision sent"), + "Decision sent text cleared after failed delivery", + ); + + // ── Step 4: retry click → delivery resolves "acked" → terminal sent ─────── + // No re-render needed. `_deliveryFn` is the closure returned by `makeDeliveryFn()`. + // Each invocation of that closure creates a fresh promise and updates `resolveDelivery`, + // so clicking the re-enabled button starts a new delivery loop via the same seam. + await act(async () => { + fireEvent.click(allowBtnAfterFail); + }); + // Card is back to "Decision sent" for the second attempt + assert.ok( + container.textContent?.includes("Decision sent"), + "Decision sent shown during second delivery attempt", + ); + + // Resolve the second delivery as "acked" → terminal state + await act(async () => { + resolveDelivery("acked"); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // Buttons stay hidden — "acked" is terminal, card awaits harness kind-40003 edit + assert.equal( + container.querySelector('[data-testid="permission-decision-opt-allow"]'), + null, + "buttons stay hidden after acked — waiting for harness resolution", + ); + assert.ok( + container.textContent?.includes("Decision sent"), + "Decision sent persists after acked — terminal state", + ); +}); diff --git a/desktop/src/features/messages/ui/PermissionRequestCardBlock.tsx b/desktop/src/features/messages/ui/PermissionRequestCardBlock.tsx index 618d6dcd5d..60af78a314 100644 --- a/desktop/src/features/messages/ui/PermissionRequestCardBlock.tsx +++ b/desktop/src/features/messages/ui/PermissionRequestCardBlock.tsx @@ -8,6 +8,7 @@ */ import * as React from "react"; +import type { startPermissionDecisionDelivery } from "@/features/agents/lib/permissionDecisionDelivery"; import type { TimelineMessage } from "@/features/messages/types"; import { getPermissionRequestAgentPubkey } from "@/features/messages/ui/permissionRequestAuthPubkey"; import { useIdentityQuery } from "@/shared/api/hooks"; @@ -23,6 +24,13 @@ export type PermissionRequestCardBlockProps = { isKnownAgentPubkey: (pubkey: string) => boolean; /** Channel ID for routing the decision click; falsy → no card. */ channelId: string | null | undefined; + /** + * Delivery function injected by tests to control the outcome without a real + * relay. Production callers omit this. + * + * @internal — test seam only; not part of the public API. + */ + _deliveryFn?: typeof startPermissionDecisionDelivery; }; export const PermissionRequestCardBlock = React.memo( @@ -30,6 +38,7 @@ export const PermissionRequestCardBlock = React.memo( message, isKnownAgentPubkey, channelId, + _deliveryFn, }: PermissionRequestCardBlockProps) { const identityQuery = useIdentityQuery(); const viewerPubkey = identityQuery.data?.pubkey; @@ -68,6 +77,7 @@ export const PermissionRequestCardBlock = React.memo( channelId={channelId} isOwner={isOwner} request={request} + _deliveryFn={_deliveryFn} /> ); @@ -75,5 +85,6 @@ export const PermissionRequestCardBlock = React.memo( (prev, next) => prev.message === next.message && prev.isKnownAgentPubkey === next.isKnownAgentPubkey && - prev.channelId === next.channelId, + prev.channelId === next.channelId && + prev._deliveryFn === next._deliveryFn, ); diff --git a/desktop/src/shared/ui/permission-request-card.tsx b/desktop/src/shared/ui/permission-request-card.tsx index 1d3dc64dd4..df31c6d5b4 100644 --- a/desktop/src/shared/ui/permission-request-card.tsx +++ b/desktop/src/shared/ui/permission-request-card.tsx @@ -48,6 +48,14 @@ export type PermissionRequestCardProps = { * Absent or false → read-only card (buttons suppressed). */ isOwner?: boolean; + /** + * Delivery function injected by tests so component tests can control + * the outcome without a real relay. Production callers omit this and + * get `startPermissionDecisionDelivery` by default. + * + * @internal — test seam only; not part of the public API. + */ + _deliveryFn?: typeof startPermissionDecisionDelivery; }; /** @@ -99,12 +107,18 @@ function PermissionButtons({ channelId, request, nowSecs, + deliveryFn = startPermissionDecisionDelivery, }: { agentPubkey: string; channelId: string; request: PermissionRequestPending; /** Current time in seconds (driven by a parent ticking state). */ nowSecs: number; + /** + * Delivery function — defaults to `startPermissionDecisionDelivery`. + * Injected by tests to control the outcome without a real relay. + */ + deliveryFn?: typeof startPermissionDecisionDelivery; }) { const [submitted, setSubmitted] = React.useState(null); @@ -150,7 +164,7 @@ function PermissionButtons({ // decision on a card that expired between renders. if (request.expiresAt <= Date.now() / 1000) return; setSubmitted(optionId); - void startPermissionDecisionDelivery({ + void deliveryFn({ agentPubkey, channelId, requestNonce: request.requestNonce, @@ -215,6 +229,7 @@ export function PermissionRequestCard({ agentPubkey, channelId, isOwner, + _deliveryFn, }: PermissionRequestCardProps) { if (request.state === "resolved") { const resolvedLabel = outcomeLabel( @@ -255,6 +270,7 @@ export function PermissionRequestCard({ className={className} isOwner={isOwner} request={request} + deliveryFn={_deliveryFn} /> ); } @@ -269,12 +285,14 @@ function PendingPermissionRequestCard({ agentPubkey, channelId, isOwner, + deliveryFn, }: { className?: string; request: PermissionRequestPending; agentPubkey: string; channelId: string; isOwner?: boolean; + deliveryFn?: typeof startPermissionDecisionDelivery; }) { const [nowSecs, setNowSecs] = React.useState(() => Date.now() / 1000); @@ -314,6 +332,7 @@ function PendingPermissionRequestCard({ channelId={channelId} nowSecs={nowSecs} request={request} + deliveryFn={deliveryFn} /> ) : (