fix: start the AUTH prompt timeout when the dialog is shown, not when it arrives

The host renders one dialog at a time, but every prompt's 60s deadline started
when its challenge arrived. When several relays challenged at once, prompts 2..N
counted down while queued and invisible, then expired unseen -- a silent deny the
user had no way to act on.

Reaching one after it expired was worse than useless: complete() is a no-op on a
resolved deferred, so the click did nothing at all. No auth sent, no "always
allow" rule written, no feedback.

RelayAuthPrompt.markShown() now opens the answer window. It is gated on a host
actually collecting the flow -- with no UI nobody can ever answer, which is what
the timeout has always existed for, so that case keeps the arrival clock -- and
capped by queueWaitMs so a host that stops rendering mid-queue cannot suspend a
relay coroutine forever.

A second challenge for the same (relay, account) now rides along on the owner's
answer with no deadline of its own. Giving it one would let it complete the
shared deferred and tear the dialog away while the user was still reading it.

Three regression tests cover the queued-clock, the no-host timeout, and the
rider.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01USYujXpjNrQdEyK39Q48Z1
This commit is contained in:
Claude
2026-08-04 01:12:39 +00:00
parent 434a454cda
commit 0e3fa8ac60
4 changed files with 147 additions and 7 deletions
@@ -38,9 +38,23 @@ Shipped as designed. Where it diverged or went further:
where the new sentence belongs — or silently dropped a new `%1$s`. New keys fall
back to the new English until Crowdin catches up. The 401 now-orphaned
translations were deleted from the 11 locale files that carried them.
- **Not done:** the 60-second silent `DISMISS` (item 6 below) is still a silent
deny with the event left pending in the outbox. It needs a decision about what
the user should see, not just a layout.
- **The 60s timeout now runs from display, not from arrival.** Found while
answering "what happens if several auths are requested inside one 60s window?".
The host shows one dialog at a time but every prompt's deadline started when its
challenge arrived, so a burst of relays meant prompt 2..N counted down while
invisible. They expired unseen — a silent deny — and if the user did reach one
after it expired, the click was swallowed whole: `complete()` is a no-op on a
resolved deferred, so no auth was sent and not even the "always allow" rule was
written. `RelayAuthPrompt.markShown()` now starts the window, gated on a host
actually collecting (no UI → the old arrival clock, which is what the timeout was
always for) and capped by `queueWaitMs` so a stuck queue can't suspend a
connection forever. A second challenge for the same (relay, account) rides along
on the owner's answer with no deadline of its own — running one would let it
resolve the shared deferred and tear down a dialog mid-read.
- **Still not done:** what a timeout should *look like*. It is now an honest 60s of
visible time rather than a clock the user never saw, but it is still a dialog
that vanishes and an event left pending in the outbox with no feedback. That
needs a product decision, not a layout.
Verified: `:amethyst:testFdroidDebugUnitTest` 1096 tests green (38 in the relay-auth
suites, 5 of them new), `:commons:jvmTest` 1446 green, `spotlessApply` clean.
@@ -113,7 +113,10 @@ fun RelayAuthPromptHost(
}
}
// One dialog at a time. Everything else waits its turn, and tells the bus when its turn comes so
// its answer window starts from the moment it is visible rather than from the challenge.
queue.firstOrNull { !it.isResolved }?.let { prompt ->
LaunchedEffect(prompt) { prompt.markShown() }
RelayAuthPromptDialog(prompt, accountViewModel, nav) { choice ->
prompt.respond(choice)
queue.remove(prompt)
@@ -26,6 +26,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.withTimeoutOrNull
/** What the user chose when asked whether to authenticate with a relay. */
@@ -64,6 +65,20 @@ class RelayAuthPrompt(
reply.complete(choice)
}
private val shown = CompletableDeferred<Unit>()
/**
* Called by the host the moment this prompt is actually put on screen. The answer window is
* measured from here, not from when the challenge arrived — the host shows one dialog at a time,
* so a prompt can sit queued behind another for a long while, and its clock must not be running
* during that.
*/
fun markShown() {
shown.complete(Unit)
}
internal suspend fun awaitShown() = shown.await()
/** True once answered by the user or resolved by the bus (e.g. timed out). */
val isResolved: Boolean get() = reply.isCompleted
@@ -82,6 +97,7 @@ class RelayAuthPrompt(
*/
class RelayAuthPromptBus(
private val timeoutMs: Long = DEFAULT_TIMEOUT_MS,
private val queueWaitMs: Long = DEFAULT_QUEUE_WAIT_MS,
) {
// 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
@@ -112,16 +128,58 @@ class RelayAuthPromptBus(
?: CompletableDeferred<UserAuthChoice>().also { inFlight[key] = it } to true
}
if (isOwner) mutablePrompts.emit(RelayAuthPrompt(relayUrl, purposes, askingAccount, isMyOwnRelay, deferred))
// Only the owner surfaces a dialog and owns its deadline. A second challenge for the same
// (relay, account) just rides along on the owner's answer — it must NOT run a deadline of its
// own, because resolving the shared deferred would tear down a dialog the user is still
// looking at.
if (!isOwner) return rideAlong(deferred)
val prompt = RelayAuthPrompt(relayUrl, purposes, askingAccount, isMyOwnRelay, deferred)
mutablePrompts.emit(prompt)
return try {
awaitOrTimeout(deferred)
awaitOrTimeout(prompt, deferred)
} finally {
if (isOwner) synchronized(inFlight) { inFlight.remove(key) }
synchronized(inFlight) { inFlight.remove(key) }
}
}
private suspend fun awaitOrTimeout(deferred: CompletableDeferred<UserAuthChoice>): UserAuthChoice {
/**
* Waits for the owner's answer without imposing a deadline that could resolve the shared prompt.
* The cap exists only so a cancelled owner can't strand this caller forever; it deliberately
* returns [UserAuthChoice.DISMISS] *locally* rather than completing the deferred.
*/
private suspend fun rideAlong(deferred: CompletableDeferred<UserAuthChoice>): UserAuthChoice = withTimeoutOrNull(queueWaitMs + 2 * timeoutMs) { deferred.await() } ?: UserAuthChoice.DISMISS
/**
* Waits for the user's answer, giving them [timeoutMs] **from the moment the dialog is on screen**
* rather than from the moment the challenge arrived.
*
* That distinction is the whole point. The host renders one dialog at a time, so when several
* relays challenge at once every prompt but the first is queued and invisible — and with a single
* deadline measured from arrival, those queued prompts expired without ever being shown. The user
* saw nothing, the relay was silently denied, and a click on a dialog that had already expired was
* swallowed whole: `complete()` is a no-op on a resolved deferred, so the auth was never sent and
* not even the "always allow" rule was written.
*/
private suspend fun awaitOrTimeout(
prompt: RelayAuthPrompt,
deferred: CompletableDeferred<UserAuthChoice>,
): UserAuthChoice {
// Is anything able to display this? With no host collecting — a headless background process,
// or a cold start before the UI subscribes — nobody can ever answer, and the relay coroutine
// must not hang. That case is what the timeout has always been for, so it keeps the old clock.
val hasHost = withTimeoutOrNull(timeoutMs) { mutablePrompts.subscriptionCount.first { it > 0 } } != null
if (hasHost) {
// Wait for this prompt's turn at the front of the host's queue. Capped, so a host that
// stops rendering (backgrounded mid-queue) still can't suspend the connection forever.
withTimeoutOrNull(queueWaitMs) { prompt.awaitShown() }
}
// The answer window proper. If the user already answered while we were waiting above, this
// returns immediately.
withTimeoutOrNull(timeoutMs) { deferred.await() }?.let { return it }
// Timed out: resolve the deferred so any UI still showing this prompt can drop it, and so a
// concurrent waiter on the same deferred gets an answer too. complete() is a no-op if a late
// user response already won the race.
@@ -131,5 +189,8 @@ class RelayAuthPromptBus(
companion object {
const val DEFAULT_TIMEOUT_MS = 60_000L
/** How long a prompt may wait its turn behind other dialogs before we give up on it. */
const val DEFAULT_QUEUE_WAIT_MS = 5 * 60_000L
}
}
@@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.relayClient.authCommand.model
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.take
import kotlinx.coroutines.flow.toList
@@ -113,4 +114,65 @@ class RelayAuthPromptBusTest {
bus.prompts.first().respond(UserAuthChoice.ALLOW_ONCE)
assertEquals(UserAuthChoice.ALLOW_ONCE, caller.await())
}
@Test
fun aQueuedPromptsClockStartsWhenItIsShown() =
runTest {
val bus = RelayAuthPromptBus(timeoutMs = 1_000L)
val relayA = NormalizedRelayUrl("wss://a.relay.test")
val relayB = NormalizedRelayUrl("wss://b.relay.test")
val surfaced = async { bus.prompts.take(2).toList() }
val callerA = async { bus.requestDecision(relayA, emptyList(), alice, isMyOwnRelay = false) }
val callerB = async { bus.requestDecision(relayB, emptyList(), alice, isMyOwnRelay = false) }
val prompts = surfaced.await()
// The host renders one dialog at a time, so B sits queued and invisible while the user
// works through A. Only A is on screen, so only A's clock is running.
prompts[0].markShown()
delay(600)
prompts[0].respond(UserAuthChoice.ALLOW_ONCE)
assertEquals(UserAuthChoice.ALLOW_ONCE, callerA.await())
// B's turn. Its own window opens now — well past the point where a single deadline
// measured from the challenge would already have expired it unseen.
prompts[1].markShown()
delay(600)
prompts[1].respond(UserAuthChoice.ALWAYS_ALLOW)
assertEquals(UserAuthChoice.ALWAYS_ALLOW, callerB.await())
}
@Test
fun aPromptNoHostCanEverShowStillTimesOut() =
runTest {
// Nothing collects the flow, so nobody can answer. The relay coroutine must not hang
// waiting for a dialog that will never appear.
val bus = RelayAuthPromptBus(timeoutMs = 1_000L, queueWaitMs = 60_000L)
assertEquals(UserAuthChoice.DISMISS, bus.ask())
}
@Test
fun aSecondChallengeNeverResolvesTheDialogTheUserIsLookingAt() =
runTest {
val bus = RelayAuthPromptBus(timeoutMs = 1_000L)
val surfaced = async { bus.prompts.first() }
val owner = async { bus.ask() }
val rider = async { bus.ask() }
val prompt = surfaced.await()
// The prompt is queued behind another dialog well past the rider's old deadline. The
// rider has no dialog of its own, so it must simply wait: if it ran its own clock it
// would complete the shared deferred at 1000ms and yank this prompt away before the
// user ever saw it.
delay(1_500)
prompt.markShown()
delay(300)
prompt.respond(UserAuthChoice.ALWAYS_ALLOW)
assertEquals(UserAuthChoice.ALWAYS_ALLOW, owner.await())
assertEquals(UserAuthChoice.ALWAYS_ALLOW, rider.await())
}
}