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 40d7927264..a2bebb9feb 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 @@ -597,6 +597,10 @@ fun ComposeView( composeState.value = composeState.value.copy(inProgress = true) } + // composeState and its inProgress flag are shared between the chats opened in this view, and sending is not cancelled + // when the chat is switched - a send may only clear or reset the state while it still holds the message that was sent + fun composeHasSentMessage(): Boolean = chatModel.chatId.value == chat.id && composeState.value.inProgress + suspend fun sendMemberContactInvitation() { val mc = checkLinkPreview() sending() @@ -604,10 +608,10 @@ fun ComposeView( if (contact != null) { withContext(Dispatchers.Main) { chatsCtx.updateContact(chat.remoteHostId, contact) - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -624,10 +628,10 @@ fun ComposeView( if (contact != null) { withContext(Dispatchers.Main) { chatsCtx.updateContact(chat.remoteHostId, contact) - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -669,10 +673,10 @@ fun ComposeView( chatModel.channelRelayHostnames.remove(groupInfo.groupId) chatModel.groupMembers.value = relayResults.map { it.relayMember } chatModel.populateGroupMembersIndexes() - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -932,16 +936,38 @@ fun ComposeView( val wasForwarding = cs.forwarding val forwardingFromChatId = (cs.contextItem as? ComposeContextItem.ForwardingItems)?.fromChatInfo?.id val lastFailed = lastMessageFailedToSend - if (lastFailed == null) { - clearState(live) - } else { - composeState.value = lastFailed - } - val draft = chatModel.draft.value - if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { - composeState.value = draft - } else { - clearCurrentDraft() + // composeState is shared between the chats opened in this view, and this runs after the send API call, so the user + // could have switched chats or typed another message in the meantime - only the message that was sent may be + // 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 + // 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) + if (sentMessageInCompose) { + if (lastFailed == null) { + clearState(live) + } else { + composeState.value = lastFailed + } + } + val draft = chatModel.draft.value + if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { + if (sentMessageInCompose) composeState.value = draft + } else { + clearCurrentDraft() + // 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 + if (chatIsOpen && composeState.value.empty) { + composeState.value = lastFailed + } else if (saveLastDraft) { + chatModel.draft.value = lastFailed + chatModel.draftChatId.value = draftChatId(chat.id, chatScope) + } + } + } } return sent } @@ -1319,7 +1345,15 @@ fun ComposeView( deleteUnusedFiles() } else if (cs.inProgress) { clearPrevDraft(prevChatId) - composeState.value = cs.copy(inProgress = false, progressByTimeout = false) + // the message being sent must not be kept in the compose state, it is shared with the chat opened next; + // if it fails to send it is restored in this chat or saved as its draft + clearState() + // clearState() does not load the draft of the chat opened next, and without this it is never shown and is + // dropped when that chat is left + val draft = chatModel.draft.value + if (draft != null && chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)) { + composeState.value = draft + } } else if (!cs.empty) { if (cs.preview is ComposePreview.VoicePreview && !cs.preview.finished) { recState.value = RecordingState.NotStarted diff --git a/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md b/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md new file mode 100644 index 0000000000..7a27fdd9d1 --- /dev/null +++ b/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md @@ -0,0 +1,244 @@ +# Fix: message being sent leaks into another chat's compose/draft, and erases what is typed there + +Branch: `nd/fix-inflight-send-writes-other-chat-compose` (off `origin/stable`) +Date: 2026-07-25 +PR: #7308 + +Line references are against `origin/stable` at `8dc387cb5`, with this fix +applied. Android and desktop only +(`multiplatform/.../views/chat/ComposeView.kt`); iOS has the same defect +but is not addressed here. + +## Problem + +Two reported symptoms, one cause. Both need a send that is still in +flight when the chat is switched (slow network, large file, or the send +just hanging with the progress circle showing): + +1. **The message ends up in another chat's draft.** Reply to a message + (or just type), press send, switch to another chat while it is + sending: the text *and the reply context* appear in that chat's input, + and leaving it saves them as that chat's draft. No forwarding + involved. +2. **A late send erases what you typed.** Press send, the progress circle + keeps spinning, switch to another chat and back, type a new message — + when the original send finally succeeds, the newly typed message is + erased. + +## Cause + +The compose state is shared, and the send outlives the chat: + +- `ChatView.kt:134` — one `MutableState` per `ChatView` + instance, `rememberSaveable` with no keys, reused for every chat that + the view displays. +- `Utils.kt:43-46` — `withLongRunningApi` launches on + `CoroutineScope(Dispatchers.Default)`, a standalone scope with no tie + to the composition or to the chat, and `sendMessage` + (`ComposeView.kt:972-976`) uses it. Leaving the chat never cancels an + in-flight send. + +Two writes then act on the wrong chat: + +- **On the chat switch** — `ComposeView.kt:1343-1347`: the `cs.inProgress` + branch used to keep the message in the shared compose state + (`composeState.value = cs.copy(inProgress = false, progressByTimeout = false)`) + and only cleared the *previous* chat's saved draft. The text and the + quote were therefore sitting in the input of the chat opened next, and + `ComposeView.kt:1348-1358` (`!cs.empty`) then saved them as *that* + chat's draft on the next switch. Symptom 1. +- **When the send completes** — `ComposeView.kt:943-968`, running in the + detached coroutine after the switch: `clearState(live)` on success, or + `composeState.value = lastFailed` on failure, where `lastFailed = + cs.copy(inProgress = false, preview = preview)` + (`ComposeView.kt:729`) **keeps `contextItem`, i.e. the reply**. On + success this wipes whatever is in the input now — including a message + typed after coming back (symptom 2); on failure it dumps the old + message into whichever chat is open (symptom 1 again). + +The same function was already inconsistent about which chat it acts on: +its draft bookkeeping (`clearCurrentDraft()`, and the forwarding +condition) uses the **captured** `chat` — the chat the message was +composed in — while its `composeState` writes hit whatever chat is +displayed at that moment. + +## Fix + +Two changes, both in `ComposeView.kt`. + +**1. Do not keep the message being sent in the shared compose state** +(`ComposeView.kt:1343-1347`). On switching away with a send in flight the +compose state is cleared, so nothing leaks into the chat opened next: + +```kotlin +} 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; + // if it fails to send it is restored in this chat or saved as its draft + clearState() +} +``` + +`clearState()` is used rather than assigning an empty `ComposeState` so that +the link preview state is reset too (`pendingLinkUrl` still points at the +sent message's link, and its fetch would otherwise set a preview on the +input of the chat opened next), and so that the attachment size limit is +carried over the same way as everywhere else. + +In-flight content is deliberately **not** saved as a draft here: the +message has been submitted and will most likely be sent, and a draft is +for messages that are not sent yet. + +`clearState()` alone would leave the chat opened next with an empty input +even when it has a draft: this branch, like the live message one above it, +returns before the branch that loads a draft +(`chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)`), +so that draft was never shown - and, being still in the slot but not in +any compose state, it was then dropped by `clearPrevDraft` on the next +chat switch. It is loaded here instead. This is not the one-slot +limitation below: nothing else is competing for the slot, the draft is +simply lost. + +**2. Only touch the compose state if it still holds the message that was +sent** (`ComposeView.kt:936-968`): + +```kotlin +withContext(Dispatchers.Main) { + val chatIsOpen = chatModel.chatId.value == chat.id + val liveSend = live || cs.liveMessage != null + val sentMessageInCompose = chatIsOpen && (liveSend || composeState.value.inProgress) + if (sentMessageInCompose) { + if (lastFailed == null) { + clearState(live) + } else { + composeState.value = lastFailed + } + } + val draft = chatModel.draft.value + if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { + if (sentMessageInCompose) composeState.value = draft + } else { + clearCurrentDraft() + if (!sentMessageInCompose && 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 + if (chatIsOpen && composeState.value.empty) { + composeState.value = lastFailed + } else if (saveLastDraft) { + chatModel.draft.value = lastFailed + chatModel.draftChatId.value = draftChatId(chat.id, chatScope) + } + } + } +} +``` + +Both the checks and the changes run on `Dispatchers.Main` (the block has +no suspension points), so they cannot be interleaved with the user +switching chats or typing - `KeyChangeEffect`, which does change 1, runs +there too. + +`inProgress` is the marker that the compose state is still the submitted +message: it is set by `sending()` (`ComposeView.kt:596-598`), preserved by +`copy` while sending (the only other write during a send is +`progressByTimeout` at `ComposeView.kt:1610-1617`), reset when switching +away (change 1), and never set by typing a new message. So a chat switch +*or* newly typed text both make the guard false. + +A **failed** send is different from an in-flight one - the message was not +sent, so it is an unsent message. It is put back into the input if that +chat is open and nothing else is being composed there, and kept as that +chat's draft otherwise, so it never appears in another chat (see the +limitations below for when it is still dropped). Staying in the chat is +unaffected: the guard is true there +and the failed message is restored into the input as before, keeping +"preserving long message when failed to send" (`e61babdc8`) working. + +Deliberately unchanged: + +- The condition of the forwarding branch. Gating the whole branch would + send a forward that completed after the user left to `clearCurrentDraft()` + instead, **deleting** the destination chat's draft that the branch + exists to preserve - only the compose write inside it is gated. +- Live message sends (`live`, or `cs.liveMessage != null` for the send + that finalises a live message when leaving the chat, `ComposeView.kt:1338-1342`), + as long as their chat is the one open. They never call `sending()`, so a + guard based on `inProgress` would change their behaviour: failed live + sends would stop restoring and would write a draft on every failing + keystroke send. That is why `liveSend` is an alternative to `inProgress` + inside the guard, and why it is excluded from the restore/draft branch - + not gating it there would produce exactly that draft-per-keystroke. + + What they are **not** exempt from is `chatIsOpen`. An earlier revision + had `live || cs.liveMessage != null` outside it, which holds only while + a live message is always sent to the chat that is open. #7323 removes + that: the live message committed by a chat switch is sent to the chat it + was composed in, while this view already shows another one, so an + unguarded clause here would clear *that* chat's compose state - the leak + this fix exists to prevent. Standalone this changes nothing except a + live send that completes after its chat was left, which now leaves the + opened chat alone. `sendMessageAsync` reads `composeState` inside the + coroutine (`ComposeView.kt:684`), so that branch cannot clear the state + itself without racing the send; #7323 adds the `composed` parameter that + makes the captured state explicit. + +**3. The same check where the flag is shared** (`ComposeView.kt:600-602` +and the three senders that connect a prepared chat). They call the same +`sending()`, so an unguarded `clearState()` or `inProgress` reset from +one of them corrupts the state of a send started in the chat opened next. + +## Behaviour after the fix + +| situation | before | after | +| --- | --- | --- | +| send, stay in chat, succeeds | input cleared | input cleared (unchanged) | +| send, stay in chat, fails | message restored in input | message restored in input (unchanged) | +| send, switch chats, succeeds | message left in the other chat's input, saved as its draft | other chat untouched | +| send, switch chats, fails | message dumped into the other chat's input | message restored in the chat it was composed in, or kept as its draft | +| send hangs, switch away and back, type, then it succeeds | typed message erased | typed message kept | +| forward send, still in destination chat | destination chat's draft restored | unchanged | +| live message sent on leaving the chat | compose state cleared by the send | unchanged | + +## Limitations + +Kept deliberately, to not grow the change: + +- A message that failed to send is dropped, rather than kept, when the + "Message draft" privacy setting is off, when the destination chat of a + failed forward already has a draft (its own draft is preserved + instead), and when the single draft slot is later taken by another + chat - drafts are one global slot, so the last write wins. +- The three senders that connect a prepared chat share the same + `sending()` flag, so they use the same check (`ComposeView.kt:604-616`, + `618-640`, `659-685`). Without it a connect completing after the chat + was switched would clear `inProgress` for a send started in the chat + opened next, and that sent message would then stay in the input. They + have no failed-message restore, so their typed message is dropped when + the chat is switched instead of being carried into the next chat. +- Typing in the same chat while its own send is in flight is still + cleared when the send completes: `inProgress` is preserved by `copy`, + so the guard stays true. Unchanged from before, and different from the + reported symptom, which needs the chat to be switched. +## Verification + +- `./gradlew :common:compileKotlinDesktop` — passes. +- Manual (needs a slow or failing send — e.g. airplane mode, or a large + file). On desktop any chat switch exercises it; on Android only an + in-place switch does (member info → open chat), because leaving to the + chat list destroys the view: + 1. Reply + type in A, send, switch to B while sending. B's input must + stay empty; leaving B must not create a draft in B. If the send + failed, A must hold the message (with the reply) as its draft. + 2. Send in A with the network off so the circle keeps spinning, switch + to B and back to A, type a new message, restore the network. The + typed message must survive the old send completing. + 3. Regression: ordinary send in A (input clears), failed send while + staying in A (message comes back in the input), forward into a chat + that has a draft (draft restored after sending). + +Rebased onto the scope-aware draft ids introduced by #7309: the draft +written here for a message that failed to send uses +`draftChatId(chat.id, chatScope)`, like every other draft write. + +Related: `plans/2026-07-25-fix-forward-moves-draft-to-target-chat.md` +(PR #7307) — different cause (stale `chat` captured by the desktop +`onDispose`), same shared-compose-state design.