From d1ba04ad52ae9ba5a5b5a69a206630c165b29c50 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 22:19:13 +0000 Subject: [PATCH] =?UTF-8?q?fix(relayauth):=20pre-merge=20audit=20=E2=80=94?= =?UTF-8?q?=20auth=20retry=20budget=20+=20lost-prompt=20window?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs found in a pre-merge audit: - quartz PoolEventOutboxState: auth-required NAKs only spared the `responses` budget, but `tries` (grown by every send/re-pump and NOT auth-aware) still accumulated across reconnects, so a slow/flapping AUTH handshake could exhaust Tries.isDone() and drop the event — with a spurious onEventGaveUp — before AUTH landed. Now an auth-required NAK resets the relay's retry budget (it responded, so it's up and just wants auth). Regression test added. - RelayAuthPromptBus used a replay=0 SharedFlow, so a challenge that resolved to ASK before RelayAuthPromptHost subscribed (cold start / account switch) was dropped and the auth coroutine stalled the full timeout then DISMISSed. Add replay so late subscribers recover pending prompts (the host already filters resolved ones). Regression test added. Also record the as-built design (Always/Never/Custom + toggles, venues, give-up toast, known deny-relay-outbox limitation) in the plan doc. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EZjmYpgHP4pf79Sav5QT8a --- ...2026-07-01-auth-permission-architecture.md | 27 ++++++++++++++++++- .../authCommand/model/RelayAuthPromptBus.kt | 6 ++++- .../model/RelayAuthPromptBusTest.kt | 16 +++++++++++ .../relay/client/pool/PoolEventOutboxState.kt | 15 ++++++----- .../client/pool/PoolEventOutboxAuthTest.kt | 22 +++++++++++++++ 5 files changed, 78 insertions(+), 8 deletions(-) diff --git a/amethyst/plans/2026-07-01-auth-permission-architecture.md b/amethyst/plans/2026-07-01-auth-permission-architecture.md index 8575a3961d..f3d431047a 100644 --- a/amethyst/plans/2026-07-01-auth-permission-architecture.md +++ b/amethyst/plans/2026-07-01-auth-permission-architecture.md @@ -2,7 +2,32 @@ **Date:** 2026-07-01 **Module:** `amethyst` (+ shared bits in `commons`) -**Status:** Design / proposal +**Status:** Implemented — see "As-built" below for where the shipped design diverged from this proposal. + +## As-built (final) + +The implementation kept this doc's core ideas (purpose derivation, a prompt bus, +per-relay overrides, grant rationale) but the policy model was reshaped during +review: + +- **Global mode is `RelayAuthPolicy { ALWAYS, NEVER, CUSTOM }`** — the earlier + `IF_IN_MY_LIST` / `TRUSTED_FOLLOWS` values were dropped. `CUSTOM` applies a + `RelayAuthCustomToggles` set of independent switches: **my relays & venues**, + **read posts from follows**, **message follows**, **message strangers** + (off by default). New-install default is `CUSTOM` with the first three on. +- **`AuthPurpose` is a `data class` (kind + counterparties + venues)** over an + `AuthPurposeKind` enum (SEND_DM, NOTIFY_INBOX, READ_OUTBOX, POST_VENUE, + READ_VENUE, MY_OWN_RELAY, OTHER) — not a sealed interface. Venues (NIP-28 + public chats, NIP-72 communities, NIP-53 live activities) are first-class. +- **Settings screen** uses the app's settings design system (`SettingsSection` + card + `SettingsSwitchTile`) for the toggles and a grouped, lazily-rendered + per-relay list (NIP-11 icon, `displayUrl`, tap → relay info). +- **Give-up signal**: quartz's outbox surfaces `onEventGaveUp`, toasted by + `RelayPublishFailureToast`. `auth-required` NAKs never burn the retry budget + (they reset it) so a slow AUTH handshake can't drop the event. +- **Known limitation**: an event queued to a relay the user then *denies* stays + pending in the outbox (auth-required never gives up); evicting it would need a + quartz "give up on relay for this event" API — deferred. ## Context diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt index 5e8e6857a5..e09d791f25 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt @@ -74,7 +74,11 @@ class RelayAuthPrompt( class RelayAuthPromptBus( private val timeoutMs: Long = DEFAULT_TIMEOUT_MS, ) { - private val mutablePrompts = MutableSharedFlow(extraBufferCapacity = 32) + // replay so a challenge raised *before* the UI host subscribes — cold start, an account switch, + // any moment no RelayAuthPromptHost is collecting — isn't dropped (which would stall the auth + // coroutine the full timeout and then silently DISMISS). The host filters out any already- + // resolved prompt it replays, so re-delivering stale ones is harmless. + private val mutablePrompts = MutableSharedFlow(replay = 32, extraBufferCapacity = 32) val prompts: SharedFlow = mutablePrompts private val inFlight = mutableMapOf>() diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt index 707489f24a..e3e71b44de 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.relayClient.authCommand.model import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import kotlinx.coroutines.async import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Test @@ -68,4 +69,19 @@ class RelayAuthPromptBusTest { // No one ever responds; the call must not hang, it resolves to DISMISS. assertEquals(UserAuthChoice.DISMISS, bus.requestDecision(relay, emptyList())) } + + @Test + fun retainsPromptForALateSubscriberSoItIsNotLost() = + runTest { + val bus = RelayAuthPromptBus() + + // A challenge fires while NO host is collecting (cold start / account switch). + val caller = async { bus.requestDecision(relay, emptyList()) } + runCurrent() // let the emit happen with no subscriber present + + // A host subscribes late; the replayed prompt must still reach it (not be lost, which + // would strand the caller until the timeout and then silently DISMISS). + bus.prompts.first().respond(UserAuthChoice.ALLOW_ONCE) + assertEquals(UserAuthChoice.ALLOW_ONCE, caller.await()) + } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxState.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxState.kt index c52a3eb896..f3f857d8c4 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxState.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxState.kt @@ -74,12 +74,15 @@ class PoolEventOutboxState( relaysRemaining = relaysRemaining - url failures = failures - url } else if (message.isAuthRequired()) { - // NIP-42 AUTH challenge in flight — don't count toward the try cap. - // RelayAuthenticator signs + relay re-issues OK; syncFilters() then - // re-pumps this outbox so the original publish is retried. Leave - // relaysRemaining and failures untouched, otherwise a relay that NAKs - // every unauthed EVENT would exhaust the retry budget and drop the - // event before AUTH lands. + // NIP-42 AUTH challenge in flight. The relay responded, so it's up and simply wants + // auth first: RelayAuthenticator signs, the relay re-issues OK, and syncFilters() + // re-pumps this outbox to retry the publish. Reset the retry budget for this relay + // (clear both tries and responses) so the send attempts accumulated across reconnects / + // slow AUTH rounds can't exhaust the cap and drop the event before AUTH lands. Note + // newTry() (the send path) grows `tries` and is NOT auth-aware, so only clearing + // `responses` would still let a re-pumped event give up here. relaysRemaining is left + // as-is: the event must stay pending for this relay until AUTH unlocks it. + failures = failures - url } else { val currentTries = failures[url] if (currentTries != null) { diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxAuthTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxAuthTest.kt index f4b0b94dc4..f8250eb9a1 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxAuthTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxAuthTest.kt @@ -103,6 +103,28 @@ class PoolEventOutboxAuthTest { assertNull(outbox.pendingRelaysFor(ev.id)) } + @Test + fun authRequiredResetsTheTriesBudgetAcrossManyResends() { + val outbox = PoolEventOutbox() + val ev = event("ff".repeat(32)) + + outbox.publish(ev, setOf(relay)) // first try + + // A flapping relay / slow AUTH handshake re-pumps the still-pending event many more times + // than the 4-try cap, each NAK'd auth-required. newTry() (the send path) grows `tries` and + // is not auth-aware, so unless auth-required resets the retry budget these sends would trip + // Tries.isDone() and drop the event (with a spurious give-up) before AUTH ever lands. + repeat(8) { i -> + assertNull(outbox.onSent(relay, EventCmd(ev)), "must not give up on re-pump $i") + outbox.nak(ev, relay, "auth-required: authenticate first") + } + assertEquals(setOf(relay), outbox.pendingRelaysFor(ev.id)) + + // AUTH finally completes -> the event delivers. + outbox.ok(ev, relay) + assertNull(outbox.pendingRelaysFor(ev.id)) + } + @Test fun terminalRejectionStillDiscardsImmediately() { val outbox = PoolEventOutbox()