feat(desktop): surface tool-permission asks as thread-reply cards (#4938)

[wpfleger96]

Part of #4938. Desktop half of tool-permission handling: agents can no longer act on session/request_permission without a decision. When an ask-policy agent hits a permission request, the harness publishes a clickable permission card as a thread reply in the originating channel; the owner clicks Allow-once or Reject, and the card edits itself to the resolved outcome. The buzz-agent wire half merged in #5712 — this PR builds against its behavior on current main and touches nothing under crates/buzz-agent/**.

Permission policy

Three policies — allow | ask | reject — resolved per agent with precedence per-agent opt-in > GlobalAgentConfig > built-in default. Desktop's built-in default is ask (surface a card); headless / bare-CLI callers default to reject since they have no UI to answer one.

• desktop/src-tauri/src/managed_agents/permission_policy.rs — PermissionPolicy enum, PermissionPolicySource attribution, and resolve_effective_permission_policy. BUZZ_ACP_PERMISSION_POLICY is a reserved env key so the env-vars UI cannot silently diverge the running harness from the saved setting.
• ManagedAgentRecord.permission_policy + GlobalAgentConfig.permission_policy (Rust and TypeScript), surfaced through AgentPermissionPolicyField and the fleet-wide default in AgentDefaultsEditor.
• Injected as BUZZ_ACP_PERMISSION_POLICY at both local spawn and remote provider deploy. apply_deploy_result stamps the exact policy sent as the confirmed applied receipt, so a later global-default flip is detectable as drift against the live worker; a failed redeploy retains the last confirmed policy rather than clearing known truth.

Permission-request cards

The card is a frozen two-action contract: exactly one allow_once and one reject_once, enforced on both the producer and the parser so the two sides can never diverge and no durable machine-wide rule (allow_always) can ever surface as an actionable button.

• crates/buzz-acp/src/acp.rs — select_card_actions picks exactly one validated allow_once + one reject_once from the adapter's option list and fails closed (denies synchronously, emits no card) when either is absent or ambiguous. The read-loop decision gate accepts a click only when it matches one of those two snapshotted actions (CardActions::accepts), not on mere membership in the original option list — so decision acceptance is kind-checked, never label- or adapter-trusted. The sentinel builders enforce the frozen byte bounds: labels truncate on a char boundary at 200 UTF-8 bytes, and sessionId (straight from the adapter's unbounded session/new response), turnId, requestNonce, chosenOptionId, and total serialized content fail closed when over-limit.
• desktop/src/shared/lib/permissionRequest.ts — parser for the frozen v1 schema: discriminated union PermissionRequestPending | PermissionRequestResolved; rejects unknown versions and enforces exactly two unique option IDs, an exact label-key count, and chosenOptionId membership in optionIds. Every untrusted string leaf (labels, each optionId, requestNonce, sessionId, turnId, chosenOptionId) is bounded at 200 UTF-8 bytes and the total serialized content at 4096 UTF-8 bytes — the same unit and values the harness enforces (SENTINEL_STRING_MAX_BYTES / SENTINEL_CONTENT_MAX_BYTES), so a card the producer emits always parses. agentPubkey/channelId derive from the signed event envelope, never from sentinel JSON.
• desktop/src/shared/lib/computePermissionRequest.ts — signer gate (the kind-9 must be agent-signed) plus edit-authenticity gate (a kind-40003 edit must be signed by the same agent as the original kind-9; owner or attacker edits are rejected). A resolved edit must also correlate to this card: its originalEventId must equal the message ID and its requestNonce/sessionId/turnId must match the pending body it overlays, so a same-signer agent cannot cross-apply a resolution meant for a different card.
• desktop/src/shared/ui/permission-request-card.tsx — pending card renders harness-provided labels[optionId] buttons (Allow-once / Reject — no Allow-for-Session, no durable-rule disclosure) and an expiry countdown to expiresAt; the resolved card shows the outcome. Actionable buttons render only for the verified owner; a click is fire-and-forget with a "Decision sent" disable and relay-error re-enable.
• PermissionRequestCardBlock.tsx wraps the card with owner identity resolution and keeps the MessageRow memo budget clean; editSignerPubkey and the pre-edit body are threaded through formatTimelineMessages for the edit-authenticity and correlation checks.

