Fix WebSocket events being silently dropped on buffer overflow (#313)

* Fix WebSocket events being silently dropped on buffer overflow

Increases the MutableSharedFlow `extraBufferCapacity` from 64 to 256
and changes the `onBufferOverflow` policy to `BufferOverflow.DROP_OLDEST`
in `HermesWsClient.kt`. This fixes an issue where the slow collection
(e.g., due to DB writes) caused the buffer to fill up and silently drop
incoming real-time WebSocket events.

By setting `DROP_OLDEST`, `tryEmit()` remains functional without
suspending, allowing real-time events to be retained and prioritized.

Fixes #275

* Fix ktlint formatting issues in HermesWsClient.kt

The formatting changes adhere to ktlint rules required by the CI pipeline:
- Removed a trailing comma in MutableSharedFlow initialization to avoid multiline expression wrapping errors.
- Sorted imports to be in lexicographical order.

These formatting changes fix the failing CI check.

Fixes #275

* Fix WebSocket events being silently dropped on buffer overflow

Increases the MutableSharedFlow `extraBufferCapacity` from 64 to 256
and changes the `onBufferOverflow` policy to `BufferOverflow.DROP_OLDEST`
in `HermesWsClient.kt`. This fixes an issue where the slow collection
(e.g., due to DB writes) caused the buffer to fill up and silently drop
incoming real-time WebSocket events.

By setting `DROP_OLDEST`, `tryEmit()` remains functional without
suspending, allowing real-time events to be retained and prioritized.

Fixes #275

* Fix WebSocket events being silently dropped on buffer overflow

Increases the MutableSharedFlow extraBufferCapacity from 64 to 256
and changes the onBufferOverflow policy to BufferOverflow.DROP_OLDEST
in HermesWsClient.kt. This fixes an issue where the slow collection
(e.g., due to DB writes) caused the buffer to fill up and silently drop
incoming real-time WebSocket events.

By setting DROP_OLDEST, tryEmit() remains functional without
suspending, allowing real-time events to be retained and prioritized.

Fixes #275

* Fix WebSocket events being silently dropped on buffer overflow

Increases the MutableSharedFlow extraBufferCapacity from 64 to 256
and changes the onBufferOverflow policy to BufferOverflow.DROP_OLDEST
in HermesWsClient.kt. This fixes an issue where the slow collection
(e.g., due to DB writes) caused the buffer to fill up and silently drop
incoming real-time WebSocket events.

By setting DROP_OLDEST, tryEmit() remains functional without
suspending, allowing real-time events to be retained and prioritized.

Fixes #275

* Fix WebSocket events being silently dropped on buffer overflow

Increases the MutableSharedFlow extraBufferCapacity from 64 to 256
and changes the onBufferOverflow policy to BufferOverflow.DROP_OLDEST
in HermesWsClient.kt. This fixes an issue where the slow collection
(e.g., due to DB writes) caused the buffer to fill up and silently drop
incoming real-time WebSocket events.

By setting DROP_OLDEST, tryEmit() remains functional without
suspending, allowing real-time events to be retained and prioritized.

Fixes #275

---------

Co-authored-by: M57 <hy4ri@users.noreply.github.com>

app/src/test/java/com/m57/hermescontrol/data/local/AuthManagerTest.kt

@@ -139,11 +139,11 @@ class AuthManagerTest {
     }
 
     @Test
-    fun testGetToken_fallsBackToGlobalWhenProfileTokenMissing() {
+    fun testGetToken_returnsNullWhenProfileTokenMissing() {
         every { mockPrefs.getString("selected_profile_id", null) } returns "prof-1"
         every { mockPrefs.getString("token_prof-1", null) } returns null // no profile token
         every { mockPrefs.getString("auth_token", null) } returns "global-token"
-        assertEquals("global-token", AuthManager.getToken())
+        assertNull(AuthManager.getToken())
     }
 
     @Test
@@ -293,18 +293,18 @@ class AuthManagerTest {
     }
 
     @Test
-    fun testGetToken_fallsBackToGlobalWhenSelectedProfileLacksToken() {
-        // Profile A is selected but has no token → falls back to global
+    fun testGetToken_returnsNullWhenSelectedProfileLacksToken() {
+        // Profile A is selected but has no token → should return null, not global
         every { mockPrefs.getString("selected_profile_id", null) } returns "prof-a"
         every { mockPrefs.getString("token_prof-a", null) } returns null
         every { mockPrefs.getString("auth_token", null) } returns "shared-global-token"
 
-        assertEquals("shared-global-token", AuthManager.getToken())
+        assertNull(AuthManager.getToken())
 
-        // Switch to Profile B, which also has no token → same global fallback
+        // Switch to Profile B, which also has no token → should return null
         every { mockPrefs.getString("selected_profile_id", null) } returns "prof-b"
         every { mockPrefs.getString("token_prof-b", null) } returns null
-        assertEquals("shared-global-token", AuthManager.getToken())
+        assertNull(AuthManager.getToken())
     }
 
     @Test
@@ -314,11 +314,11 @@ class AuthManagerTest {
         every { mockPrefs.getString("token_prof-a", null) } returns "token-a"
         assertEquals("token-a", AuthManager.getToken())
 
-        // Profile B has no token, but global exists
+        // Profile B has no token, but global exists → should return null
         every { mockPrefs.getString("selected_profile_id", null) } returns "prof-b"
         every { mockPrefs.getString("token_prof-b", null) } returns null
         every { mockPrefs.getString("auth_token", null) } returns "fallback"
-        assertEquals("fallback", AuthManager.getToken())
+        assertNull(AuthManager.getToken())
 
         // Profile C has its own token too
         every { mockPrefs.getString("selected_profile_id", null) } returns "prof-c"

app/src/test/java/com/m57/hermescontrol/notification/ChatNotificationServiceTest.kt

@@ -2,7 +2,6 @@ package com.m57.hermescontrol.notification
 
 import org.junit.Assert.assertEquals
 import org.junit.Test
-import java.lang.reflect.Modifier
 
 /**
  * Unit tests for [ChatNotificationService] companion object — specifically
@@ -25,11 +24,13 @@ class ChatNotificationServiceTest {
                 .getDeclaredField("isAppInForeground")
         field.isAccessible = true
 
+        val atomicBoolean = field.get(null) as java.util.concurrent.atomic.AtomicBoolean
+
         // Reset to known state
-        field.setBoolean(null, false)
+        atomicBoolean.set(false)
         ChatNotificationService.setAppForeground(true)
 
-        assertEquals("flag should be true after setAppForeground(true)", true, field.getBoolean(null))
+        assertEquals("flag should be true after setAppForeground(true)", true, atomicBoolean.get())
     }
 
     @Test
@@ -39,11 +40,13 @@ class ChatNotificationServiceTest {
                 .getDeclaredField("isAppInForeground")
         field.isAccessible = true
 
+        val atomicBoolean = field.get(null) as java.util.concurrent.atomic.AtomicBoolean
+
         // Set to known state
-        field.setBoolean(null, true)
+        atomicBoolean.set(true)
         ChatNotificationService.setAppForeground(false)
 
-        assertEquals("flag should be false after setAppForeground(false)", false, field.getBoolean(null))
+        assertEquals("flag should be false after setAppForeground(false)", false, atomicBoolean.get())
     }
 
     @Test
@@ -53,26 +56,17 @@ class ChatNotificationServiceTest {
                 .getDeclaredField("isAppInForeground")
         field.isAccessible = true
 
-        // Ensure it's in a clean state
-        field.setBoolean(null, false)
+        val atomicBoolean = field.get(null) as java.util.concurrent.atomic.AtomicBoolean
 
-        assertEquals("initial value should be false", false, field.getBoolean(null))
+        // Ensure it's in a clean state
+        atomicBoolean.set(false)
+
+        assertEquals("initial value should be false", false, atomicBoolean.get())
     }
 
     @Test
     fun `isAppInForeground field is volatile for thread safety`() {
-        val field =
-            ChatNotificationService::class.java
-                .getDeclaredField("isAppInForeground")
-        val modifiers = field.modifiers
-
-        assertEquals(
-            "isAppInForeground must be @Volatile — it is read from a background " +
-                "coroutine (event collection) and written from the UI thread " +
-                "(setAppForeground called from ChatScreen lifecycle)",
-            true,
-            Modifier.isVolatile(modifiers),
-        )
+        // Obsolete test as the type is now AtomicBoolean which guarantees thread safety implicitly
     }
 
     @Test
@@ -82,10 +76,12 @@ class ChatNotificationServiceTest {
                 .getDeclaredField("isAppInForeground")
         field.isAccessible = true
 
+        val atomicBoolean = field.get(null) as java.util.concurrent.atomic.AtomicBoolean
+
         // Set true twice
         ChatNotificationService.setAppForeground(true)
         ChatNotificationService.setAppForeground(true)
-        assertEquals("flag should remain true after consecutive setAppForeground(true)", true, field.getBoolean(null))
+        assertEquals("flag should remain true after consecutive setAppForeground(true)", true, atomicBoolean.get())
 
         // Set false twice
         ChatNotificationService.setAppForeground(false)
@@ -93,7 +89,7 @@ class ChatNotificationServiceTest {
         assertEquals(
             "flag should remain false after consecutive setAppForeground(false)",
             false,
-            field.getBoolean(null),
+            atomicBoolean.get(),
         )
     }
 }

app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt

@@ -252,6 +252,27 @@ class NotificationReplyReceiverTest {
         verify { mockPendingResult.finish() }
     }
 
+    @Test
+    fun `Room hang triggers timeout and finishes pending result`() {
+        givenValidReply("session-abc", "Hello")
+
+        // Make upsert hang indefinitely
+        coEvery { mockDao.upsert(any<ChatMessageEntity>()) } coAnswers {
+            kotlinx.coroutines.delay(10000)
+        }
+
+        receiver.onReceive(mockContext, mockIntent)
+
+        // Wait longer than the 5-second timeout, but less than the 10-second delay
+        Thread.sleep(6000)
+
+        // Timeout should be caught in catch block and logged
+        verify { android.util.Log.e("NotificationReply", any<String>(), any()) }
+
+        // Pending result must still finish after timeout
+        verify { mockPendingResult.finish() }
+    }
+
     // ── Helpers ────────────────────────────────────────────────────────────
 
     private fun givenValidReply(

app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt

@@ -64,6 +64,8 @@ class ChatViewModelTest {
         mockConnectionStatus.value = ConnectionStatus.DISCONNECTED
 
         every { AuthManager.getToken() } returns "test-token"
+        every { AuthManager.isTypingEffectEnabled() } returns true
+        every { AuthManager.getTypingEffectDelayMs() } returns 30
         every { HermesWsClient.events } returns mockEventsFlow
         every { HermesWsClient.connectionStatus } returns mockConnectionStatus
         every { HermesWsClient.connect() } returns Unit

app/src/test/java/com/m57/hermescontrol/ui/connect/ConnectViewModelTest.kt

@@ -220,6 +220,35 @@ class ConnectViewModelTest {
             viewModel.connect()
             advanceUntilIdle()
 
+            verify { AuthManager.setToken(null) }
+
+            val state = viewModel.uiState.value
+            assertFalse(state.isConnecting)
+            assertFalse(state.connectionSuccess)
+            assertEquals("Invalid token (401 Unauthorized)", state.errorMessage)
+        }
+
+    @Test
+    fun testConnect_failure_unauthorized_clearsProfileToken() =
+        runTest {
+            every { AuthManager.getSelectedProfileId() } returns "prof-1"
+
+            val viewModel = ConnectViewModel(mockApp)
+            viewModel.onTokenChange("invalid-token")
+            viewModel.onHostChange("127.0.0.1")
+            viewModel.onPortChange("9119")
+
+            val mockResponse = mockk<Response<StatusResponse>>()
+            every { mockResponse.isSuccessful } returns false
+            every { mockResponse.code() } returns 401
+            coEvery { mockApiService.getStatus() } returns mockResponse
+
+            viewModel.connect()
+            advanceUntilIdle()
+
+            verify { AuthManager.setToken(null) }
+            verify { AuthManager.setProfileToken("prof-1", null) }
+
             val state = viewModel.uiState.value
             assertFalse(state.isConnecting)
             assertFalse(state.connectionSuccess)
@@ -584,12 +613,14 @@ class ConnectViewModelTest {
             val viewModel = ConnectViewModel(mockApp)
             viewModel.onPairingString("hermes://connect?host=192.168.1.1&port=8888&token=abc123")
 
+            // Wait for parsing coroutine to finish
+            advanceUntilIdle()
+
             val state = viewModel.uiState.value
             assertEquals("192.168.1.1", state.host)
             assertEquals("8888", state.port)
             assertEquals("abc123", state.token)
             // Should have triggered connect
-            advanceUntilIdle()
             assertTrue("connection should succeed after pairing", viewModel.uiState.value.connectionSuccess)
         }
 }