test(desktop): add delivery-seam + component regression test for failed-outcome recovery

Thufir's mutation showed that deleting the `outcome === "failed"` clear
in `permission-request-card.tsx` left all 7 PermissionRequestCardBlock
jsdom tests green, meaning Carl's P1 regression (stuck Decision sent, no
retry path) could silently return.

Three files changed, zero production behavior change:

- `permission-request-card.tsx`: add `_deliveryFn` seam prop (defaults to
  `startPermissionDecisionDelivery`); threaded through PermissionRequestCard
  → PendingPermissionRequestCard → PermissionButtons. Matches the existing
  isKnownAgentPubkey injection pattern.
- `PermissionRequestCardBlock.tsx`: accept and forward `_deliveryFn`; added
  to memo equality check.
- `PermissionRequestCardBlock.test.mjs`: new 8th test
  `test_failed_delivery_re_enables_buttons_and_successful_retry_reaches_sent`
  — renders the card, clicks allow, resolves delivery as "failed", asserts
  buttons return and Decision sent clears, retries, resolves "acked", asserts
  terminal sent state with buttons hidden.

Mutation proof: deleting `if (outcome === "failed") setSubmitted(null)`
fails the new test at "buttons re-enabled after failed delivery"; existing
7 remain green.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Duncan
2026-08-28 18:14:16 -04:00
parent ecd441ba8c
commit 9301abf030
3 changed files with 158 additions and 2 deletions

View File

@@ -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",
);
});

View File

@@ -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}
/>
</AttachmentGroup>
);
@@ -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,
);

View File

@@ -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<string | null>(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}
/>
) : (
<div className="mt-1 text-xs text-muted-foreground">