Transcript labels

describePermissionOutcome / describePermissionTerminalReason render harness display labels (label ?? verb) instead of raw ACP kind strings; the legacy compound-key path falls back to verb-only ("Approved" / "Denied").

NIP-AO

Documents the permission sentinel card lifecycle: kind-9 pending publish, kind-40003 resolved edit, admission gating, the two-action contract, and sentinel authenticity.

Decision delivery

The desktop sends its permission_decision fire-and-forget over the observer control channel. If the harness socket is down when the decision is published, the decision is lost and the card strands. Both halves of the lifecycle are closed so a decision is delivered exactly once regardless of transient socket loss.

• crates/buzz-acp — inbound dedup: a retransmitted decision whose nonce was already applied (the deciding turn has ended) acks success-shaped (already_decided) instead of failing no_active_turn / channel_closed. Backed by a recently-decided nonce set with a retention window, so a late retransmit of a correctly-resolved decision never surfaces as an error.
• desktop/src/features/agents/lib/retransmitPermissionDecision.ts — a pure retransmit-until-acked orchestrator (modeled on awaitLiveSwitchOutcome), wrapped by permissionDecisionDelivery.ts which injects the real scheduler/sender. Retransmission is bounded by the card's expiresAt. sent and already_decided resolve the loop as "acked" (the harness routed or already applied the decision). The four failure statuses (no_active_turn, channel_full, channel_closed, no_channel) resolve it as "failed": the harness answered authoritatively but could not route the click; retransmitting the same nonce cannot change that, so the loop stops and the card re-enables for owner retry. A rejected send (transport failure) is swallowed and the next tick retries — the loop survives a briefly-down socket.
• AuthorizationEnvelope.expiresAt rides the observer frame through plain serde_json::to_vec (no byte bound, no fixture) and is plumbed onto the card so the orchestrator can bound its own deadline. Because expires_at is Option<u64>, an archived or pre-upgrade frame may not carry it; resolveDecisionDeadlineSecs falls back to a 300s freshness window measured from the frame timestamp. Live frames always carry it since harness and desktop ship together in this PR.

[LucasMoskun]

Field validation from Buzz Desktop 0.5.7 on macOS, using a locally managed Claude agent with Claude Code 2.1.220 against a self-hosted relay:

The #4609 behavior created an abrupt upgrade regression for an existing managed agent. Before the desktop update, the agent could use the relay and perform implementation work. After updating, the same agent launched in dontAsk; every unapproved request was rejected with no desktop approval surface.

The failure progression demonstrates why the full permission policy in this PR is needed rather than only the narrow #5263 workaround:

1. Preauthorizing Bash(buzz) and Bash(buzz:*) in the trusted local Claude settings restored relay reads and replies.
2. The next workflow failed because even read-only git status was denied.
3. Preauthorizing git restored repository commands, but native Edit/Write operations remained subject to the same problem.
4. The only complete manual workaround was an owner-controlled blanket local allowlist including bare Bash, Edit, Write, and the other permission-requiring Claude tools.

That workaround restores autonomy, but it is all-or-nothing, hidden from the Buzz UI, and easy for users to misdiagnose as a relay, model, or harness-switching problem. The per-agent and fleet-wide ask | allow | reject policy plus actionable desktop cards in this PR is the correct product boundary: explicit owner intent rather than either silent approval or silent rejection.

Suggested upgrade/regression coverage:

• Start with an existing managed agent created before fix(acp): reject unattended permission requests #4609, then upgrade into the new permission-policy build.
• Verify the effective inherited/per-agent policy is visible in the UI and changing it triggers the required restart.
• With allow, verify a fresh worker can run standalone buzz, git, and file-edit operations without an interactive prompt.
• With ask, verify concurrent approval cards render and resolve correctly for a locally managed agent connected to a self-hosted relay.
• With reject, verify requests remain fail-closed.
• Verify the selected policy follows the actual effective harness when a user switches Claude and Codex, rather than only the global default.

