From 26c60a10c0eb3f50e8cc70f52c99e5afe2d4dc22 Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Wed, 30 Sep 2026 01:09:34 +0300 Subject: [PATCH] refactor(#1375): type command dispatch, slash exec and attachment RPC params (#1394) --- .../hermescontrol/data/ws/HermesWsClient.kt | 14 ++- .../data/ws/contract/ChatCommands.kt | 40 +++++++ .../data/ws/contract/RpcMethod.kt | 45 ++++++++ .../hermescontrol/ui/chat/ChatViewModel.kt | 73 +++++++------ .../data/ws/RawRpcParamsRatchetTest.kt | 10 +- .../data/ws/contract/ChatCommandsTypesTest.kt | 103 ++++++++++++++++++ .../ui/chat/ChatViewModelTest.kt | 8 +- .../ui/chat/SlashCommandDispatchRpcTest.kt | 35 +++--- 8 files changed, 263 insertions(+), 65 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/data/ws/contract/ChatCommands.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/contract/ChatCommandsTypesTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt index 4175356e..71e219b0 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt @@ -830,6 +830,17 @@ object HermesWsClient { return deferred } + /** + * Non-suspending typed twin of [request] for callers that must enqueue inside a + * non-suspending owner check and await the raw result themselves. + */ + fun requestTyped( + method: RpcMethod, + params: P, + timeoutMs: Long = REQUEST_TIMEOUT_MS, + suppressErrorEvent: Boolean = false, + ): CompletableDeferred = request(method.name, encodeParams(method, params), timeoutMs, suppressErrorEvent) + /** * Send a typed JSON-RPC request and await its deserialized result. * @@ -844,8 +855,7 @@ object HermesWsClient { timeoutMs: Long = REQUEST_TIMEOUT_MS, suppressErrorEvent: Boolean = false, ): R { - val encoded = encodeParams(method, params) - val deferred = request(method.name, encoded, timeoutMs, suppressErrorEvent) + val deferred = requestTyped(method, params, timeoutMs, suppressErrorEvent) val result = try { deferred.await() diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/ChatCommands.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/ChatCommands.kt new file mode 100644 index 00000000..c3f32fda --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/ChatCommands.kt @@ -0,0 +1,40 @@ +package com.m57.hermescontrol.data.ws.contract + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** `commands.catalog` is sent without explicit params; the active profile is injected by WsProfileParams. */ +@Serializable +data object CommandsCatalogParams + +/** `arg` is sent whenever non-null, including an empty string, exactly like the old map payload. */ +@Serializable +data class CommandDispatchParams( + @SerialName("name") val name: String, + @SerialName("arg") val arg: String? = null, + @SerialName("session_id") val sessionId: String? = null, +) + +@Serializable +data class SlashExecParams( + @SerialName("session_id") val sessionId: String, + @SerialName("command") val command: String, +) + +/** `data_url` carries the bytes for an uploaded file; `path` is for gateway-visible files. */ +@Serializable +data class FileAttachParams( + @SerialName("session_id") val sessionId: String, + @SerialName("path") val path: String? = null, + @SerialName("data_url") val dataUrl: String? = null, + @SerialName("name") val name: String? = null, +) + +/** The mobile client always sends the payload as `content_base64`; the `data` alias is not modelled. */ +@Serializable +data class ImageAttachBytesParams( + @SerialName("session_id") val sessionId: String, + @SerialName("content_base64") val contentBase64: String? = null, + @SerialName("filename") val filename: String? = null, + @SerialName("ext") val ext: String? = null, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt index 68bbc53e..f611d435 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt @@ -427,6 +427,46 @@ object RpcMethods { JsonElement.serializer(), ) + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val COMMANDS_CATALOG: RpcMethod = + RpcMethod( + WsMethods.COMMANDS_CATALOG, + CommandsCatalogParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val COMMAND_DISPATCH: RpcMethod = + RpcMethod( + WsMethods.COMMAND_DISPATCH, + CommandDispatchParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val SLASH_EXEC: RpcMethod = + RpcMethod( + WsMethods.SLASH_EXEC, + SlashExecParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val FILE_ATTACH: RpcMethod = + RpcMethod( + WsMethods.FILE_ATTACH, + FileAttachParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val IMAGE_ATTACH_BYTES: RpcMethod = + RpcMethod( + WsMethods.IMAGE_ATTACH_BYTES, + ImageAttachBytesParams.serializer(), + JsonElement.serializer(), + ) + /** Every typed method. GatewayContractTest iterates this; future migrations append here. */ val all: List> = listOf( @@ -482,5 +522,10 @@ object RpcMethods { CONFIG_GET, MODEL_OPTIONS, CLIENT_CAPABILITIES, + COMMANDS_CATALOG, + COMMAND_DISPATCH, + SLASH_EXEC, + FILE_ATTACH, + IMAGE_ATTACH_BYTES, ) } 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 b2d7721d..a0c31cbe 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 @@ -37,8 +37,12 @@ import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.JsonRpcError import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.CommandDispatchParams +import com.m57.hermescontrol.data.ws.contract.CommandsCatalogParams import com.m57.hermescontrol.data.ws.contract.ConfigGetParams import com.m57.hermescontrol.data.ws.contract.ConfigSetParams +import com.m57.hermescontrol.data.ws.contract.FileAttachParams +import com.m57.hermescontrol.data.ws.contract.ImageAttachBytesParams import com.m57.hermescontrol.data.ws.contract.PromptBtwParams import com.m57.hermescontrol.data.ws.contract.RpcMethod import com.m57.hermescontrol.data.ws.contract.RpcMethods @@ -51,6 +55,7 @@ import com.m57.hermescontrol.data.ws.contract.SessionIdParams import com.m57.hermescontrol.data.ws.contract.SessionInterruptParams import com.m57.hermescontrol.data.ws.contract.SessionListParams import com.m57.hermescontrol.data.ws.contract.SessionResumeParams +import com.m57.hermescontrol.data.ws.contract.SlashExecParams import com.m57.hermescontrol.data.ws.contract.TypedRpcSender import com.m57.hermescontrol.data.ws.toAny import com.m57.hermescontrol.data.ws.toJsonElement @@ -3400,13 +3405,13 @@ class ChatViewModel( if (attachment.isImage) { val deferred = enqueueOwned { - wsClient.request( - WsMethods.IMAGE_ATTACH_BYTES, - mapOf( - "session_id" to owner.agentSessionId, - "content_base64" to "data:${attachment.mimeType};base64,$b64", - "filename" to attachment.name, - "ext" to attachment.fileExtension, + wsClient.requestTyped( + RpcMethods.IMAGE_ATTACH_BYTES, + ImageAttachBytesParams( + sessionId = owner.agentSessionId, + contentBase64 = "data:${attachment.mimeType};base64,$b64", + filename = attachment.name, + ext = attachment.fileExtension, ), ) } ?: return@launch @@ -3419,12 +3424,12 @@ class ChatViewModel( } else { val deferred = enqueueOwned { - wsClient.request( - WsMethods.FILE_ATTACH, - mapOf( - "session_id" to owner.agentSessionId, - "data_url" to "data:${attachment.mimeType};base64,$b64", - "name" to attachment.name, + wsClient.requestTyped( + RpcMethods.FILE_ATTACH, + FileAttachParams( + sessionId = owner.agentSessionId, + dataUrl = "data:${attachment.mimeType};base64,$b64", + name = attachment.name, ), ) } ?: return@launch @@ -4076,12 +4081,11 @@ class ChatViewModel( // #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) + wsClient.call( + RpcMethods.COMMAND_DISPATCH, + CommandDispatchParams(name = name, arg = arg, sessionId = sessionId), + ) + handleDispatchResult(result.toAny()) } catch (e: HermesWsClient.HermesRpcException) { val msg = e.message.orEmpty() // Registry miss on command.dispatch: the backend emits exactly @@ -4093,15 +4097,14 @@ class ChatViewModel( // 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 + wsClient.call( + RpcMethods.SLASH_EXEC, + SlashExecParams( + sessionId = sessionId, + command = "/$name${if (arg.isNotEmpty()) " $arg" else ""}", + ), + ) + val output = (result.toAny() as? Map<*, *>)?.get("output") as? String if (!output.isNullOrBlank()) addAssistantMessage(output) } catch (e2: HermesWsClient.HermesRpcException) { addAssistantMessage("/$name: ${e2.message}") @@ -4173,12 +4176,11 @@ class ChatViewModel( viewModelScope.launch(ioDispatcher) { try { val result = - wsClient - .request( - WsMethods.COMMAND_DISPATCH, - mapOf("name" to "undo", "arg" to count, "session_id" to sessionId), - ).await() - handleDispatchResult(result) + wsClient.call( + RpcMethods.COMMAND_DISPATCH, + CommandDispatchParams(name = "undo", arg = count, sessionId = sessionId), + ) + handleDispatchResult(result.toAny()) } catch (e: HermesWsClient.HermesRpcException) { addAssistantMessage(e.message ?: "Failed to undo.") } catch (e: Exception) { @@ -4474,7 +4476,8 @@ class ChatViewModel( private fun fetchCommandCatalog() { viewModelScope.launch(ioDispatcher) { wsClient.send( - WsMethods.COMMANDS_CATALOG, + RpcMethods.COMMANDS_CATALOG, + CommandsCatalogParams, onSent = { id -> trackRequest(id, WsMethods.COMMANDS_CATALOG) }, ) } diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt index 0775f856..271b4c35 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt @@ -314,14 +314,6 @@ class RawRpcParamsRatchetTest { * Notice HermesWsClient.kt retains its other raw uses: `CLIENT_CAPABILITIES`, `PROMPT_SUBMIT`, * and `SESSION_REDIRECT`. */ - val BASELINE: List = - listOf( - "ui/chat/ChatViewModel.kt::COMMANDS_CATALOG", - "ui/chat/ChatViewModel.kt::COMMAND_DISPATCH", - "ui/chat/ChatViewModel.kt::COMMAND_DISPATCH", - "ui/chat/ChatViewModel.kt::FILE_ATTACH", - "ui/chat/ChatViewModel.kt::IMAGE_ATTACH_BYTES", - "ui/chat/ChatViewModel.kt::SLASH_EXEC", - ) + val BASELINE: List = emptyList() } } diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ChatCommandsTypesTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ChatCommandsTypesTest.kt new file mode 100644 index 00000000..58689dae --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ChatCommandsTypesTest.kt @@ -0,0 +1,103 @@ +package com.m57.hermescontrol.data.ws.contract + +import com.m57.hermescontrol.data.remote.OkHttpProvider +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatCommandsTypesTest { + private val json = OkHttpProvider.json + + @Test + fun commandsCatalogEncodesAsEmptyObject() { + assertEquals( + buildJsonObject { }, + json.encodeToJsonElement(CommandsCatalogParams.serializer(), CommandsCatalogParams), + ) + } + + @Test + fun commandDispatchKeepsEmptyArgAndOmitsNullOnes() { + assertEquals( + buildJsonObject { + put("name", "help") + put("arg", "") + put("session_id", "s1") + }, + json.encodeToJsonElement( + CommandDispatchParams.serializer(), + CommandDispatchParams(name = "help", arg = "", sessionId = "s1"), + ), + ) + assertEquals( + buildJsonObject { put("name", "help") }, + json.encodeToJsonElement(CommandDispatchParams.serializer(), CommandDispatchParams(name = "help")), + ) + } + + @Test + fun slashExecEncodesSessionAndCommand() { + assertEquals( + buildJsonObject { + put("session_id", "s1") + put("command", "/status") + }, + json.encodeToJsonElement( + SlashExecParams.serializer(), + SlashExecParams(sessionId = "s1", command = "/status"), + ), + ) + } + + @Test + fun fileAttachSendsDataUrlAndNameWithoutPath() { + assertEquals( + buildJsonObject { + put("session_id", "s1") + put("data_url", "data:text/plain;base64,QQ==") + put("name", "a.txt") + }, + json.encodeToJsonElement( + FileAttachParams.serializer(), + FileAttachParams(sessionId = "s1", dataUrl = "data:text/plain;base64,QQ==", name = "a.txt"), + ), + ) + } + + @Test + fun imageAttachBytesEncodesContentBase64FilenameAndExt() { + assertEquals( + buildJsonObject { + put("session_id", "s1") + put("content_base64", "data:image/png;base64,QQ==") + put("filename", "p.png") + put("ext", "png") + }, + json.encodeToJsonElement( + ImageAttachBytesParams.serializer(), + ImageAttachBytesParams( + sessionId = "s1", + contentBase64 = "data:image/png;base64,QQ==", + filename = "p.png", + ext = "png", + ), + ), + ) + } + + @Test + fun descriptorsAreRegisteredWithContractNames() { + listOf( + RpcMethods.COMMANDS_CATALOG to "commands.catalog", + RpcMethods.COMMAND_DISPATCH to "command.dispatch", + RpcMethods.SLASH_EXEC to "slash.exec", + RpcMethods.FILE_ATTACH to "file.attach", + RpcMethods.IMAGE_ATTACH_BYTES to "image.attach_bytes", + ).forEach { (method, name) -> + assertTrue("$name not in RpcMethods.all", method in RpcMethods.all) + assertEquals(name, method.name) + } + } +} 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 10f52473..2dbe0d6b 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 @@ -1483,7 +1483,11 @@ class ChatViewModelTest { every { HermesWsClient.request( WsMethods.COMMAND_DISPATCH, - mapOf("name" to "undo", "arg" to "2", "session_id" to sessionId), + buildJsonObject { + put("name", "undo") + put("arg", "2") + put("session_id", sessionId) + }, any(), ) } returns @@ -7238,7 +7242,7 @@ class ChatViewModelTest { verify { HermesWsClient.request(WsMethods.IMAGE_ATTACH_BYTES, any()) } assertEquals( "session_id must be forwarded to image.attach_bytes", - sessionId, + JsonPrimitive(sessionId), paramsSlot.captured["session_id"], ) assertTrue("encoded attachment cache must be cleaned", attachmentCacheDir.listFiles().isNullOrEmpty()) 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 5c301f69..029b9311 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 @@ -36,6 +36,7 @@ import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain +import kotlinx.serialization.json.JsonPrimitive import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -218,9 +219,9 @@ class SlashCommandDispatchRpcTest { val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) val params = paramsCalls[dispatchIndex] - assertEquals("help", params["name"]) - assertEquals("", params["arg"]) - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive("help"), params["name"]) + assertEquals(JsonPrimitive(""), params["arg"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) } @Test @@ -482,8 +483,8 @@ class SlashCommandDispatchRpcTest { assertTrue("expected SLASH_EXEC fallback, got $methodCalls", execIndex >= 0) assertTrue("dispatch must precede fallback", dispatchIndex < execIndex) val params = paramsCalls[execIndex] - assertEquals("/status", params["command"]) - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive("/status"), params["command"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) // And the slash.exec output must be surfaced to the user. val last = @@ -629,9 +630,9 @@ class SlashCommandDispatchRpcTest { val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) val params = paramsCalls[dispatchIndex] - assertEquals("init", params["name"]) - assertEquals("", params["arg"]) - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive("init"), params["name"]) + assertEquals(JsonPrimitive(""), params["arg"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) } @Test @@ -654,9 +655,9 @@ class SlashCommandDispatchRpcTest { val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) val params = paramsCalls[dispatchIndex] - assertEquals("init", params["name"]) - assertEquals("extra context here", params["arg"]) - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive("init"), params["name"]) + assertEquals(JsonPrimitive("extra context here"), params["arg"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) } @Test @@ -682,9 +683,9 @@ class SlashCommandDispatchRpcTest { val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) val params = paramsCalls[dispatchIndex] - assertEquals("focus", params["name"]) - assertEquals("", params["arg"]) - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive("focus"), params["name"]) + assertEquals(JsonPrimitive(""), params["arg"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) } @Test @@ -707,9 +708,9 @@ class SlashCommandDispatchRpcTest { val dispatchIndex = methodCalls.indexOf(WsMethods.COMMAND_DISPATCH) assertTrue("expected COMMAND_DISPATCH, got $methodCalls", dispatchIndex >= 0) val params = paramsCalls[dispatchIndex] - assertEquals("focus", params["name"]) - assertEquals("on", params["arg"]) - assertEquals(sessionId, params["session_id"]) + assertEquals(JsonPrimitive("focus"), params["name"]) + assertEquals(JsonPrimitive("on"), params["arg"]) + assertEquals(JsonPrimitive(sessionId), params["session_id"]) } @Test