android, desktop: fix live message sent to the chat opened after switching (#7323)

* android, desktop: fix live message sent to the chat opened after switching

A live message is sent when the chat is switched, but by then this view
already shows the chat that was opened - the effect that sends it runs
with that chat, so the message typed in one chat was sent to another.

The chat the message was composed in is passed to the send, and it is
resolved by the chat id from before the switch. If that chat is no longer
there the message is not sent at all, rather than sent to the chat opened
instead. The draft cleared after sending is the one of that chat too.

* plan: correct references; clear the draft of the chat the message is sent to

* plan: note the blast radius and how to resolve the overlap with #7308

* android, desktop: only pass the chat to what a live message can reach

A live message has no context item, so the forwarding, editing and
reporting branches of the send cannot run for it - they keep using the
chat of the view, and the chat it was composed in is passed only to the
message send, to the update of an already sent live message, and to
clearing the draft after sending.

* android, desktop: give the opened chat its own compose state while the live message is sent

Sending the live message to the chat it was composed in is not enough on
its own: composeState is shared between the chats opened in this view, and
the chat switch branch of KeyChangeEffect is the only one that neither
resets it nor loads the opened chat's draft - the branch that loads a
draft is later in the same if chain and cannot be reached.

sendMessageAsync then made it visible. It runs on Dispatchers.Default, so
its writes land after the switch: the whole composed state (via
cs.copy(liveMessage = null)) and its spinner (sending()) were written to
the compose state of a view that already shows another chat, which then
displayed the text composed in the previous one until the send completed.
The draft it should have shown was still in the model, and the next switch
away dropped it.

- sendMessageAsync takes composed, and sendMessage takes it as null by
  default, so only the chat switch passes a state and every other sender
  reads it inside the coroutine, where the send read it before. The chat
  switch captures it on the main thread before replacing it - otherwise
  the send would read the compose state of the chat that was opened and
  send its draft to the previous chat.
- checkLinkPreview takes that state too. It re-read composeState rather
  than what was passed in, and every text live message reaches it through
  updateMsgContent, so it would have rebuilt the message from the opened
  chat's draft instead of committing what was composed.
- every composeState write in sendMessageAsync is guarded by
  composeIsForSend() (toChat.id == chat.id), which compares the two chats
  instead of checking which one is open, so this send never takes the
  compose state back if that chat is opened again before it completes.
- the chat switch branch then resets composeState to the opened chat's
  draft, or to an empty state, like the branches below it do.

* plan: document the compose state handoff; correct the #7308 overlap resolution

The note on resolving the overlap with #7308 said its cs.liveMessage !=
null clause "already covers the send made by the chat switch". It does
not - in #7308 that clause sits outside the chatIsOpen check, which is
correct only while a live message is always sent to the chat that is
open, the assumption this fix removes. Read as written it exempts the
chat switch send from the guard that protects the opened chat, and a
merge that follows it reintroduces the leak.

Also records what manual test 2 was found failing on, and adds a slow
send variant so the window between the switch and the send completing is
long enough to type in the chat that was opened.

* android, desktop: keep reading the current state where a forward appends it

checkLinkPreview taking the composed state is needed where a live message
reaches it, but the forwarding branch is not one of those - it cannot run
with a chat other than the view's - and forwardItem suspends before it. So
there the captured state is stale by a network round trip, and text typed
while the forward was in flight stopped being appended to the message it
adds, while still being cleared when the send completed.

* plan: correct references and the claims that no longer hold

Line references were against the base this branch forked from, before
#7308 landed. Also: the live message loop no longer exits because the send
clears liveMessage - the chat switch replaces the compose state, on the
main thread, before the send runs; checkLinkPreview is not passed the
captured state everywhere; and chatsCtx.getChat can only return null in a
secondary context, which is not how "the chat is gone" reads.

Adds the two manual checks the review implied: a live message carrying a
link preview, which is what breaks if checkLinkPreview stops reading the
state it was given, and returning to the chat before the send completes.

* android, desktop: narrow the change to what the fix needs

- the draft id a failed message is saved under keeps using chat: that
  branch is behind !liveSend, which the send made by a chat switch never
  satisfies, so toChat is always chat where it is read;
- the state the chat switch installs no longer carries maxFileSize over.
  That field is kept in sync on chat switch by LaunchedEffect(chat.chatInfo),
  which is why the branches below this one construct it without one, and
  the paths where that effect does not re-run are the ones where the send
  overwrites the state anyway;
- sendMessageAsync takes its cs as a parameter rather than aliasing a
  separately named one.

* plan: follow the narrowed change

* android, desktop: shorten the comments

The threading mechanism behind cs is explained where it is used, so the
function comment only has to say what it is; the rest is rewording.
This commit is contained in:
Narasimha-sc
2026-08-06 23:08:15 +01:00
committed by GitHub
parent 0b2f030dc6
commit f921bd47bb
2 changed files with 284 additions and 19 deletions
@@ -497,8 +497,8 @@ fun ComposeView(
}
}
fun clearCurrentDraft() {
if (chatModel.draftChatId.value == draftChatId(chat.id, chatScope)) {
fun clearCurrentDraft(forChat: Chat = chat) {
if (chatModel.draftChatId.value == draftChatId(forChat.id, chatScope)) {
chatModel.draft.value = null
chatModel.draftChatId.value = null
}
@@ -575,9 +575,10 @@ fun ComposeView(
}
// TODO [short links] connectCheckLinkPreview
fun checkLinkPreview(): MsgContent {
val msgText = composeState.value.message.text
return when (val composePreview = composeState.value.preview) {
// the state is passed in by a send that must not read the current one - see sendMessageAsync
fun checkLinkPreview(cs: ComposeState = composeState.value): MsgContent {
val msgText = cs.message.text
return when (val composePreview = cs.preview) {
is ComposePreview.CLinkPreview -> {
val parsedMsg = parseToMarkdown(msgText)
val url = getMessageLinks(parsedMsg).first
@@ -680,8 +681,12 @@ fun ComposeView(
}
}
suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false): List<ChatItem>? {
val cs = composeState.value
// toChat is the chat the message was composed in - it differs from the one this view shows only for the live message
// committed by a chat switch, which has no context item, so the forwarding, editing and reporting branches below
// cannot run with a different chat. cs is that send's state, captured before the switch replaced it.
suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false, toChat: Chat = chat, cs: ComposeState = composeState.value): List<ChatItem>? {
// a send for another chat may not write to composeState, even after that chat is opened again - it was handed over
fun composeIsForSend(): Boolean = toChat.id == chat.id
var sent: List<ChatItem>?
var lastMessageFailedToSend: ComposeState? = null
val msgText = text ?: cs.message.text
@@ -731,8 +736,8 @@ fun ComposeView(
fun updateMsgContent(msgContent: MsgContent): MsgContent {
return when (msgContent) {
is MsgContent.MCText -> checkLinkPreview()
is MsgContent.MCLink -> checkLinkPreview()
is MsgContent.MCText -> checkLinkPreview(cs)
is MsgContent.MCLink -> checkLinkPreview(cs)
is MsgContent.MCImage -> MsgContent.MCImage(msgText, image = msgContent.image)
is MsgContent.MCVideo -> MsgContent.MCVideo(msgText, image = msgContent.image, duration = msgContent.duration)
is MsgContent.MCVoice -> MsgContent.MCVoice(msgText, duration = msgContent.duration)
@@ -789,12 +794,12 @@ fun ComposeView(
}
val liveMessage = cs.liveMessage
if (!live) {
if (!live && composeIsForSend()) {
if (liveMessage != null) composeState.value = cs.copy(liveMessage = null)
sending()
}
if (!cs.forwarding || chatModel.draft.value?.forwarding == true) {
clearCurrentDraft()
clearCurrentDraft(toChat)
}
if (cs.contextItem is ComposeContextItem.ForwardingItems) {
@@ -805,6 +810,8 @@ fun ComposeView(
if (cs.message.text.isNotEmpty()) {
sent?.mapIndexed { index, message ->
if (index == sent!!.lastIndex) {
// the current state, not cs: forwarding is never reached from the chat switch, and keeps what was typed
// while it was in flight
send(chat, checkLinkPreview(), quoted = message.id, live = false, ttl = ttl, mentions = cs.memberMentions, sign = sign)
} else {
message
@@ -818,7 +825,7 @@ fun ComposeView(
sent = if (updatedMessage != null) listOf(updatedMessage) else null
lastMessageFailedToSend = if (updatedMessage == null) constructFailedMessage(cs) else null
} else if (liveMessage != null && liveMessage.sent) {
val updatedMessage = updateMessage(liveMessage.chatItem, chat, live)
val updatedMessage = updateMessage(liveMessage.chatItem, toChat, live)
sent = if (updatedMessage != null) listOf(updatedMessage) else null
} else if (cs.contextItem is ComposeContextItem.ReportedItem) {
sent = sendReport(cs.contextItem.reason, cs.contextItem.chatItem.id)
@@ -828,7 +835,7 @@ fun ComposeView(
val remoteHost = chatModel.currentRemoteHost.value
when (val preview = cs.preview) {
ComposePreview.NoPreview -> msgs.add(MsgContent.MCText(msgText))
is ComposePreview.CLinkPreview -> msgs.add(checkLinkPreview())
is ComposePreview.CLinkPreview -> msgs.add(checkLinkPreview(cs))
is ComposePreview.ChatLinkPreview -> {
val linkStr = preview.chatLink.connLinkStr
val text = if (msgText.isEmpty()) linkStr else "$msgText\n$linkStr"
@@ -919,7 +926,7 @@ fun ComposeView(
localPath = file.filePath
)
}
val sendResult = send(chat, content, if (index == 0) quotedItemId else null, file,
val sendResult = send(toChat, content, if (index == 0) quotedItemId else null, file,
live = if (content !is MsgContent.MCVoice && index == msgs.lastIndex) live else false,
ttl = ttl,
mentions = cs.memberMentions,
@@ -941,7 +948,7 @@ fun ComposeView(
// cleared or restored. On Main, so that these checks and changes are not interleaved with the user switching
// chats or typing.
withContext(Dispatchers.Main) {
val chatIsOpen = chatModel.chatId.value == chat.id
val chatIsOpen = composeIsForSend() && chatModel.chatId.value == chat.id
// a live message is held in the compose state of the chat it is sent to, but only while that chat is the one open
val liveSend = live || cs.liveMessage != null
val sentMessageInCompose = chatIsOpen && (liveSend || composeState.value.inProgress)
@@ -956,7 +963,7 @@ fun ComposeView(
if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) {
if (sentMessageInCompose) composeState.value = draft
} else {
clearCurrentDraft()
clearCurrentDraft(toChat)
// liveSend excluded: a failing keystroke send would otherwise write a draft on every attempt
if (!sentMessageInCompose && !liveSend && lastFailed != null) {
// the message was not sent, so it is restored in the chat it was composed in, or kept as its draft if another chat is open
@@ -972,9 +979,11 @@ fun ComposeView(
return sent
}
fun sendMessage(ttl: Int?, sign: Boolean = false) {
// toChat and composed are for the chat switch, which hands the compose state over to the chat it opened; passing
// toChat without doing that leaves the sent message in the input
fun sendMessage(ttl: Int?, sign: Boolean = false, toChat: Chat = chat, composed: ComposeState? = null) {
withLongRunningApi(slow = 120_000) {
sendMessageAsync(null, false, ttl, sign)
sendMessageAsync(null, false, ttl, sign, toChat, composed ?: composeState.value)
}
}
@@ -1339,10 +1348,18 @@ fun ComposeView(
KeyChangeEffect(chatModel.chatId.value) { prevChatId ->
val cs = composeState.value
if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) {
sendMessage(null)
// the chat is already switched, so the live message goes to the chat with the id it had before the switch
val liveMessageChat = if (prevChatId == null || prevChatId == chat.id) chat else chatsCtx.getChat(prevChatId)
// if that chat is gone there is nowhere to send it, and it must not be sent to the chat opened instead
// cs is captured on this thread, before the compose state is replaced below
if (liveMessageChat != null) sendMessage(null, toChat = liveMessageChat, composed = cs) else clearState()
resetLinkPreview()
clearPrevDraft(prevChatId)
deleteUnusedFiles()
// the sent message belongs to the chat it was composed in; the chat opened next shows its own draft
val draft = chatModel.draft.value
composeState.value = if (draft != null && chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)) draft
else ComposeState(useLinkPreviews = useLinkPreviews)
} else if (cs.inProgress) {
clearPrevDraft(prevChatId)
// the message being sent must not be kept in the compose state, it is shared with the chat opened next;
@@ -0,0 +1,248 @@
# Fix: live message is sent to the chat opened after switching chats
Branch: `nd/fix-live-message-sent-to-wrong-chat` (off `origin/stable`)
Date: 2026-07-29
Line references are against `origin/stable` at `970ef8932`, with this fix
applied. Android and desktop (`commonMain/ComposeView.kt`).
## Problem
Typing a live message and switching to another chat sends that message to
the chat that was opened, without the user sending anything. Reported on
desktop, where every chat switch reuses the same view.
## Cause
A live message is committed when the chat is switched
(`ComposeView.kt:1353-1369`), which before this change was:
```
if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) {
sendMessage(null)
```
`KeyChangeEffect` is `LaunchedEffect(key1) { block(prev) }`
(`Utils.kt:683-698`), so when the key changes, `remember(key1)` rebuilds
it from the lambda of the composition that is running *now* - and by then
`chatModel.chatId` is already the new chat, `ChatView` has recomposed
`ComposeView` with the new `chat`, and the block that runs captured that
one.
`sendMessage(null)``sendMessageAsync``send(chat, ...)` uses the
captured `chat`, while the message content comes from `composeState`,
which is shared between the chats opened in this view. So the content of
the chat that was left is sent to the chat that was opened:
- `liveMessage.sent == false` - a new message is created in the wrong
chat, which is what is seen;
- `liveMessage.sent == true` - `apiUpdateChatItem` is called with the new
chat's type and id and the item id from the previous chat, which the
backend cannot resolve.
The same mismatch made the post-send `clearCurrentDraft()` clear the
draft of the chat opened after the switch, deleting a draft that was
never sent.
## Fix
`sendMessageAsync` and `sendMessage` take the chat the message was
composed in, defaulting to the chat this view shows
(`ComposeView.kt:685-694`, `988-993`). Only what a live message can reach
uses it: the message send, the update of an already sent live message,
and the two places that clear the draft after sending. Live messages have
no context item (`SendMsgView.kt:156-165` only offers the button when the
compose is empty and has none), so the forwarding, editing and reporting
branches cannot run with a chat other than the view's and keep using
`chat` - the parameter is not threaded through them.
The chat switch resolves the chat by the id it had before the switch:
```kotlin
val liveMessageChat = if (prevChatId == null || prevChatId == chat.id) chat else chatsCtx.getChat(prevChatId)
// if that chat is gone there is nowhere to send it, and it must not be sent to the chat opened instead
if (liveMessageChat != null) sendMessage(null, toChat = liveMessageChat, composed = cs) else clearState()
```
`prevChatId == chat.id` keeps the view's own chat, which is what secondary
(member support) chat views need - they share the group's chat id, and
only their `chat` carries the scope.
If the previous chat can no longer be found the message is not sent at
all, and the compose state is cleared so it does not leak into the chat
that was opened. Sending it to the chat that is open now is the defect
being fixed, so it is not used as a fallback.
### Handing the compose state over to the opened chat
Sending to the right chat is not enough on its own: `composeState` is
shared between the chats opened in this view, and this is the only branch
of `KeyChangeEffect` that neither resets it nor loads the opened chat's
draft - the branch that loads a draft (`else if (chatModel.draftChatId
.value == draftChatId(chatModel.chatId.value, chatScope) ...)`) is later
in the same `if` chain and cannot be reached. So the live message stayed
in the compose state of a view that now shows another chat, and that
chat's draft was never read.
`sendMessageAsync` then made it visible. It runs on `Dispatchers.Default`,
so its writes land after the switch:
```kotlin
val liveMessage = cs.liveMessage
if (!live) {
if (liveMessage != null) composeState.value = cs.copy(liveMessage = null) // the whole composed state
sending() // and its spinner
}
```
The opened chat's input showed the text composed in the previous one until
the send completed and `clearState()` emptied it; the draft it should have
shown was still in the model, and the next switch away dropped it. This
predates this fix - without it the same writes happen, and there
`clearCurrentDraft()` resolves to the opened chat and deletes its draft
outright.
Four changes, all following from "this send no longer owns the compose
state":
- `sendMessageAsync` takes its `cs` as a parameter defaulting to
`composeState.value`, and `sendMessage` takes `composed: ComposeState? =
null`, so only the chat switch passes a state and every other sender
still reads it inside the coroutine, exactly where the send read it
before. The chat switch
captures it on the main thread before replacing it - without that the
send would read the compose state of the chat that was opened and send
*its draft* to the previous chat.
- `checkLinkPreview` takes that state too. It re-read `composeState`
rather than what was passed in, and it is reached by every text live
message through `updateMsgContent`, so with the compose state handed
over it would have rebuilt the message from the opened chat's draft, or
from nothing - overwriting the live message instead of committing it.
Only the calls a live message can reach pass the state. The forwarding
call site keeps reading the current one - it is unreachable from the
chat switch, and `forwardItem` suspends before it, so passing the
captured state there would drop what was typed while the forward was in
flight. The three senders that connect a prepared chat keep reading the
current one too.
- Every `composeState` write in `sendMessageAsync` is guarded by
`composeIsForSend()` (`toChat.id == chat.id`): directly for the two at
the start, and through `chatIsOpen` for the clear/restore at the end,
which #7308 already routes through `sentMessageInCompose`. It compares
the two chats rather than checking which one is open, so the send made
by a chat switch never takes the compose state back, not even if that
chat is opened again before the send completes.
`clearCurrentDraft(toChat)` is already keyed on the chat and needs no
guard. Whether the *view's own* send may still write when its chat has
been switched away is #7308's question, not this one's.
- The chat-switch branch then resets `composeState` to the opened chat's
draft, or to an empty state, like the branches below it do.
## Blast radius
`toChat` defaults to the chat this view shows, so every other send passes
no chat: the send button (`SendMsgView.kt`), the live updates while typing
(`sendMessageAsync(live = true)`), forwarding, editing and reporting. For
all of them `composeIsForSend()` is true, so every guard added here is a
no-op and they behave exactly as before. Only the send started by the chat
switch passes a different chat, and only the branches it can reach were
changed.
The one change not behind that guard is `checkLinkPreview` reading the
state passed in. It matters only where the two can differ, which is after
a suspension: the forwarding branch waits on `forwardItem`, so that call
site deliberately keeps reading the current state (it is unreachable from
the chat switch anyway). The other call sites are reached with nothing
suspending since the state was captured.
The live message update loop is not affected: it is started once
(`SendMsgView.kt:523-559`) with the `::updateLiveMessage` reference of the
composition in which live mode started, so its updates already go to the
chat the message belongs to. It exits because the chat switch replaces the
compose state with the opened chat's, which has no `liveMessage` - on the
main thread, as the chat is switched, rather than when the send completes
as before. Only that send was created fresh on every composition, which is
why it was the one going to the wrong chat.
Not covered, and unchanged: a live message in a member support chat that
is closed without changing the chat id is never committed - the effect
that commits it is keyed on the chat id, which does not change when that
view is closed.
`chatsCtx.getChat` searches the context's own list, and a secondary
context (member support, reports) is built with an empty one, so there it
can only return null. That branch is not reached from a support chat in
practice - it shares the group's chat id, so `prevChatId == chat.id` holds
and the view's own `chat` is used - and if it ever were, the message is
discarded rather than sent to the chat that was opened, which is the
behaviour intended for "the chat is gone" anyway.
## Verification
- `./gradlew :common:compileKotlinDesktop` — passes.
- Manual:
1. Start a live message in **A**, type, and switch to **B** while
typing. The message must appear in **A**; nothing is sent in **B**,
and B's input and draft are untouched.
2. Repeat with a draft already saved in **B** - it must still be there
after the switch. This is the case that was found failing: B showed
the text composed in A, then emptied when the send completed, and B's
draft was dropped on the next switch. Watch B's input from the moment
of the switch, not only after the send finishes.
3. Slow or failing send (network off) while doing 1 and 2, so the window
between the switch and the send completing is long enough to type in
**B** - what is typed there must survive the send completing.
4. The live message must carry a **link preview**: type a URL in **A**,
let the preview load, then switch. The message committed to A must be
the text that was composed - not the opened chat's draft, and not
empty. Every text live message is rebuilt through
`updateMsgContent` -> `checkLinkPreview`, so this is what breaks if
that one stops reading the state it was given.
5. Switch **back**: live message in A, switch to B, return to A and type
something new before the send completes. What is typed in A must
survive - the send handed the compose state over at the switch and
must not take it back.
6. Regressions: an ordinary send goes to the chat it was typed in;
forwarding still targets the chat it was forwarded to, and text typed
while a forward is in flight is still appended to it; reporting a
message still reports it in the chat it belongs to; sending in a
member support chat still goes to that scope.
## Merged with #7308
#7308 (a send that is still in flight when the chat is switched) landed in
`stable` first, so this branch was merged with it. Both changed the end of
`sendMessageAsync`, and the two guards are **not** the same rule - the
merge keeps both:
- here, `composeIsForSend()` = `toChat.id == chat.id` - is this send for
the chat this view shows, or for another one;
- in #7308, `chatIsOpen` = `chatModel.chatId.value == chat.id` - is the
chat this view shows still the one open.
`chatIsOpen` becomes the conjunction,
`composeIsForSend() && chatModel.chatId.value == chat.id`. Where `toChat`
is `chat` - every send but the one made by a chat switch - that reduces to
#7308's own check, so its behaviour is unchanged.
Nothing else in that block had to move. #7308 already routes both compose
writes through `sentMessageInCompose`, which derives from `chatIsOpen`, so
guarding `chatIsOpen` guards them; the rest of the change there is one
call site taking `toChat`, `clearCurrentDraft`. The draft id a failed
message is saved under keeps using `chat`: that branch is behind
`!liveSend`, which the send made by a chat switch never satisfies, so
`toChat` is always `chat` where it is read.
An earlier revision of this note said that #7308's `cs.liveMessage != null`
clause "already covers the send made by the chat switch". **It did not.**
At the time that clause sat outside the `chatIsOpen` check:
```kotlin
val sentMessageInCompose = live || cs.liveMessage != null || (chatIsOpen && composeState.value.inProgress)
```
which is correct only while a live message is always sent to the chat that
is open - the assumption this fix removes. Read as written, the clause
*exempts* the chat-switch send from the very guard that protects the
opened chat, and a merge that followed it reintroduced the leak described
above. #7308 shipped with the live clauses moved inside `chatIsOpen`,
which was a no-op on its own branch and is what makes this merge work.