The final point matters because our same reproduction also encountered #5054: a stale per-agent agent_command_override could leave the actual harness different from the UI/global runtime selection. Permission policy and harness identity should be displayed from the same effective spawn configuration.

[wolfyy970]

This is the right boundary for portable agents: the definition can request tools, while the execution target decides how permission requests are handled. I would not merge this head yet because four paths can make the saved policy differ from what actually runs.

• Desktop reserves BUZZ_ACP_PERMISSION_POLICY, but not BUZZ_ACP_PERMISSION_MODE. Definition env is written after the policy, so acceptEdits can bypass Ask or Reject before ACP asks Buzz. Managed local and provider launches should derive the mode from the selected policy and strip user overrides. Bare CLI use can keep the explicit mode.
• The permission receiver is taken by the first prompt and not restored. A new session's initial_message can consume it before the real prompt. Heartbeats have no receiver at all, but can still emit an actionable card. Only emit an actionable Ask request when a routable decision channel exists.
• Permission responses use several awaited writes. Cancel can drop one after partial output, then send another cancellation response. Every permission response path needs the same persistent Writing or poisoned state so cancellation cannot produce a second answer.
• Remote deployment records the policy in its launch payload, but the UI later recomputes the current desired value. Changing the global default can therefore display Reject while the remote process still runs Allow. Keep the applied policy in the deployment receipt and show Redeploy required when it drifts.

The UI should also say that this governs ACP permission requests, not tools that run without asking Buzz. Unknown option kinds and allow_always should not appear as an ordinary green Allow button.

The focused permission tests pass, but they do not cover these paths. The missing regressions are: definition mode override, initial-message then main-prompt approval, heartbeat Ask, cancel during a blocked permission write, and remote desired-versus-applied drift.

[wolfyy970]

@wpfleger96 I prepared the first review fix as signed commit aeb8f71eb directly on top of your head.

It reserves BUZZ_ACP_PERMISSION_MODE across Desktop configuration, removes any inherited process value before launch, and rejects Reject + acceptEdits before startup. The focused ACP and Desktop tests, formatting, and strict Clippy pass; the adversarial re-review is clean.

You can cherry-pick it as-is. I kept it dependent on #5106 rather than opening another competing PR.

[wolfyy970]

I pushed the second independent fix from my review as wolfyy970@a83200bf7.

It keeps the permission-decision route alive across the initial message and the main prompt. Ask now fails closed and non-actionable when no live route exists, including heartbeat tasks.

The focused Ask and permission tests pass, strict clippy passes, and the adversarial review is clean. This can be cherry-picked after aeb8f71.

[wolfyy970]

The third review fix is wolfyy970@eeaedd910.

If a control signal interrupts a permission response after stdin has accepted some bytes, cleanup now reports the result as uncertain, sends no second JSON-RPC response, and forces process replacement. The regression backpressures a real child pipe and proves there is exactly one write attempt.

All 740 buzz-acp library tests and strict Clippy pass. The adversarial review is clean. This can be cherry-picked after a83200b.

[wesbillman]

Review at exact head e87f265d258a1816995f743e4fee85d4a2ee6c02 (and separately inspected the proposed follow-up stack through eeaedd910f3df5cdd3e6b937f6e920ee0f6acfa9).

The proposed three-commit stack addresses the mode-override, decision-route lifetime, and interrupted-write findings, but I do not think this is release-safe yet:

1. Persist and display the remotely applied policy, not only current desired policy. build_deploy_payload resolves a policy into launch.policy_env, but ManagedAgentSummary later recomputes from the mutable record/global config. If the fleet default changes after deployment, Desktop can display reject while the remote worker still runs allow. The deployment receipt/record needs the applied value, and the UI should show desired-vs-applied drift as redeploy-required. Add a regression covering deploy under Allow, mutate global default to Reject, and verify the UI continues to truthfully expose applied Allow plus drift.

2. Do not render unknown or persistent options as ordinary green Allow buttons. PermissionDecisionButtons currently classifies everything not starting with reject as Allow. That makes unknown kinds and allow_always look equivalent to allow_once. Only recognized one-shot and reject choices should be actionable under this feature, or persistent grants must receive explicit differentiated semantics/copy. Unknown kinds should fail closed/non-actionable. Add reducer/render tests for allow_once, reject_once, allow_always, and unknown kinds.

