* 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 <m57@hermes-control.app>
This commit is contained in:
M57andM57 authored and GitHub committed
1 parent
e273488adc
commit
d9f20f8eb2
5 files changed
+281
-93
No files matched your search
@@ -24,3 +24,66 @@ data class CommandCategory(
|
||||
val name: String,
|
||||
val pairs: List<List<String>>,
|
||||
)
|
||||
|
||||
/**
|
||||
* 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<String> =
|
||||
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
|
||||
}
|
||||
}
|
||||
@@ -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(" "),
|
||||
|
||||
@@ -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: <name>" (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}")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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") },
|
||||
|
||||
@@ -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<Map<String, Any>>()
|
||||
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<Any?>(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<String>()
|
||||
val paramsSlot = slot<Map<String, Any>>()
|
||||
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<Any?>(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<String>()
|
||||
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<String>()
|
||||
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<Any?>()
|
||||
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<String>()
|
||||
val paramsSlot = slot<Map<String, Any>>()
|
||||
every {
|
||||
HermesWsClient.request(capture(methodSlot), capture(paramsSlot), any())
|
||||
} answers {
|
||||
val m = methodSlot.captured
|
||||
val d = CompletableDeferred<Any?>()
|
||||
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<Any?>(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<String>(0)
|
||||
val d = CompletableDeferred<Any?>()
|
||||
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<String>(0)
|
||||
val d = CompletableDeferred<Any?>()
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user