fix(acp): close Thufir addendum gaps — unconditional first attempt, interactive delivery-seam test

F2 (retransmit structural fix): move delivery-deadline check to after the first
attempt. The first publish is now unconditional — an already-expired deadline
supplied by the caller (e.g. ordinary-timeout path where entry_deadline was past
at resolution time) no longer silently skips the one relay write that resolves
the card. Retries still consult the deadline.

F2 (test fixes): replace the channel-closed bounded-exit test with two focused
tests:
- retransmit_resolved_edit_unconditional_first_attempt_on_expired_deadline:
  supplies an already-expired deadline, verifies the kind-40003 event is still
  published. Mutation: revert !first_attempt guard → zero publishes.
- retransmit_resolved_edit_always_uncertain_bounded_exit: uses test_pair_silent
  (always Uncertain) under paused time, advances past the 4s delivery window,
  verifies the loop exits. Mutation: large RESOLVED_DELIVERY_WINDOW_SECS → hangs.

F3 (interactive delivery-seam): add _deliveryFn testability seam to
PermissionDecisionButtons and thread it through LifecycleActivity. Add
test_f3_interactive_delivery_seam_allow_once_and_reject_once_fire_delivery:
builds an acp_read transcript card (four option kinds), renders via
LifecycleActivity with a mock _deliveryFn, clicks allow_once button, asserts
delivery called with opt-allow-once + nonce-interactive; clicks reject_once
button in a fresh render, asserts delivery called with opt-reject-once.
allow_always/reject_always produce no buttons (ACTIONABLE_KINDS contract).
JS suite: 10/10 pass. Mutation: remove kind from ACTIONABLE_KINDS → button
absent → delivery assertion fails.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

crates/buzz-acp/src/acp.rs