3. Release validation must use actual adapters, not only scripts. This PR changes the mode sent into runtime-specific ACP adapters and claims to restore permission-requiring tools. Before release, exercise exact built buzz-acp plus current managed Claude/Codex/Buzz Agent/Goose versions under Ask, Allow, and Reject. At minimum prove: Buzz read/reply; a representative workspace read and edit; Ask card round-trip; Allow selects only offered allow_once; Reject denies; unattended escalation remains blocked; network/filesystem boundaries remain intact. Codex's separate workspace-write network defect still requires its adapter fix.

This is the correct architectural boundary and the harness work is impressively defensive, but CI green does not establish cross-adapter behavior. Please land the existing three fixes, resolve the two remaining truth/UI issues, and attach exact-runtime evidence before merging as the full recovery.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at head 242dbf2680fee3fd19ca02ffdfaa60dc9579db9b, against base 59328d5ae38a51a618dd2fddd7faf1343d42096f.

The contract remains owner-controlled, one-request consent: Desktop defaults to Ask, headless defaults to Reject, only Allow-once / Reject are actionable, and the operation and eventual outcome must be truthful. The earlier pre-ACK overwrite, terminal-delivery deadline, and permanent observer option findings are repaired in source. Actual command/path context is now carried, but its truncation disclosure is broken. The main merge also introduced concrete integration regressions.

[P1] Restore the deploy-policy helper and reconcile all launch call sites

agents.rs:1172 still imports deploy::extract_applied_permission_policy, and the production provider path calls it at agents/provider_deploy.rs:107, but the main merge deleted its definition from agents_deploy.rs. The deploy module has neither that definition nor a replacement export. This is an unresolved import in the normal Desktop build, not just missing test coverage.

The same merge left three test call sites with the old arity: agents_deploy.rs:409–417 supplies seven arguments to the now-eight-argument build_launch_block_for_policy; :438–445 and :474–481 supply six to the now-seven-argument build_launch_block. Restoring the helper alone therefore still leaves the Tauri test target uncompilable.

Restore the exact-payload receipt helper and its invariant tests, and update all launch callers to supply the permission policy. Preserve the confirmed-policy receipt rather than removing its validation to silence the compiler. Validate both the normal Desktop target and the full Tauri test target after repair. These failures are established by source/signature resolution; no compilation was run in this review.

[P1] Do not acknowledge fan-out when the nonce-owning thread missed the decision

handle_permission_decision_control now sends each decision to every same-channel task. It records the nonce and returns sent if any enqueue succeeds, even when another queue returns Full. Non-owning read loops intentionally discard unknown nonces (acp.rs:2287–2300,2394–2399), so a successful sibling enqueue is not evidence that the owning loop received anything.

Concrete sequence: thread A has a pending card and is temporarily awaiting a different permission's ACP write (acp.rs:2348–2375, bounded at 30 seconds). Decisions for sibling cards fill A's eight-slot queue (lib.rs:4551–4558; PERMISSION_MAP_CAP = 8). A decision for A's own nonce then gets Full from A but Sent from a sibling whose queue has space. That sibling discards it, yet the dispatcher reports success and records the nonce as delivered. Desktop stops retransmission on sent, leaving A's unreceived decision to expire fail-closed while its card says Decision sent.

Tie success/receipt to delivery to the nonce-owning loop, or otherwise make this mixed-result case retryable/visibly failed instead of successful. Add the missing owner-queue-Full + sibling-queue-Sent case; the current two-thread test (lib.rs:11185–11225) exercises only queues with space. This is a concrete partial-fan-out loss, not the previously excluded speculative enqueue-versus-prompt-completion race.

[P2] Preserve the truncation marker through the final sentinel byte cap

