diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 8382cf6..b966987 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -885,9 +885,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { it.copy( busy = false, openChat = it.openChat?.copy(lines = current.history(chat.threadId)), - error = if (sent.failed.isEmpty()) null - else "Delivered to ${sent.delivered.size} of " + - "${sent.delivered.size + sent.failed.size} devices.", + error = describeDelivery(sent), ) } }.onFailure { failure -> update { it.copy(busy = false, error = describe(failure)) } } @@ -1197,14 +1195,30 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { it.copy( busy = false, openChat = it.openChat?.copy(lines = current.history(chat.threadId)), - error = if (sent.failed.isEmpty()) null - else "Delivered to ${sent.delivered.size} of " + - "${sent.delivered.size + sent.failed.size} devices.", + error = describeDelivery(sent), ) } }.onFailure { failure -> update { it.copy(busy = false, error = describe(failure)) } } } + + /** + * What to say about a send. + * + * ⚠️ "Delivered to 0 of 1 devices" is a ratio pretending to be an + * explanation. The case that actually happens — the other person is signed + * out, so they have no devices at all — deserves the sentence, because the + * message is not queued anywhere and will not arrive later. + */ + private fun describeDelivery(sent: io.flamenet.client.Conversations.Sent): String? = when { + sent.failed.isEmpty() -> null + sent.delivered.isEmpty() -> + "Not delivered — they are not signed in on any device. " + + "This message was not queued; send it again once they are back." + else -> "Delivered to ${sent.delivered.size} of " + + "${sent.delivered.size + sent.failed.size} devices." + } + /** * Bytes for a received attachment, fetched and cached on first use. * @@ -1266,9 +1280,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { // Partial delivery is stated, not hidden. "Sent" when one // of someone's three devices got it is a lie the user acts // on. - error = if (sent.failed.isEmpty()) null - else "Delivered to ${sent.delivered.size} of " + - "${sent.delivered.size + sent.failed.size} devices.", + error = describeDelivery(sent), ) } }.onFailure { failure -> diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index 361e7bb..39e16ea 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -370,6 +370,26 @@ class Conversations( val failed = LinkedHashMap() val outgoing = mutableListOf() + // 🔴 **Nobody to send to is a FAILURE, and it used to report success.** + // A message is addressed per device. With no devices the loop below ran + // zero times, `outgoing` stayed empty, no request was made — and `Sent` + // came back with an empty `failed`, which every caller reads as "it + // went". The message was written into our own thread and simply never + // existed anywhere else. + // + // That is the whole bug behind "I wasn't signed in on the other end and + // the message got lost": signing out deregisters the device, so the + // recipient genuinely has none, and this is the one path that has to + // notice. iOS already threw here; this side never did. + // + // ⚠️ Recorded in `failed` rather than thrown, because the group senders + // above call this once per member with no try around it — throwing + // would abandon everyone after the first person who is signed out. + if (bundles.none { it.deviceId != identity.deviceId }) { + failed["user:$toUserId"] = "not signed in on any device" + return Sent(delivered, failed, sealed = false) + } + // Seal only if we hold their key. A silent fall back to an // unsealed send is the failure §14.5 warns about, so the outcome is // returned rather than assumed — the UI can then say which it was diff --git a/client/src/test/kotlin/io/flamenet/client/SendToNobodyTest.kt b/client/src/test/kotlin/io/flamenet/client/SendToNobodyTest.kt new file mode 100644 index 0000000..fa121dc --- /dev/null +++ b/client/src/test/kotlin/io/flamenet/client/SendToNobodyTest.kt @@ -0,0 +1,100 @@ +package io.flamenet.client + +import io.flamenet.e2e.DeviceIdentity +import io.flamenet.e2e.Vault +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * A message nobody can receive must not report success. + * + * 🔴 **This shipped, and it lost real messages.** Signing out deregisters the + * device, so a signed-out recipient genuinely has no devices. Envelopes are + * addressed per device, so `sendPlaintext` looped over an empty bundle list, + * made no request at all, and returned `Sent(delivered = [], failed = {})`. + * Every caller reads an empty `failed` as "it went" — the message was written + * into the sender's own thread and existed nowhere else. It was reported as + * "I wasn't signed in on the other end and the message got lost", and the same + * path is why a group text vanished: the group senders call this once per + * member. + * + * ⚠️ **iOS never had this bug** — `transmit` throws on an empty device list. + * The two platforms disagreed about a delivery guarantee for as long as both + * have existed, and nothing failed, because the only tests that covered + * sending had both ends signed in. That is the actual lesson here: a test that + * only exercises the happy path cannot tell a delivery from a no-op, since + * both of them are silent. + * + * So this asserts the *unhappy* path directly, on the seam where the two + * platforms drifted. + */ +class SendToNobodyTest { + + private lateinit var server: MockWebServer + + @BeforeTest fun start() { server = MockWebServer().also { it.start() } } + @AfterTest fun stop() { server.shutdown() } + + private class MapStorage : Vault.Storage { + private val records = mutableMapOf() + override fun read(name: String) = records[name] + override fun write(name: String, bytes: ByteArray) { records[name] = bytes } + override fun delete(name: String) { records.remove(name) } + override fun names() = records.keys.toList() + } + + private fun key(): SecretKey = + KeyGenerator.getInstance("AES").apply { init(256) }.generateKey() + + private fun conversations(): Conversations { + val identity = DeviceIdentity.create() + val vault = Vault(MapStorage(), key()).also { it.saveIdentity(identity) } + val relay = RelayClient(server.url("/").toString(), "alice", "app pass word") + return Conversations(identity, vault, relay) + } + + private fun json(body: String) = MockResponse() + .setResponseCode(200) + .setHeader("Content-Type", "application/json") + .setBody(body) + + @Test + fun `sending to somebody with no registered devices is reported as failed`() { + // What the relay answers for a user who is signed out everywhere. + server.enqueue(json("""{"devices":[]}""")) + + val sent = conversations().send(toUserId = 99, text = "are you there?") + + assertTrue(sent.delivered.isEmpty(), "nothing can have been delivered") + assertFalse( + sent.failed.isEmpty(), + "a send that reached nobody reported an empty `failed`, which every " + + "caller reads as success — this is the message-loss bug", + ) + } + + /** + * ⚠️ And it must not quietly post the envelope anyway. The bug was not only + * in the return value: with no devices there is nothing to address, so a + * request here would mean the relay was being handed an envelope for a + * device that does not exist. + */ + @Test + fun `no envelope is posted when there is nobody to address`() { + server.enqueue(json("""{"devices":[]}""")) + + conversations().send(toUserId = 99, text = "are you there?") + + // Exactly one request: the device lookup. Nothing after it. + assertEquals(1, server.requestCount, + "a send to nobody should stop after the lookup") + } +}