From e4be8b146e693879985f977a6679a12316cb7db1 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Tue, 15 Sep 2026 13:09:11 +0300 Subject: [PATCH] fix(#1119): handle gateway server requests (#1125) --- .../m57/hermescontrol/data/ws/EventParser.kt | 19 +- .../hermescontrol/data/ws/HermesWsClient.kt | 66 ++++++ .../hermescontrol/data/ws/JsonRpcModels.kt | 4 +- .../com/m57/hermescontrol/data/ws/WsEvent.kt | 41 +++- .../ui/chat/ChatApprovalsDelegate.kt | 74 ++++++- .../ui/chat/ChatClarifyDelegate.kt | 39 +++- .../ui/chat/ChatCredentialPromptsDelegate.kt | 127 +++++++---- .../m57/hermescontrol/ui/chat/ChatMessage.kt | 2 + .../hermescontrol/ui/chat/ChatViewModel.kt | 206 +++++++++++++++++- .../ui/chat/ChatWsEventReducer.kt | 8 + .../ui/chat/components/MessageCards.kt | 161 ++++++++------ .../hermescontrol/data/ws/EventParserTest.kt | 51 +++++ .../data/ws/HermesWsClientTest.kt | 105 +++++++++ .../ui/chat/ChatApprovalsDelegateTest.kt | 111 ++++++++++ .../ui/chat/ChatPromptProtocolTest.kt | 176 +++++++++++++++ .../ui/chat/ChatViewModelTest.kt | 98 +++++++++ 16 files changed, 1169 insertions(+), 119 deletions(-) create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPromptProtocolTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt index 22bdd752..3306955b 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt @@ -19,9 +19,17 @@ object EventParser { response: JsonRpcResponse, rawJson: String = "", ): WsEvent { - // ── RPC response (has id) ──────────────────────────────────────── + // JSON-RPC is peer-to-peer: an inbound frame with both `id` and `method` + // is a request from the gateway, not a response to one of our calls. val id = response.id - if (id != null) { + if (response.method != null && id != null) { + @Suppress("UNCHECKED_CAST") + val requestParams = response.params?.toAny() as? Map ?: emptyMap() + return WsEvent.ServerRequest(id, response.method, requestParams) + } + + // A response has no method and carries a result or error member. + if (response.method == null && id != null && (response.result != null || response.error != null)) { return if (response.error != null) { WsEvent.RpcError(id, response.error) } else { @@ -220,6 +228,13 @@ object EventParser { WsEvent.ClarifyExpire(clarifyId, sessionId) } + "request.cancel" -> { + val requestId = payload?.get("id") as? String ?: "" + val requestMethod = payload?.get("method") as? String ?: "" + val reason = payload?.get("reason") as? String ?: "" + WsEvent.ServerRequestCancelled(requestId, requestMethod, reason, sessionId) + } + "status.update" -> { val status = payload?.get("status") as? String WsEvent.StatusUpdate(status, payload) 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 c694ed03..65d3ecc8 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 @@ -33,6 +33,8 @@ import kotlinx.serialization.encodeToString import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response @@ -808,6 +810,55 @@ object HermesWsClient { } } + /** + * Answer a gateway-originated JSON-RPC request. This deliberately bypasses + * [send]: server requests already have an id allocated by the gateway and + * responses must not contain a method or receive a new client id. + */ + fun respondToServerRequest( + id: String, + result: JsonElement, + ): Boolean { + if (id.isBlank()) return false + val json = + buildJsonObject { + put("jsonrpc", "2.0") + put("id", id) + put("result", result) + }.toString() + synchronized(outboundLock) { + val ws = webSocket + if (ws == null || !connected.get()) return false + return ws.send(json) + } + } + + /** Answer a gateway-originated request with a standard JSON-RPC error. */ + fun respondToServerRequestError( + id: String, + code: Int, + message: String, + ): Boolean { + if (id.isBlank()) return false + val json = + buildJsonObject { + put("jsonrpc", "2.0") + put("id", id) + put( + "error", + buildJsonObject { + put("code", code) + put("message", message) + }, + ) + }.toString() + synchronized(outboundLock) { + val ws = webSocket + if (ws == null || !connected.get()) return false + return ws.send(json) + } + } + // ── Send helpers ───────────────────────────────────────────────────── /** @@ -1249,6 +1300,21 @@ object HermesWsClient { resolvePending(rpcId, rpc.result, null) } + // Resume/replay responses carry still-open server requests + // separately from the event ring. Re-deliver them through the + // same generic path before exposing the RPC result. + @Suppress("UNCHECKED_CAST") + val openRequests = + (rpc.result?.toAny() as? Map<*, *>)?.get("open_requests") as? List> + openRequests?.forEach { openRequest -> + val openId = openRequest["id"] as? String ?: return@forEach + val method = openRequest["method"] as? String ?: return@forEach + + @Suppress("UNCHECKED_CAST") + val params = openRequest["params"] as? Map ?: emptyMap() + parsedEvents.tryEmit(WsEvent.ServerRequest(openId, method, params, replayed = true)) + } + if (rpc.id == null) { @Suppress("UNCHECKED_CAST") val params = rpc.params?.toAny() as? Map diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/JsonRpcModels.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/JsonRpcModels.kt index b8e91c84..796df16c 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/JsonRpcModels.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/JsonRpcModels.kt @@ -23,7 +23,9 @@ data class JsonRpcRequest( ) /** - * Incoming JSON-RPC 2.0 response **or** server-pushed notification. + * Incoming JSON-RPC 2.0 response, server-to-client request, or notification. + * Frames carrying both [id] and [method] are classified as server requests by + * [EventParser], never as responses to a client RPC. */ @Serializable data class JsonRpcResponse( diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt index 94dd2b25..237e4558 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt @@ -152,6 +152,8 @@ sealed class WsEvent { val questionId: String? = null, val multiSelect: Boolean = false, val questions: List = emptyList(), + val serverRequestId: String? = null, + val lockedAnswers: Map = emptyMap(), ) : WsEvent() data class ClarifyExpire( @@ -213,6 +215,25 @@ sealed class WsEvent { val sessionId: String? = null, ) : WsEvent() + /** + * Generic JSON-RPC server-to-client request. The gateway keeps the request + * open until the client sends a response frame with this exact [id]. + */ + data class ServerRequest( + val id: String, + val method: String, + val params: Map = emptyMap(), + val replayed: Boolean = false, + ) : WsEvent() + + /** The gateway withdrew a still-open server request (timeout/interruption). */ + data class ServerRequestCancelled( + val id: String, + val method: String, + val reason: String, + val sessionId: String? = null, + ) : WsEvent() + // ── RPC responses ──────────────────────────────────────────────────── data class RpcResult( @@ -256,18 +277,21 @@ sealed class WsEvent { * restricts to once/deny only (desktop `smartDenied` parity). */ val smartDenied: Boolean? = null, + /** JSON-RPC server-request id (`srq-*`) used for the response frame. */ + val serverRequestId: String? = null, ) : WsEvent() // ── Sudo / secret requests ───────────────────────────────────────── /** * Backend needs the user's sudo password to continue a turn - * (desktop: `sudo.request` → `sudo.respond {request_id, password}`). - * Mobile previously dropped this and the agent hung forever. + * New gateways use a same-ID JSON-RPC result frame; legacy gateways fall + * back to `sudo.respond`. */ data class SudoRequest( val requestId: String?, val sessionId: String?, + val serverRequestId: String? = null, ) : WsEvent() /** @@ -278,12 +302,13 @@ sealed class WsEvent { data class SudoExpire( val requestId: String?, val sessionId: String?, + val serverRequestId: String? = null, ) : WsEvent() /** * Backend needs a secret value (password / token) to continue a turn - * (desktop: `secret.request` → `secret.respond {request_id, value}`). - * Mobile previously dropped this and the agent hung forever. + * New gateways use a same-ID JSON-RPC result frame; legacy gateways fall + * back to `secret.respond`. * `envVar`/`prompt` mirror desktop: title = envVar ?: secretTitle, * body = prompt ?: secretDesc. */ @@ -292,6 +317,7 @@ sealed class WsEvent { val sessionId: String?, val envVar: String? = null, val prompt: String? = null, + val serverRequestId: String? = null, ) : WsEvent() /** @@ -301,6 +327,7 @@ sealed class WsEvent { data class SecretExpire( val requestId: String?, val sessionId: String?, + val serverRequestId: String? = null, ) : WsEvent() // ── Credential Vault prompts (issue #1090) ────────────────────────── @@ -310,11 +337,13 @@ sealed class WsEvent { val sessionId: String? = null, val backend: String? = null, val displayName: String? = null, + val serverRequestId: String? = null, ) : WsEvent() data class VaultUnlockExpire( val requestId: String?, val sessionId: String? = null, + val serverRequestId: String? = null, ) : WsEvent() data class VaultSaveLoginRequest( @@ -322,11 +351,13 @@ sealed class WsEvent { val sessionId: String? = null, val origin: String? = null, val site: String? = null, + val serverRequestId: String? = null, ) : WsEvent() data class VaultSaveLoginExpire( val requestId: String?, val sessionId: String? = null, + val serverRequestId: String? = null, ) : WsEvent() data class VaultCodeRequest( @@ -334,11 +365,13 @@ sealed class WsEvent { val sessionId: String? = null, val site: String? = null, val hint: String? = null, + val serverRequestId: String? = null, ) : WsEvent() data class VaultCodeExpire( val requestId: String?, val sessionId: String? = null, + val serverRequestId: String? = null, ) : WsEvent() // ── Gateway-level errors ─────────────────────────────────────────── diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt index 0144d71b..32d68d66 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegate.kt @@ -7,6 +7,9 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put /** * Owns approval rendering, deduplication, user response submission, and @@ -22,8 +25,49 @@ class ChatApprovalsDelegate( private val wsSend: (method: String, params: Map, onSent: ((String) -> Unit)?) -> Unit, private val trackRequest: (id: String, method: String) -> Unit, private val addSystemMessage: (text: String) -> Unit, + private val respondToServerRequest: ((String, JsonElement) -> Unit)? = null, ) { fun handleApprovalRequest(event: WsEvent.ApprovalRequest) { + val existingIndex = + uiState.value.messages.indexOfLast { message -> + val approval = message.approvalInfo ?: return@indexOfLast false + (event.serverRequestId != null && approval.serverRequestId == event.serverRequestId) || + (event.requestId != null && approval.requestId == event.requestId) + } + if (existingIndex >= 0) { + // `approval.pending` can arrive before the live server request during + // reconnect. Upgrade that legacy card with the direct server id so + // the user's next action answers the same JSON-RPC request instead of + // creating a duplicate prompt. + if (event.serverRequestId != null) { + uiState.update { state -> + state.copy( + messages = + state.messages.mapIndexed { index, message -> + if (index != existingIndex) { + message + } else { + val approval = message.approvalInfo + message.copy( + approvalInfo = + approval?.copy( + command = event.command ?: approval.command, + description = event.description ?: approval.description, + patternKeys = event.patternKeys ?: approval.patternKeys, + requestId = event.requestId ?: approval.requestId, + serverRequestId = event.serverRequestId, + choices = event.choices ?: approval.choices, + allowPermanent = event.allowPermanent ?: approval.allowPermanent, + smartDenied = event.smartDenied ?: approval.smartDenied, + ), + ) + } + }, + ) + } + } + return + } val description = event.description ?: event.command ?: "Unknown command" val content = "**Approval Required**\n$description" val msg = @@ -36,6 +80,7 @@ class ChatApprovalsDelegate( description = event.description, patternKeys = event.patternKeys, requestId = event.requestId, + serverRequestId = event.serverRequestId, choices = event.choices, allowPermanent = event.allowPermanent, smartDenied = event.smartDenied, @@ -51,7 +96,7 @@ class ChatApprovalsDelegate( // so the backend knows this client holds the prompt. Fire-and-forget. val requestId = event.requestId val sessionId = runtimeSessionId() ?: event.sessionId ?: uiState.value.currentSessionId - if (requestId != null && sessionId != null) { + if (event.serverRequestId == null && requestId != null && sessionId != null) { scope.launch(ioDispatcher) { runCatching { wsSend( @@ -118,6 +163,7 @@ class ChatApprovalsDelegate( // (any non-deny still unblocks, but stay on-spec going forward). val choice = if (action == "approve") "once" else action val requestId = approvalMsg.approvalInfo?.requestId + val serverRequestId = approvalMsg.approvalInfo?.serverRequestId // Clear buttons immediately uiState.update { s -> @@ -134,6 +180,16 @@ class ChatApprovalsDelegate( } scope.launch(ioDispatcher) { + if (serverRequestId != null && respondToServerRequest != null) { + respondToServerRequest.invoke( + serverRequestId, + buildJsonObject { + put("choice", choice) + put("all", false) + }, + ) + return@launch + } val params = mutableMapOf( "session_id" to sessionId, @@ -149,6 +205,21 @@ class ChatApprovalsDelegate( replayPendingApproval(sessionId) } } + + fun cancelServerRequest(requestId: String) { + uiState.update { state -> + state.copy( + messages = + state.messages.map { message -> + if (message.approvalInfo?.serverRequestId == requestId) { + message.copy(approvalInfo = null) + } else { + message + } + }, + ) + } + } } /** @@ -189,6 +260,7 @@ internal fun parseApprovalMap( patternKeys = patternKeys, sessionId = sessionId, requestId = map["request_id"] as? String, + serverRequestId = null, choices = choices, allowPermanent = allowPermanent, smartDenied = smartDenied, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatClarifyDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatClarifyDelegate.kt index e24f6b6b..1c284fcf 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatClarifyDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatClarifyDelegate.kt @@ -7,6 +7,9 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put class ChatClarifyDelegate( private val uiState: MutableStateFlow, @@ -15,6 +18,7 @@ class ChatClarifyDelegate( private val persistMessage: suspend (ChatMessage, String) -> Unit, private val wsClient: HermesWsClient = HermesWsClient, private val trackRequest: (String, String) -> Unit = { _, _ -> }, + private val respondToServerRequest: ((String, JsonElement) -> Unit)? = null, ) { fun respondToClarify(option: String) { val clarify = uiState.value.clarifyRequest @@ -41,6 +45,8 @@ class ChatClarifyDelegate( val sessionId = uiState.value.currentSessionId ?: return val clarify = uiState.value.clarifyRequest val clarifyId = clarify?.clarifyId + val serverRequestId = clarify?.serverRequestId + val lockedAnswers = clarify?.lockedAnswers.orEmpty() val isBatch = !clarify?.questions.isNullOrEmpty() val questions = clarify?.resolvedQuestions.orEmpty() uiState.update { it.copy(clarifyRequest = null) } @@ -49,7 +55,9 @@ class ChatClarifyDelegate( if (questions.size > 1) { questions .mapIndexed { index, q -> - val ans = answers[q.qid]?.trim().orEmpty() + val ans = + answers[q.qid]?.trim()?.takeIf { it.isNotEmpty() } + ?: lockedAnswers[q.qid].orEmpty() "${index + 1}. ${ans.ifEmpty { "(Skipped)" }}" }.joinToString("\n") } else { @@ -75,6 +83,35 @@ class ChatClarifyDelegate( } scope.launch(ioDispatcher) { + if (serverRequestId != null && respondToServerRequest != null) { + val finalAnswers = + buildMap { + putAll(lockedAnswers) + answers.forEach { (qid, answer) -> + answer.trim().takeIf { it.isNotEmpty() }?.let { put(qid, it) } + } + } + val result = + if (isBatch) { + buildJsonObject { + put( + "answers", + buildJsonObject { + for ((qid, answer) in finalAnswers) { + put(qid, answer) + } + }, + ) + } + } else { + buildJsonObject { + put("answer", answers.values.firstOrNull()?.trim() ?: singleFallbackAnswer.orEmpty()) + } + } + respondToServerRequest.invoke(serverRequestId, result) + return@launch + } + if (isBatch) { for (q in questions) { val ans = answers[q.qid]?.trim().orEmpty() diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt index fdaa51c0..c6e03cf1 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatCredentialPromptsDelegate.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.put @@ -21,15 +22,29 @@ class ChatCredentialPromptsDelegate( private val uiState: MutableStateFlow, private val wsSend: (method: String, params: Map, onSent: ((String) -> Unit)?) -> Unit, private val trackRequest: (id: String, method: String) -> Unit, + private val respondToServerRequest: ((String, JsonElement) -> Unit)? = null, ) { + private fun sendResponse( + serverRequestId: String?, + result: JsonElement, + legacyMethod: String, + legacyParams: Map, + ) { + if (serverRequestId != null && respondToServerRequest != null) { + respondToServerRequest.invoke(serverRequestId, result) + } else { + wsSend(legacyMethod, legacyParams) { id -> trackRequest(id, legacyMethod) } + } + } + /** - * The agent needs the user's sudo password. Surface a secure dialog and - * reply via sudo.respond. + * New gateways use a same-ID JSON-RPC response; legacy notifications fall + * back to sudo.respond. */ fun handleSudoRequest(event: WsEvent.SudoRequest) { uiState.update { it.copy( - sudoPrompt = SudoPromptUi(event.requestId, event.sessionId), + sudoPrompt = SudoPromptUi(event.requestId, event.sessionId, event.serverRequestId), isAgentTyping = false, ) } @@ -42,9 +57,10 @@ class ChatCredentialPromptsDelegate( fun handleSudoExpire(event: WsEvent.SudoExpire) { uiState.update { state -> val current = state.sudoPrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { + if (event.serverRequestId != null && current.serverRequestId != event.serverRequestId) { + return@update state + } + if (event.serverRequestId == null && event.requestId != null && current.requestId != event.requestId) { return@update state } state.copy(sudoPrompt = null) @@ -52,8 +68,8 @@ class ChatCredentialPromptsDelegate( } /** - * The agent needs a secret value (token/password). Surface a secure dialog - * and reply via secret.respond. + * New gateways use a same-ID JSON-RPC response; legacy notifications fall + * back to secret.respond. */ fun handleSecretRequest(event: WsEvent.SecretRequest) { uiState.update { @@ -64,6 +80,7 @@ class ChatCredentialPromptsDelegate( event.sessionId, event.envVar, event.prompt, + event.serverRequestId, ), isAgentTyping = false, ) @@ -76,9 +93,10 @@ class ChatCredentialPromptsDelegate( fun handleSecretExpire(event: WsEvent.SecretExpire) { uiState.update { state -> val current = state.secretPrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { + if (event.serverRequestId != null && current.serverRequestId != event.serverRequestId) { + return@update state + } + if (event.serverRequestId == null && event.requestId != null && current.requestId != event.requestId) { return@update state } state.copy(secretPrompt = null) @@ -101,10 +119,12 @@ class ChatCredentialPromptsDelegate( "password" to "", ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", "") }, WsMethods.SUDO_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.SUDO_RESPOND) } + ) } } @@ -124,10 +144,12 @@ class ChatCredentialPromptsDelegate( "value" to "", ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", "") }, WsMethods.SECRET_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.SECRET_RESPOND) } + ) } } @@ -149,10 +171,12 @@ class ChatCredentialPromptsDelegate( "password" to password, ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", password) }, WsMethods.SUDO_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.SUDO_RESPOND) } + ) } } @@ -173,10 +197,12 @@ class ChatCredentialPromptsDelegate( "value" to value, ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", value) }, WsMethods.SECRET_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.SECRET_RESPOND) } + ) } } @@ -184,7 +210,8 @@ class ChatCredentialPromptsDelegate( /** * The agent needs to unlock a password manager (1Password, Bitwarden). - * Surface an interactive prompt card in chat and reply via vault.unlock.respond. + * New gateways use a same-ID JSON-RPC response; legacy notifications fall + * back to vault.unlock.respond. */ fun handleVaultUnlockRequest(event: WsEvent.VaultUnlockRequest) { uiState.update { @@ -195,6 +222,7 @@ class ChatCredentialPromptsDelegate( sessionId = event.sessionId, backend = event.backend, displayName = event.displayName, + serverRequestId = event.serverRequestId, ), isAgentTyping = false, ) @@ -204,9 +232,10 @@ class ChatCredentialPromptsDelegate( fun handleVaultUnlockExpire(event: WsEvent.VaultUnlockExpire) { uiState.update { state -> val current = state.vaultUnlockPrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { + if (event.serverRequestId != null && current.serverRequestId != event.serverRequestId) { + return@update state + } + if (event.serverRequestId == null && event.requestId != null && current.requestId != event.requestId) { return@update state } state.copy(vaultUnlockPrompt = null) @@ -228,10 +257,12 @@ class ChatCredentialPromptsDelegate( "password" to "", ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", "") }, WsMethods.VAULT_UNLOCK_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.VAULT_UNLOCK_RESPOND) } + ) } } @@ -249,10 +280,12 @@ class ChatCredentialPromptsDelegate( "password" to password, ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", password) }, WsMethods.VAULT_UNLOCK_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.VAULT_UNLOCK_RESPOND) } + ) } } @@ -268,6 +301,7 @@ class ChatCredentialPromptsDelegate( sessionId = event.sessionId, origin = event.origin, site = event.site, + serverRequestId = event.serverRequestId, ), isAgentTyping = false, ) @@ -277,9 +311,10 @@ class ChatCredentialPromptsDelegate( fun handleVaultSaveLoginExpire(event: WsEvent.VaultSaveLoginExpire) { uiState.update { state -> val current = state.vaultSaveLoginPrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { + if (event.serverRequestId != null && current.serverRequestId != event.serverRequestId) { + return@update state + } + if (event.serverRequestId == null && event.requestId != null && current.requestId != event.requestId) { return@update state } state.copy(vaultSaveLoginPrompt = null) @@ -301,10 +336,12 @@ class ChatCredentialPromptsDelegate( "login" to "", ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", "") }, WsMethods.VAULT_SAVE_LOGIN_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.VAULT_SAVE_LOGIN_RESPOND) } + ) } } @@ -330,10 +367,12 @@ class ChatCredentialPromptsDelegate( "login" to loginJson, ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", loginJson) }, WsMethods.VAULT_SAVE_LOGIN_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.VAULT_SAVE_LOGIN_RESPOND) } + ) } } @@ -349,6 +388,7 @@ class ChatCredentialPromptsDelegate( sessionId = event.sessionId, site = event.site, hint = event.hint, + serverRequestId = event.serverRequestId, ), isAgentTyping = false, ) @@ -358,9 +398,10 @@ class ChatCredentialPromptsDelegate( fun handleVaultCodeExpire(event: WsEvent.VaultCodeExpire) { uiState.update { state -> val current = state.vaultCodePrompt ?: return@update state - if (event.requestId != null && current.requestId != null && - event.requestId != current.requestId - ) { + if (event.serverRequestId != null && current.serverRequestId != event.serverRequestId) { + return@update state + } + if (event.serverRequestId == null && event.requestId != null && current.requestId != event.requestId) { return@update state } state.copy(vaultCodePrompt = null) @@ -382,10 +423,12 @@ class ChatCredentialPromptsDelegate( "code" to "", ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", "") }, WsMethods.VAULT_CODE_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.VAULT_CODE_RESPOND) } + ) } } @@ -403,10 +446,12 @@ class ChatCredentialPromptsDelegate( "code" to code, ) prompt.requestId?.let { id -> params["request_id"] = id } - wsSend( + sendResponse( + prompt.serverRequestId, + buildJsonObject { put("value", code) }, WsMethods.VAULT_CODE_RESPOND, params, - ) { id -> trackRequest(id, WsMethods.VAULT_CODE_RESPOND) } + ) } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt index 8239f426..783ddba0 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt @@ -14,6 +14,8 @@ data class ApprovalInfo( val patternKeys: List?, /** Backend `request_id` — sent back in `approval.respond` to pin the exact pending. */ val requestId: String? = null, + /** JSON-RPC server-request id (`srq-*`) used for the response frame. */ + val serverRequestId: String? = null, /** Backend-advertised choices (`once`/`session`/`always`/`deny`). Null = legacy Run/Deny. */ val choices: List? = null, /** False → hide "Always allow" (tirith warning, desktop parity). */ 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 3cf0199e..75824201 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 @@ -43,6 +43,7 @@ import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.decodeFromJsonElement import kotlinx.serialization.json.intOrNull import kotlinx.serialization.json.jsonPrimitive @@ -204,6 +205,8 @@ data class ClarifyUi( val questionId: String? = null, val multiSelect: Boolean = false, val questions: List = emptyList(), + val serverRequestId: String? = null, + val lockedAnswers: Map = emptyMap(), ) { /** * Normalized list of questions to display. Guarantees at least one question @@ -251,6 +254,7 @@ private const val CLARIFY_DISMISS_RESPONSE = "The user cancelled — no answer p data class SudoPromptUi( val requestId: String?, val sessionId: String?, + val serverRequestId: String? = null, ) /** Transient — not persisted. Holds a pending secret (token/password) request. */ @@ -259,6 +263,7 @@ data class SecretPromptUi( val sessionId: String?, val envVar: String? = null, val prompt: String? = null, + val serverRequestId: String? = null, ) /** Transient — not persisted. Holds a pending vault unlock request (issue #1090). */ @@ -267,6 +272,7 @@ data class VaultUnlockPromptUi( val sessionId: String?, val backend: String? = null, val displayName: String? = null, + val serverRequestId: String? = null, ) /** Transient — not persisted. Holds a pending vault save login request (issue #1090). */ @@ -275,6 +281,7 @@ data class VaultSaveLoginPromptUi( val sessionId: String?, val origin: String? = null, val site: String? = null, + val serverRequestId: String? = null, ) /** Transient — not persisted. Holds a pending vault 2FA/MFA code request (issue #1090). */ @@ -283,6 +290,7 @@ data class VaultCodePromptUi( val sessionId: String?, val site: String? = null, val hint: String? = null, + val serverRequestId: String? = null, ) /** @@ -469,6 +477,7 @@ class ChatViewModel( uiState = _uiState, wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, trackRequest = { id, method -> trackRequest(id, method) }, + respondToServerRequest = { id, result -> wsClient.respondToServerRequest(id, result) }, ) private val approvalsDelegate = @@ -480,6 +489,7 @@ class ChatViewModel( wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, trackRequest = { id, method -> trackRequest(id, method) }, addSystemMessage = { text -> addSystemMessage(text) }, + respondToServerRequest = { id, result -> wsClient.respondToServerRequest(id, result) }, ) private val clarifyDelegate = @@ -490,6 +500,7 @@ class ChatViewModel( persistMessage = { msg, sid -> repo.persistMessage(msg, sid) }, wsClient = wsClient, trackRequest = { id, method -> trackRequest(id, method) }, + respondToServerRequest = { id, result -> wsClient.respondToServerRequest(id, result) }, ) private val subagentsDelegate = @@ -941,6 +952,14 @@ class ChatViewModel( streamingController.resetStreaming() } + is WsEvent.ServerRequest -> { + handleServerRequest(event) + } + + is WsEvent.ServerRequestCancelled -> { + handleServerRequestCancelled(event) + } + is WsEvent.ApprovalRequest -> { approvalsDelegate.handleApprovalRequest(event) } @@ -1015,6 +1034,184 @@ class ChatViewModel( } } + /** Route the generic transport event into the existing prompt delegates. */ + private fun handleServerRequest(request: WsEvent.ServerRequest) { + val params = request.params + val sessionId = params["session_id"] as? String + when (request.method) { + "clarify" -> { + @Suppress("UNCHECKED_CAST") + val rawQuestions = params["questions"] as? List<*> + + @Suppress("UNCHECKED_CAST") + val lockedAnswers = + (params["answers"] as? Map<*, *>) + ?.entries + ?.mapNotNull { (key, value) -> + if (key is String && value is String) key to value else null + }?.toMap() + .orEmpty() + val questions = + rawQuestions.orEmpty().mapIndexedNotNull { index, item -> + val map = item as? Map<*, *> ?: return@mapIndexedNotNull null + val question = map["question"] as? String ?: return@mapIndexedNotNull null + + @Suppress("UNCHECKED_CAST") + val choices = (map["choices"] as? List<*>)?.filterIsInstance() ?: emptyList() + WsEvent.ClarifyQuestion( + qid = map["qid"] as? String ?: "q$index", + question = question, + choices = choices, + multiSelect = map["multi_select"] as? Boolean ?: false, + ) + } + val first = questions.firstOrNull() + handleWsEvent( + WsEvent.ClarifyRequest( + text = first?.question ?: params["question"] as? String, + options = first?.choices ?: (params["choices"] as? List<*>)?.filterIsInstance(), + clarifyId = params["clarify_id"] as? String ?: params["request_id"] as? String, + sessionId = sessionId, + questionId = first?.qid, + multiSelect = first?.multiSelect ?: (params["multi_select"] as? Boolean ?: false), + questions = questions, + serverRequestId = request.id, + lockedAnswers = lockedAnswers, + ), + ) + } + + "approval" -> { + @Suppress("UNCHECKED_CAST") + val choices = (params["choices"] as? List<*>)?.filterIsInstance() + handleWsEvent( + WsEvent.ApprovalRequest( + command = params["command"] as? String, + description = params["description"] as? String, + patternKeys = (params["pattern_keys"] as? List<*>)?.filterIsInstance(), + sessionId = sessionId, + requestId = params["request_id"] as? String, + serverRequestId = request.id, + choices = choices, + allowPermanent = params["allow_permanent"] as? Boolean, + smartDenied = params["smart_denied"] as? Boolean, + ), + ) + } + + "sudo" -> { + handleWsEvent( + WsEvent.SudoRequest( + requestId = params["request_id"] as? String, + sessionId = sessionId, + serverRequestId = request.id, + ), + ) + } + + "secret" -> { + handleWsEvent( + WsEvent.SecretRequest( + requestId = params["request_id"] as? String, + sessionId = sessionId, + envVar = params["env_var"] as? String, + prompt = params["prompt"] as? String, + serverRequestId = request.id, + ), + ) + } + + "vault.code" -> { + handleWsEvent( + WsEvent.VaultCodeRequest( + requestId = params["request_id"] as? String, + sessionId = sessionId, + site = params["site"] as? String, + hint = params["hint"] as? String, + serverRequestId = request.id, + ), + ) + } + + "vault.save_login" -> { + handleWsEvent( + WsEvent.VaultSaveLoginRequest( + requestId = params["request_id"] as? String, + sessionId = sessionId, + origin = params["origin"] as? String, + site = params["site"] as? String, + serverRequestId = request.id, + ), + ) + } + + "vault.unlock_prompt" -> { + handleWsEvent( + WsEvent.VaultUnlockRequest( + requestId = params["request_id"] as? String, + sessionId = sessionId, + backend = params["backend"] as? String, + displayName = params["display_name"] as? String, + serverRequestId = request.id, + ), + ) + } + + else -> { + wsClient.respondToServerRequestError( + request.id, + -32601, + "no handler for server request: ${request.method}", + ) + } + } + } + + private fun handleServerRequestCancelled(event: WsEvent.ServerRequestCancelled) { + when (event.method) { + "clarify" -> { + if (_uiState.value.clarifyRequest?.serverRequestId == event.id) { + _uiState.update { it.copy(clarifyRequest = null) } + streamingController.resetStreaming() + } + } + + "approval" -> { + approvalsDelegate.cancelServerRequest(event.id) + } + + "sudo" -> { + credentialPromptsDelegate.handleSudoExpire( + WsEvent.SudoExpire(null, event.sessionId, serverRequestId = event.id), + ) + } + + "secret" -> { + credentialPromptsDelegate.handleSecretExpire( + WsEvent.SecretExpire(null, event.sessionId, serverRequestId = event.id), + ) + } + + "vault.code" -> { + credentialPromptsDelegate.handleVaultCodeExpire( + WsEvent.VaultCodeExpire(null, event.sessionId, serverRequestId = event.id), + ) + } + + "vault.save_login" -> { + credentialPromptsDelegate.handleVaultSaveLoginExpire( + WsEvent.VaultSaveLoginExpire(null, event.sessionId, serverRequestId = event.id), + ) + } + + "vault.unlock_prompt" -> { + credentialPromptsDelegate.handleVaultUnlockExpire( + WsEvent.VaultUnlockExpire(null, event.sessionId, serverRequestId = event.id), + ) + } + } + } + // ── Message streaming ──────────────────────────────────────────────── /** @@ -3452,8 +3649,8 @@ class ChatViewModel( * * The backend's clarify tool blocks the agent thread waiting for a response * (CLI timeout is 120s). A silent dismiss would leave the agent hanging - * until that timeout, so we send a cancel sentinel - * ([CLARIFY_DISMISS_RESPONSE]) over `clarify.respond` to unblock it. + * until that timeout, so we send an empty result frame for new gateways or + * the legacy cancel sentinel for old notification-based gateways. * * This is a *reject*, not an instruction to proceed — the agent is told no * answer was provided and should re-ask or back off, NOT charge ahead. @@ -3467,6 +3664,7 @@ class ChatViewModel( val sessionId = _uiState.value.currentSessionId ?: return val clarify = _uiState.value.clarifyRequest val clarifyId = clarify?.clarifyId + val serverRequestId = clarify?.serverRequestId // Raw batch questions (non-empty only for true batch payloads). // Legacy singles keep questions empty and rely on questionId (nullable). val isBatch = !clarify?.questions.isNullOrEmpty() @@ -3476,6 +3674,10 @@ class ChatViewModel( addSystemMessage("Clarify dismissed — no answer sent", persist = true) viewModelScope.launch(ioDispatcher) { + if (serverRequestId != null) { + wsClient.respondToServerRequest(serverRequestId, buildJsonObject {}) + return@launch + } if (isBatch) { // Send dismissal for every question in the batch for (q in displayQuestions) { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt index 068e4b61..f79882f0 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt @@ -164,6 +164,12 @@ object ChatWsEventReducer { // RpcResult is handled by the ViewModel (needs pending request context) is WsEvent.RpcResult -> ReducerResult(state = state, streamingState = streamingState) + // Server requests are dispatched by ChatViewModel to the existing + // prompt delegates; the reducer must remain side-effect free. + is WsEvent.ServerRequest -> ReducerResult(state = state, streamingState = streamingState) + + is WsEvent.ServerRequestCancelled -> ReducerResult(state = state, streamingState = streamingState) + // ApprovalRequest is handled by the ViewModel (needs active session + WS client) is WsEvent.ApprovalRequest -> ReducerResult(state = state, streamingState = streamingState) @@ -706,6 +712,8 @@ object ChatWsEventReducer { questionId = event.questionId, multiSelect = event.multiSelect, questions = questions, + serverRequestId = event.serverRequestId, + lockedAnswers = event.lockedAnswers, ), isAgentTyping = false, ), diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt index 8ec0a493..dd0477ab 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt @@ -473,6 +473,7 @@ fun ClarifyBubble( horizontalAlignment = Alignment.CenterHorizontally, ) { questions.forEachIndexed { index, q -> + val lockedAnswer = clarifyRequest.lockedAnswers[q.qid] if (index > 0) { HorizontalDivider( modifier = Modifier.padding(vertical = 12.dp), @@ -497,79 +498,105 @@ fun ClarifyBubble( ) } - if (q.multiSelect) { - Spacer(Modifier.height(2.dp)) - Text( - text = "Select all that apply", - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.fillMaxWidth(), - ) - } - - if (q.choices.isNotEmpty()) { - Spacer(Modifier.height(8.dp)) - FlowRow( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.spacedBy(6.dp), - verticalArrangement = Arrangement.spacedBy(6.dp), + if (lockedAnswer != null) { + Surface( + modifier = Modifier.fillMaxWidth().padding(top = 8.dp), + color = MaterialTheme.colorScheme.secondaryContainer, + shape = RoundedCornerShape(8.dp), ) { - q.choices.forEach { choice -> - val selectedSet = selectedChoicesByQid[q.qid] ?: emptySet() - val isSelected = selectedSet.contains(choice) - FilterChip( - selected = isSelected, - onClick = { - if (q.multiSelect) { - val updated = if (isSelected) selectedSet - choice else selectedSet + choice - selectedChoicesByQid = selectedChoicesByQid + (q.qid to updated) - } else { - if (!isBatch && customTextByQid[q.qid].isNullOrBlank()) { - // Fast 1-tap respond for lone single-select question when no custom text is entered - onRespondSingle(choice) - } else { - val updated = if (isSelected) emptySet() else setOf(choice) - selectedChoicesByQid = selectedChoicesByQid + (q.qid to updated) - } - } - }, - label = { Text(choice) }, - leadingIcon = - if (isSelected) { - { - Icon( - imageVector = Icons.Default.Check, - contentDescription = null, - modifier = Modifier.size(16.dp), - ) - } - } else { - null - }, + Row( + modifier = Modifier.padding(horizontal = 10.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon( + imageVector = Icons.Default.Check, + contentDescription = null, + tint = MaterialTheme.colorScheme.primary, + modifier = Modifier.size(18.dp), + ) + Spacer(Modifier.width(6.dp)) + Text( + text = "Answered: $lockedAnswer", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSecondaryContainer, ) } } - } - - Spacer(Modifier.height(8.dp)) - val typed = customTextByQid[q.qid].orEmpty() - OutlinedTextField( - value = typed, - onValueChange = { newText -> - customTextByQid = customTextByQid + (q.qid to newText) - }, - label = { + } else { + if (q.multiSelect) { + Spacer(Modifier.height(2.dp)) Text( - if (q.choices.isEmpty()) { - stringResource(R.string.message_your_response) - } else { - "Other (optional)" - }, + text = "Select all that apply", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.fillMaxWidth(), ) - }, - modifier = Modifier.fillMaxWidth(), - singleLine = true, - ) + } + + if (q.choices.isNotEmpty()) { + Spacer(Modifier.height(8.dp)) + FlowRow( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(6.dp), + verticalArrangement = Arrangement.spacedBy(6.dp), + ) { + q.choices.forEach { choice -> + val selectedSet = selectedChoicesByQid[q.qid] ?: emptySet() + val isSelected = selectedSet.contains(choice) + FilterChip( + selected = isSelected, + onClick = { + if (q.multiSelect) { + val updated = if (isSelected) selectedSet - choice else selectedSet + choice + selectedChoicesByQid = selectedChoicesByQid + (q.qid to updated) + } else { + if (!isBatch && customTextByQid[q.qid].isNullOrBlank()) { + // Fast 1-tap respond for lone single-select question when no custom text is entered + onRespondSingle(choice) + } else { + val updated = if (isSelected) emptySet() else setOf(choice) + selectedChoicesByQid = selectedChoicesByQid + (q.qid to updated) + } + } + }, + label = { Text(choice) }, + leadingIcon = + if (isSelected) { + { + Icon( + imageVector = Icons.Default.Check, + contentDescription = null, + modifier = Modifier.size(16.dp), + ) + } + } else { + null + }, + ) + } + } + } + + Spacer(Modifier.height(8.dp)) + val typed = customTextByQid[q.qid].orEmpty() + OutlinedTextField( + value = typed, + onValueChange = { newText -> + customTextByQid = customTextByQid + (q.qid to newText) + }, + label = { + Text( + if (q.choices.isEmpty()) { + stringResource(R.string.message_your_response) + } else { + "Other (optional)" + }, + ) + }, + modifier = Modifier.fillMaxWidth(), + singleLine = true, + ) + } } Spacer(Modifier.height(12.dp)) diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt index 6f90080f..5f79c483 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt @@ -59,6 +59,57 @@ class EventParserTest { assertEquals(mapOf("status" to "success"), rpcResult.result) } + @Test + fun testParseServerRequest_withIdAndMethod_isNotRpcResult() { + val event = + EventParser.parse( + createJsonRpcResponse( + jsonrpc = "2.0", + id = "srq-123", + method = "clarify", + params = mapOf("session_id" to "session-1", "question" to "Continue?"), + ), + ) + + assertTrue(event is WsEvent.ServerRequest) + assertFalse(event is WsEvent.RpcResult) + val request = event as WsEvent.ServerRequest + assertEquals("srq-123", request.id) + assertEquals("clarify", request.method) + assertEquals("session-1", request.params["session_id"]) + assertEquals("Continue?", request.params["question"]) + } + + @Test + fun testParseRequestCancel_notification_returnsCancellationEvent() { + val event = + EventParser.parse( + createJsonRpcResponse( + jsonrpc = "2.0", + id = null, + method = "event", + params = + mapOf( + "type" to "request.cancel", + "session_id" to "session-1", + "payload" to + mapOf( + "id" to "srq-123", + "method" to "clarify", + "reason" to "timeout", + ), + ), + ), + ) + + assertTrue(event is WsEvent.ServerRequestCancelled) + val cancelled = event as WsEvent.ServerRequestCancelled + assertEquals("srq-123", cancelled.id) + assertEquals("clarify", cancelled.method) + assertEquals("timeout", cancelled.reason) + assertEquals("session-1", cancelled.sessionId) + } + @Test fun testParseRpcError_returnsRpcErrorEvent() { val error = JsonRpcError(code = -32600, message = "Invalid Request") diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt index 72945a3e..dff70f1a 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt @@ -19,13 +19,18 @@ import io.mockk.verify import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job +import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout +import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive +import kotlinx.serialization.json.put import okhttp3.Response import okhttp3.WebSocket import okhttp3.WebSocketListener @@ -164,6 +169,106 @@ class HermesWsClientTest { assertTrue(msg.contains(id)) } + @Test + fun testOpenRequestsReplayUsesTheLiveServerRequestDispatcher() = + runBlocking { + mockWebServer.enqueue( + MockResponse().withWebSocketUpgrade( + object : WebSocketListener() { + override fun onOpen( + webSocket: WebSocket, + response: okhttp3.Response, + ) { + webSocket.send( + """{"jsonrpc":"2.0","id":"42","result":{"open_requests":[{"id":"srq-secret","method":"secret","params":{"session_id":"session-1","env_var":"API_KEY","prompt":"Enter API key"}}]}}""", + ) + } + }, + ), + ) + + val received = + async { + withTimeout(5_000) { + HermesWsClient.events.first { + it is WsEvent.ServerRequest && it.id == "srq-secret" + } + } + } + HermesWsClient.connect() + val event = received.await() as WsEvent.ServerRequest + + assertEquals("secret", event.method) + assertEquals("session-1", event.params["session_id"]) + assertEquals("API_KEY", event.params["env_var"]) + } + + @Test + fun testRespondToServerRequest_preservesIdAndOmitsMethod() { + val socket = mockk() + every { socket.send(any()) } returns true + every { socket.close(any(), any()) } returns true + val socketField = HermesWsClient::class.java.getDeclaredField("webSocket").apply { isAccessible = true } + val connectedField = HermesWsClient::class.java.getDeclaredField("connected").apply { isAccessible = true } + socketField.set(HermesWsClient, socket) + (connectedField.get(HermesWsClient) as java.util.concurrent.atomic.AtomicBoolean).set(true) + + val sent = + HermesWsClient.respondToServerRequest( + "srq-abc123", + buildJsonObject { put("value", "hello") }, + ) + + assertTrue(sent) + verify { + socket.send( + match { raw -> + val frame = Json.parseToJsonElement(raw).jsonObject + frame["jsonrpc"]?.jsonPrimitive?.content == "2.0" && + frame["id"]?.jsonPrimitive?.content == "srq-abc123" && + frame["result"] + ?.jsonObject + ?.get("value") + ?.jsonPrimitive + ?.content == "hello" && + !frame.containsKey("method") + }, + ) + } + } + + @Test + fun testRespondToServerRequestError_preservesIdAndErrorShape() { + val socket = mockk() + every { socket.send(any()) } returns true + every { socket.close(any(), any()) } returns true + val socketField = HermesWsClient::class.java.getDeclaredField("webSocket").apply { isAccessible = true } + val connectedField = HermesWsClient::class.java.getDeclaredField("connected").apply { isAccessible = true } + socketField.set(HermesWsClient, socket) + (connectedField.get(HermesWsClient) as java.util.concurrent.atomic.AtomicBoolean).set(true) + + assertTrue(HermesWsClient.respondToServerRequestError("srq-abc123", -32601, "unsupported")) + verify { + socket.send( + match { raw -> + val frame = Json.parseToJsonElement(raw).jsonObject + frame["id"]?.jsonPrimitive?.content == "srq-abc123" && + frame["error"] + ?.jsonObject + ?.get("code") + ?.jsonPrimitive + ?.content == "-32601" && + frame["error"] + ?.jsonObject + ?.get("message") + ?.jsonPrimitive + ?.content == "unsupported" && + !frame.containsKey("method") + }, + ) + } + } + @Test fun testFailedSocketSendQueuesMessageForReconnect() { val staleSocket = mockk(relaxed = true) diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt index 6623c4bf..bc797046 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatApprovalsDelegateTest.kt @@ -8,8 +8,12 @@ import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -196,4 +200,111 @@ class ChatApprovalsDelegateTest { delegate.handleApprovalRespondResult(mapOf("resolved" to 1)) assertEquals(listOf("Approval submitted"), systemMessages) } + + @Test + fun cancelServerRequest_matchesServerRequestIdOnly() { + delegate.handleApprovalRequest( + WsEvent.ApprovalRequest( + command = "rm -rf /tmp/x", + description = "dangerous", + patternKeys = null, + sessionId = "session-1", + requestId = "approval-1", + serverRequestId = "srq-approval-1", + ), + ) + + delegate.cancelServerRequest("srq-other") + assertNotNull( + uiState.value.messages + .last() + .approvalInfo, + ) + + delegate.cancelServerRequest("srq-approval-1") + assertNull( + uiState.value.messages + .last() + .approvalInfo, + ) + } + + @Test + fun liveServerApproval_upgradesMatchingPendingApprovalWithoutDuplicating() { + delegate.handleApprovalRequest( + WsEvent.ApprovalRequest( + command = "git status", + description = "inspect", + patternKeys = null, + sessionId = "session-1", + requestId = "approval-business-id", + ), + ) + delegate.handleApprovalRequest( + WsEvent.ApprovalRequest( + command = "git status", + description = "inspect", + patternKeys = null, + sessionId = "session-1", + requestId = "approval-business-id", + serverRequestId = "srq-approval", + ), + ) + + assertEquals(1, uiState.value.messages.size) + assertEquals( + "srq-approval", + uiState.value.messages + .single() + .approvalInfo + ?.serverRequestId, + ) + } + + @Test + fun serverApproval_usesChoiceAndAllResultPayload() { + val responses = mutableListOf>() + val serverDelegate = + ChatApprovalsDelegate( + scope = testScope, + ioDispatcher = testDispatcher, + uiState = uiState, + runtimeSessionId = { "session-1" }, + wsSend = { _, _, _ -> error("new approval must not use legacy response") }, + trackRequest = { _, _ -> }, + addSystemMessage = { }, + respondToServerRequest = { id, result -> responses += id to result }, + ) + serverDelegate.handleApprovalRequest( + WsEvent.ApprovalRequest( + command = "rm -rf /tmp/x", + description = "dangerous", + patternKeys = null, + sessionId = "session-1", + requestId = "approval-1", + serverRequestId = "srq-approval", + ), + ) + + serverDelegate.respondToApproval("approve") + testDispatcher.scheduler.advanceUntilIdle() + + assertEquals("srq-approval", responses.single().first) + assertEquals( + "once", + responses + .single() + .second.jsonObject["choice"] + ?.jsonPrimitive + ?.content, + ) + assertEquals( + "false", + responses + .single() + .second.jsonObject["all"] + ?.jsonPrimitive + ?.content, + ) + } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPromptProtocolTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPromptProtocolTest.kt new file mode 100644 index 00000000..55f61e9d --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPromptProtocolTest.kt @@ -0,0 +1,176 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.ws.WsEvent +import com.m57.hermescontrol.data.ws.WsMethods +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ChatPromptProtocolTest { + private val dispatcher = StandardTestDispatcher() + + @Test + fun clarifyServerResponse_mergesLockedAnswers_withoutEmptyOverwrite() { + val scope = TestScope(dispatcher) + val uiState = + MutableStateFlow( + ChatUiState( + currentSessionId = "session-1", + clarifyRequest = + ClarifyUi( + text = "", + questions = + listOf( + ClarifyQuestionUi(qid = "q0", question = "First?"), + ClarifyQuestionUi(qid = "q1", question = "Second?"), + ), + serverRequestId = "srq-clarify", + lockedAnswers = mapOf("q0" to "yes"), + ), + ), + ) + var responseId: String? = null + var response: JsonElement? = null + + val delegate = + ChatClarifyDelegate( + uiState = uiState, + scope = scope, + ioDispatcher = dispatcher, + persistMessage = { _, _ -> }, + respondToServerRequest = { id, result -> + responseId = id + response = result + }, + ) + + delegate.respondToClarifyBatch(mapOf("q0" to "", "q1" to "new answer")) + dispatcher.scheduler.advanceUntilIdle() + + assertEquals("srq-clarify", responseId) + val answers = response?.jsonObject?.get("answers")?.jsonObject + assertNotNull(answers) + assertEquals("yes", answers?.get("q0")?.jsonPrimitive?.content) + assertEquals("new answer", answers?.get("q1")?.jsonPrimitive?.content) + } + + @Test + fun credentialPrompt_serverRequestId_controls_response_and_cancellation() { + val scope = TestScope(dispatcher) + val uiState = MutableStateFlow(ChatUiState(currentSessionId = "session-1")) + val sentMethods = mutableListOf() + var responseId: String? = null + var response: JsonElement? = null + val delegate = + ChatCredentialPromptsDelegate( + scope = scope, + ioDispatcher = dispatcher, + uiState = uiState, + wsSend = { method, _, _ -> sentMethods += method }, + trackRequest = { _, _ -> }, + respondToServerRequest = { id, result -> + responseId = id + response = result + }, + ) + + delegate.handleSudoRequest( + WsEvent.SudoRequest( + requestId = "legacy-sudo-id", + sessionId = "session-1", + serverRequestId = "srq-sudo", + ), + ) + delegate.handleSudoExpire(WsEvent.SudoExpire(null, "session-1", serverRequestId = "srq-other")) + assertNotNull(uiState.value.sudoPrompt) + + delegate.respondToSudo("password") + dispatcher.scheduler.advanceUntilIdle() + assertEquals("srq-sudo", responseId) + assertEquals( + "password", + response + ?.jsonObject + ?.get("value") + ?.jsonPrimitive + ?.content, + ) + assertNull(uiState.value.sudoPrompt) + assertEquals(emptyList(), sentMethods) + + delegate.handleSudoRequest(WsEvent.SudoRequest("legacy-sudo-2", "session-1")) + delegate.respondToSudo("legacy-password") + dispatcher.scheduler.advanceUntilIdle() + assertEquals(listOf(WsMethods.SUDO_RESPOND), sentMethods) + } + + @Test + fun secretAndVaultServerRequests_useValueResultPayloads() { + val scope = TestScope(dispatcher) + val uiState = MutableStateFlow(ChatUiState(currentSessionId = "session-1")) + val responses = mutableListOf>() + val delegate = + ChatCredentialPromptsDelegate( + scope = scope, + ioDispatcher = dispatcher, + uiState = uiState, + wsSend = { _, _, _ -> error("new prompt must not use legacy response") }, + trackRequest = { _, _ -> }, + respondToServerRequest = { id, result -> responses += id to result }, + ) + + delegate.handleSecretRequest(WsEvent.SecretRequest(null, "session-1", serverRequestId = "srq-secret")) + delegate.respondToSecret("secret-value") + delegate.handleVaultCodeRequest(WsEvent.VaultCodeRequest(null, "session-1", serverRequestId = "srq-code")) + delegate.respondToVaultCode("123456") + delegate.handleVaultUnlockRequest( + WsEvent.VaultUnlockRequest(null, "session-1", serverRequestId = "srq-unlock"), + ) + delegate.respondToVaultUnlock("vault-password") + delegate.handleVaultSaveLoginRequest( + WsEvent.VaultSaveLoginRequest(null, "session-1", serverRequestId = "srq-save"), + ) + delegate.respondToVaultSaveLogin("alice", "login-password") + dispatcher.scheduler.advanceUntilIdle() + + assertEquals(listOf("srq-secret", "srq-code", "srq-unlock", "srq-save"), responses.map { it.first }) + assertEquals( + "secret-value", + responses[0] + .second.jsonObject["value"] + ?.jsonPrimitive + ?.content, + ) + assertEquals( + "123456", + responses[1] + .second.jsonObject["value"] + ?.jsonPrimitive + ?.content, + ) + assertEquals( + "vault-password", + responses[2] + .second.jsonObject["value"] + ?.jsonPrimitive + ?.content, + ) + assertEquals( + "{\"identifier\":\"alice\",\"password\":\"login-password\"}", + responses[3] + .second.jsonObject["value"] + ?.jsonPrimitive + ?.content, + ) + } +} 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 14c82d19..7b2d7811 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 @@ -49,6 +49,8 @@ import kotlinx.coroutines.test.setMain import kotlinx.coroutines.withTimeout import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive import kotlinx.serialization.json.put import org.junit.After import org.junit.Assert.assertArrayEquals @@ -188,6 +190,8 @@ class ChatViewModelTest { arg<((String) -> Unit)?>(2)?.invoke(id) id } + every { HermesWsClient.respondToServerRequest(any(), any()) } returns true + every { HermesWsClient.respondToServerRequestError(any(), any(), any()) } returns true // Stub model-options so preloadModelOptions() (fired at GatewayReady) is safe. val mockApi = mockk(relaxed = true) @@ -1494,6 +1498,100 @@ class ChatViewModelTest { } } + @Test + fun testServerRequestClarifyReplay_restoresLockedAnswersAndMergesRemainingAnswer() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + + mockEventsFlow.emit( + WsEvent.ServerRequest( + id = "srq-clarify-replay", + method = "clarify", + params = + mapOf( + "session_id" to sessionId, + "questions" to + listOf( + mapOf("qid" to "q0", "question" to "First?", "choices" to listOf("yes")), + mapOf("qid" to "q1", "question" to "Second?", "choices" to emptyList()), + ), + "answers" to mapOf("q0" to "yes", "invalid" to 42), + ), + replayed = true, + ), + ) + advanceUntilIdle() + + val clarify = viewModel.uiState.value.clarifyRequest + assertEquals("srq-clarify-replay", clarify?.serverRequestId) + assertEquals(mapOf("q0" to "yes"), clarify?.lockedAnswers) + assertEquals(2, clarify?.resolvedQuestions?.size) + + viewModel.respondToClarifyBatch(mapOf("q0" to "", "q1" to "new answer")) + advanceUntilIdle() + + verify { + HermesWsClient.respondToServerRequest( + "srq-clarify-replay", + withArg { result -> + val answers = result.jsonObject["answers"]?.jsonObject + assertEquals("yes", answers?.get("q0")?.jsonPrimitive?.content) + assertEquals("new answer", answers?.get("q1")?.jsonPrimitive?.content) + assertFalse(answers?.containsKey("invalid") == true) + }, + ) + } + } + + @Test + fun serverRequestCancel_clearsOnlyMatchingClarifyPrompt() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + mockEventsFlow.emit( + WsEvent.ServerRequest( + id = "srq-clarify-cancel", + method = "clarify", + params = + mapOf( + "session_id" to sessionId, + "question" to "Continue?", + ), + ), + ) + advanceUntilIdle() + assertEquals( + "srq-clarify-cancel", + viewModel.uiState.value.clarifyRequest + ?.serverRequestId, + ) + + mockEventsFlow.emit( + WsEvent.ServerRequestCancelled( + id = "srq-other", + method = "clarify", + reason = "timeout", + sessionId = sessionId, + ), + ) + advanceUntilIdle() + assertEquals( + "srq-clarify-cancel", + viewModel.uiState.value.clarifyRequest + ?.serverRequestId, + ) + + mockEventsFlow.emit( + WsEvent.ServerRequestCancelled( + id = "srq-clarify-cancel", + method = "clarify", + reason = "timeout", + sessionId = sessionId, + ), + ) + advanceUntilIdle() + assertNull(viewModel.uiState.value.clarifyRequest) + } + @Test fun testClarifyRequestCustomResponse() = runTest {