description_from_request_permission reserves three bytes for (…), but that wrapper is five UTF-8 bytes. With the existing test's title fake__shell (11 bytes) and a 500-byte ASCII command, the context budget becomes 186 bytes and the formatted description becomes 202 bytes. The pending/resolved builders then recap it at 200 bytes (acp.rs:4145,4182); because byte 200 falls inside the ellipsis, truncate_to_bytes backs up before the ellipsis. The published card loses both its truncation marker and closing parenthesis.

The owner sees a command prefix without the required notice that the rest was omitted. Budget the actual UTF-8 suffix before truncating and preserve the omission marker in the final serialized sentinel. Add a producer-to-card regression asserting both the 200-byte bound and the surviving marker for ASCII and multibyte input. The current pure bound test at acp.rs:8119–8152 already contradicts this arithmetic, while the production-seam bound test checks size only.

[P2] Retire observer permission cards by terminal turn, not entire channel

agentSessionTranscript.ts:727–737 passes only the channel to retireAllLivePermissionCards. That helper disables every actionable permission card in the channel and deletes its nonce/legacy correlation indexes (agentSessionTranscriptPermissions.ts:248–292). The merged runtime now permits concurrent thread-scoped turns and emits the exact terminal turn ID (pool.rs:4824–4829), but the reducer ignores it.

Reproduction: enable thread-scoped sessions with multiple workers; thread A waits for permission while thread B in the same agent/channel completes normally. B's completion removes A's observer buttons even though A remains pending. A's separate thread card remains usable, but a subsequent A outcome cannot repair its observer row because handlePermissionWrite drops the now-unknown nonce (agentSessionTranscriptPermissions.ts:353–384). The real channel panel combines these events before reducing them (ManagedAgentSessionPanel.tsx:99–119).

Scope retirement and index cleanup to the terminal turn, using the appropriate session/process identity for error events. Cover A pending → B completed → A still actionable → A resolves, including archive reconstruction.

Validation and scope: read-only inspection of immutable GitHub metadata, diffs and exact-head source, with independent harness lifecycle/routing, UI, and deploy/policy integration lanes. Reviewed the agreed corrective findings and new main-merge interactions across Desktop thread/observer surfaces, local/remote policy projection and confirmed receipts, owner/option gates, publishing/pending/writing/terminal transitions, and concurrent thread routing. Reused prior exact-head review evidence for unchanged paths rather than reopening the design. No checkout, build, tests, PR code execution, live relay/provider workflow, screenshots, or assistive-technology validation. Mobile/web permission controls remain outside this Desktop delivery. Source inspection of regression tests is not a passing test run.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at head 00a04c13b46fd04a9c6da6b47642d45d093d44c3, against base 59328d5ae38a51a618dd2fddd7faf1343d42096f.

This is a bounded corrective re-review of the eight-file delta since 242dbf2680fee3fd19ca02ffdfaa60dc9579db9b, plus verification of the remaining prior findings. The missing deployment-policy helper and three old-arity launch test calls are repaired in source. The helper reads the exact rebuilt payload before provider invocation, and the confirmed applied-policy receipt remains intact; this is not a claim that the entire build/test suite passes.

The three findings below remain in byte-identical current-head paths. They are remaining prior blockers, not new design requirements. The contract is unchanged: owner-controlled one-request consent; Desktop defaults to Ask, headless to Reject; exactly Allow-once / Reject are actionable; operation and eventual outcome must be truthful; remote applied policy reflects the confirmed deployment payload.

[P1] Do not acknowledge fan-out when the nonce-owning thread missed the decision

handle_permission_decision_control still sends each decision to every same-channel task. It records the nonce and returns sent if any enqueue succeeds, even when another queue returns Full. Non-owning read loops intentionally discard unknown nonces (acp.rs:2287–2300,2394–2399), so a successful sibling enqueue is not evidence that the owning loop received anything.

Concrete sequence: thread A has a pending card and is temporarily awaiting a different permission's ACP write (acp.rs:2348–2375, bounded at 30 seconds). Decisions for sibling cards fill A's eight-slot queue (lib.rs:4551–4558; PERMISSION_MAP_CAP = 8). A decision for A's own nonce then gets Full from A but Sent from a sibling whose queue has space. That sibling discards it, yet the dispatcher reports success and records the nonce as delivered. Desktop stops retransmission on sent, leaving A's unreceived decision to expire fail-closed while its card says Decision sent.

