The transcript records a message id and a reply reference
🔴 The wire always carried both and Session.Line never stored them. Content.groupText has taken a messageId since §6.9 shipped and parse 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. All three were "codec done" and none worked. A reply from the iPhone app arrived as a bare message with the quote silently removed, which is worse than the missing id: the reader had no way to know anything had been dropped. 🔴 Also fixes a real bug in sendGroup. It built the envelope twice — once to send, 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. One id, minted by the caller, used by both. Both fields 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". Refs paulhitt/flamenet-messenger-ios#58, #10 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
0dec029917
commit
3b7e69c0eb
@@ -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 |
|
||||
|
||||
@@ -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:<id>" 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
|
||||
}
|
||||
|
||||
|
||||
@@ -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<String>()
|
||||
val failed = LinkedHashMap<String, String>()
|
||||
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
|
||||
|
||||
@@ -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"))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user