From fa17c95296d2750a923e5a31f6a6d66c90db9fac Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:50:13 +0300 Subject: [PATCH] fix(chat): support batch clarify.request payload and route question_id on respond (fixes #1045) (#1046) --- .../m57/hermescontrol/data/ws/EventParser.kt | 18 +++- .../com/m57/hermescontrol/data/ws/WsEvent.kt | 1 + .../hermescontrol/ui/chat/ChatViewModel.kt | 9 ++ .../ui/chat/ChatWsEventReducer.kt | 1 + .../hermescontrol/data/ws/EventParserTest.kt | 36 +++++++ .../ui/chat/ChatViewModelTest.kt | 95 +++++++++++++++++++ 6 files changed, 159 insertions(+), 1 deletion(-) 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 c85fe7ea..55195363 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 @@ -134,17 +134,33 @@ object EventParser { "clarify.request" -> { // Gateway sends "question"/"choices" — fall back to "text"/"options" for any // older client or test that still uses the legacy field names. (Issue #206) + // Batch clarify (issue #18450): gateway sends "questions" array of {qid, question, choices, multi_select}. + val rawQuestions = payload?.get("questions") as? List<*> + val firstQuestion = rawQuestions?.firstOrNull() as? Map<*, *> + val text = payload?.get("question") as? String ?: payload?.get("text") as? String + ?: if (rawQuestions != null && rawQuestions.size > 1) { + rawQuestions + .mapIndexedNotNull { index, item -> + val q = (item as? Map<*, *>)?.get("question") as? String + q?.let { "${index + 1}. $it" } + }.joinToString("\n\n") + } else { + firstQuestion?.get("question") as? String + } + val rawOptions = payload?.get("choices") ?: payload?.get("options") + ?: firstQuestion?.get("choices") val clarifyId = payload?.get("clarify_id") as? String ?: payload?.get("request_id") as? String + val questionId = firstQuestion?.get("qid") as? String @Suppress("UNCHECKED_CAST") val options = (rawOptions as? List<*>)?.filterIsInstance() - WsEvent.ClarifyRequest(text, options, clarifyId, sessionId) + WsEvent.ClarifyRequest(text, options, clarifyId, sessionId, questionId) } "status.update" -> { 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 4b0a4ab3..323040a3 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 @@ -140,6 +140,7 @@ sealed class WsEvent { val options: List?, val clarifyId: String? = null, val sessionId: String? = null, + val questionId: String? = null, ) : WsEvent() /** 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 c96f4924..6df12b52 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 @@ -379,6 +379,7 @@ data class ClarifyUi( val text: String, val options: List, val clarifyId: String? = null, + val questionId: String? = null, ) /** @@ -3236,6 +3237,7 @@ class ChatViewModel( fun dismissClarify() { val sessionId = _uiState.value.currentSessionId ?: return val clarifyId = _uiState.value.clarifyRequest?.clarifyId + val questionId = _uiState.value.clarifyRequest?.questionId _uiState.update { it.copy(clarifyRequest = null) } addSystemMessage("Clarify dismissed — no answer sent", persist = true) @@ -3251,6 +3253,9 @@ class ChatViewModel( params["clarify_id"] = clarifyId params["request_id"] = clarifyId } + if (questionId != null) { + params["question_id"] = questionId + } wsClient.send( method = WsMethods.CLARIFY_RESPOND, params = params, @@ -3262,6 +3267,7 @@ class ChatViewModel( fun respondToClarify(option: String) { val sessionId = _uiState.value.currentSessionId ?: return val clarifyId = _uiState.value.clarifyRequest?.clarifyId + val questionId = _uiState.value.clarifyRequest?.questionId _uiState.update { it.copy(clarifyRequest = null) } val userMessage = @@ -3292,6 +3298,9 @@ class ChatViewModel( params["clarify_id"] = clarifyId params["request_id"] = clarifyId } + if (questionId != null) { + params["question_id"] = questionId + } wsClient.send( method = WsMethods.CLARIFY_RESPOND, params = params, 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 1ce569d0..e9e1b4ce 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 @@ -633,6 +633,7 @@ object ChatWsEventReducer { text = event.text.orEmpty(), options = event.options.orEmpty(), clarifyId = event.clarifyId, + questionId = event.questionId, ), isAgentTyping = false, ), 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 dc922da5..675fdd8f 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 @@ -291,6 +291,42 @@ class EventParserTest { assertEquals(listOf("staging", "production"), clarifyEvent.options) } + @Test + fun testParseClarifyRequest_withBatchQuestions_parsesSuccessfully() { + val response = + createJsonRpcResponse( + jsonrpc = "2.0", + id = null, + result = null, + error = null, + method = "event", + params = + mapOf( + "type" to "clarify.request", + "payload" to + mapOf( + "request_id" to "req-batch-1", + "questions" to + listOf( + mapOf( + "qid" to "q0", + "question" to "Which database?", + "choices" to listOf("Postgres", "SQLite"), + "multi_select" to false, + ), + ), + ), + ), + ) + val event = EventParser.parse(response) + assertTrue(event is WsEvent.ClarifyRequest) + val clarifyEvent = event as WsEvent.ClarifyRequest + assertEquals("Which database?", clarifyEvent.text) + assertEquals(listOf("Postgres", "SQLite"), clarifyEvent.options) + assertEquals("req-batch-1", clarifyEvent.clarifyId) + assertEquals("q0", clarifyEvent.questionId) + } + // ── TEST-07: Untested subtypes ───────────────────────────────────── @Test 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 0382f8d4..3df66a2b 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 @@ -1217,6 +1217,54 @@ class ChatViewModelTest { } } + @Test + fun testClarifyRequestWithQuestionIdAndRespond() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + + mockEventsFlow.emit( + WsEvent.ClarifyRequest( + text = "Pick one:", + options = listOf("Option 1"), + clarifyId = "clarify-batch-1", + sessionId = sessionId, + questionId = "q0", + ), + ) + advanceUntilIdle() + + assertEquals( + "Pick one:", + viewModel.uiState.value.clarifyRequest + ?.text, + ) + assertEquals( + "q0", + viewModel.uiState.value.clarifyRequest + ?.questionId, + ) + + viewModel.respondToClarify("Option 1") + advanceUntilIdle() + + assertNull(viewModel.uiState.value.clarifyRequest) + verify { + HermesWsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = + mapOf( + "session_id" to sessionId, + "response" to "Option 1", + "answer" to "Option 1", + "clarify_id" to "clarify-batch-1", + "request_id" to "clarify-batch-1", + "question_id" to "q0", + ), + onSent = any(), + ) + } + } + @Test fun testClarifyRequestCustomResponse() = runTest { @@ -1316,6 +1364,53 @@ class ChatViewModelTest { } } + @Test + fun testClarifyDismissWithQuestionIdInformsAgent() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + + mockEventsFlow.emit( + WsEvent.ClarifyRequest( + "Please choose:", + listOf("Yes", "No"), + "clarify-789", + sessionId, + questionId = "q0", + ), + ) + advanceUntilIdle() + assertEquals( + "clarify-789", + viewModel.uiState.value.clarifyRequest + ?.clarifyId, + ) + assertEquals( + "q0", + viewModel.uiState.value.clarifyRequest + ?.questionId, + ) + + viewModel.dismissClarify() + advanceUntilIdle() + + assertNull(viewModel.uiState.value.clarifyRequest) + verify { + HermesWsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = + mapOf( + "session_id" to sessionId, + "response" to "The user cancelled — no answer provided.", + "answer" to "The user cancelled — no answer provided.", + "clarify_id" to "clarify-789", + "request_id" to "clarify-789", + "question_id" to "q0", + ), + onSent = any(), + ) + } + } + // ── Attachments ────────────────────────────────────────────────────────── /** Add [count] dummy attachments so a test starts with a populated list. */