diff --git a/desktop/src/features/agents/lib/retransmitPermissionDecision.test.mjs b/desktop/src/features/agents/lib/retransmitPermissionDecision.test.mjs index 667fac6732..b901b9d49f 100644 --- a/desktop/src/features/agents/lib/retransmitPermissionDecision.test.mjs +++ b/desktop/src/features/agents/lib/retransmitPermissionDecision.test.mjs @@ -17,20 +17,26 @@ function frame(overrides = {}) { /** * Controllable harness mirroring the real wiring: a single-listener pub/sub * whose unsubscribe genuinely detaches, a manual retransmit tick, a manual - * deadline flag, and a send counter. + * deadline flag, and a send counter. `failSends` rejects the first N send + * attempts, mirroring a socket that is briefly down. */ -function harness({ nonce = NONCE } = {}) { +function harness({ nonce = NONCE, failSends = 0 } = {}) { let listener = null; let tickCb = null; let unsubscribeCalls = 0; let cancelRetransmitCalls = 0; let sendCalls = 0; let expired = false; + let remainingFailures = failSends; const outcome = retransmitPermissionDecision({ requestNonce: nonce, send: () => { sendCalls += 1; + if (remainingFailures > 0) { + remainingFailures -= 1; + return Promise.reject(new Error("send failed: socket down")); + } return Promise.resolve(); }, subscribe: (fn) => { @@ -144,3 +150,57 @@ test("retransmitPermissionDecision ignores a control_result for a different nonc h.push(frame()); assert.equal(await h.outcome, "acked"); }); + +test("retransmitPermissionDecision survives a rejected first send and acks when a later tick's send resolves", async () => { + // The causal case: the owner clicks while the relay socket is down. The first + // send rejects, but the loop must stay live so a later retransmit — once the + // socket recovers — delivers and acks. + const unhandled = []; + const onUnhandled = (reason) => unhandled.push(reason); + process.on("unhandledRejection", onUnhandled); + try { + const h = harness({ failSends: 1 }); + await drainMicrotasks(); + assert.equal(h.sendCalls, 1, "first send fired and rejected"); + + // A later tick, after the socket recovers, resends successfully. + h.tick(); + await drainMicrotasks(); + assert.equal(h.sendCalls, 2, "the loop retries after a rejected send"); + + h.push(frame()); + assert.equal(await h.outcome, "acked"); + assert.equal(h.unsubscribeCalls, 1); + assert.equal(h.cancelRetransmitCalls, 1); + } finally { + process.off("unhandledRejection", onUnhandled); + } + assert.deepEqual(unhandled, [], "a rejected send must not surface unhandled"); +}); + +test("retransmitPermissionDecision expires cleanly when every send rejects", async () => { + const unhandled = []; + const onUnhandled = (reason) => unhandled.push(reason); + process.on("unhandledRejection", onUnhandled); + try { + // Every send rejects (permanent transport failure). The loop must not throw + // — it keeps retrying until the deadline, then resolves "expired". + const h = harness({ failSends: Infinity }); + await drainMicrotasks(); + assert.equal(h.sendCalls, 1, "first send fired and rejected"); + + h.tick(); + await drainMicrotasks(); + assert.equal(h.sendCalls, 2, "keeps retrying through rejection"); + + h.expire(); + h.tick(); + assert.equal(await h.outcome, "expired"); + assert.equal(h.sendCalls, 2, "no resend past the deadline"); + assert.equal(h.unsubscribeCalls, 1, "listener torn down at expiry"); + assert.equal(h.cancelRetransmitCalls, 1, "scheduler torn down at expiry"); + } finally { + process.off("unhandledRejection", onUnhandled); + } + assert.deepEqual(unhandled, [], "rejected sends must not surface unhandled"); +}); diff --git a/desktop/src/features/agents/lib/retransmitPermissionDecision.ts b/desktop/src/features/agents/lib/retransmitPermissionDecision.ts index 079583656c..9192adb1f7 100644 --- a/desktop/src/features/agents/lib/retransmitPermissionDecision.ts +++ b/desktop/src/features/agents/lib/retransmitPermissionDecision.ts @@ -25,11 +25,19 @@ import type { ControlResultFrame } from "@/shared/api/types"; * concurrent `control_result` for a different card carries a different nonce and * is inert, mirroring the `requestId` guard in `awaitLiveSwitchOutcome`. * + * A send rejection is the transport failure this loop exists to survive, so the + * orchestrator never throws: the first send and every retransmit run through + * the same guarded path, and a rejected attempt is swallowed — the next tick + * retries. This guarantees exactly one loop per click (the UI's disabled state + * guards double-click) and no unhandled rejection on any path. Permanent send + * failure therefore ends the same way as silence: the loop retries until + * `expiresAt`, then resolves `"expired"` and the card fails closed. + * * The loop is isolated from React and the relay so it can be unit tested with a * fake clock and synthetic frames. The caller injects the send, the * `control_result` subscription, and the retransmit scheduler. */ -export async function retransmitPermissionDecision({ +export function retransmitPermissionDecision({ requestNonce, send, subscribe, @@ -53,7 +61,7 @@ export async function retransmitPermissionDecision({ */ deadlineReached: () => boolean; }): Promise<"acked" | "expired"> { - const settled = new Promise<"acked" | "expired">((resolve) => { + return new Promise<"acked" | "expired">((resolve) => { let unsubscribe = () => {}; let cancelRetransmit = () => {}; const finish = (outcome: "acked" | "expired") => { @@ -61,6 +69,16 @@ export async function retransmitPermissionDecision({ unsubscribe(); resolve(outcome); }; + // Attempt one transmit, respecting the deadline and swallowing a rejected + // send. A rejection is a failed transmit, not a fatal error: the next tick + // retries, so the loop survives a socket that is briefly down. + const transmit = () => { + if (deadlineReached()) { + finish("expired"); + return; + } + void send().catch(() => {}); + }; unsubscribe = subscribe((frame) => { // A `control_result` for THIS nonce means the harness received the // decision and has an authoritative answer — stop retransmitting. Frames @@ -74,19 +92,9 @@ export async function retransmitPermissionDecision({ } finish("acked"); }); - cancelRetransmit = scheduleRetransmit(() => { - // Never resend past the card's expiry: it times out on its own and a - // late decision would be rejected. Stop and report the deadline. - if (deadlineReached()) { - finish("expired"); - return; - } - void send(); - }); + cancelRetransmit = scheduleRetransmit(transmit); + + // Fire the first send immediately, then let the scheduler drive resends. + transmit(); }); - - // Fire the first send immediately, then let the scheduler drive resends. - await send(); - - return settled; } diff --git a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx index 548700550b..278b36efb4 100644 --- a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx +++ b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx @@ -153,9 +153,9 @@ function PermissionDecisionButtons({ optionId, deadlineSecs, }).catch(() => { - // First send rejected by the relay. Re-enable so the user can - // retry; the harness's 300 s fail-closed timeout handles - // permanent loss. + // The delivery loop never rejects on a failed send — it retries + // until ack or the card's 300 s fail-closed expiry. This only + // fires on an unexpected error; re-enable so the user can retry. setPending(null); }); }} diff --git a/desktop/src/shared/ui/permission-request-card.tsx b/desktop/src/shared/ui/permission-request-card.tsx index df1ca3a731..2283b48f7c 100644 --- a/desktop/src/shared/ui/permission-request-card.tsx +++ b/desktop/src/shared/ui/permission-request-card.tsx @@ -145,9 +145,9 @@ function PermissionButtons({ optionId, deadlineSecs: request.expiresAt, }).catch(() => { - // First send rejected by the relay. Re-enable so the user - // can retry; the retransmit loop only starts once the send - // resolves. + // The delivery loop never rejects on a failed send — it + // retries until ack or expiry. This only fires on an + // unexpected error; re-enable so the user can retry. setSubmitted(null); }); }}