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 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
adc23e3540
commit
d7ca7b084e
3 files changed
+62
-2
No files matched your search
@@ -73,6 +73,7 @@ import androidx.compose.material3.TextButton
|
|||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
import androidx.compose.runtime.DisposableEffect
|
import androidx.compose.runtime.DisposableEffect
|
||||||
import androidx.compose.runtime.LaunchedEffect
|
import androidx.compose.runtime.LaunchedEffect
|
||||||
|
import androidx.compose.runtime.SideEffect
|
||||||
import androidx.compose.runtime.derivedStateOf
|
import androidx.compose.runtime.derivedStateOf
|
||||||
import androidx.compose.runtime.getValue
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.runtime.mutableIntStateOf
|
import androidx.compose.runtime.mutableIntStateOf
|
||||||
@@ -131,9 +132,22 @@ fun ChatScreen(
|
|||||||
val context = LocalContext.current
|
val context = LocalContext.current
|
||||||
val lifecycleOwner = androidx.lifecycle.compose.LocalLifecycleOwner.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.
|
// 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
|
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
|
val target = if (!sessionId.isNullOrBlank()) sessionId else pendingSessionId
|
||||||
if (!target.isNullOrBlank()) {
|
if (!target.isNullOrBlank()) {
|
||||||
viewModel.switchSession(target)
|
viewModel.switchSession(target)
|
||||||
|
|||||||
@@ -137,6 +137,16 @@ class ChatViewModel(
|
|||||||
|
|
||||||
// ── WS Event Handling ────────────────────────────────────────────────
|
// ── 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) {
|
private fun handleWsEvent(event: WsEvent) {
|
||||||
when (event) {
|
when (event) {
|
||||||
is WsEvent.GatewayReady -> {
|
is WsEvent.GatewayReady -> {
|
||||||
@@ -144,7 +154,13 @@ class ChatViewModel(
|
|||||||
addSystemMessage("Connected to Hermes")
|
addSystemMessage("Connected to Hermes")
|
||||||
loadSessions()
|
loadSessions()
|
||||||
if (_uiState.value.currentSessionId == null) {
|
if (_uiState.value.currentSessionId == null) {
|
||||||
createNewSession()
|
val initial = initialSessionId
|
||||||
|
if (!initial.isNullOrBlank()) {
|
||||||
|
initialSessionId = null
|
||||||
|
switchSession(initial)
|
||||||
|
} else {
|
||||||
|
createNewSession()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -128,6 +128,36 @@ class ChatViewModelTest {
|
|||||||
verify { HermesWsClient.send(WsMethods.SESSION_CREATE, any(), any()) }
|
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
|
@Test
|
||||||
fun testSessionCreateRpcResult() =
|
fun testSessionCreateRpcResult() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
Reference in new issue
Block a user