diff --git a/PARITY.md b/PARITY.md index 9c89334..4802ecb 100644 --- a/PARITY.md +++ b/PARITY.md @@ -97,6 +97,7 @@ From `E2E/E2EInbox.swift`, which is the real feature list. | Edit a sent message (§6.15) | 🟡 codec done, in progress — an inbound edit is recognised and not drawn as a duplicate bubble; applying it needs the same `mid` | | Drafts that survive a relaunch | ✅ sealed in the vault, not SharedPreferences | | Forwarding (§6.16) | 🟡 codec done, in progress — an inbound forward is recognised and shown as one; originating needs #10 | +| Group message ids and quotes (§6.9) | ✅ transcript stores both; sent and queued copies share one id | | Photos | ✅ send via the system picker, render inline | | Voice notes (record + play) | ✅ AAC/MP4, tap to record, tap to send | | Stories, and seen-state | ✅ 24h, sent per-contact, expired bytes deleted | diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt index 14fd02a..e3e4e05 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -76,6 +76,24 @@ class Session( val delivery: Delivery? = null, /** A sender-generated preview of a link in this message (§6.12). */ val link: Content.LinkPreview? = null, + /** + * The sender's own id for this message (§6.9). + * + * 🔴 **The wire always carried one and this type never stored it.** + * `Content.groupText` has taken a `messageId` since §6.9 shipped and + * the parser reads it back — the transcript was where it stopped. So + * nothing here could name an earlier message: no replies to resolve, no + * reactions to fold, no edits to apply. + */ + val messageId: String? = null, + /** + * What this message quotes, when it is a reply (§6.9). + * + * Also never stored, so a reply from the iPhone app arrived as a bare + * message with the quote silently removed — and the reader had no way + * to know anything had been dropped. + */ + val about: Content.Reference? = null, ) /** Where an outbound message got to. */ @@ -107,6 +125,8 @@ class Session( delivery = m.optString("delivery").takeIf { it.isNotBlank() } ?.let { runCatching { Delivery.valueOf(it) }.getOrNull() }, link = m.optJSONObject("lnk")?.let { Content.LinkPreview.fromJson(it) }, + messageId = m.optString("mid").takeIf { it.isNotEmpty() }, + about = m.optJSONObject("re")?.let { Content.Reference.fromJson(it) }, ) } } @@ -121,6 +141,10 @@ class Session( at: Long = System.currentTimeMillis(), delivery: Delivery? = null, link: Content.LinkPreview? = null, + /** The sender's own id for this message (§6.9); null for none. */ + messageId: String? = null, + /** What this message quotes, when it is a reply (§6.9). */ + about: Content.Reference? = null, ) { vault.append(threadId, JSONObject() .put("author", author) @@ -132,7 +156,12 @@ class Session( } } .also { if (attachment != null) it.put("kind", attachment.kind).put("att", attachment.toJson()) } - .also { if (link != null) it.put("lnk", link.toJson()) }) + .also { if (link != null) it.put("lnk", link.toJson()) } + // ⚠️ Written only when present. A null on every message would grow + // the transcript for nothing and make "no id" indistinguishable + // from "an id of nothing". + .also { if (messageId != null) it.put("mid", messageId) } + .also { if (about != null) it.put("re", about.toJson()) }) // 🔴 A reply arriving cancels the reminder, and it is done HERE because // this is the one place every inbound message passes through. Hanging it @@ -647,9 +676,15 @@ class Session( .getOrNull() fun sendGroup(group: RelayClient.Group, text: String): Conversations.Sent { - val sent = conversations.sendToGroup(group, text, userId) + // 🔴 One id for the whole fan-out, minted here so the SAME one reaches + // both the send and the outbox entry below. Building the envelope twice + // with two ids means a queued retry arrives as a different message from + // the one everybody else already has. + val messageId = java.util.UUID.randomUUID().toString() + val sent = conversations.sendToGroup(group, text, userId, messageId) val at = System.currentTimeMillis() record(Vault.threadForGroup(group.id), text, mine = true, author = userId, at = at, + messageId = messageId, delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING) // 🔴 **The group half of the outbox promise.** Members with no // registered device are filed in the failure map as "user:" by the @@ -657,7 +692,7 @@ class Session( // the group thread to restate. A member whose devices took it is not // queued — resending to them would duplicate the message. val body = io.flamenet.e2e.Content.groupText( - text, group.id, groupName = vault.groupName(group.id)) + text, group.id, messageId = messageId, groupName = vault.groupName(group.id)) sent.failed.keys.filter { it.startsWith("user:") } .mapNotNull { it.removePrefix("user:").toLongOrNull() } .forEach { member -> @@ -800,7 +835,9 @@ class Session( record(thread, message.content.text, mine = false, attachment = message.content.attachment, sticker = message.content.sticker?.let { Sticker(it.pack, it.id) }, - author = message.fromUser) + author = message.fromUser, + messageId = message.content.messageId, + about = message.content.about) touched += thread } diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index 1b5ee14..11dd4ab 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -212,10 +212,22 @@ class Conversations( * at the sender as an incoming one. The sender's own copy is written * locally by the caller. */ - fun sendToGroup(group: RelayClient.Group, text: String, myUserId: Long): Sent { + /** + * @param messageId one id for the whole fan-out (§6.9). It is ONE message: + * a reply or a reaction from any member has to name the same thing, so + * per-recipient ids would make that impossible. + * + * ⚠️ Taken as a parameter rather than minted here, because the caller has + * to build the same envelope again for the outbox. Two constructions with + * two ids means a queued retry arrives as a *different* message from the + * one everybody else got. + */ + fun sendToGroup(group: RelayClient.Group, text: String, myUserId: Long, + messageId: String? = null): Sent { val delivered = mutableListOf() val failed = LinkedHashMap() - val body = Content.groupText(text, group.id, groupName = vault.groupName(group.id)) + val body = Content.groupText(text, group.id, messageId = messageId, + groupName = vault.groupName(group.id)) for (member in group.memberIds) { if (member == myUserId) continue diff --git a/engine/src/test/kotlin/io/flamenet/e2e/GroupMessageIdentityTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/GroupMessageIdentityTest.kt new file mode 100644 index 0000000..1046370 --- /dev/null +++ b/engine/src/test/kotlin/io/flamenet/e2e/GroupMessageIdentityTest.kt @@ -0,0 +1,99 @@ +package io.flamenet.e2e + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import org.json.JSONObject + +/** + * Group messages carrying an id of their own (§6.9). + * + * 🔴 **The wire always carried one and the transcript never stored it.** + * `Content.groupText` has taken a `messageId` since §6.9 shipped and `parse` + * reads it back — `Session.Line` was where it stopped. So nothing in a group + * could name an earlier message: no reply to resolve, no reaction to fold, no + * edit to apply. All three were "codec done" and none of them worked. + */ +class GroupMessageIdentityTest { + + @Test + fun `a group message round trips its id`() { + val content = Content.parse(Content.groupText("meet at 6", "g1", messageId = "abc")) + assertEquals("abc", content.messageId) + assertEquals("g1", content.groupId) + } + + /** + * 🔴 One id for the whole fan-out. A group message is ONE message: a reply + * or a reaction from any member has to name the same thing, so + * per-recipient ids would make that impossible. + */ + fun bodiesFor(memberCount: Int, messageId: String) = + (1..memberCount).map { Content.groupText("meet at 6", "g1", messageId = messageId) } + + @Test + fun `every copy of a group message carries the same id`() { + val ids = bodiesFor(3, "shared").map { Content.parse(it).messageId } + assertEquals(setOf("shared"), ids.toSet(), + "members received different ids for one message") + } + + /** + * 🔴 **The queued retry has to be the same message.** `sendGroup` builds + * the envelope twice — once to send and once for the outbox — so an id + * minted inside either would differ between them, and a member who was + * offline would eventually receive a *different* message from everybody + * else, which nothing could reconcile with a reply or a reaction. + */ + @Test + fun `the queued copy carries the same id as the sent one`() { + val messageId = "shared" + val sent = Content.groupText("meet at 6", "g1", messageId = messageId) + val queued = Content.groupText("meet at 6", "g1", messageId = messageId) + assertEquals(Content.parse(sent).messageId, Content.parse(queued).messageId) + } + + @Test + fun `a group reply round trips its quote`() { + val about = Content.Reference("earlier", 9L, "what they said") + val content = Content.parse( + Content.groupText("yes", "g1", messageId = "m", about = about)) + assertEquals("earlier", content.about?.messageId) + assertEquals("what they said", content.about?.preview) + } + + /** + * ⚠️ A reference survives the transcript's JSON round trip, which is the + * part that was missing — the codec was always fine. + */ + @Test + fun `a reference survives being stored and read back`() { + val about = Content.Reference("earlier", 9L, "what they said") + val stored = JSONObject().put("re", about.toJson()) + val read = Content.Reference.fromJson(stored.getJSONObject("re")) + + assertEquals("earlier", read.messageId) + assertEquals(9L, read.author) + assertEquals("what they said", read.preview) + } + + /** + * ⚠️ A message stored before ids existed reads back with none, rather than + * with an empty string that would then look like an id and match nothing. + */ + @Test + fun `a message stored before ids existed has none`() { + val stored = JSONObject().put("text", "hi").put("mine", false).put("at", 1L) + assertNull(stored.optString("mid").takeIf { it.isNotEmpty() }) + assertNull(stored.optJSONObject("re")) + } + + @Test + fun `a message stored with an id reads it back`() { + val stored = JSONObject().put("text", "hi").put("mine", false).put("at", 1L) + .put("mid", "abc") + assertNotNull(stored.optString("mid").takeIf { it.isNotEmpty() }) + assertEquals("abc", stored.optString("mid")) + } +}