Tie success/receipt to delivery to the nonce-owning loop, or otherwise make this mixed-result case retryable/visibly failed instead of successful. Add the missing owner-queue-Full + sibling-queue-Sent case; the current two-thread test (lib.rs:11185–11225) exercises only queues with space. This is a concrete partial-fan-out loss, not the previously excluded speculative enqueue-versus-prompt-completion race.

[P2] Preserve the truncation marker through the final sentinel byte cap

description_from_request_permission reserves three bytes for (…), but that wrapper is five UTF-8 bytes. With the existing test's title fake__shell (11 bytes) and a 500-byte ASCII command, the context budget becomes 186 bytes and the formatted description becomes 202 bytes. The pending/resolved builders then recap it at 200 bytes (acp.rs:4145,4182); because byte 200 falls inside the ellipsis, truncate_to_bytes backs up before the ellipsis. The published card loses both its truncation marker and closing parenthesis.

The owner sees a command prefix without the required notice that the rest was omitted. Budget the actual UTF-8 suffix before truncating and preserve the omission marker in the final serialized sentinel. Add a producer-to-card regression asserting both the 200-byte bound and the surviving marker for ASCII and multibyte input. The current pure bound test at acp.rs:8119–8152 already contradicts this arithmetic, while the production-seam bound test checks size only.

[P2] Retire observer permission cards by terminal turn, not entire channel

agentSessionTranscript.ts:727–737 passes only the channel to retireAllLivePermissionCards. That helper disables every actionable permission card in the channel and deletes its nonce/legacy correlation indexes (agentSessionTranscriptPermissions.ts:248–292). The merged runtime now permits concurrent thread-scoped turns and emits the exact terminal turn ID (pool.rs:4824–4829), but the reducer ignores it.

Reproduction: enable thread-scoped sessions with multiple workers; thread A waits for permission while thread B in the same agent/channel completes normally. B's completion removes A's observer buttons even though A remains pending. A's separate thread card remains usable, but a subsequent A outcome cannot repair its observer row because handlePermissionWrite drops the now-unknown nonce (agentSessionTranscriptPermissions.ts:353–384). The real channel panel combines these events before reducing them (ManagedAgentSessionPanel.tsx:99–119).

Scope retirement and index cleanup to the terminal turn, using the appropriate session/process identity for error events. Cover A pending → B completed → A still actionable → A resolves, including archive reconstruction.

Additional validation defect reported by the late independent audit: two new buzz-acp tests call .sign(&keys).unwrap() without awaiting the signing future (acp.rs:12307–12309,12369–12371), and the resolved-edit path retains an unused entry_deadline binding (acp.rs:1620). The collaborator reports exact-head Unit Tests E0599 failures and Rust/Windows lint failures under -D warnings for these lines. I verified the cited source expressions, but did not independently retrieve those CI logs or run compilation. Repair the signing calls (await the async API or use the synchronous signing API already used in this file), remove the obsolete binding, and validate the full affected Rust targets. These are in the unchanged acp.rs blob, not introduced by the eight-file corrective delta; they do not reopen the product design or replace the three user-visible blockers above.

Validation and scope: read-only immutable GitHub metadata, diffs and exact-head source, with direct deployment/helper/type/fixture corrective inspection. The delegated audit returned after initial submission and corroborated the deployment/helper/type/fixture conclusions; this body was updated in place rather than publishing a duplicate review. Rechecked the permission producer → fan-out → read-loop → Desktop acknowledgement path, description → pending/resolved sentinel cap, and terminal turn → live/archive observer reducer. Reused previous verified evidence for unchanged owner/option gates, first-valid pre-ACK decision, terminal-delivery retries, signed edit provenance and local/remote policy projection. No checkout, build, tests, PR-code execution or live relay/provider/UI workflow was performed. Mobile/web permission controls and the previously excluded speculative enqueue-versus-prompt-completion race are outside this corrective review. Source inspection is not a passing test run.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head a0309cdbd39f8f37a32dc1c285a479608d41c970 against base 5aed49b505a7e27f3b0e34dafa53d6c4e8cdcd64, focusing on the corrections since 00a04c13b46fd04a9c6da6b47642d45d093d44c3. Contract: owner-only, one-request Allow-once/Reject consent; truthful pending/delivery/outcome states; sibling turns must not retire each other’s requests.