@@ -4255,9 +4255,11 @@ fn build_kind40003_sentinel(
 ///
 /// `delivery_deadline` is computed at resolution time as
 /// `Instant::now() + RESOLVED_DELIVERY_WINDOW_SECS`, independent of the original
-/// card/click deadline. This guarantees at least one publish attempt for every
-/// terminal outcome, including ordinary timeouts (where the card deadline is
-/// already past when `finish_permission` fires).
+/// card/click deadline. The first publish attempt is **unconditional** — the
+/// deadline is only consulted before each *retry* so that the relay always sees
+/// at least one publication even when the caller supplies an already-expired
+/// deadline (e.g. during ordinary timeouts where `entry_deadline` was already
+/// past when `finish_permission` fired).
 ///
 /// Spawned detached so it never blocks the read loop. `event` is consumed and
 /// resent by clone each attempt so the signature and id are stable across retries.
@@ -4266,8 +4268,13 @@ async fn retransmit_resolved_edit(
     event: nostr::Event,
     delivery_deadline: tokio::time::Instant,
 ) {
+    let mut first_attempt = true;
     loop {
-        if tokio::time::Instant::now() >= delivery_deadline {
+        // The first attempt is unconditional — an already-expired deadline must
+        // not prevent the single relay write that resolves the card. Subsequent
+        // retries (Uncertain outcome) are gated by the deadline so the loop
+        // terminates once the delivery window closes.
+        if !first_attempt && tokio::time::Instant::now() >= delivery_deadline {
             tracing::warn!(
                 target: "acp::permission",
                 "resolved edit {} not accepted before delivery window — giving up",
@@ -4275,10 +4282,11 @@ async fn retransmit_resolved_edit(
             );
             return;
         }
+        first_attempt = false;
         // Per-attempt ACK deadline: min(fixed publish timeout, delivery_deadline).
         // Capping each attempt at SENTINEL_PUBLISH_TIMEOUT sweeps a stuck waiter
-        // promptly so the same signed event is resent, while the loop-top check
-        // keeps the overall delivery window as the outer bound.
+        // promptly so the same signed event is resent, while the deadline check
+        // above keeps the overall delivery window as the outer bound.
         let attempt_deadline = (tokio::time::Instant::now()
             + std::time::Duration::from_secs(SENTINEL_PUBLISH_TIMEOUT_SECS))
         .min(delivery_deadline);
@@ -12280,69 +12288,116 @@ mod tests {
         let _ = std::fs::remove_file(&capture_file);
     }
 
+    // ── F2: unconditional first attempt with an already-expired deadline ──────
+    //
+    // The retransmit loop must publish the resolved edit at least once even when
+    // the delivery_deadline has already elapsed at call time. This covers the
+    // ordinary-timeout path where the card's entry_deadline expired before
+    // `finish_permission` called `retransmit_resolved_edit`.
+    //
+    // Mutation proof: reverting the `!first_attempt &&` guard (i.e. making the
+    // deadline check unconditional at loop-top) causes the loop to return
+    // immediately on an already-expired deadline without publishing — the
+    // assertion that a kind-40003 event was emitted goes red.
+
+    #[tokio::test(start_paused = true)]
+    async fn retransmit_resolved_edit_unconditional_first_attempt_on_expired_deadline() {
+        let keys = Keys::generate();
+
+        let event = nostr::EventBuilder::new(nostr::Kind::from(40003), "resolved-expired")
+            .sign(&keys)
+            .unwrap();
+        let event_id = event.id.to_hex();
+
+        // Use an accepting publisher — we only need to confirm the event is
+        // attempted once despite the expired deadline.
+        let (publisher, mut event_rx) = crate::relay::RelayEventPublisher::test_pair();
+
+        let collected: std::sync::Arc<std::sync::Mutex<Vec<String>>> =
+            std::sync::Arc::new(std::sync::Mutex::new(Vec::new()));
+        let collected_drain = collected.clone();
+        tokio::spawn(async move {
+            while let Some(ev) = event_rx.recv().await {
+                if ev.kind.as_u16() == 40003 {
+                    collected_drain.lock().unwrap().push(ev.id.to_hex());
+                }
+            }
+        });
+
+        // Supply an already-expired deadline.
+        // Under start_paused, Instant::now() is fixed at epoch; subtract 1ns.
+        let already_expired = tokio::time::Instant::now() - std::time::Duration::from_nanos(1);
+        let handle = tokio::spawn(retransmit_resolved_edit(publisher, event, already_expired));
+
+        // Advance time past the per-attempt timeout so the spawned task drains.
+        tokio::time::advance(std::time::Duration::from_secs(
+            SENTINEL_PUBLISH_TIMEOUT_SECS + 1,
+        ))
+        .await;
+        tokio::task::yield_now().await;
+        let _ = tokio::time::timeout(std::time::Duration::from_millis(200), handle).await;
+        tokio::task::yield_now().await;
+
+        let seen = collected.lock().unwrap().clone();
+        assert!(
+            !seen.is_empty(),
+            "retransmit must publish even when delivery_deadline is already expired at call time; \
+             mutation: unconditional loop-top deadline check → zero publishes → this goes red"
+        );
+        assert_eq!(
+            seen[0], event_id,
+            "published event must carry the same stable signed id"
+        );
+    }
+
     // ── F2: always-Uncertain bounded-exit ─────────────────────────────────────
     //
-    // A retransmit loop that never receives `Accepted` must still terminate
-    // once the delivery window (300s) elapses. Under paused time we advance past
-    // the window and confirm the loop exits — i.e. the spawned task finishes
-    // without hanging forever.
+    // A retransmit loop that perpetually receives `Uncertain` (via test_pair_silent
+    // which drops ack_tx so every await resolves as RecvError → Uncertain) must
+    // still terminate once the delivery window elapses. Under paused tokio time
+    // we advance past the window and confirm the loop exits.
     //
-    // Same signed event ID across retries: the same `nostr::Event` struct
-    // (identical id + signature) is re-submitted on every `Uncertain` retry.
-    // The relay's idempotency guarantee only holds when the id is stable.
-    //
-    // Mutation proof: replacing `RESOLVED_DELIVERY_WINDOW_SECS` with a very
-    // large value (e.g. `u64::MAX / 2`) makes the loop effectively infinite
-    // under the test's time budget — the `JoinHandle` would never complete.
+    // Mutation proof: replacing `RESOLVED_DELIVERY_WINDOW_SECS` with a very large
+    // value (e.g. `u64::MAX / 2`) makes the loop effectively infinite under the
+    // test's time budget — the JoinHandle would never complete and the timeout
+    // assertion fails.
 
     #[tokio::test(start_paused = true)]
     async fn retransmit_resolved_edit_always_uncertain_bounded_exit() {
-        // Verify two properties of `retransmit_resolved_edit`:
-        //
-        // 1. **Bounded exit**: the loop terminates when the delivery window closes.
-        //    Exercised by dropping the publisher's event receiver immediately so
-        //    the first `register_publish_ack` call returns `Err` (channel closed)
-        //    — the loop treats that as terminal and exits.
-        //
-        // 2. **Same signed event ID across retries**: the same `nostr::Event`
-        //    struct (identical id + signature) must be re-submitted on every
-        //    attempt. Verified by comparing `id.to_hex()` on the original and a
-        //    clone — these must match.
         let keys = Keys::generate();
 
-        let event = nostr::EventBuilder::new(nostr::Kind::from(40003), "resolved")
+        let event = nostr::EventBuilder::new(nostr::Kind::from(40003), "resolved-uncertain")
             .sign(&keys)
             .unwrap();
 
-        // Drop the receiver immediately so the publisher channel is closed before
-        // the first attempt — the loop exits on the `Err(channel closed)` path.
-        let (publisher, event_rx) = crate::relay::RelayEventPublisher::test_pair();
-        drop(event_rx);
+        // test_pair_silent drops ack_tx on each PublishEventAcked → every
+        // ack_rx.await yields Err(RecvError) → unwrapped as Uncertain.
+        let (publisher, _event_rx) = crate::relay::RelayEventPublisher::test_pair_silent();
 
-        let delivery_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(4);
+        // Short delivery window to advance past quickly.
+        let delivery_window = std::time::Duration::from_secs(4);
+        let delivery_deadline = tokio::time::Instant::now() + delivery_window;
         let handle = tokio::spawn(retransmit_resolved_edit(
             publisher,
-            event.clone(),
+            event,
             delivery_deadline,
         ));
 
-        // Advance time to let the spawned task run its first iteration.
-        tokio::time::advance(std::time::Duration::from_millis(100)).await;
+        // Each Uncertain attempt is followed by RESOLVED_RETRANSMIT_BACKOFF sleep.
+        // Under paused time the runtime auto-advances through parked timers.
+        // Advance well past the delivery window to drain all retry cycles.
+        tokio::time::advance(
+            delivery_window + std::time::Duration::from_secs(SENTINEL_PUBLISH_TIMEOUT_SECS * 3),
+        )
+        .await;
+        tokio::task::yield_now().await;
 
-        // The loop must exit (channel closed = terminal).
-        let join_result = tokio::time::timeout(std::time::Duration::from_millis(500), handle).await;
+        let join_result = tokio::time::timeout(std::time::Duration::from_secs(1), handle).await;
         assert!(
             join_result.is_ok(),
-            "retransmit loop must exit when publisher channel closes (bounded-exit property)"
-        );
-
-        // Same event ID across clones — the relay idempotency guarantee requires
-        // the same signed event (same id) on every retry attempt.
-        let id_original = event.id.to_hex();
-        let id_cloned = event.clone().id.to_hex();
-        assert_eq!(
-            id_original, id_cloned,
-            "event ID must be stable across clones (same signed event retransmitted on retry)"
+            "retransmit loop must exit once the delivery window elapses even when every \
+             attempt returns Uncertain; mutation: large RESOLVED_DELIVERY_WINDOW_SECS → \
+             loop never completes within test budget → timeout fires"
         );
     }
 

desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs

@@ -1,6 +1,7 @@
 import assert from "node:assert/strict";
-import test from "node:test";
+import { after, afterEach, before, mock, test } from "node:test";
 
+import { JSDOM } from "jsdom";
 import React from "react";
 import { renderToStaticMarkup } from "react-dom/server";
 
@@ -43,9 +44,62 @@ function pendingPermissionItem(options) {
 }
 
 // ---------------------------------------------------------------------------
-// allow_once — renders a green actionable Allow button
+// jsdom + fake-timer setup (required for the interactive delivery-seam tests)
+// The static renderToStaticMarkup tests do not use document/window but the
+// setup is harmless for them: it only assigns globals they never read.
 // ---------------------------------------------------------------------------
 
+const dom = new JSDOM("<!doctype html><html><body></body></html>", {
+  url: "http://localhost",
+});
+
+// Deterministic wall-clock epoch — far from real time to avoid expiry surprises.
+// expiresAt is set to FAKE_NOW_SECS + 9_999_999 in the interactive tests so
+// the card never expires during the test.
+const FAKE_NOW_MS = 1_000_000_000_000;
+
+before(() => {
+  mock.timers.enable({ apis: ["setInterval", "Date"], now: FAKE_NOW_MS });
+
+  Object.assign(globalThis, {
+    document: dom.window.document,
+    HTMLElement: dom.window.HTMLElement,
+    IS_REACT_ACT_ENVIRONMENT: true,
+    window: dom.window,
+    MutationObserver: class {
+      observe() {}
+      disconnect() {}
+      takeRecords() {
+        return [];
+      }
+    },
+    ResizeObserver: class {
+      observe() {}
+      unobserve() {}
+      disconnect() {}
+    },
+  });
+  dom.window.matchMedia = () => ({
+    matches: false,
+    addEventListener() {},
+    removeEventListener() {},
+  });
+  dom.window.MutationObserver = globalThis.MutationObserver;
+  dom.window.ResizeObserver = globalThis.ResizeObserver;
+});
+
+afterEach(async () => {
+  const { cleanup } = await import("@testing-library/react");
+  cleanup();
+  mock.timers.reset();
+  mock.timers.enable({ apis: ["setInterval", "Date"], now: FAKE_NOW_MS });
+});
+
+after(() => {
+  mock.timers.reset();
+  dom.window.close();
+});
+
 test("test_allow_once_renders_actionable_allow_button", () => {
   const html = renderToStaticMarkup(
     React.createElement(LifecycleActivity, {
@@ -462,3 +516,175 @@ test("test_f3_cross_layer_four_options_acp_read_to_lifecycle_activity_two_button
     `cross-layer four-option card must render exactly 2 buttons; got ${buttonCount}`,
   );
 });
+
+// ---------------------------------------------------------------------------
+// F3 interactive delivery-seam: acp_read → buildTranscript → LifecycleActivity
+// click buttons → assert _deliveryFn called with ruled allow_once/reject_once IDs
+//
+// Mutation proof: removing `allow_once` from ACTIONABLE_KINDS → the allow_once
+// button is not rendered → fireEvent.click finds no element → first delivery
+// assertion fails. Removing `reject_once` → same for reject_once.
+// Removing both → zero delivery calls → both assertions fail.
+// ---------------------------------------------------------------------------
+
+test("test_f3_interactive_delivery_seam_allow_once_and_reject_once_fire_delivery", 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;
+
+  // Record delivery calls: { optionId, requestNonce }[]
+  const deliveryCalls = [];
+  function mockDeliveryFn({ optionId, requestNonce }) {
+    deliveryCalls.push({ optionId, requestNonce });
+    // Resolve as "acked" so the component doesn't re-enable the button.
+    return Promise.resolve("acked");
+  }
+
+  // Build the transcript card from a real acp_read event carrying all four
+  // option kinds — same wire shape as the static cross-layer test above.
+  const acpReadEvent = {
+    seq: 1,
+    timestamp: "2026-09-01T10:00:00.000Z",
+    kind: "acp_read",
+    agentIndex: 0,
+    channelId: "ch-interactive",
+    sessionId: "sess-interactive",
+    turnId: "turn-interactive",
+    payload: {
+      jsonrpc: "2.0",
+      id: "req-interactive",
+      method: "session/request_permission",
+      params: {
+        title: "Tool requires approval",
+        toolCallId: "tc-interactive",
+        options: [
+          {
+            optionId: "opt-allow-once",
+            kind: "allow_once",
+            name: "Allow once",
+          },
+          { optionId: "opt-reject-once", kind: "reject_once", name: "Deny" },
+          {
+            optionId: "opt-allow-always",
+            kind: "allow_always",
+            name: "Always allow",
+          },
+          {
+            optionId: "opt-reject-always",
+            kind: "reject_always",
+            name: "Always deny",
+          },
+        ],
+      },
+    },
+    authorization: {
+      requestNonce: "nonce-interactive",
+      actionable: true,
+      expiresAt: FUTURE_EXPIRY,
+    },
+  };
+
+  const transcript = buildTranscript([acpReadEvent]);
+  const card = transcript.find((item) => item.renderClass === "permission");
+  assert.ok(card, "transcript must contain a permission card");
+  assert.ok(card.actionable, "card must be actionable");
+
+  let container;
+  await act(async () => {
+    ({ container } = render(
+      createElement(LifecycleActivity, {
+        ...BASE_PROPS,
+        item: card,
+        _deliveryFn: mockDeliveryFn,
+      }),
+    ));
+  });
+
+  // ── Click allow_once — must call delivery with opt-allow-once ─────────────
+  const allowBtn = container.querySelector(
+    '[data-testid="permission-decision-opt-allow-once"]',
+  );
+  assert.ok(
+    allowBtn !== null,
+    "allow_once button must be present (ACTIONABLE_KINDS must include allow_once)",
+  );
+  await act(async () => {
+    fireEvent.click(allowBtn);
+    // Drain microtasks so the async delivery fn resolves.
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+  assert.equal(
+    deliveryCalls.length,
+    1,
+    "exactly one delivery call after clicking allow_once; mutation: remove allow_once from ACTIONABLE_KINDS → zero calls",
+  );
+  assert.equal(
+    deliveryCalls[0].optionId,
+    "opt-allow-once",
+    "delivery must be called with the allow_once optionId; mutation: wrong id → fails",
+  );
+  assert.equal(
+    deliveryCalls[0].requestNonce,
+    "nonce-interactive",
+    "delivery must carry the card's requestNonce",
+  );
+
+  // ── allow_always must NOT have a button (not in ACTIONABLE_KINDS) ─────────
+  assert.equal(
+    container.querySelector(
+      '[data-testid="permission-decision-opt-allow-always"]',
+    ),
+    null,
+    "allow_always must not render a clickable button",
+  );
+
+  // ── reject_always must NOT have a button either ───────────────────────────
+  assert.equal(
+    container.querySelector(
+      '[data-testid="permission-decision-opt-reject-always"]',
+    ),
+    null,
+    "reject_always must not render a clickable button",
+  );
+
+  // ── Render a fresh card and click reject_once ──────────────────────────────
+  // Use a separate render to avoid the pending-state from the allow_once click
+  // disabling the reject_once button.
+  deliveryCalls.length = 0;
+  let container2;
+  await act(async () => {
+    ({ container: container2 } = render(
+      createElement(LifecycleActivity, {
+        ...BASE_PROPS,
+        item: card,
+        _deliveryFn: mockDeliveryFn,
+      }),
+    ));
+  });
+
+  const rejectBtn = container2.querySelector(
+    '[data-testid="permission-decision-opt-reject-once"]',
+  );
+  assert.ok(
+    rejectBtn !== null,
+    "reject_once button must be present (ACTIONABLE_KINDS must include reject_once)",
+  );
+  await act(async () => {
+    fireEvent.click(rejectBtn);
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+  assert.equal(
+    deliveryCalls.length,
+    1,
+    "exactly one delivery call after clicking reject_once; mutation: remove reject_once from ACTIONABLE_KINDS → zero calls",
+  );
+  assert.equal(
+    deliveryCalls[0].optionId,
+    "opt-reject-once",
+    "delivery must be called with the reject_once optionId; mutation: wrong id → fails",
+  );
+});

desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx

@@ -82,6 +82,7 @@ function PermissionDecisionButtons({
   requestNonce,
   deliveryFailed,
   deadlineSecs,
+  _deliveryFn,
 }: {
   agentPubkey: string;
   channelId: string;
@@ -97,7 +98,14 @@ function PermissionDecisionButtons({
    * Effective expiry deadline (unix seconds) bounding the retransmit loop.
    */
   deadlineSecs: number;
+  /**
+   * Seam for testing — injects a mock delivery function without needing
+   * `mock.module`. Production callers omit this; the real
+   * `startPermissionDecisionDelivery` is used by default.
+   */
+  _deliveryFn?: typeof startPermissionDecisionDelivery;
 }) {
+  const deliveryFn = _deliveryFn ?? startPermissionDecisionDelivery;
   const [pending, setPending] = React.useState<string | null>(null);
 
   // Re-enable buttons when the reducer signals delivery failure (non-`sent`
@@ -140,7 +148,7 @@ function PermissionDecisionButtons({
             disabled={pending !== null}
             onClick={() => {
               setPending(optionId);
-              void startPermissionDecisionDelivery({
+              void deliveryFn({
                 agentPubkey,
                 channelId,
                 requestNonce,
@@ -172,7 +180,15 @@ function PermissionDecisionButtons({
   );
 }
 
-export function LifecycleActivity(props: ActivityRenderClassItemProps) {
+export function LifecycleActivity(
+  props: ActivityRenderClassItemProps & {
+    /**
+     * Seam for testing — injected mock delivery function threaded through to
+     * `PermissionDecisionButtons`. Production callers omit this prop.
+     */
+    _deliveryFn?: typeof startPermissionDecisionDelivery;
+  },
+) {
   if (props.item.type === "tool") {
     return <ToolActivity {...props} />;
   }
@@ -230,6 +246,7 @@ export function LifecycleActivity(props: ActivityRenderClassItemProps) {
               props.item.timestamp,
               Date.now() / 1000,
             )}
+            _deliveryFn={props._deliveryFn}
           />
         ) : null}
         {/* Row 5: decision — only when outcome is resolved */}