fix(desktop): survive rejected send in permission retransmit loop

The retransmit orchestrator installed its listener and interval, then
awaited the first send outside the loop's settle path. A rejected first
send propagated to the caller with the loop never torn down: the
listener and interval stayed live firing unhandled `void send()`, while
both UI callers re-enabled the buttons — so a retry with the opposite
option left two loops retransmitting different optionIds under one nonce.

Route the first send through the same guarded transmit path as every
tick and swallow a rejected send: a rejection is the transport failure
this loop exists to survive, so the next tick retries. The orchestrator
never rejects, guaranteeing exactly one loop per click and no unhandled
promise on any path. Permanent send failure ends like silence — retry
until `expiresAt`, then the card fails closed.

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 13:16:54 -04:00
parent 111bc91de5
commit 247f897d74
4 changed files with 92 additions and 24 deletions

View File

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

View File

@@ -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;
}

View File

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

View File

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