diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTracker.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTracker.kt index 7e27dda358..3f0c4f1564 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTracker.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTracker.kt @@ -24,16 +24,46 @@ import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.RelayInsertConfirmationCollector +import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.MachineReadablePrefix import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +/** + * One relay's refusal of a published event — its `OK false` reason, verbatim, plus the NIP-01 + * machine-readable [prefix] when the relay supplied one. + * + * This is the only thing a relay ever says about *why* a message didn't land. A Buzz tenant ban + * (kind 9040) or timeout (9042) reaches the client here and nowhere else: the relay enforces it at + * publish time and answers `restricted: ...`, with no event for the client to fold. + */ +@Immutable +data class RelayRejection( + val relay: NormalizedRelayUrl, + val reason: String, + val prefix: MachineReadablePrefix? = MachineReadablePrefix.parse(reason), +) { + /** True when retrying the same event unchanged could plausibly succeed. */ + val isTransient: Boolean + get() = + when (prefix) { + MachineReadablePrefix.RATE_LIMITED, MachineReadablePrefix.ERROR -> true + // A refusal we can act on: authenticate, then republish. + MachineReadablePrefix.AUTH_REQUIRED -> true + // blocked/restricted (bans, timeouts, non-members), invalid, unsupported, pow: + // resending the identical event changes nothing. Unprefixed reasons are unknown, + // and claiming a retry will help would be a guess. + else -> false + } +} + /** Delivery progress of one recipient's gift wrap (NIP-17 DMs). */ @Immutable data class RecipientDelivery( val recipient: HexKey, val targetRelays: Set, val acceptedRelays: Set = emptySet(), + val rejections: List = emptyList(), // The sender's own self-copy wrap: shown in the delivery detail (it matters // for multi-device sync) but excluded from "delivered to everyone" and the // k/n count, which describe the OTHER participants. @@ -41,6 +71,10 @@ data class RecipientDelivery( ) { val isDelivered: Boolean get() = acceptedRelays.isNotEmpty() + + /** Every target relay refused and none accepted — this recipient's wrap did not land. */ + val isRefused: Boolean + get() = acceptedRelays.isEmpty() && rejections.isNotEmpty() && rejections.mapTo(HashSet()) { it.relay }.containsAll(targetRelays) } /** @@ -52,6 +86,7 @@ data class RecipientDelivery( data class ChatDelivery( val targetRelays: Set, val acceptedRelays: Set = emptySet(), + val rejections: List = emptyList(), val recipients: List? = null, ) { /** The other participants' wraps (self-copy excluded); null for rooms. */ @@ -67,6 +102,32 @@ data class ChatDelivery( targetRelays.isNotEmpty() && acceptedRelays.containsAll(targetRelays) } } + + /** + * The message reached nobody: every target relay answered `OK false` and none accepted. + * + * Deliberately strict. A relay that simply hasn't answered yet leaves this false — the message + * is still in flight, not refused — and one relay refusing while another accepts is a delivered + * message, so the refusal belongs in the detail view rather than on the bubble. The strictness + * is what makes the true case worth showing: on a single-relay room (a Buzz workspace channel) + * it fires on the first refusal. + */ + val isRefused: Boolean + get() { + val others = otherRecipients + return if (others != null) { + others.isNotEmpty() && others.all { it.isRefused } + } else { + acceptedRelays.isEmpty() && rejections.isNotEmpty() && rejections.mapTo(HashSet()) { it.relay }.containsAll(targetRelays) + } + } + + /** A representative refusal to show on the bubble; the detail view lists them all. */ + val firstRejection: RelayRejection? + get() = rejections.firstOrNull() ?: otherRecipients?.firstNotNullOfOrNull { it.rejections.firstOrNull() } + + /** Rejections keyed by relay, for the per-relay detail rows. */ + fun rejectionFor(relay: NormalizedRelayUrl): RelayRejection? = rejections.firstOrNull { it.relay == relay } } /** @@ -108,7 +169,10 @@ class ChatDeliveryTracker( private var knownIds = setOf() private val okCollector = - RelayInsertConfirmationCollector(client) { eventId, relay -> + RelayInsertConfirmationCollector( + client, + onRelayRejected = { eventId, relay, reason -> onRejected(eventId, relay.url, reason) }, + ) { eventId, relay -> onAccepted(eventId, relay.url) } @@ -142,6 +206,7 @@ class ChatDeliveryTracker( ChatDelivery( targetRelays = (current?.targetRelays ?: emptySet()) + targetRelays, acceptedRelays = current?.acceptedRelays ?: emptySet(), + rejections = current?.rejections ?: emptyList(), recipients = (current?.recipients ?: emptyList()) + RecipientDelivery(recipient, targetRelays, isSelf = isSelf), ) @@ -208,10 +273,17 @@ class ChatDeliveryTracker( flow.value = delivery.copy( acceptedRelays = delivery.acceptedRelays + relay, + // A relay that accepted is no longer refusing: the auth-required -> AUTH -> + // republish round trip refuses first and then stores, and leaving the stale + // refusal behind would report a delivered message as rejected. + rejections = delivery.rejections.filterNot { it.relay == relay }, recipients = delivery.recipients?.map { if (it.recipient == recipient) { - it.copy(acceptedRelays = it.acceptedRelays + relay) + it.copy( + acceptedRelays = it.acceptedRelays + relay, + rejections = it.rejections.filterNot { r -> r.relay == relay }, + ) } else { it } @@ -220,11 +292,69 @@ class ChatDeliveryTracker( } else { val flow = deliveries[eventId] ?: return val delivery = flow.value ?: return - flow.value = delivery.copy(acceptedRelays = delivery.acceptedRelays + relay) + flow.value = + delivery.copy( + acceptedRelays = delivery.acceptedRelays + relay, + rejections = delivery.rejections.filterNot { it.relay == relay }, + ) } } } + /** + * A relay refused [eventId]. Recorded per relay so the bubble can say the message didn't land and + * the detail view can quote the reason — the only place a relay-side ban, timeout, rate limit or + * unsupported kind ever becomes visible to the sender. + */ + private fun onRejected( + eventId: HexKey, + relay: NormalizedRelayUrl, + reason: String, + ) { + // `duplicate:` means the relay already holds the event, which is delivery, not refusal. + // Relays disagree on whether to pair it with OK true or OK false, so normalize here. + if (MachineReadablePrefix.parse(reason) == MachineReadablePrefix.DUPLICATE) { + onAccepted(eventId, relay) + return + } + + // Lock-free negative path, as in [onAccepted]. + if (eventId !in wrapIndex && eventId !in knownIds) return + + synchronized(lock) { + val rejection = RelayRejection(relay, reason) + val wrapTarget = wrapIndex[eventId] + if (wrapTarget != null) { + val (noteId, recipient) = wrapTarget + val flow = deliveries[noteId] ?: return + val delivery = flow.value ?: return + + flow.value = + delivery.copy( + rejections = delivery.rejections.addingOnce(rejection), + recipients = + delivery.recipients?.map { + if (it.recipient == recipient) { + it.copy(rejections = it.rejections.addingOnce(rejection)) + } else { + it + } + }, + ) + } else { + val flow = deliveries[eventId] ?: return + val delivery = flow.value ?: return + flow.value = delivery.copy(rejections = delivery.rejections.addingOnce(rejection)) + } + } + } + + /** + * Appends [rejection] unless this relay already has one recorded — a relay can repeat an `OK + * false` across reconnects/retries, and the detail view shows one row per relay. + */ + private fun List.addingOnce(rejection: RelayRejection): List = if (any { it.relay == rejection.relay }) this else this + rejection + // Must run under [lock]. Also creates entries for ids queried by the UI // before their send registers (compose can win that race), so both paths // share the same flow instance. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatDeliveryTicks.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatDeliveryTicks.kt index 703a7c6b4e..bef03194f7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatDeliveryTicks.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatDeliveryTicks.kt @@ -52,6 +52,7 @@ import com.vitorpamplona.amethyst.commons.model.Channel import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.service.relayClient.chatDelivery.ChatDelivery import com.vitorpamplona.amethyst.service.relayClient.chatDelivery.RecipientDelivery +import com.vitorpamplona.amethyst.service.relayClient.chatDelivery.RelayRejection import com.vitorpamplona.amethyst.ui.components.ClickableBox import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.note.UserPicture @@ -78,6 +79,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.displayUrl * - clock: published, no relay has accepted yet * - single check: accepted somewhere (at least one relay OK / seen-on relay) * - double check (green): every recipient's / target relay accepted + * - error (red): every target relay refused it — the message did not land anywhere * * Old messages we didn't track this session simply show no tick, but the time still * opens the dialog (which lists the relays it was seen on, if any). @@ -195,6 +197,17 @@ private fun ChatDeliveryDetailDialog( baseNote.inGatherers?.firstNotNullOfOrNull { (it as? Channel)?.relays()?.takeIf { r -> r.isNotEmpty() } } ?: emptySet() } + // Lead with the verdict when the message didn't land: the per-relay rows below carry + // the detail, but the headline answer to "did this send?" should not have to be + // inferred from a list of ticks. + if (delivery?.isRefused == true) { + Text( + text = stringRes(R.string.chat_delivery_refused_summary), + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodyMedium, + ) + } + val recipients = delivery?.recipients when { !recipients.isNullOrEmpty() -> @@ -207,6 +220,7 @@ private fun ChatDeliveryDetailDialog( RelayDeliveryRow( relay = relay, accepted = relay in delivery.acceptedRelays || relay in seenOnRelays, + rejection = delivery.rejectionFor(relay), ) } @@ -246,7 +260,13 @@ private fun ChatDeliveryDetailDialog( onDismiss() }, ) { - Text(stringRes(R.string.broadcast)) + // Same action either way — re-publish the event (or, for a wrapped message, its + // delivering envelope). Only the label changes: after a refusal the user is + // retrying a failed send, not re-broadcasting a delivered one. A refusal the + // relay will repeat verbatim (a ban, an unsupported kind) keeps the neutral + // label, so the button doesn't promise a retry that cannot work. + val retryable = delivery?.isRefused == true && delivery.firstRejection?.isTransient == true + Text(stringRes(if (retryable) R.string.retry else R.string.broadcast)) } } }, @@ -280,7 +300,7 @@ private fun RecipientDeliveryRow( } } - DeliveryStatusTick(recipient.isDelivered) + DeliveryStatusTick(accepted = recipient.isDelivered, refused = recipient.isRefused) } } @@ -288,28 +308,44 @@ private fun RecipientDeliveryRow( private fun RelayDeliveryRow( relay: NormalizedRelayUrl, accepted: Boolean, + rejection: RelayRejection? = null, ) { - Row( - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(8.dp), - modifier = Modifier.fillMaxWidth(), - ) { - Text( - text = relay.displayUrl(), - modifier = Modifier.weight(1f), - maxLines = 1, - ) + Column(modifier = Modifier.fillMaxWidth()) { + Row( + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.fillMaxWidth(), + ) { + Text( + text = relay.displayUrl(), + modifier = Modifier.weight(1f), + maxLines = 1, + ) - DeliveryStatusTick(accepted) + DeliveryStatusTick(accepted = accepted, refused = !accepted && rejection != null) + } + + // The relay's own words. This is the only explanation of a refusal that exists — a Buzz ban + // or timeout is enforced at publish time and never arrives as an event we could fold. + if (!accepted && rejection != null) { + Text( + text = stringRes(R.string.chat_delivery_refused_reason, rejection.reason), + color = MaterialTheme.colorScheme.error, + fontSize = Font12SP, + ) + } } } @Composable -private fun DeliveryStatusTick(delivered: Boolean) { - if (delivered) { - TickIcon(MaterialSymbols.Done, R.string.chat_delivery_accepted, MaterialTheme.colorScheme.allGoodColor) - } else { - TickIcon(MaterialSymbols.Schedule, R.string.chat_delivery_pending, MaterialTheme.colorScheme.placeholderText) +private fun DeliveryStatusTick( + accepted: Boolean, + refused: Boolean = false, +) { + when { + accepted -> TickIcon(MaterialSymbols.Done, R.string.chat_delivery_accepted, MaterialTheme.colorScheme.allGoodColor) + refused -> TickIcon(MaterialSymbols.ErrorOutline, R.string.chat_delivery_refused, MaterialTheme.colorScheme.error) + else -> TickIcon(MaterialSymbols.Schedule, R.string.chat_delivery_pending, MaterialTheme.colorScheme.placeholderText) } } @@ -341,6 +377,7 @@ private fun RenderDeliveryTicks( DeliveryLadderTick( pending = deliveredCount == 0 && !seenSomewhere, fullyAccepted = delivery.isFullyAccepted, + refused = delivery.isRefused && !seenSomewhere, ) Text( text = "$deliveredCount/${others.size}", @@ -357,16 +394,22 @@ private fun RenderDeliveryTicks( DeliveryLadderTick( pending = !acceptedSomewhere, fullyAccepted = delivery.isFullyAccepted, + refused = delivery.isRefused && !seenSomewhere, ) } -/** The shared pending -> accepted-somewhere -> fully-accepted tick selection. */ +/** The shared refused / pending -> accepted-somewhere -> fully-accepted tick selection. */ @Composable private fun DeliveryLadderTick( pending: Boolean, fullyAccepted: Boolean, + refused: Boolean = false, ) { when { + // Checked first: a refusal is the one outcome the clock would misreport as "still trying". + refused -> + TickIcon(MaterialSymbols.ErrorOutline, R.string.chat_delivery_refused, MaterialTheme.colorScheme.error) + pending -> TickIcon(MaterialSymbols.Schedule, R.string.chat_delivery_pending, MaterialTheme.colorScheme.placeholderText) diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 5542ee2916..73ae261bd1 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -3643,6 +3643,9 @@ No relay information for this message Accepted by at least one relay Delivered to all recipients\' relays + Refused by the relay — this message was not delivered + Not delivered. The relay refused this message. + Relay said: %1$s Add expiration date Remove expiration date diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTrackerTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTrackerTest.kt new file mode 100644 index 0000000000..c89519c0cf --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/chatDelivery/ChatDeliveryTrackerTest.kt @@ -0,0 +1,293 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.relayClient.chatDelivery + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.relay.client.EmptyNostrClient +import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient +import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener +import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient +import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.MachineReadablePrefix +import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.OkMessage +import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.Command +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * How an outgoing chat message's delivery state answers "did this send?". + * + * The gap this closes: the tracker only ever listened for `OK true`, so a refused message sat on the + * pending clock forever, indistinguishable from one still in flight. That is the whole of what a + * banned Buzz member saw — the relay enforces a tenant ban (kind 9040) or timeout (9042) at publish + * time and answers `OK false`, with no event for the client to fold, so the refusal reaches the user + * here or nowhere. + * + * Drives real `OK` frames through the real listener the tracker installs, rather than poking at its + * internals. + */ +class ChatDeliveryTrackerTest { + private val relayA = NormalizedRelayUrl("wss://a.example/") + private val relayB = NormalizedRelayUrl("wss://b.example/") + private val noteId: HexKey = "aa".repeat(32) + private val wrapId: HexKey = "bb".repeat(32) + private val alice: HexKey = "c1".repeat(32) + private val bob: HexKey = "c2".repeat(32) + + private class Harness { + val client = CapturingClient() + val tracker = ChatDeliveryTracker(client) + + fun ok( + eventId: HexKey, + relay: NormalizedRelayUrl, + ) = deliver(eventId, relay, OkMessage.accepted(eventId)) + + fun refuse( + eventId: HexKey, + relay: NormalizedRelayUrl, + reason: String, + ) = deliver(eventId, relay, OkMessage(eventId, false, reason)) + + private fun deliver( + eventId: HexKey, + relay: NormalizedRelayUrl, + msg: OkMessage, + ) { + client.listener!!.onIncomingMessage(relayClient(relay), msg.toJson(), msg) + } + + fun stateOf(id: HexKey) = tracker.currentFor(id) + } + + @Test + fun aRefusalIsRecordedWithTheRelaysOwnReason() { + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA)) + + h.refuse(noteId, relayA, "restricted: you are banned from posting here") + + val delivery = h.stateOf(noteId)!! + assertEquals(1, delivery.rejections.size) + assertEquals(relayA, delivery.rejections[0].relay) + assertEquals("restricted: you are banned from posting here", delivery.rejections[0].reason) + assertEquals(MachineReadablePrefix.RESTRICTED, delivery.rejections[0].prefix) + // The single-relay room case — a Buzz workspace channel — so one refusal is the whole answer. + assertTrue(delivery.isRefused) + assertFalse(delivery.isFullyAccepted) + } + + @Test + fun aBanIsNotOfferedAsRetryableButARateLimitIs() { + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA)) + h.refuse(noteId, relayA, "blocked: pubkey is banned") + assertFalse(h.stateOf(noteId)!!.firstRejection!!.isTransient) + + val other = "dd".repeat(32) + h.tracker.trackPublic(other, setOf(relayA)) + h.refuse(other, relayA, "rate-limited: slow down") + assertTrue(h.stateOf(other)!!.firstRejection!!.isTransient) + } + + @Test + fun oneRelayRefusingWhileAnotherAcceptsIsStillDelivered() { + // The refusal is worth recording (it shows in the detail rows) but the message landed, so the + // bubble must not claim otherwise. + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA, relayB)) + + h.refuse(noteId, relayA, "invalid: kind not accepted here") + h.ok(noteId, relayB) + + val delivery = h.stateOf(noteId)!! + assertEquals(setOf(relayB), delivery.acceptedRelays) + assertEquals(1, delivery.rejections.size) + assertFalse(delivery.isRefused) + } + + @Test + fun aRelayThatHasNotAnsweredYetKeepsTheMessagePendingRatherThanRefused() { + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA, relayB)) + + h.refuse(noteId, relayA, "error: try again later") + + // relayB is still out. Refused means "nowhere left to land", which isn't true yet. + assertFalse(h.stateOf(noteId)!!.isRefused) + + h.refuse(noteId, relayB, "error: try again later") + assertTrue(h.stateOf(noteId)!!.isRefused) + } + + @Test + fun duplicateCountsAsDeliveredEvenWhenSentAsARefusal() { + // Relays disagree on whether `duplicate:` rides an OK true or an OK false; either way the + // relay holds the event, so reporting it as a failed send would be wrong. + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA)) + + h.refuse(noteId, relayA, "duplicate: have this event") + + val delivery = h.stateOf(noteId)!! + assertEquals(setOf(relayA), delivery.acceptedRelays) + assertTrue(delivery.rejections.isEmpty()) + assertFalse(delivery.isRefused) + assertTrue(delivery.isFullyAccepted) + } + + @Test + fun anAcceptanceAfterARefusalClearsIt() { + // The auth-required -> AUTH -> republish round trip: the same relay refuses, then stores. + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA)) + + h.refuse(noteId, relayA, "auth-required: we only accept events from authenticated users") + assertTrue(h.stateOf(noteId)!!.isRefused) + + h.ok(noteId, relayA) + + val delivery = h.stateOf(noteId)!! + assertTrue(delivery.rejections.isEmpty()) + assertFalse(delivery.isRefused) + assertTrue(delivery.isFullyAccepted) + } + + @Test + fun aRepeatedRefusalFromOneRelayIsRecordedOnce() { + val h = Harness() + h.tracker.trackPublic(noteId, setOf(relayA)) + + h.refuse(noteId, relayA, "blocked: nope") + h.refuse(noteId, relayA, "blocked: nope") + + assertEquals(1, h.stateOf(noteId)!!.rejections.size) + } + + @Test + fun aWrappedRoomMessageAttributesTheRefusalToTheDisplayedNote() { + // Concord: the relay OKs the encrypted wrap, but the feed shows the inner rumor. + val h = Harness() + h.tracker.trackWrappedPublic(noteId, wrapId, setOf(relayA)) + + h.refuse(wrapId, relayA, "restricted: not a member") + + val delivery = h.stateOf(noteId)!! + assertEquals(1, delivery.rejections.size) + assertTrue(delivery.isRefused) + } + + @Test + fun aDmRefusalIsAttributedToTheRecipientWhoseWrapItWas() { + val h = Harness() + val aliceWrap = "e1".repeat(32) + val bobWrap = "e2".repeat(32) + h.tracker.trackWrap(noteId, alice, aliceWrap, setOf(relayA)) + h.tracker.trackWrap(noteId, bob, bobWrap, setOf(relayB)) + + h.refuse(aliceWrap, relayA, "blocked: sender not allowed") + h.ok(bobWrap, relayB) + + val delivery = h.stateOf(noteId)!! + val byRecipient = delivery.otherRecipients!!.associateBy { it.recipient } + assertTrue(byRecipient.getValue(alice).isRefused) + assertFalse(byRecipient.getValue(alice).isDelivered) + assertTrue(byRecipient.getValue(bob).isDelivered) + assertFalse(byRecipient.getValue(bob).isRefused) + // Reached one of the two, so the message as a whole is neither refused nor fully delivered. + assertFalse(delivery.isRefused) + assertFalse(delivery.isFullyAccepted) + } + + @Test + fun aDmRefusedForEveryRecipientIsRefused() { + val h = Harness() + val aliceWrap = "e1".repeat(32) + val bobWrap = "e2".repeat(32) + h.tracker.trackWrap(noteId, alice, aliceWrap, setOf(relayA)) + h.tracker.trackWrap(noteId, bob, bobWrap, setOf(relayB)) + + h.refuse(aliceWrap, relayA, "blocked: nope") + h.refuse(bobWrap, relayB, "blocked: nope") + + assertTrue(h.stateOf(noteId)!!.isRefused) + } + + @Test + fun theSendersSelfCopyDoesNotDecideTheVerdict() { + // The self-copy exists for multi-device sync; "did this reach the other person" is about the + // other participants, refusals included. + val h = Harness() + val selfWrap = "e3".repeat(32) + val aliceWrap = "e1".repeat(32) + h.tracker.trackWrap(noteId, alice, aliceWrap, setOf(relayA)) + h.tracker.trackWrap(noteId, bob, selfWrap, setOf(relayB), isSelf = true) + + h.ok(selfWrap, relayB) + h.refuse(aliceWrap, relayA, "blocked: nope") + + assertTrue(h.stateOf(noteId)!!.isRefused) + } + + @Test + fun refusalsForUntrackedEventsAreIgnored() { + val h = Harness() + h.refuse("ff".repeat(32), relayA, "blocked: nope") + assertNull(h.stateOf("ff".repeat(32))?.rejections?.firstOrNull()) + } + + /** Minimal [INostrClient] (delegating to [EmptyNostrClient]) that just captures the listener. */ + private class CapturingClient( + private val delegate: INostrClient = EmptyNostrClient(), + ) : INostrClient by delegate { + var listener: RelayConnectionListener? = null + + override fun addConnectionListener(listener: RelayConnectionListener) { + this.listener = listener + } + + override fun removeConnectionListener(listener: RelayConnectionListener) { + if (this.listener === listener) this.listener = null + } + } +} + +private fun relayClient(relayUrl: NormalizedRelayUrl): IRelayClient = + object : IRelayClient { + override val url = relayUrl + + override fun connect() = error("unused") + + override fun needsToReconnect() = error("unused") + + override fun connectAndSyncFiltersIfDisconnected(ignoreRetryDelays: Boolean) = error("unused") + + override fun isConnected() = error("unused") + + override fun sendOrConnectAndSync(cmd: Command) = error("unused") + + override fun sendIfConnected(cmd: Command) = error("unused") + + override fun disconnect() = error("unused") + } diff --git a/docs/community-status-matrix.md b/docs/community-status-matrix.md index cb6a67ab34..2638284423 100644 --- a/docs/community-status-matrix.md +++ b/docs/community-status-matrix.md @@ -225,7 +225,7 @@ others), and **invisible** (nothing renders it at all). | channel type (`stream`/`forum`/`dm`/`workflow`) | drives the workspace list's sections, icons and DM titling | **shown** | | `closed` / `restricted` / `hidden` / `livekit` | editable toggles in `RelayGroupMetadataScreen`; not surfaced as reader-facing state | implied | | Agent (NIP-OA) virtual membership | `AgentAttestationScreen` shows held/issued attestations; the channel still reports `NONE` | partial | -| Tenant **banned** (9040) / **timed out** (9042) | **nothing.** Consumed into `LocalCache` as generic regular events — no state object, no UI. And no send path surfaces the relay's `OK false`: `NotifyCoordinator` only routes NIP-42-billed NOTIFYs, `BroadcastTracker` only backs the explicit Broadcast action. A banned member's message just never lands. | **invisible** | +| Tenant **banned** (9040) / **timed out** (9042) | The message's own bubble. The relay enforces both at publish time and answers `OK false`; `ChatDeliveryTracker` records that refusal, the bubble turns to an error tick instead of the pending clock, and the delivery dialog quotes the relay's reason verbatim. Still **no state object** folds 9040/9042, so nothing warns you *before* you type — inherent, since the relay only says so on publish. | **shown** (on send) | | Tenant role (owner/admin/member) | `BuzzCommunityMembership` drops the role by design and there is no community roster view. "Add people" exists (9030); `removeCommunityMember` (9031) has **no caller**; change-role (9032), ban/unban, timeout/untimeout have **no send path at all** | **invisible** | | **Presence** (online/away/offline) | nothing — 20001 is still the geohash-presence slot in `EventFactory` | **invisible** | | **Archived identity** (8002 / 13535) | nothing | **invisible** | @@ -302,13 +302,20 @@ UI-only (the state is tracked correctly, nothing tells the user): NIP-43 role, so a tenant *admin* with no per-channel 39001 row reads as channel `MEMBER`. Correct as a safety default (it avoids over-granting), but it means the community-admin status has no channel-level expression. -7. **A ban is never explained to the person banned.** *Concord half fixed:* every reason posting - is blocked is now a `PostingGate` (`commons/.../model/chats/PostingGate.kt`) rendered by - `PostingGateNotice`, and `canPost()` is defined as `postingGate() == Allowed` so the two can't - drift. *Buzz half open:* a tenant ban/timeout produces an `OK false` that no send path - surfaces, so the message silently never lands — there is no local state to derive a gate from, - which is why it needs the send-receipt work in §8.9's neighborhood rather than a read-side - notice. `PostingGate` has no `Banned`/`TimedOut` producer for Buzz until then. +7. ~~**A ban is never explained to the person banned.**~~ **Fixed, from both ends.** *Concord:* + every reason posting is blocked is a `PostingGate` (`commons/.../model/chats/PostingGate.kt`) + rendered by `PostingGateNotice`, with `canPost()` defined as `postingGate() == Allowed` so the + two can't drift. *Buzz:* the refusal is the only signal that exists — the relay enforces a ban + or timeout at publish time and answers `OK false` with no event to fold — so + `ChatDeliveryTracker` now records rejections alongside acceptances + (`RelayInsertConfirmationCollector` gained an `onRelayRejected` callback; it previously dropped + every `OK false` on the floor). The bubble shows an error tick rather than a permanent pending + clock, and the delivery dialog quotes the reason. `duplicate:` is normalized to acceptance + (relays disagree on which OK flag carries it) and a later acceptance clears an earlier refusal + (the auth-required → AUTH → republish round trip). Retry is offered only when the NIP-01 prefix + says a resend could work — `rate-limited`, `error`, `auth-required` — not for `blocked` / + `restricted`, where the relay would repeat itself. This is a general fix: every relay refusal + on a chat message now reaches the sender, with a ban as one case. 8. **Stranding is silent.** Recovery is automatic and log-only, so a member left out of a Refounding sees an ordinary empty community rather than "you were left behind on epoch N". 9. **Buzz tenant moderation has no write path.** 9031 remove-member has no caller; 9032 diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/RelayInsertConfirmationCollector.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/RelayInsertConfirmationCollector.kt index 5014eada6f..59d8766d4c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/RelayInsertConfirmationCollector.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/RelayInsertConfirmationCollector.kt @@ -29,10 +29,22 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.OkMessage import com.vitorpamplona.quartz.utils.Log /** - * Listens to INostrClient's onEvent messages for caching purposes. + * Listens to a relay's `OK` frames for every event this client publishes. + * + * [onRelayReceived] fires on `OK true` — the relay stored the event. [onRelayRejected], when + * supplied, fires on `OK false` with the relay's verbatim reason (NIP-01 suggests a + * machine-readable prefix; parse it with `MachineReadablePrefix.parse`). A refusal is the only + * signal a relay ever gives that a published event did not land, so a caller that ignores it + * cannot distinguish "refused" from "still in flight". + * + * The reason is passed through untouched. In particular `duplicate:` is reported as a rejection + * here because that is what the frame said, even though it means the relay already holds the + * event — callers that care (delivery ticks) treat it as acceptance themselves, since relays + * disagree on which OK flag to pair it with. */ class RelayInsertConfirmationCollector( val client: INostrClient, + val onRelayRejected: ((eventId: HexKey, relay: IRelayClient, reason: String) -> Unit)? = null, val onRelayReceived: (eventId: HexKey, relay: IRelayClient) -> Unit, ) { private val clientListener = @@ -42,8 +54,11 @@ class RelayInsertConfirmationCollector( msgStr: String, msg: Message, ) { - if (msg is OkMessage && msg.success) { + if (msg !is OkMessage) return + if (msg.success) { onRelayReceived(msg.eventId, relay) + } else { + onRelayRejected?.invoke(msg.eventId, relay, msg.message) } } }