fix(desktop): propagate control_result status; failure re-enables card for retry

retransmitPermissionDecision settled "acked" on any matching control_result
frame without reading frame.status. The four failure statuses (no_active_turn,
channel_full, channel_closed, no_channel) mean the harness received the frame
but could not route the decision — retransmitting the same nonce cannot change
that. The loop now resolves "failed" for those statuses and "acked" only for
"sent" / "already_decided", so the card re-enables for owner retry on failure.

Thread card (permission-request-card.tsx): .then checks the outcome and clears
submitted on "failed" so the owner can retry. LifecycleActivity fast-path does
the same before the reducer deliveryFailed path fires. Comments in both callers
updated to reflect the non-throwing-but-tri-outcome contract.

A11y: status text nodes ("Decision sent" / "Timed out" in thread card) now carry
role="status" / aria-live="polite" so AT hears the async confirmation.

NIP-AO: adds already_decided to the permission_decision status set with one line
of meaning; updates the failure-status sentence to name the retry semantics.

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 16:10:37 -04:00
parent 247f897d74
commit 77f74405e8
6 changed files with 152 additions and 32 deletions

View File

@@ -41,9 +41,10 @@ export function resolveDecisionDeadlineSecs(
* the pure {@link retransmitPermissionDecision} orchestrator.
*
* Fire-and-forget from the caller's view: the returned promise resolves when
* the harness acknowledges the nonce or the card's deadline passes. The card's
* UI reaction (resolve / retry) is driven separately by the `control_result`
* reducer path, so callers need not await this.
* the harness acknowledges the nonce, the harness returns an authoritative
* failure (the card re-enables for retry), or the card's deadline passes.
* The card's UI reaction (resolve / retry) is driven separately by the
* `control_result` reducer path, so callers need not await this.
*/
export function startPermissionDecisionDelivery({
agentPubkey,
@@ -57,7 +58,7 @@ export function startPermissionDecisionDelivery({
requestNonce: string;
optionId: string;
deadlineSecs: number;
}): Promise<"acked" | "expired"> {
}): Promise<"acked" | "expired" | "failed"> {
return retransmitPermissionDecision({
requestNonce,
send: () =>

View File

@@ -204,3 +204,77 @@ test("retransmitPermissionDecision expires cleanly when every send rejects", asy
}
assert.deepEqual(unhandled, [], "rejected sends must not surface unhandled");
});
test("retransmitPermissionDecision resolves failed on a negative control_result status", async () => {
// Carl's regression: a failure status (no_active_turn / channel_full /
// channel_closed / no_channel) means the harness answered authoritatively but
// could not route the decision. The loop must stop retransmitting (re-sending
// the same nonce cannot change the routing refusal) and resolve "failed" so
// the card can re-enable for owner retry.
for (const status of [
"no_active_turn",
"channel_full",
"channel_closed",
"no_channel",
]) {
const h = harness();
await drainMicrotasks();
assert.equal(h.sendCalls, 1, `first send fired (${status})`);
h.push(frame({ status }));
assert.equal(
await h.outcome,
"failed",
`negative status "${status}" must resolve "failed"`,
);
assert.equal(h.unsubscribeCalls, 1, `listener torn down on "${status}"`);
assert.equal(
h.cancelRetransmitCalls,
1,
`scheduler torn down on "${status}"`,
);
// A tick after failure must NOT resend — the loop has settled.
h.tick();
await drainMicrotasks();
assert.equal(
h.sendCalls,
1,
`no resend after failure settle on "${status}"`,
);
}
});
test("retransmitPermissionDecision: negative result then retry delivers acked", async () => {
// Carl's exact regression: a failure reply leaves the card actionable, the
// owner retries (new harness() = fresh loop), and the second attempt succeeds.
const first = harness();
await drainMicrotasks();
first.push(frame({ status: "no_active_turn" }));
assert.equal(await first.outcome, "failed");
// Owner retries — fresh orchestrator instance.
const second = harness();
await drainMicrotasks();
second.push(frame({ status: "sent" }));
assert.equal(await second.outcome, "acked");
assert.equal(second.unsubscribeCalls, 1);
assert.equal(second.cancelRetransmitCalls, 1);
});
test("retransmitPermissionDecision: frame after failure settlement is inert", async () => {
// A late duplicate `control_result` for the same nonce arriving after the
// loop has already settled (via a failure status) must not re-resolve.
const h = harness();
await drainMicrotasks();
h.push(frame({ status: "no_active_turn" }));
assert.equal(await h.outcome, "failed");
// The unsubscribe detaches the listener, so a late push is dropped.
// Verify the loop doesn't try to double-resolve or re-enable a second loop.
h.push(frame({ status: "sent" })); // arrives after settle — inert
h.tick(); // tick after settle must not resend
await drainMicrotasks();
assert.equal(h.sendCalls, 1, "no resend after settled failure");
assert.equal(h.unsubscribeCalls, 1, "listener detached exactly once");
});

View File

@@ -13,13 +13,16 @@ import type { ControlResultFrame } from "@/shared/api/types";
* fail-closed timeout even though the owner decided promptly.
*
* This orchestrator resends the decision on a fixed cadence until it observes a
* `control_result` for THIS nonce, then stops. Any matching frame settles the
* loop — the harness has an authoritative answer (`sent` / `already_decided` =
* applied; a failure status is handled by the card's retry path), and resending
* cannot change it. If no reply arrives before the card's own `expiresAt`
* deadline the loop resolves `"expired"`: the card times out on its own and a
* decision applied past expiry would be rejected anyway, so retransmitting past
* it is pointless.
* `control_result` for THIS nonce, then stops. The outcome depends on the frame's
* status: `sent` and `already_decided` mean the harness routed or already applied
* the decision — the loop resolves `"acked"`. The four failure statuses
* (`no_active_turn`, `channel_full`, `channel_closed`, `no_channel`) mean the
* harness received the frame but could not route it — the loop resolves
* `"failed"`, stopping retransmission (re-sending the same nonce cannot change an
* authoritative routing refusal), and the card returns to the actionable state for
* owner retry. If no reply arrives before the card's own `expiresAt` deadline the
* loop resolves `"expired"`: the card times out on its own and a decision applied
* past expiry would be rejected anyway, so retransmitting past it is pointless.
*
* A nonce guard scopes the settling frame to this exact decision: a replayed or
* concurrent `control_result` for a different card carries a different nonce and
@@ -60,11 +63,11 @@ export function retransmitPermissionDecision({
* retransmit so the loop never resends past expiry.
*/
deadlineReached: () => boolean;
}): Promise<"acked" | "expired"> {
return new Promise<"acked" | "expired">((resolve) => {
}): Promise<"acked" | "expired" | "failed"> {
return new Promise<"acked" | "expired" | "failed">((resolve) => {
let unsubscribe = () => {};
let cancelRetransmit = () => {};
const finish = (outcome: "acked" | "expired") => {
const finish = (outcome: "acked" | "expired" | "failed") => {
cancelRetransmit();
unsubscribe();
resolve(outcome);
@@ -90,7 +93,13 @@ export function retransmitPermissionDecision({
) {
return;
}
finish("acked");
// `sent` and `already_decided` are both success: the harness routed or
// has already applied the decision. The four failure statuses indicate the
// harness received the frame but could not route it — retransmitting the
// same nonce cannot change that, so stop and let the card retry.
const success =
frame.status === "sent" || frame.status === "already_decided";
finish(success ? "acked" : "failed");
});
cancelRetransmit = scheduleRetransmit(transmit);

View File

@@ -152,12 +152,22 @@ function PermissionDecisionButtons({
requestNonce,
optionId,
deadlineSecs,
}).catch(() => {
// 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);
});
})
.then((outcome) => {
// `"failed"` means the harness received the frame but could
// not route it — re-enable so the user can retry. The reducer
// `deliveryFailed` path also re-enables via the `control_result`
// frame; this fast path handles the case before the reducer
// fires. `"acked"` / `"expired"` are terminal; the transcript
// item updates via the observer relay and no retry is needed.
if (outcome === "failed") setPending(null);
})
.catch(() => {
// The delivery loop never rejects — it resolves one of
// "acked" | "expired" | "failed". This branch guards against
// any unexpected error and re-enables for safety.
setPending(null);
});
}}
>
{pending === optionId ? "…" : displayLabel}

View File

@@ -112,13 +112,25 @@ function PermissionButtons({
if (expired) {
return (
<div className="mt-1.5 text-xs text-muted-foreground">Timed out</div>
<div
aria-live="polite"
className="mt-1.5 text-xs text-muted-foreground"
role="status"
>
Timed out
</div>
);
}
if (submitted !== null) {
return (
<div className="mt-1.5 text-xs text-muted-foreground">Decision sent</div>
<div
aria-live="polite"
className="mt-1.5 text-xs text-muted-foreground"
role="status"
>
Decision sent
</div>
);
}
@@ -144,12 +156,22 @@ function PermissionButtons({
requestNonce: request.requestNonce,
optionId,
deadlineSecs: request.expiresAt,
}).catch(() => {
// 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);
});
})
.then((outcome) => {
// `"failed"` means the harness received the frame but could
// not route it (no_active_turn / channel_full / etc.) —
// re-enable so the owner can retry. `"acked"` and `"expired"`
// are terminal: the harness applied the decision or the card
// timed out; the card transitions away via the kind-40003 edit
// or expiry countdown and no retry is needed.
if (outcome === "failed") setSubmitted(null);
})
.catch(() => {
// The delivery loop never rejects — it resolves one of
// "acked" | "expired" | "failed". This branch guards against
// any unexpected error and re-enables for safety.
setSubmitted(null);
});
}}
>
{label}

View File

@@ -284,15 +284,19 @@ event to confirm receipt. This is an `acp_read`-style telemetry frame (kind =
```json
{
"type": "permission_decision",
"status": "sent" | "no_active_turn" | "channel_full" | "channel_closed" | "no_channel",
"status": "sent" | "already_decided" | "no_active_turn" | "channel_full" | "channel_closed" | "no_channel",
"requestNonce": "<nonce>",
"optionId": "<optionId>"
}
```
`status: "sent"` means the decision was delivered to the in-flight read loop.
Other statuses indicate delivery failure; the per-request timeout will fail the
entry closed.
`status: "already_decided"` means the nonce was already applied by a prior delivery
(a retransmit reached the harness after the first copy was accepted); treat it as
success. The four remaining statuses (`no_active_turn`, `channel_full`,
`channel_closed`, `no_channel`) indicate delivery failure — the harness received
the frame but could not route the decision; the desktop should re-enable the card
so the owner can retry.
## Ephemerality Contract