P2: Keep observer-card controls disabled while channel_full retries remain active

handlePermissionDecisionResult, lines 500–522 still classifies channel_full as a delivery failure and increments deliveryFailed. But the corrected retransmit orchestrator, lines 114–121 deliberately keeps the original decision’s retry loop alive for that status.

These consumers receive the same production frame. LifecycleActivity, lines 111–118 clears its pending selection when the reducer increments the token, re-enabling both buttons. Another click then starts a second independent delivery loop (lines 148–165); each loop captures its own option ID, but both correlate acknowledgements only by request nonce.

Reproduction by source trace: fill the owning turn’s decision queue, click Allow once in the activity card, and deliver control_result(channel_full). Both buttons re-enable although Allow is still being retried. Click Reject while the queue remains full, then let it drain before the original Allow loop’s next tick. The older Allow can be delivered first and applied; its nonce-only sent acknowledgement also settles the Reject loop. The visible retry control therefore permits a competing decision while the supposedly failed choice is still live. The harness’s existing first-valid-decision rule is correct; the UI must not offer this second choice as a retry while the first remains in flight.

Smallest fix: make channel_full non-failing in the transcript reducer as well, leaving deliveryFailed unchanged and the existing selection disabled. Preserve re-enable behavior for no_active_turn, channel_closed, and no_channel. Add a reducer-to-rendered-card regression: click, feed channel_full, assert both controls stay disabled and no second delivery starts; then verify the terminal failure statuses still allow a retry. The pure orchestrator tests alone do not exercise this consumer.

Prior findings resolved

• The native mixed owner-Full/sibling-Sent fan-out no longer reports success or records the nonce; it retries until all relevant queues accept or the deadline expires.
• The combined description reserves the correct UTF-8 overhead and survives the sentinel cap with omission disclosure. The two un-awaited signing calls and unused tuple binding are repaired.
• Turn completion/error retirement uses the owning turn ID rather than disabling every pending card in the channel; the sibling card retains its nonce for a later outcome.

Source-only review: exact-head/base text and regression-test source inspected; no checkout, build, test execution, mutation run, or live workflow performed. Unchanged policy/launch-receipt, owner/edit-provenance, two-option, and terminal-retry paths were cross-checked against prior verified blob identities. Mobile/web controls and unrelated base-merge changes are outside this corrective review.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Corrective review clear

Reviewed head 2e2f4df8933101bc8b25ac7c90fdc113156cd63a against base 0dbd036f5bff33e7ade75e7639f3218d424a6e73, following up on the prior review. No remaining actionable blocker found within the agreed corrective scope.

• The queue-full P2 is resolved. agentSessionTranscriptPermissions.ts:510–515 now leaves deliveryFailed unchanged on channel_full, matching the retransmit loop and keeping the original selection disabled. no_active_turn, channel_closed, and no_channel retain manual-retry behavior. The native producer and both desktop consumers now agree on this status distinction.
• The requested regression is present. LifecycleActivity.render.test.mjs:537–761 binds the real transcript reducer to the mounted production card, checks both controls stay disabled after queue saturation, and checks a routing refusal re-enables retry. Its delivery promise is intentionally stalled; this is reducer-to-card coverage, not a single end-to-end retransmit/ACK/expiry test.
• Policy integration remains intact through the base merge. Instance Save preserves its display-only policy contract; local launch stamps the injected policy; remote deployment derives its applied receipt from the actual launch payload and preserves the prior receipt on failure. Previously reviewed signer/correlation, bounded-schema, turn-retirement, and native dispatch paths retain their reviewed blobs.

This was source/metadata review only, including independent desktop and native integration lanes. No checkout, build, test, mutation, or live workflow was executed. Unrelated merged-base effort behavior and previously excluded surfaces were not reopened. This is a clear review comment, not approval.