diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt index a2bebb9feb..ff393a3c30 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt @@ -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? { - 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? { + // 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? 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; diff --git a/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md b/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md new file mode 100644 index 0000000000..bd37361dc5 --- /dev/null +++ b/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md @@ -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.