diff --git a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs index 35c3490310..587a38703b 100644 --- a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs +++ b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs @@ -517,6 +517,249 @@ test("test_f3_cross_layer_four_options_acp_read_to_lifecycle_activity_two_button ); }); +// --------------------------------------------------------------------------- +// Cross-layer reducer+mounted regression: channel_full leaves buttons disabled +// +// Proves that the full pipeline — acp_read → buildTranscript reducer → +// LifecycleActivity component — correctly leaves both buttons DISABLED after +// a `channel_full` control_result, matching the "transient, retransmit +// orchestrator keeps going" contract. +// +// The companion case proves authoritative failures (`no_active_turn`) DO +// re-enable buttons — so the effect path is also covered. +// +// Mutation proof: restoring `channel_full` to increment `deliveryFailed` in +// `handlePermissionDecisionResult` → the card acquires `deliveryFailed: 1` → +// the component re-renders with `deliveryFailed={1}` → the useEffect fires → +// `setPending(null)` re-enables both buttons → the disabled assertion fails. +// --------------------------------------------------------------------------- + +test("test_channel_full_reducer_to_component_buttons_stay_disabled", async () => { + const { createElement, act } = await import("react"); + const { render, fireEvent } = await import("@testing-library/react"); + + const FAKE_NOW_SECS = Math.floor(FAKE_NOW_MS / 1000); + const FUTURE_EXPIRY = FAKE_NOW_SECS + 9_999_999; + const nonce = "nonce-cross-layer-cf"; + + // Base acp_read event. + const acpReadEvent = { + seq: 1, + timestamp: "2026-09-01T10:00:00.000Z", + kind: "acp_read", + agentIndex: 0, + channelId: "ch-cross-cf", + sessionId: "sess-cross-cf", + turnId: "turn-cross-cf", + payload: { + jsonrpc: "2.0", + id: "req-cross-cf", + method: "session/request_permission", + params: { + title: "Tool requires approval", + toolCallId: "tc-cross-cf", + options: [ + { + optionId: "opt-allow-once", + kind: "allow_once", + name: "Allow once", + }, + { optionId: "opt-reject-once", kind: "reject_once", name: "Deny" }, + ], + }, + }, + authorization: { + requestNonce: nonce, + actionable: true, + expiresAt: FUTURE_EXPIRY, + }, + }; + + // `channel_full` control_result — transient; must NOT set deliveryFailed. + const channelFullResult = { + seq: 2, + timestamp: "2026-09-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-cross-cf", + sessionId: "sess-cross-cf", + turnId: "turn-cross-cf", + payload: { + type: "permission_decision", + status: "channel_full", + requestNonce: nonce, + optionId: "opt-allow-once", + }, + }; + + // `no_active_turn` control_result — authoritative failure; MUST set deliveryFailed. + const authoritativeFailure = { + seq: 2, + timestamp: "2026-09-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-cross-cf", + sessionId: "sess-cross-cf", + turnId: "turn-cross-cf", + payload: { + type: "permission_decision", + status: "no_active_turn", + requestNonce: nonce, + optionId: "opt-allow-once", + }, + }; + + // Build both card states through the real transcript reducer. + const cardAfterChannelFull = buildTranscript([ + acpReadEvent, + channelFullResult, + ]).find((i) => i.renderClass === "permission"); + const cardAfterAuthoritativeFailure = buildTranscript([ + acpReadEvent, + authoritativeFailure, + ]).find((i) => i.renderClass === "permission"); + assert.ok(cardAfterChannelFull, "card must exist after channel_full"); + assert.ok( + cardAfterAuthoritativeFailure, + "card must exist after authoritative failure", + ); + + // Reducer-level gate: channel_full must NOT set deliveryFailed. + assert.equal( + cardAfterChannelFull.deliveryFailed, + undefined, + "channel_full must not set deliveryFailed in the reducer (mutation: restoring increment → 1 here → test fails)", + ); + // Reducer-level gate: no_active_turn MUST set deliveryFailed. + assert.equal( + cardAfterAuthoritativeFailure.deliveryFailed, + 1, + "no_active_turn must set deliveryFailed in the reducer", + ); + + // ── Component: channel_full → buttons stay disabled ─────────────────────── + // Start with the initial card (no deliveryFailed), click Allow to set pending. + const initialCard = buildTranscript([acpReadEvent]).find( + (i) => i.renderClass === "permission", + ); + + // Track delivery calls to ensure no second delivery is started. + const deliveryCalls = []; + // The first delivery is intentionally stalled — never resolves. + function stalledDelivery({ optionId }) { + deliveryCalls.push(optionId); + return new Promise(() => {}); + } + + let container, rerender; + await act(async () => { + ({ container, rerender } = render( + createElement(LifecycleActivity, { + ...BASE_PROPS, + item: initialCard, + _deliveryFn: stalledDelivery, + }), + )); + }); + + // Click Allow — sets pending, disables both buttons. + const allowBtn = container.querySelector( + '[data-testid="permission-decision-opt-allow-once"]', + ); + assert.ok(allowBtn, "allow_once button must be present before click"); + await act(async () => { + fireEvent.click(allowBtn); + await Promise.resolve(); + }); + assert.equal(deliveryCalls.length, 1, "first delivery must fire on click"); + + // Now rerender with the post-channel_full card (deliveryFailed undefined). + // The useEffect must NOT fire (deliveryFailed didn't change), so pending stays + // set and both buttons remain disabled. + await act(async () => { + rerender( + createElement(LifecycleActivity, { + ...BASE_PROPS, + item: cardAfterChannelFull, + _deliveryFn: stalledDelivery, + }), + ); + await Promise.resolve(); + }); + + const allowBtnAfterCF = container.querySelector( + '[data-testid="permission-decision-opt-allow-once"]', + ); + const denyBtnAfterCF = container.querySelector( + '[data-testid="permission-decision-opt-reject-once"]', + ); + assert.ok(allowBtnAfterCF, "allow button must still be in DOM"); + assert.ok(denyBtnAfterCF, "deny button must still be in DOM"); + assert.ok( + allowBtnAfterCF.disabled, + "allow button must remain DISABLED after channel_full (mutation: increment deliveryFailed → setPending(null) fires → button enabled → this fails)", + ); + assert.ok( + denyBtnAfterCF.disabled, + "deny button must remain DISABLED after channel_full — both buttons stay disabled during automatic retry", + ); + assert.equal( + deliveryCalls.length, + 1, + "no second delivery must start after channel_full — retransmit orchestrator handles resend, not a second click", + ); + + // ── Companion: authoritative failure re-enables buttons ─────────────────── + // Render a fresh card, click, then rerender with deliveryFailed: 1. + const deliveryCalls2 = []; + function stalledDelivery2({ optionId }) { + deliveryCalls2.push(optionId); + return new Promise(() => {}); + } + + let container2, rerender2; + await act(async () => { + ({ container: container2, rerender: rerender2 } = render( + createElement(LifecycleActivity, { + ...BASE_PROPS, + item: initialCard, + _deliveryFn: stalledDelivery2, + }), + )); + }); + + const allowBtn2 = container2.querySelector( + '[data-testid="permission-decision-opt-allow-once"]', + ); + assert.ok(allowBtn2, "allow button must be present for companion case"); + await act(async () => { + fireEvent.click(allowBtn2); + await Promise.resolve(); + }); + + // Rerender with authoritative failure card (deliveryFailed: 1). + // useEffect sees deliveryFailed change 0→1 → setPending(null) → buttons enabled. + await act(async () => { + rerender2( + createElement(LifecycleActivity, { + ...BASE_PROPS, + item: cardAfterAuthoritativeFailure, + _deliveryFn: stalledDelivery2, + }), + ); + await Promise.resolve(); + await Promise.resolve(); + }); + + const allowBtnAfterFail = container2.querySelector( + '[data-testid="permission-decision-opt-allow-once"]', + ); + assert.ok( + !allowBtnAfterFail.disabled, + "allow button must be RE-ENABLED after no_active_turn — user can retry", + ); +}); + // --------------------------------------------------------------------------- // F3 interactive delivery-seam: acp_read → buildTranscript → LifecycleActivity // click buttons → assert _deliveryFn called with ruled allow_once/reject_once IDs diff --git a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx index 3e74dcd735..1be598ee1e 100644 --- a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx +++ b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx @@ -90,8 +90,11 @@ function PermissionDecisionButtons({ requestNonce: string; /** * Monotonically increasing failure token from the reducer — incremented on - * every non-`sent` `control_result`. Keying the effect on this number (not a - * boolean) ensures a second failure after a retry also re-enables buttons. + * every authoritative delivery failure (`no_active_turn`, `channel_closed`, + * `no_channel`). The transient `channel_full` status does NOT increment this + * token; the retransmit orchestrator handles that status automatically. + * Keying the effect on this number (not a boolean) ensures a second failure + * after a retry also re-enables buttons. */ deliveryFailed?: number; /** @@ -108,9 +111,11 @@ function PermissionDecisionButtons({ const deliveryFn = _deliveryFn ?? startPermissionDecisionDelivery; const [pending, setPending] = React.useState(null); - // Re-enable buttons when the reducer signals delivery failure (non-`sent` - // control_result status). The relay send succeeded but the harness couldn't - // route the click — the user should be able to retry. + // Re-enable buttons when the reducer signals an authoritative delivery + // failure (`no_active_turn`, `channel_closed`, `no_channel`). The transient + // `channel_full` status does NOT increment this token — the retransmit + // orchestrator stays subscribed and keeps resending automatically, so buttons + // must remain disabled until the retry settles or the deadline expires. React.useEffect(() => { if (deliveryFailed) { setPending(null); diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index 6bbe5357b5..c44ab71a52 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -2403,7 +2403,7 @@ test("buildTranscript_control_result_second_failure_increments_delivery_failed", turnId: "turn-1", payload: { type: "permission_decision", - status: "channel_closed", + status: "no_channel", requestNonce: nonce, optionId: "allow_once", }, @@ -2499,6 +2499,53 @@ test("buildTranscript_control_result_already_decided_does_not_mark_delivery_fail ); }); +test("buildTranscript_control_result_channel_full_does_not_mark_delivery_failed", () => { + // `channel_full` is a transient queue-saturation status — the retransmit + // orchestrator stays subscribed and keeps resending automatically. The card + // must remain DISABLED (deliveryFailed must NOT be incremented) so a second + // click cannot start a racing delivery loop while the first is still alive. + // + // Mutation proof: restoring `channel_full` to increment `deliveryFailed` in + // `handlePermissionDecisionResult` → `deliveryFailed` becomes 1 → this + // assertion fails at `expected undefined, got 1`. + const nonce = "nonce-channel-full"; + const events = [ + makePermissionRequestWithAuth(1, "req-cf", nonce), + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "channel_full", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + undefined, + "deliveryFailed must not be set on channel_full — it is transient; buttons must stay disabled while the retransmit loop is active", + ); + // Card must remain actionable (the request is still live). + assert.equal( + card.actionable, + true, + "card must remain actionable after channel_full — the retransmit loop is still in progress", + ); +}); + // ─── permission index cleanup + FOREIGN-nonce tests (Pass 4) ───────────────── import { buildTranscriptState } from "./agentSessionTranscript.ts"; diff --git a/desktop/src/features/agents/ui/agentSessionTranscriptPermissions.ts b/desktop/src/features/agents/ui/agentSessionTranscriptPermissions.ts index 9686f6ee8f..df79868fac 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscriptPermissions.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscriptPermissions.ts @@ -481,17 +481,25 @@ export function handlePermissionWrite( /** * Handle a `control_result` frame for a `permission_decision` delivery. - * A non-success status means the click did not reach the harness — marks the - * card with an incremented `deliveryFailed` counter so buttons re-enable for - * retry. * - * `sent` and `already_decided` are both success: `sent` means the harness - * forwarded the decision to the live read loop; `already_decided` means a - * retransmit matched a nonce the harness had previously forwarded (delivery - * suppressed — the deciding task has since ended). Neither may fail the card — - * an `already_decided` that incremented `deliveryFailed` would flip a - * correctly-resolved card back to a clickable/failed state, the exact P1 the - * retransmit loop exists to avoid. + * Three statuses are treated as authoritative failures — the harness cannot + * route the decision at all, so the card's `deliveryFailed` token is + * incremented and the buttons re-enable for a manual retry: + * `no_active_turn`, `channel_closed`, `no_channel` + * + * Two statuses are success and must not fail the card: + * `sent` — the harness forwarded the decision to the live read loop. + * `already_decided` — a retransmit matched a nonce the harness had already + * applied (suppressed); incrementing `deliveryFailed` here would flip a + * correctly-resolved card back to clickable, the exact P1 this exists to + * prevent. + * + * `channel_full` is a transient queue-saturation condition (one or more + * eligible queues could not accept the frame). The retransmit orchestrator + * (`retransmitPermissionDecision.ts`) stays subscribed and keeps resending + * until the harness accepts or the deadline expires. The card must remain + * DISABLED while the automatic retry is in progress — do NOT increment + * `deliveryFailed` here. */ export function handlePermissionDecisionResult( d: PermissionDraftSlice, @@ -500,8 +508,12 @@ export function handlePermissionDecisionResult( const frameType = asString(payload.type); if (frameType !== "permission_decision") return; const deliveryStatus = asString(payload.status); + // Success statuses — no card update needed. if (deliveryStatus === "sent" || deliveryStatus === "already_decided") return; - // Delivery failed — find the card by nonce and mark it retryable. + // Transient queue-saturation — retransmit orchestrator keeps retrying; + // do not re-enable the buttons mid-retry. + if (deliveryStatus === "channel_full") return; + // Authoritative failure — find the card by nonce and mark it retryable. const nonce = asString(payload.requestNonce); if (!nonce) return; const itemId = d.pendingPermissionsByNonce.get(nonce);