From d9f20f8eb2a7bfbc2c903eb42738fa40660c0abd Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Wed, 15 Jul 2026 23:43:00 +0300 Subject: [PATCH] fix(#576): surface command.dispatch 4018 errors instead of silent dead-tap (#603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#576): surface command.dispatch 4018 errors instead of silent dead-tap * fix(#576): fall back to slash.exec for command.dispatch 4018 registry misses * fix(#576): block cli_only/TUI-only commands at dispatch (blocklist deliverable) * review(#603): address agy findings — tighten 4018 match, case-insensitive filter, add double-fault + blank-output tests * fix(#576): surface command dispatch outputs and update tests --------- Co-authored-by: M57 --- .../hermescontrol/data/ws/CommandModels.kt | 63 ++++++ .../m57/hermescontrol/ui/chat/ChatScreen.kt | 47 +---- .../hermescontrol/ui/chat/ChatViewModel.kt | 58 +++++- .../ui/chat/ChatViewModelTest.kt | 19 +- .../ui/chat/SlashCommandDispatchRpcTest.kt | 187 ++++++++++++++---- 5 files changed, 281 insertions(+), 93 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/CommandModels.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/CommandModels.kt index c4ec920d..541795eb 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/CommandModels.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/CommandModels.kt @@ -24,3 +24,66 @@ data class CommandCategory( val name: String, val pairs: List>, ) + +/** + * Desktop/CLI-only and TUI-only slash commands that do NOT function on mobile + * (issue #574). Source of truth for the backend `cli_only` / TUI-only flags + * returned by `commands.catalog`. + * + * Two consumers: + * 1. The slash-command suggestion menu hides these (so users don't discover + * dead commands). + * 2. [com.m57.hermescontrol.ui.chat.ChatViewModel.handleSlashCommand] blocks + * them at dispatch time (issue #576, deliverable #3) — a user who types one + * anyway gets a clear "not supported on mobile" message instead of a doomed + * RPC that produces TUI-only output or a confusing error. + */ +object CommandBlocklist { + val UNSUPPORTED: Set = + setOf( + "/clear", + "/redraw", + "/history", + "/save", + "/prompt", + "/snapshot", + "/handoff", + "/journey", + "/config", + "/statusbar", + "/timestamps", + "/verbose", + "/skin", + "/indicator", + "/busy", + "/tools", + "/toolsets", + "/skills", + "/pet", + "/hatch", + "/cron", + "/reload", + "/browser", + "/plugins", + "/billing", + "/platforms", + "/copy", + "/paste", + "/image", + "/quit", + // TUI-only extras (meaningless outside the TUI) + "/compact", + "/logs", + "/mouse", + ) + + /** + * True if [command] (e.g. "/clear" or "/CLEAR arg") is blocked on mobile. + * Matches on the exact command name (the token before the first space), + * so "/reload" is blocked while "/reload-mcp" (a supported command) is not. + */ + fun contains(command: String): Boolean { + val name = command.split(" ", limit = 2)[0].lowercase() + return name in UNSUPPORTED + } +} 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 22909fbb..4c38aaee 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 @@ -127,6 +127,7 @@ import coil.compose.AsyncImage import com.m57.hermescontrol.NavigationController import com.m57.hermescontrol.R import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.data.ws.CommandBlocklist import com.m57.hermescontrol.data.ws.CommandCatalog import com.m57.hermescontrol.data.ws.ConnectionStatus import com.m57.hermescontrol.data.ws.HermesWsClient @@ -739,45 +740,13 @@ private fun ChatInputBar( Column { // Commands hidden from the suggestion menu — desktop/CLI-only and // TUI-only commands that don't function on mobile (issue #574). - // Source of truth: backend `cli_only` / TUI-only flags in the catalog. - val hiddenSlashDisplay = - setOf( - "/clear", - "/redraw", - "/history", - "/save", - "/prompt", - "/snapshot", - "/handoff", - "/journey", - "/config", - "/statusbar", - "/timestamps", - "/verbose", - "/skin", - "/indicator", - "/busy", - "/tools", - "/toolsets", - "/skills", - "/pet", - "/hatch", - "/cron", - "/reload", - "/browser", - "/plugins", - "/billing", - "/platforms", - "/copy", - "/paste", - "/image", - "/quit", - // TUI-only extras (meaningless outside the TUI) - "/compact", - "/logs", - "/mouse", - ) - val commandNames = commandCatalog.pairs.map { it[0] }.filter { it !in hiddenSlashDisplay } + // Single source of truth: CommandBlocklist.UNSUPPORTED, which is + // also enforced at dispatch time (issue #576, deliverable #3). + val hiddenSlashDisplay = CommandBlocklist.UNSUPPORTED + val commandNames = + commandCatalog.pairs + .map { it[0] } + .filter { it.lowercase() !in hiddenSlashDisplay } androidx.compose.animation.AnimatedVisibility( visible = inputFieldValue.text.startsWith("/") && !inputFieldValue.text.contains(" "), 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 bd1fb1f1..a8232e6d 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 @@ -18,6 +18,7 @@ import com.m57.hermescontrol.data.remote.NetworkResult import com.m57.hermescontrol.data.remote.OkHttpProvider import com.m57.hermescontrol.data.remote.safeApiCall import com.m57.hermescontrol.data.session.ActiveSessionHolder +import com.m57.hermescontrol.data.ws.CommandBlocklist import com.m57.hermescontrol.data.ws.CommandCatalog import com.m57.hermescontrol.data.ws.ConnectionStatus import com.m57.hermescontrol.data.ws.HermesWsClient @@ -855,6 +856,17 @@ class ChatViewModel( } } + // Block desktop/CLI-only + TUI-only commands that don't function on + // mobile (issue #576, deliverable #3). These are also hidden from the + // suggestion menu, but a user can still type one — intercept it here + // (before any RPC fires) with a clear message instead of a doomed call. + if (CommandBlocklist.contains(command)) { + addAssistantMessage( + "⚠️ ${command.split(" ", limit = 2)[0]} is not supported on mobile", + ) + return + } + when (val result = slashDispatcher.dispatch(command)) { is SlashResult.Interrupt -> { interruptSession() @@ -912,11 +924,47 @@ class ChatViewModel( val name = parts[0].lowercase().removePrefix("/") val arg = parts.getOrElse(1) { "" } viewModelScope.launch(Dispatchers.IO) { - wsClient.send( - WsMethods.COMMAND_DISPATCH, - mapOf("name" to name, "arg" to arg, "session_id" to sessionId), - onSent = { id -> trackRequest(id, WsMethods.COMMAND_DISPATCH) }, - ) + try { + // Primary path: command.dispatch handles quick/plugin/bundle/ + // skill commands + a few hardcoded ones. It returns a hard 4018 + // "not a ... command" for everything that lives only in the TUI + // slash worker (the 29 commands that 4018'd on mobile — issue + // #576). For those we fall back to slash.exec, which runs the + // full COMMAND_REGISTRY through the worker. + val result = + wsClient.request( + WsMethods.COMMAND_DISPATCH, + mapOf("name" to name, "arg" to arg, "session_id" to sessionId), + ).await() + handleDispatchResult(result) + } catch (e: HermesWsClient.HermesRpcException) { + val msg = e.message.orEmpty() + // Registry miss on command.dispatch: the backend emits exactly + // "not a quick/plugin/bundle/skill command: " (tui_gateway + // server.py L12408). Match that precise phrase so unrelated + // errors can't accidentally trigger the slash.exec fallback. + if (msg.contains("not a quick/plugin/bundle/skill command")) { + // Registry miss on command.dispatch -> retry via slash.exec, + // which routes the full CLI command set through the worker. + try { + val result = + wsClient.request( + WsMethods.SLASH_EXEC, + mapOf( + "command" to "/$name${if (arg.isNotEmpty()) " $arg" else ""}", + "session_id" to sessionId, + ), + ).await() + val output = (result as? Map<*, *>)?.get("output") as? String + if (!output.isNullOrBlank()) addAssistantMessage(output) + } catch (e2: HermesWsClient.HermesRpcException) { + addAssistantMessage("⚠️ /$name: ${e2.message}") + } + } else { + // Legit error from command.dispatch (busy, no history, etc.) + addAssistantMessage("⚠️ /$name: ${e.message}") + } + } } } 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 5a21598f..f2f00e4e 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 @@ -179,24 +179,17 @@ class ChatViewModelTest { fun testSlashCommand_help_addsHelpMessage() = runTest { val (viewModel, sessionId) = createViewModelWithSession() - var dispatchReqId = "dispatch-help" - every { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } answers { - arg<((String) -> Unit)?>(2)?.invoke(dispatchReqId) - dispatchReqId - } + every { + HermesWsClient.request(WsMethods.COMMAND_DISPATCH, any(), any()) + } returns + CompletableDeferred( + mapOf("type" to "exec", "output" to "**Available Commands:**\n\u2022 `/status`\n\u2022 `/new`"), + ) viewModel.sendMessage("/help") advanceUntilIdle() - mockEventsFlow.emit( - WsEvent.RpcResult( - dispatchReqId, - mapOf("type" to "exec", "output" to "**Available Commands:**\n\u2022 `/status`\n\u2022 `/new`"), - ), - ) - advanceUntilIdle() - assertTrue( viewModel.uiState.value.messages .any { it.content.contains("Available Commands") }, diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt index 569aa1f9..d043f4fd 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt @@ -15,6 +15,7 @@ import io.mockk.mockkObject import io.mockk.mockkStatic import io.mockk.slot import io.mockk.unmockkAll +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow @@ -129,20 +130,17 @@ class SlashCommandDispatchRpcTest { val paramsSlot = slot>() var captured = false every { - HermesWsClient.send(capture(methodSlot), capture(paramsSlot), any()) + HermesWsClient.request(capture(methodSlot), capture(paramsSlot), any()) } answers { captured = true - reqCount++ - val id = "req-dispatch-$reqCount" - arg<((String) -> Unit)?>(2)?.invoke(id) - id + CompletableDeferred(Unit) } // /help is NOT client-special-cased -> RpcDispatch vm.sendMessage("/help") advanceUntilIdle() - assertTrue("expected a COMMAND_DISPATCH send", captured) + assertTrue("expected a COMMAND_DISPATCH request", captured) assertEquals(WsMethods.COMMAND_DISPATCH, methodSlot.captured) val params = paramsSlot.captured assertEquals("help", params["name"]) @@ -158,12 +156,9 @@ class SlashCommandDispatchRpcTest { val methodSlot = slot() val paramsSlot = slot>() every { - HermesWsClient.send(capture(methodSlot), capture(paramsSlot), any()) + HermesWsClient.request(capture(methodSlot), capture(paramsSlot), any()) } answers { - reqCount++ - val id = "req-dispatch-$reqCount" - arg<((String) -> Unit)?>(2)?.invoke(id) - id + CompletableDeferred(Unit) } vm.sendMessage("/queue do the thing") @@ -177,41 +172,161 @@ class SlashCommandDispatchRpcTest { } @Test - fun `client-special-cased commands do NOT hit COMMAND_DISPATCH`() = + fun `non-registry-miss backend error is surfaced directly (no fallback)`() = runTest { val (vm, _) = createViewModelWithSession() - val dispatched = mutableListOf() every { - HermesWsClient.send(any(), any(), any()) + HermesWsClient.request(any(), any(), any()) } answers { - // track method via the captured value isn't possible here; re-stub below - reqCount++ - val id = "req-id-$reqCount" - arg<((String) -> Unit)?>(2)?.invoke(id) - id - } - // Narrow stub to record COMMAND_DISPATCH calls - val seen = mutableListOf() - every { - HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) - } answers { - seen.add("dispatch") - reqCount++ - val id = "req-id-$reqCount" - arg<((String) -> Unit)?>(2)?.invoke(id) - id + val d = CompletableDeferred() + d.completeExceptionally( + // A real, actionable error (not the "not a command" 4018 + // that triggers the slash.exec fallback). + HermesWsClient.HermesRpcException("session busy — /interrupt first"), + ) + d } - vm.sendMessage("/stop") - advanceUntilIdle() - vm.sendMessage("/new") + vm.sendMessage("/help") advanceUntilIdle() - // /stop -> Interrupt, /new -> NewSession: neither should RpcDispatch. + val last = vm.uiState.value.messages.lastOrNull() + assertEquals("⚠️ /help: session busy — /interrupt first", last?.content) + } + + @Test + fun `registry-miss 4018 on command dispatch falls back to slash_exec and surfaces output`() = + runTest { + val (vm, sessionId) = createViewModelWithSession() + + val methodSlot = slot() + val paramsSlot = slot>() + every { + HermesWsClient.request(capture(methodSlot), capture(paramsSlot), any()) + } answers { + val m = methodSlot.captured + val d = CompletableDeferred() + if (m == WsMethods.COMMAND_DISPATCH) { + // Backend rejects /status with the registry-miss 4018 (issue #576). + d.completeExceptionally( + HermesWsClient.HermesRpcException( + "not a quick/plugin/bundle/skill command: status", + ), + ) + } else { + // slash.exec runs the full COMMAND_REGISTRY and returns output. + d.complete(mapOf("output" to "STATUS: gateway reachable")) + } + d + } + + vm.sendMessage("/status") + advanceUntilIdle() + + // The fallback must have hit slash.exec with the full command string. + // (methodSlot/paramsSlot hold the LAST call = slash.exec.) + assertEquals(WsMethods.SLASH_EXEC, methodSlot.captured) + val params = paramsSlot.captured + assertEquals("/status", params["command"]) + assertEquals(sessionId, params["session_id"]) + + // And the slash.exec output must be surfaced to the user. + val last = vm.uiState.value.messages.lastOrNull() + assertEquals("STATUS: gateway reachable", last?.content) + } + + @Test + fun `blocklisted cli_only or TUI-only command is rejected with friendly message and no RPC`() = + runTest { + val (vm, _) = createViewModelWithSession() + + var rpcCalls = 0 + every { + HermesWsClient.request(any(), any(), any()) + } answers { + rpcCalls++ + CompletableDeferred(Unit) + } + + // /redraw is TUI-only (issue #574) — hidden from suggestions but a + // user can still type it. It must be blocked before any RPC fires. + vm.sendMessage("/redraw") + advanceUntilIdle() + + val last = vm.uiState.value.messages.lastOrNull() assertTrue( - "client-handled commands must not be forwarded to the backend", - seen.isEmpty(), + "expected a 'not supported on mobile' message, got: ${last?.content}", + last?.content?.contains("not supported on mobile") == true, ) + assertEquals("no RPC should fire for a blocklisted command", 0, rpcCalls) + } + + @Test + fun `slash_exec double-fault surfaces the secondary error`() = + runTest { + val (vm, _) = createViewModelWithSession() + + every { + HermesWsClient.request(any(), any(), any()) + } answers { + val m = arg(0) + val d = CompletableDeferred() + if (m == WsMethods.COMMAND_DISPATCH) { + // Registry miss -> triggers the slash.exec fallback. + d.completeExceptionally( + HermesWsClient.HermesRpcException( + "not a quick/plugin/bundle/skill command: status", + ), + ) + } else { + // slash.exec ALSO fails (e.g. worker can't start). + d.completeExceptionally( + HermesWsClient.HermesRpcException("slash worker start failed: boom"), + ) + } + d + } + + vm.sendMessage("/status") + advanceUntilIdle() + + // Both RPCs fail -> the secondary slash.exec error must surface. + val last = vm.uiState.value.messages.lastOrNull() + assertEquals("⚠️ /status: slash worker start failed: boom", last?.content) + } + + @Test + fun `slash_exec blank output appends no assistant message`() = + runTest { + val (vm, _) = createViewModelWithSession() + + every { + HermesWsClient.request(any(), any(), any()) + } answers { + val m = arg(0) + val d = CompletableDeferred() + if (m == WsMethods.COMMAND_DISPATCH) { + d.completeExceptionally( + HermesWsClient.HermesRpcException( + "not a quick/plugin/bundle/skill command: status", + ), + ) + } else { + // slash.exec succeeded but returned no/empty output. + d.complete(mapOf("output" to "")) + } + d + } + + val before = vm.uiState.value.messages.size + vm.sendMessage("/status") + advanceUntilIdle() + + // The user's "/status" message is added, but blank slash.exec output + // must NOT append an assistant bubble — so the last message is still + // the user's own command, and only one message was added. + assertEquals(before + 1, vm.uiState.value.messages.size) + assertEquals("/status", vm.uiState.value.messages.lastOrNull()?.content) } }