Hold an undelivered message and send it when they come back
Asked: how does the sender know to try again when the other person has no registered device - especially since they are friends. They should not have to. Marking it undelivered and stopping there moves the problem onto the person who did nothing wrong: they would have to remember, notice their friend was back, and retype it. So the message goes into an outbox and is retried on every poll tick. Nothing expires and nothing is discarded - a contact who reinstalls in a month gets it in a month. Timing out would mean deleting something somebody wrote in order to tidy up a label. The queue has to live on the sender's phone, and that is forced by the encryption rather than chosen: every envelope is sealed to a particular device's keys, so somebody with no device has no keys to seal to, and the relay could only queue plaintext. The consequence is that it goes out the next time THIS app runs and polls, not the instant the recipient signs in - so the label says "Undelivered - will send" rather than implying it is in flight. Delivery is now a tri-state. PENDING is quiet and grey because it is the normal thing that happens when a friend is signed out and there is nothing to do about it; only FAILED is a warning, and the outbox never produces one. OutboxTest covers it end to end, including delivery against a real second identity rather than a hand-written bundle: queued while away, sent when they register, and the bubble restated - a version that delivered the message but left it reading "undelivered" would pass a test that only checked the queue. Engine, client and app suites green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
co-authored by
Claude Opus 5
Claude Opus 5
parent
f711dd279b
commit
94337cbc07
@@ -1426,18 +1426,39 @@ private fun Bubble(
|
||||
// of it. Only `false` marks — null means nothing was claimed, which
|
||||
// is every received message and everything stored before the field
|
||||
// existed.
|
||||
if (line.delivered == false) {
|
||||
Icon(
|
||||
KitIcon.NotDelivered,
|
||||
contentDescription = "Not delivered",
|
||||
tint = Palette.Pink,
|
||||
modifier = Modifier.size(12.dp).testTag("undelivered"),
|
||||
)
|
||||
Text(
|
||||
"Not delivered",
|
||||
fontSize = 10.sp,
|
||||
color = Palette.Pink,
|
||||
)
|
||||
// ⚠️ PENDING is deliberately quiet and grey, not a warning. It is
|
||||
// the normal thing that happens when a friend is signed out, and
|
||||
// there is nothing for the sender to do about it — the outbox will
|
||||
// send it when they register a device again, however long that
|
||||
// takes. It says "undelivered" because that is true, and "will
|
||||
// send" because otherwise the reader has to guess whose job it is.
|
||||
//
|
||||
// FAILED is now unreachable from the outbox, which never expires.
|
||||
// It stays in the enum for a send that fails for a reason waiting
|
||||
// cannot fix.
|
||||
when (line.delivery) {
|
||||
Session.Delivery.PENDING -> {
|
||||
Icon(
|
||||
KitIcon.Pending,
|
||||
contentDescription = null,
|
||||
tint = Palette.SecondaryText,
|
||||
modifier = Modifier.size(12.dp).testTag("pending"),
|
||||
)
|
||||
Text("Undelivered — will send", fontSize = 10.sp,
|
||||
color = Palette.SecondaryText)
|
||||
}
|
||||
Session.Delivery.FAILED -> {
|
||||
Icon(
|
||||
KitIcon.NotDelivered,
|
||||
contentDescription = "Not delivered",
|
||||
tint = Palette.Pink,
|
||||
modifier = Modifier.size(12.dp).testTag("undelivered"),
|
||||
)
|
||||
Text("Not delivered", fontSize = 10.sp, color = Palette.Pink)
|
||||
}
|
||||
// DELIVERED and null both draw nothing: a message that arrived
|
||||
// needs no decoration, and null is every received message.
|
||||
else -> Unit
|
||||
}
|
||||
}
|
||||
Column(
|
||||
|
||||
@@ -19,6 +19,7 @@ import androidx.compose.material.icons.outlined.Mic
|
||||
import androidx.compose.material.icons.outlined.MicOff
|
||||
import androidx.compose.material.icons.outlined.MoreHoriz
|
||||
import androidx.compose.material.icons.outlined.ErrorOutline
|
||||
import androidx.compose.material.icons.outlined.Schedule
|
||||
import androidx.compose.material.icons.outlined.NotificationsNone
|
||||
import androidx.compose.material.icons.outlined.PersonAdd
|
||||
import androidx.compose.material.icons.outlined.PersonOutline
|
||||
@@ -95,4 +96,7 @@ object KitIcon {
|
||||
|
||||
/** On a bubble that reached nobody. */
|
||||
val NotDelivered: ImageVector = Icons.Outlined.ErrorOutline
|
||||
|
||||
/** Waiting in the outbox for the recipient to have a device again. */
|
||||
val Pending: ImageVector = Icons.Outlined.Schedule
|
||||
}
|
||||
|
||||
@@ -1326,6 +1326,24 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
val threads = withContext(Dispatchers.IO) {
|
||||
runCatching { current.receive() }.getOrDefault(emptySet())
|
||||
}
|
||||
// 🔴 **Retry the outbox on every tick.** A message to
|
||||
// somebody with no registered device waits here rather than
|
||||
// being lost or dumped on the sender to remember — see
|
||||
// Session.flushOutbox. This is also the honest limit of it:
|
||||
// the queue is on THIS phone, so it goes out the next time
|
||||
// this app runs and polls, not the instant the other person
|
||||
// signs in.
|
||||
val flushed = withContext(Dispatchers.IO) {
|
||||
runCatching { current.flushOutbox() }.getOrDefault(emptySet())
|
||||
}
|
||||
if (flushed.isNotEmpty()) {
|
||||
update { state ->
|
||||
state.copy(openChat = state.openChat?.let {
|
||||
if (it.threadId in flushed)
|
||||
it.copy(lines = current.history(it.threadId)) else it
|
||||
})
|
||||
}
|
||||
}
|
||||
// Read back every tick rather than only when a thread was
|
||||
// touched: a story files itself outside the thread map, so
|
||||
// `threads` is empty for a poll that brought one in.
|
||||
|
||||
@@ -40,7 +40,7 @@ class Session(
|
||||
val at: Long,
|
||||
val attachment: Attachment.Ref? = null,
|
||||
/**
|
||||
* Whether any of their devices took this.
|
||||
* What happened to this message on its way out.
|
||||
*
|
||||
* 🔴 **The thing that makes a ghost message impossible.** The old code
|
||||
* had one bit — record it or do not — and both answers are wrong. Not
|
||||
@@ -51,13 +51,24 @@ class Session(
|
||||
* group text. A message can only be a ghost while a delivered bubble
|
||||
* and an undelivered bubble are the same picture.
|
||||
*
|
||||
* ⚠️ Null for received messages and for anything written before this
|
||||
* field existed — absence of a claim, not a claim of failure. Only a
|
||||
* `false` draws the warning.
|
||||
* ⚠️ **Three states, not two, and the third is the important one.**
|
||||
* "Not delivered" on a message to a friend who is merely signed out
|
||||
* puts the work on the person who did nothing wrong — they would have
|
||||
* to remember, and check, and send it again. [Delivery.PENDING] means
|
||||
* it is in the outbox and will go on its own. Only a message that has
|
||||
* been waiting long enough to be worth mentioning becomes FAILED.
|
||||
*
|
||||
* Null for received messages and for anything written before this
|
||||
* field existed — absence of a claim, not a claim of failure.
|
||||
*/
|
||||
val delivered: Boolean? = null,
|
||||
val delivery: Delivery? = null,
|
||||
)
|
||||
|
||||
/** Where an outbound message got to. */
|
||||
enum class Delivery { PENDING, DELIVERED, FAILED }
|
||||
|
||||
|
||||
|
||||
fun history(threadId: String): List<Line> {
|
||||
val stored = vault.history(threadId)
|
||||
return (0 until stored.length()).map {
|
||||
@@ -72,7 +83,8 @@ class Session(
|
||||
// ⚠️ `if (has)` rather than `optBoolean`, which would turn a
|
||||
// message stored before this field into a claim of failure and
|
||||
// decorate every older bubble with a warning.
|
||||
delivered = if (m.has("delivered")) m.optBoolean("delivered") else null,
|
||||
delivery = m.optString("delivery").takeIf { it.isNotBlank() }
|
||||
?.let { runCatching { Delivery.valueOf(it) }.getOrNull() },
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -84,12 +96,12 @@ class Session(
|
||||
attachment: Attachment.Ref? = null,
|
||||
author: Long = 0,
|
||||
at: Long = System.currentTimeMillis(),
|
||||
delivered: Boolean? = null,
|
||||
delivery: Delivery? = null,
|
||||
) {
|
||||
vault.append(threadId, JSONObject()
|
||||
.put("author", author)
|
||||
.put("text", text).put("mine", mine).put("at", at)
|
||||
.also { if (delivered != null) it.put("delivered", delivered) }
|
||||
.also { if (delivery != null) it.put("delivery", delivery.name) }
|
||||
.also { if (attachment != null) it.put("kind", attachment.kind).put("att", attachment.toJson()) })
|
||||
}
|
||||
|
||||
@@ -105,7 +117,7 @@ class Session(
|
||||
// the image without fetching it back from the relay. Recorded whatever
|
||||
// happened, carrying the outcome — see [Line.delivered].
|
||||
record(Vault.threadForUser(peerUserId), "", mine = true,
|
||||
attachment = sent.attachment, delivered = sent.reachedSomeone)
|
||||
attachment = sent.attachment, delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING)
|
||||
return sent
|
||||
}
|
||||
|
||||
@@ -121,7 +133,7 @@ class Session(
|
||||
val sent = conversations.sendAttachment(
|
||||
peerUserId, bytes, mime, Attachment.AUDIO, duration = seconds)
|
||||
record(Vault.threadForUser(peerUserId), "", mine = true,
|
||||
attachment = sent.attachment, delivered = sent.reachedSomeone)
|
||||
attachment = sent.attachment, delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING)
|
||||
return sent
|
||||
}
|
||||
|
||||
@@ -150,11 +162,94 @@ class Session(
|
||||
*/
|
||||
fun send(peerUserId: Long, text: String): Conversations.Sent {
|
||||
val sent = conversations.send(peerUserId, text)
|
||||
record(Vault.threadForUser(peerUserId), text, mine = true,
|
||||
delivered = sent.reachedSomeone)
|
||||
val at = System.currentTimeMillis()
|
||||
record(Vault.threadForUser(peerUserId), text, mine = true, at = at,
|
||||
delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING)
|
||||
// 🔴 Queued, not abandoned. A friend who is simply signed out should
|
||||
// not cost the sender a reminder to try again later.
|
||||
if (!sent.reachedSomeone) enqueue(peerUserId, text, at)
|
||||
return sent
|
||||
}
|
||||
|
||||
// MARK: - Outbox
|
||||
|
||||
/**
|
||||
* Hold a message that reached nobody, and send it when they come back.
|
||||
*
|
||||
* 🔴 **Why this is on the phone and not the relay.** Every envelope is
|
||||
* sealed to a particular device's keys. Somebody with no registered device
|
||||
* has no keys to seal to, so there is nothing the relay could be handed to
|
||||
* hold — it could only queue plaintext, which is the thing this app exists
|
||||
* not to do. The consequence is that the sender's phone is the only place
|
||||
* the message can wait, and therefore that it goes out when THIS device
|
||||
* next runs, not the instant the recipient signs in.
|
||||
*
|
||||
* ⚠️ Which is worth being honest about in the UI: "Sending…" here means
|
||||
* "waiting on your phone", not "in flight".
|
||||
*/
|
||||
private fun enqueue(peerUserId: Long, text: String, at: Long) {
|
||||
val pending = vault.outbox()
|
||||
pending.put(JSONObject()
|
||||
.put("to", peerUserId).put("text", text).put("at", at))
|
||||
vault.saveOutbox(pending)
|
||||
}
|
||||
|
||||
/**
|
||||
* Try everything waiting. Returns the threads that changed, so the caller
|
||||
* can redraw them.
|
||||
*
|
||||
* ⚠️ **Never gives up.** There is no expiry and nothing is ever discarded:
|
||||
* a message stays marked undelivered until the recipient registers a device
|
||||
* and it goes. Timing out would mean deleting something somebody wrote in
|
||||
* order to tidy up a label, and the label is not the point — the message
|
||||
* is. A contact who reinstalls in a month gets it in a month.
|
||||
*/
|
||||
fun flushOutbox(): Set<String> {
|
||||
val pending = vault.outbox()
|
||||
if (pending.length() == 0) return emptySet()
|
||||
|
||||
val touched = mutableSetOf<String>()
|
||||
val stillWaiting = org.json.JSONArray()
|
||||
|
||||
for (i in 0 until pending.length()) {
|
||||
val item = pending.optJSONObject(i) ?: continue
|
||||
val to = item.optLong("to")
|
||||
val text = item.optString("text")
|
||||
val at = item.optLong("at")
|
||||
val threadId = Vault.threadForUser(to)
|
||||
|
||||
val sent = runCatching { conversations.send(to, text) }.getOrNull()
|
||||
if (sent?.reachedSomeone == true) {
|
||||
restate(threadId, at, Delivery.DELIVERED)
|
||||
touched += threadId
|
||||
} else {
|
||||
stillWaiting.put(item)
|
||||
}
|
||||
}
|
||||
vault.saveOutbox(stillWaiting)
|
||||
return touched
|
||||
}
|
||||
|
||||
/**
|
||||
* Move one already-recorded message to a new state.
|
||||
*
|
||||
* ⚠️ Matched on the timestamp it was written with, which is why [send]
|
||||
* stamps `at` once and uses the same value for the thread and the outbox.
|
||||
* Matching on the text would restate the wrong bubble the first time
|
||||
* somebody sends the same word twice.
|
||||
*/
|
||||
private fun restate(threadId: String, at: Long, state: Delivery) {
|
||||
val stored = vault.history(threadId)
|
||||
for (i in 0 until stored.length()) {
|
||||
val m = stored.optJSONObject(i) ?: continue
|
||||
if (m.optLong("at") == at && m.optBoolean("mine")) {
|
||||
m.put("delivery", state.name)
|
||||
vault.replaceHistory(threadId, stored)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Send to a group, and file our own copy.
|
||||
*
|
||||
@@ -175,7 +270,7 @@ class Session(
|
||||
fun sendGroup(group: RelayClient.Group, text: String): Conversations.Sent {
|
||||
val sent = conversations.sendToGroup(group, text, userId)
|
||||
record(Vault.threadForGroup(group.id), text, mine = true, author = userId,
|
||||
delivered = sent.reachedSomeone)
|
||||
delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING)
|
||||
return sent
|
||||
}
|
||||
|
||||
@@ -190,7 +285,7 @@ class Session(
|
||||
val sent = conversations.sendGroupAttachment(group, userId, bytes, mime, width, height)
|
||||
record(Vault.threadForGroup(group.id), "", mine = true,
|
||||
attachment = sent.attachment, author = userId,
|
||||
delivered = sent.reachedSomeone)
|
||||
delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING)
|
||||
return sent
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user