From d7ca7b084ecb96225f0938ffc404a5b77d380239 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Sun, 21 Jun 2026 18:32:29 +0300 Subject: [PATCH] fix(#240): notification tap race condition opens new chat instead of correct session (#241) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-pronged fix for the race between GatewayReady auto-creating a session and ChatScreen LaunchedEffect calling switchSession: 1. ChatViewModel: added `initialSessionId` field. GatewayReady now checks this before auto-creating a session — if set, it calls switchSession instead. The field is set synchronously by ChatScreen via SideEffect during composition, before any WS event can be processed. 2. ChatScreen: LaunchedEffect now keys on state.connectionStatus and waits for CONNECTED before calling switchSession. This prevents the SESSION_RESUME message from being dropped (webSocket is null before onOpen, so send() is a silent no-op). Fixes #240 Co-authored-by: M57 --- .../m57/hermescontrol/ui/chat/ChatScreen.kt | 16 +++++++++- .../hermescontrol/ui/chat/ChatViewModel.kt | 18 ++++++++++- .../ui/chat/ChatViewModelTest.kt | 30 +++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt index 1ddf7929..2421c58e 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt @@ -73,6 +73,7 @@ import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf @@ -131,9 +132,22 @@ fun ChatScreen( val context = LocalContext.current val lifecycleOwner = androidx.lifecycle.compose.LocalLifecycleOwner.current + // Publish the notification session ID to the ViewModel synchronously + // during composition — BEFORE any WebSocket event (GatewayReady) can be + // processed. This ensures GatewayReady sees the pending session and + // resumes it instead of creating a new empty chat (issue #240). + SideEffect { + viewModel.initialSessionId = sessionId + } + // Switch to the session from a notification tap or history screen when provided. + // Keyed on connectionStatus so switchSession only fires after the WS is + // connected — otherwise SESSION_RESUME is silently dropped (webSocket is + // null before onOpen). GatewayReady also handles initialSessionId as a + // fallback for the race where it fires before this effect. val pendingSessionId = NavigationController.pendingSessionId - LaunchedEffect(sessionId, pendingSessionId) { + LaunchedEffect(sessionId, pendingSessionId, state.connectionStatus) { + if (state.connectionStatus != ConnectionStatus.CONNECTED) return@LaunchedEffect val target = if (!sessionId.isNullOrBlank()) sessionId else pendingSessionId if (!target.isNullOrBlank()) { viewModel.switchSession(target) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index 853cd79c..8510a635 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -137,6 +137,16 @@ class ChatViewModel( // ── WS Event Handling ──────────────────────────────────────────────── + /** + * Session ID to resume when the WebSocket connects. Set synchronously by + * [ChatScreen] via `SideEffect` during composition — before any WS event + * can be processed. This prevents the race where [GatewayReady] fires + * before ChatScreen's `LaunchedEffect` can call [switchSession], causing + * [createNewSession] to create an empty chat that overwrites the + * notification session (issue #240). + */ + var initialSessionId: String? = null + private fun handleWsEvent(event: WsEvent) { when (event) { is WsEvent.GatewayReady -> { @@ -144,7 +154,13 @@ class ChatViewModel( addSystemMessage("Connected to Hermes") loadSessions() if (_uiState.value.currentSessionId == null) { - createNewSession() + val initial = initialSessionId + if (!initial.isNullOrBlank()) { + initialSessionId = null + switchSession(initial) + } else { + createNewSession() + } } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index 255c8af9..17e8241a 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -128,6 +128,36 @@ class ChatViewModelTest { verify { HermesWsClient.send(WsMethods.SESSION_CREATE, any(), any()) } } + @Test + fun testGatewayReady_withInitialSessionId_switchesToIt() = + runTest { + val viewModel = ChatViewModel(app, startCleanup = false) + advanceUntilIdle() + + // Set the initial session ID (normally done by ChatScreen SideEffect) + viewModel.initialSessionId = "session-from-notification" + mockConnectionStatus.value = ConnectionStatus.CONNECTED + mockEventsFlow.emit(WsEvent.GatewayReady(null)) + advanceUntilIdle() + + val state = viewModel.uiState.value + assertTrue(state.isConnected) + // Should NOT have called createNewSession — switchSession was used instead + assertEquals("session-from-notification", state.currentSessionId) + + // Verify SESSION_RESUME was sent instead of SESSION_CREATE + verify { HermesWsClient.send(WsMethods.SESSION_LIST, any(), any()) } + verify { + HermesWsClient.send( + WsMethods.SESSION_RESUME, + mapOf("session_id" to "session-from-notification"), + any(), + ) + } + // SESSION_CREATE must NOT be sent + verify(inverse = true) { HermesWsClient.send(WsMethods.SESSION_CREATE, any(), any()) } + } + @Test fun testSessionCreateRpcResult() = runTest {