fix(relayauth): pre-merge audit — auth retry budget + lost-prompt window

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZjmYpgHP4pf79Sav5QT8a
This commit is contained in:
Claude
2026-07-10 22:40:13 +00:00
parent 50d942cfce
commit d1ba04ad52
5 changed files with 78 additions and 8 deletions
@@ -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
@@ -74,7 +74,11 @@ class RelayAuthPrompt(
class RelayAuthPromptBus(
private val timeoutMs: Long = DEFAULT_TIMEOUT_MS,
) {
private val mutablePrompts = MutableSharedFlow<RelayAuthPrompt>(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<RelayAuthPrompt>(replay = 32, extraBufferCapacity = 32)
val prompts: SharedFlow<RelayAuthPrompt> = mutablePrompts
private val inFlight = mutableMapOf<NormalizedRelayUrl, CompletableDeferred<UserAuthChoice>>()
@@ -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())
}
}
@@ -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) {
@@ -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()