From 5ab7ad16df704b3e67fef668c8b5666fccaa914f Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:30:32 +0300 Subject: [PATCH] fix(chat): /update handled client-side with progress popup (issue #862) (#873) * fix(chat): /update handled client-side with progress popup (issue #862) * test(chat): stub actionProgress controller in ChatScreenTest (issue #862) --------- Co-authored-by: M57 --- .../hermescontrol/ui/chat/ChatScreenTest.kt | 8 + .../m57/hermescontrol/ui/chat/ChatScreen.kt | 30 +++ .../hermescontrol/ui/chat/ChatViewModel.kt | 58 +++++ .../ui/chat/SlashCommandDispatcher.kt | 20 +- .../ui/chat/ChatUpdateCommandTest.kt | 239 ++++++++++++++++++ .../ui/chat/SlashCommandDispatcherTest.kt | 10 + 6 files changed, 363 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatUpdateCommandTest.kt diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt index 03f2d37a..c0b7731b 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt @@ -9,9 +9,12 @@ import androidx.core.content.ContextCompat import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import com.m57.hermescontrol.data.ws.ConnectionStatus +import com.m57.hermescontrol.ui.common.ActionProgressController import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import org.junit.Before @@ -55,6 +58,11 @@ class ChatScreenTest { val mockViewModel = mockk(relaxed = true) every { mockViewModel.uiState } returns MutableStateFlow(uiState).asStateFlow() every { mockViewModel.streamingState } returns MutableStateFlow(StreamingState()).asStateFlow() + // ActionProgressDialog collects the controller's StateFlow — a relaxed + // mock proxy would crash the cast inside collectAsStateWithLifecycle. + // A real controller (never started here) stays invisible. + every { mockViewModel.actionProgress } returns + ActionProgressController(scope = CoroutineScope(Dispatchers.Main)) composeTestRule.setContent { ChatScreen( 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 44f3652b..91439dc8 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 @@ -37,6 +37,7 @@ import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Search +import androidx.compose.material3.AlertDialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -46,6 +47,7 @@ import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Surface import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf @@ -94,6 +96,7 @@ import com.m57.hermescontrol.ui.chat.components.SearchBarRow import com.m57.hermescontrol.ui.chat.components.SubagentInspectionSheet import com.m57.hermescontrol.ui.chat.components.rememberChatScrollController import com.m57.hermescontrol.ui.chat.components.tailContentKey +import com.m57.hermescontrol.ui.common.ActionProgressDialog import com.m57.hermescontrol.ui.common.AutoScrollingTitleText import com.m57.hermescontrol.ui.common.CredentialWarningBanner import com.m57.hermescontrol.ui.common.HermesScaffold @@ -717,6 +720,33 @@ fun ChatScreen( ) } + // /update from chat (issue #862): confirm, then the shared progress + // popup tracks the background update (live log tail + final state). + if (state.updateConfirmOpen) { + AlertDialog( + onDismissRequest = viewModel::closeUpdateConfirm, + title = { Text(stringResource(R.string.system_update_confirm_title)) }, + text = { Text(stringResource(R.string.system_update_confirm_desc)) }, + confirmButton = { + TextButton(onClick = { + viewModel.closeUpdateConfirm() + viewModel.applyUpdate() + }) { + Text(stringResource(R.string.system_confirm_update_now)) + } + }, + dismissButton = { + TextButton(onClick = viewModel::closeUpdateConfirm) { + Text(stringResource(R.string.system_confirm_cancel)) + } + }, + ) + } + ActionProgressDialog( + controller = viewModel.actionProgress, + title = stringResource(R.string.system_update_progress_title), + ) + // In-session model picker (issue #589) — opens on "/model". if (state.showModelPicker) { ModelPickerDialog( 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 cc5ac31b..d9341ade 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 @@ -32,6 +32,7 @@ import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods import com.m57.hermescontrol.data.ws.toJsonElement +import com.m57.hermescontrol.ui.common.ActionProgressController import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.delay @@ -261,6 +262,8 @@ data class ChatUiState( val sudoPrompt: SudoPromptUi? = null, val secretPrompt: SecretPromptUi? = null, val showSessionPicker: Boolean = false, + // /update confirm dialog (issue #862) — the command is handled client-side + val updateConfirmOpen: Boolean = false, // Search state val isSearchActive: Boolean = false, val searchQuery: String = "", @@ -454,6 +457,14 @@ class ChatViewModel( // ── Session persistence ────────────────────────────────────────────── private val repo: ChatPersistenceRepository = repo private val slashDispatcher = SlashCommandDispatcher() + + /** + * Progress popup for `/update` from chat (issue #862). The backend `/update` + * handler is interactive + session-exiting and can never answer the slash + * worker (45s timeout), so the command is intercepted client-side and + * routed through the same REST action + shared popup as the System screen. + */ + val actionProgress = ActionProgressController(scope = viewModelScope) private val searchDelegate = ChatSearchDelegate( scope = viewModelScope, @@ -1396,12 +1407,59 @@ class ChatViewModel( handleModelSwitch(command) } + is SlashResult.Update -> { + openUpdateConfirm() + } + is SlashResult.RpcDispatch -> { dispatchViaRpc(command) } } } + // ── Update from chat (issue #862) ──────────────────────────────────── + // `/update` can't travel via the slash worker (the backend handler is + // interactive + session-exiting → guaranteed 45s timeout). Intercept it + // client-side: confirm, then trigger the same REST action the System + // screen uses and track it in the shared ActionProgressDialog. + + fun openUpdateConfirm() { + _uiState.update { it.copy(updateConfirmOpen = true) } + } + + fun closeUpdateConfirm() { + _uiState.update { it.copy(updateConfirmOpen = false) } + } + + /** + * Run the backend update: `POST /api/hermes/update` returns immediately + * (`{ok, name}`) while `hermes update` runs in the background, so + * [actionProgress] polls its status log until it exits and the popup shows + * the live tail + final state. + */ + fun applyUpdate() { + actionProgress.open() + viewModelScope.launch(ioDispatcher) { + val result = safeApiCall { ApiClient.hermesApi.updateHermes() } + when (result) { + is NetworkResult.Success -> { + val name = result.data.name + if (name != null) { + actionProgress.markStarted(name) + } else { + actionProgress.fail( + "Update started but the backend did not report an action name", + ) + } + } + + is NetworkResult.Failure -> { + actionProgress.fail("Failed to start update: ${result.error.message}") + } + } + } + } + /** * Fork the active conversation via the session.branch WS RPC (issue #533). * The backend already supports session.branch; the mobile previously had diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt index 9fa3344c..1c641999 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcher.kt @@ -13,8 +13,12 @@ package com.m57.hermescontrol.ui.chat * get their own results: `/fork` goes via the `session.branch` RPC and `/model` * via the `config.set` RPC (key="model" → gateway `_apply_model_switch`; the * TUI gateway's `prompt.submit` does NOT parse slash commands, so sending it as - * a normal prompt makes the LLM treat it as text). Everything else is forwarded - * to the backend via [SlashResult.RpcDispatch]. + * a normal prompt makes the LLM treat it as text). `/update` is intercepted + * too: the backend handler is interactive + session-exiting (confirmation modal + * + relaunch as `hermes update`), so it can never produce the single response + * the slash worker waits for and always dies with a 45s "slash worker timed + * out" (issue #862). Everything else is forwarded to the backend via + * [SlashResult.RpcDispatch]. */ class SlashCommandDispatcher { fun dispatch(command: String): SlashResult { @@ -26,6 +30,7 @@ class SlashCommandDispatcher { "/new" -> SlashResult.NewSession "/fork", "/branch" -> SlashResult.SessionBranch "/model" -> SlashResult.ModelSwitch + "/update" -> SlashResult.Update else -> SlashResult.RpcDispatch } } @@ -55,4 +60,15 @@ sealed class SlashResult { * `/model --provider --session`. */ data object ModelSwitch : SlashResult() + + /** + * Trigger the backend update via the REST action API + * (`POST /api/hermes/update`, the System screen's flow) and track it in + * the shared [com.m57.hermescontrol.ui.common.ActionProgressDialog]. + * NOT sent via slash.exec — the backend `/update` handler is interactive + + * session-exiting (confirmation modal + relaunch), so it can never produce + * the single WS response the slash worker waits for and always times out + * after 45s (issue #862). + */ + data object Update : SlashResult() } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatUpdateCommandTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatUpdateCommandTest.kt new file mode 100644 index 00000000..ac4f7960 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatUpdateCommandTest.kt @@ -0,0 +1,239 @@ +package com.m57.hermescontrol.ui.chat + +import android.app.Application +import com.m57.hermescontrol.data.local.AuthManager +import com.m57.hermescontrol.data.local.HermesDatabase +import com.m57.hermescontrol.data.model.ActionResponse +import com.m57.hermescontrol.data.remote.ApiClient +import com.m57.hermescontrol.data.remote.HermesApiService +import com.m57.hermescontrol.data.ws.ConnectionStatus +import com.m57.hermescontrol.data.ws.HermesWsClient +import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.ui.chat.fakes.FakeChatPersistenceRepository +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import retrofit2.Response + +/** + * Issue #862 — `/update` from chat: the backend handler is interactive + + * session-exiting (confirmation modal + relaunch as `hermes update`), so it can + * never produce the single response the slash worker waits for — sending it via + * slash.exec/command.dispatch always dies with a 45s "slash worker timed out". + * The command must be intercepted client-side: confirm dialog → REST + * `POST /api/hermes/update` → the shared ActionProgressDialog tracks the log. + * + * NOTE: no mockkStatic(Dispatchers) — that bleed breaks later classes. The + * update flow is REST + StateFlow only; setMain on the test dispatcher is safe. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ChatUpdateCommandTest { + private val testDispatcher = StandardTestDispatcher() + private val mockEventsFlow = MutableSharedFlow(extraBufferCapacity = 64) + private val mockConnectionStatus = MutableStateFlow(ConnectionStatus.DISCONNECTED) + private lateinit var app: Application + private lateinit var fakeRepo: FakeChatPersistenceRepository + private lateinit var mockApi: HermesApiService + private var reqCount = 0 + + @Before + fun setUp() { + Dispatchers.setMain(testDispatcher) + reqCount = 0 + + mockkStatic(android.util.Log::class) + every { android.util.Log.d(any(), any()) } returns 0 + every { android.util.Log.i(any(), any()) } returns 0 + every { android.util.Log.w(any(), any()) } returns 0 + every { android.util.Log.e(any(), any(), any()) } returns 0 + + mockkObject(AuthManager) + mockkObject(HermesWsClient) + mockkObject(ApiClient) + mockkObject(HermesDatabase) + + app = mockk(relaxed = true) + fakeRepo = FakeChatPersistenceRepository() + mockApi = mockk(relaxed = true) + + mockConnectionStatus.value = ConnectionStatus.DISCONNECTED + + every { AuthManager.getToken() } returns "test-token" + every { AuthManager.isTypingEffectEnabled() } returns true + every { AuthManager.getTypingEffectDelayMs() } returns 30 + every { AuthManager.isAutoReconnect() } returns false + every { HermesWsClient.events } returns mockEventsFlow + every { HermesWsClient.connectionStatus } returns mockConnectionStatus + every { HermesWsClient.connect() } answers { + mockConnectionStatus.value = ConnectionStatus.CONNECTING + } + every { HermesWsClient.disconnect() } returns Unit + + every { HermesWsClient.send(any(), any(), any()) } answers { + reqCount++ + val id = "req-id-$reqCount" + arg<((String) -> Unit)?>(2)?.invoke(id) + id + } + every { HermesWsClient.sendMessage(any(), any(), any()) } answers { + reqCount++ + val id = "req-msg-$reqCount" + arg<((String) -> Unit)?>(2)?.invoke(id) + id + } + // Catch-all: mirror send() but complete immediately (no 120s timer). + every { HermesWsClient.request(any(), any(), any()) } answers { + HermesWsClient.send(arg(0), arg(1)) {} + CompletableDeferred(Unit) + } + every { ApiClient.hermesApi } returns mockApi + } + + @After + fun tearDown() { + Dispatchers.resetMain() + unmockkAll() + } + + private suspend fun TestScope.createViewModelWithSession(): Pair { + val vm = ChatViewModel(app, false, fakeRepo, ioDispatcher = testDispatcher) + advanceUntilIdle() + mockConnectionStatus.value = ConnectionStatus.CONNECTED + mockEventsFlow.emit(WsEvent.GatewayReady(null)) + advanceUntilIdle() + // req-id-3 = session.create (after loadSessions + fetchCommandCatalog) + mockEventsFlow.emit(WsEvent.RpcResult("req-id-3", mapOf("session_id" to "session-xyz"))) + advanceUntilIdle() + return Pair(vm, "session-xyz") + } + + @Test + fun `update in chat opens the confirm dialog and fires no WS RPC`() = + runTest { + val (vm, _) = createViewModelWithSession() + + vm.sendMessage("/update") + advanceUntilIdle() + + assertTrue("confirm dialog should open", vm.uiState.value.updateConfirmOpen) + assertEquals("/update", vm.uiState.value.messages.last().content) + + // The whole point of the fix: NOTHING goes over the wire. + verify(exactly = 0) { + HermesWsClient.request(WsMethods.COMMAND_DISPATCH, any(), any()) + } + verify(exactly = 0) { HermesWsClient.request(WsMethods.SLASH_EXEC, any(), any()) } + } + + @Test + fun `applyUpdate triggers the REST action and starts the progress popup`() = + runTest { + val vm = ChatViewModel(app, false, fakeRepo, ioDispatcher = testDispatcher) + coEvery { mockApi.updateHermes() } returns + Response.success(ActionResponse(ok = true, name = "hermes-update")) + // One running poll, then the action exits — runTest's teardown + // advances virtual time, so the loop must terminate. + coEvery { mockApi.getActionStatus("hermes-update") } returnsMany + listOf( + Response.success( + com.m57.hermescontrol.data.model.ActionStatusResponse( + name = "hermes-update", + running = true, + lines = listOf("cloning repo…"), + ), + ), + Response.success( + com.m57.hermescontrol.data.model.ActionStatusResponse( + name = "hermes-update", + running = false, + exit_code = 0, + lines = listOf("cloning repo…", "done"), + ), + ), + ) + + vm.applyUpdate() + runCurrent() + + val state = vm.actionProgress.state.value + assertTrue("popup should be visible", state.visible) + assertEquals( + com.m57.hermescontrol.ui.common.ActionProgressPhase.RUNNING, + state.phase, + ) + assertEquals("hermes-update", state.actionName) + } + + @Test + fun `applyUpdate settles on success when the action exits cleanly`() = + runTest { + val vm = ChatViewModel(app, false, fakeRepo, ioDispatcher = testDispatcher) + coEvery { mockApi.updateHermes() } returns + Response.success(ActionResponse(ok = true, name = "hermes-update")) + coEvery { mockApi.getActionStatus("hermes-update") } returns + Response.success( + com.m57.hermescontrol.data.model.ActionStatusResponse( + name = "hermes-update", + running = false, + exit_code = 0, + lines = listOf("done"), + ), + ) + + vm.applyUpdate() + advanceTimeBy(1_500) + runCurrent() + + val state = vm.actionProgress.state.value + assertEquals( + com.m57.hermescontrol.ui.common.ActionProgressPhase.SUCCEEDED, + state.phase, + ) + assertEquals(0, state.exitCode) + } + + @Test + fun `applyUpdate surfaces a rejected trigger in the popup`() = + runTest { + val vm = ChatViewModel(app, false, fakeRepo, ioDispatcher = testDispatcher) + coEvery { mockApi.updateHermes() } returns + Response.error(404, "no update endpoint".toResponseBody()) + + vm.applyUpdate() + runCurrent() + + val state = vm.actionProgress.state.value + assertEquals( + com.m57.hermescontrol.ui.common.ActionProgressPhase.FAILED, + state.phase, + ) + assertNotNull(state.error) + assertFalse(vm.uiState.value.updateConfirmOpen) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcherTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcherTest.kt index 2404a2f8..659dcbd2 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcherTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatcherTest.kt @@ -53,6 +53,16 @@ class SlashCommandDispatcherTest { assertEquals(SlashResult.ModelSwitch, dispatcher.dispatch("/Model")) } + @Test + fun `update routes to Update`() { + // /update's backend handler is interactive + session-exiting and can + // never answer the slash worker (45s timeout, issue #862) — it must be + // handled client-side via the REST action API + shared progress popup. + assertEquals(SlashResult.Update, dispatcher.dispatch("/update")) + assertEquals(SlashResult.Update, dispatcher.dispatch("/UPDATE now")) + assertEquals(SlashResult.Update, dispatcher.dispatch("/Update")) + } + @Test fun `NEW uppercase still routes to NewSession`() { // Dispatcher lower-cases before matching, so case must not matter.