fix: clarify Dismiss now informs the agent instead of silently dropping (#580)
* fix(#572): clarify Dismiss now informs agent instead of silently dropping * fix(#572): clarify Dismiss rejects the question (no answer) instead of telling agent to proceed --------- Co-authored-by: M57 <m57@hermes-control.app>
This commit is contained in:
1 parent
109fb1a04d
commit
e48778291a
2 files changed
+94
No files matched your search
@@ -105,6 +105,17 @@ data class ClarifyUi(
|
||||
val clarifyId: String? = null,
|
||||
)
|
||||
|
||||
/**
|
||||
* String sent to the agent when a clarify prompt is dismissed (the Dismiss
|
||||
* button). This is a *reject* — "I'm not answering this question" — NOT an
|
||||
* instruction to proceed. Deliberately NOT the CLI's interrupt sentinel
|
||||
* ("...Use your best judgement to proceed."): a mobile Dismiss is a
|
||||
* skip-the-question gesture, not an interrupt of the whole turn. The agent is
|
||||
* unblocked but told no answer was given, so it re-asks or backs off rather
|
||||
* than charging ahead.
|
||||
*/
|
||||
private const val CLARIFY_DISMISS_RESPONSE = "The user cancelled — no answer provided."
|
||||
|
||||
/** Transient — not persisted. Holds a pending sudo.password request. */
|
||||
data class SudoPromptUi(
|
||||
val requestId: String?,
|
||||
@@ -1225,8 +1236,47 @@ class ChatViewModel(
|
||||
_uiState.update { it.copy(showSessionPicker = !it.showSessionPicker) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Dismiss the active clarify prompt and reject it (tell the agent no answer
|
||||
* was given).
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* Unlike [respondToClarify] we do NOT append a user chat bubble: a dismiss
|
||||
* is not something the user typed, so faking a USER message would be
|
||||
* dishonest. We instead surface a short SYSTEM note so the dismissal is
|
||||
* visible in the transcript.
|
||||
*/
|
||||
fun dismissClarify() {
|
||||
val sessionId = _uiState.value.currentSessionId ?: return
|
||||
val clarifyId = _uiState.value.clarifyRequest?.clarifyId
|
||||
_uiState.update { it.copy(clarifyRequest = null) }
|
||||
|
||||
addSystemMessage("Clarify dismissed — no answer sent", persist = true)
|
||||
|
||||
viewModelScope.launch(Dispatchers.IO) {
|
||||
val params =
|
||||
mutableMapOf<String, Any>(
|
||||
"session_id" to sessionId,
|
||||
"response" to CLARIFY_DISMISS_RESPONSE,
|
||||
"answer" to CLARIFY_DISMISS_RESPONSE,
|
||||
)
|
||||
if (clarifyId != null) {
|
||||
params["clarify_id"] = clarifyId
|
||||
params["request_id"] = clarifyId
|
||||
}
|
||||
wsClient.send(
|
||||
method = WsMethods.CLARIFY_RESPOND,
|
||||
params = params,
|
||||
onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) },
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
fun respondToClarify(option: String) {
|
||||
|
||||
@@ -766,6 +766,50 @@ class ChatViewModelTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testClarifyDismissInformsAgent() =
|
||||
runTest {
|
||||
val (viewModel, sessionId) = createViewModelWithSession()
|
||||
|
||||
mockEventsFlow.emit(
|
||||
WsEvent.ClarifyRequest(
|
||||
"Please choose:",
|
||||
listOf("Yes", "No"),
|
||||
"clarify-789",
|
||||
),
|
||||
)
|
||||
advanceUntilIdle()
|
||||
assertEquals("clarify-789", viewModel.uiState.value.clarifyRequest?.clarifyId)
|
||||
|
||||
viewModel.dismissClarify()
|
||||
advanceUntilIdle()
|
||||
|
||||
// Dialog dismissed locally
|
||||
assertNull(viewModel.uiState.value.clarifyRequest)
|
||||
// Baseline has 1 "Connected" system message; dismiss adds exactly
|
||||
// ONE system note and must NOT fake a user bubble.
|
||||
val messages = viewModel.uiState.value.messages
|
||||
assertEquals(2, messages.size)
|
||||
assertEquals(MessageRole.SYSTEM, messages[0].role) // pre-existing "Connected"
|
||||
assertEquals(MessageRole.SYSTEM, messages[1].role) // dismiss trace
|
||||
assertTrue(messages[1].content.contains("dismissed", ignoreCase = true))
|
||||
|
||||
verify {
|
||||
HermesWsClient.send(
|
||||
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",
|
||||
),
|
||||
onSent = any(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Attachments ──────────────────────────────────────────────────────────
|
||||
|
||||
/** Add [count] dummy attachments so a test starts with a populated list. */
|
||||
|
||||
Reference in new issue
Block a user