From 1ab30f1e90a72774756c5bb7f3fdb1def7c34503 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:51:04 +0300 Subject: [PATCH] fix(#1174): preserve pending process state and match tool events by ID (#1180) --- .../m57/hermescontrol/data/ws/EventParser.kt | 6 +- .../com/m57/hermescontrol/data/ws/WsEvent.kt | 6 +- .../ui/chat/ChatWsEventReducer.kt | 70 ++++++------- .../ui/chat/tool/ToolRenderer.kt | 3 + .../ui/chat/tool/ToolViewBuilder.kt | 8 +- .../ui/chat/tool/render/TaskRenderers.kt | 5 + .../hermescontrol/data/ws/EventParserTest.kt | 11 ++- .../ui/chat/ChatWsEventReducerTest.kt | 97 +++++++++++++++++++ .../ui/chat/tool/ToolViewBuilderTest.kt | 15 +++ 9 files changed, 179 insertions(+), 42 deletions(-) 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 c278711e..14c064ce 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 @@ -128,14 +128,16 @@ object EventParser { } "tool.progress" -> { + val toolId = payload?.get("tool_id") as? String val name = payload?.get("name") as? String val preview = payload?.get("preview") as? String - WsEvent.ToolProgress(name, preview, sessionId) + WsEvent.ToolProgress(name, preview, sessionId, toolId) } "tool.generating" -> { + val toolId = payload?.get("tool_id") as? String val name = payload?.get("name") as? String - WsEvent.ToolGenerating(name, sessionId) + WsEvent.ToolGenerating(name, sessionId, toolId) } "subagent.spawn_requested", "subagent.start", "subagent.progress", "subagent.complete" -> { 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 237e4558..cea239f9 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 @@ -104,23 +104,25 @@ sealed class WsEvent { * Live tool execution progress with optional preview content. * * Events: `tool.progress` - * Payload: `{ name?: string, preview?: string }` + * Payload: `{ tool_id?: string, name?: string, preview?: string }` */ data class ToolProgress( val name: String? = null, val preview: String? = null, val sessionId: String? = null, + val toolId: String? = null, ) : WsEvent() /** * Tool generation active state. * * Events: `tool.generating` - * Payload: `{ name?: string }` + * Payload: `{ tool_id?: string, name?: string }` */ data class ToolGenerating( val name: String? = null, val sessionId: String? = null, + val toolId: String? = null, ) : WsEvent() /** 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 5b2f047b..1fe44662 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 @@ -639,12 +639,7 @@ object ChatWsEventReducer { OkHttpProvider.json.encodeToString(it.toJsonElement()) } ?: "" val messages = state.messages.toMutableList() - val toolIdx = - messages.indexOfLast { - it.role == MessageRole.TOOL && - it.toolName == event.name && - it.toolStatus == ToolStatus.RUNNING - } + val toolIdx = findToolIndex(messages, event.name, event.data?.get("tool_id") as? String, ToolStatus.RUNNING) if (toolIdx < 0) return ReducerResult(state = state, streamingState = streamingState) val updated = @@ -675,10 +670,10 @@ object ChatWsEventReducer { /** * Attaches [ToolOutputRiskData] to the matching tool message. * - * Finds the last RUNNING tool message with matching [WsEvent.ToolOutputRisk.name]. - * If no RUNNING message exists (tool already completed), falls back to the last - * COMPLETED tool with that name. This covers the case where the risk event arrives - * after tool.complete. + * Finds the matching stable tool ID first. If the event has no ID or no ID + * match exists, it falls back to the tool name for legacy payloads. It + * prefers RUNNING tools and then falls back to COMPLETED tools because the + * risk event may arrive after tool.complete. */ private fun onToolOutputRisk( state: ChatUiState, @@ -694,20 +689,11 @@ object ChatWsEventReducer { val messages = state.messages.toMutableList() - // Prefer RUNNING tool, fall back to COMPLETED - var toolIdx = - messages.indexOfLast { - it.role == MessageRole.TOOL && - it.toolName == event.name && - it.toolStatus == ToolStatus.RUNNING - } + // Prefer RUNNING tool, fall back to COMPLETED. Stable tool IDs win over + // name matching so overlapping calls to the same tool stay isolated. + var toolIdx = findToolIndex(messages, event.name, event.toolId, ToolStatus.RUNNING) if (toolIdx < 0) { - toolIdx = - messages.indexOfLast { - it.role == MessageRole.TOOL && - it.toolName == event.name && - it.toolStatus == ToolStatus.COMPLETED - } + toolIdx = findToolIndex(messages, event.name, event.toolId, ToolStatus.COMPLETED) } if (toolIdx < 0) return ReducerResult(state = state, streamingState = streamingState) @@ -904,12 +890,7 @@ object ChatWsEventReducer { event: WsEvent.ToolProgress, ): ReducerResult { val messages = state.messages.toMutableList() - val toolIdx = - messages.indexOfLast { - it.role == MessageRole.TOOL && - it.toolName == event.name && - it.toolStatus == ToolStatus.RUNNING - } + val toolIdx = findToolIndex(messages, event.name, event.toolId, ToolStatus.RUNNING) if (toolIdx < 0) return ReducerResult(state = state, streamingState = streamingState) messages[toolIdx] = messages[toolIdx].copy(progressPreview = event.preview ?: "") @@ -925,12 +906,7 @@ object ChatWsEventReducer { event: WsEvent.ToolGenerating, ): ReducerResult { val messages = state.messages.toMutableList() - val toolIdx = - messages.indexOfLast { - it.role == MessageRole.TOOL && - it.toolName == event.name && - it.toolStatus == ToolStatus.RUNNING - } + val toolIdx = findToolIndex(messages, event.name, event.toolId, ToolStatus.RUNNING) if (toolIdx < 0) return ReducerResult(state = state, streamingState = streamingState) messages[toolIdx] = messages[toolIdx].copy(progressPreview = "") @@ -940,6 +916,30 @@ object ChatWsEventReducer { ) } + /** Match stable tool IDs first, with name matching for legacy events. */ + private fun findToolIndex( + messages: List, + name: String?, + toolId: String?, + status: ToolStatus, + ): Int { + if (!toolId.isNullOrBlank()) { + val byId = + messages.indexOfLast { + it.role == MessageRole.TOOL && + it.toolCallId == toolId && + it.toolStatus == status + } + if (byId >= 0) return byId + } + + return messages.indexOfLast { + it.role == MessageRole.TOOL && + it.toolName == name && + it.toolStatus == status + } + } + private fun onSubagentEvent( state: ChatUiState, streamingState: StreamingState, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolRenderer.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolRenderer.kt index ebc7b4c1..09a23609 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolRenderer.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolRenderer.kt @@ -25,6 +25,9 @@ internal interface ToolRenderer { /** Expanded body text. */ fun detail(call: ToolCall): String? = null + /** Expanded body text while the call is still running. */ + fun pendingDetail(call: ToolCall): String? = null + /** Structured extras (streams, diffs, search hits) for special row layouts. */ fun extras( call: ToolCall, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilder.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilder.kt index 14ae55fe..d4b436fe 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilder.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilder.kt @@ -64,7 +64,13 @@ object ToolViewBuilder { val detailBody = ToolJson.stripDividerLines( - renderer.detail(call) ?: ToolJson.fallbackDetailText(call.rawArgs, call.rawResult), + if (running) { + renderer.pendingDetail(call) + ?: renderer.detail(call) + ?: ToolJson.fallbackDetailText(call.rawArgs, call.rawResult) + } else { + renderer.detail(call) ?: ToolJson.fallbackDetailText(call.rawArgs, call.rawResult) + }, ) val detail = if (error.isNotEmpty()) { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/render/TaskRenderers.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/render/TaskRenderers.kt index e8311ea7..b56eab28 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/render/TaskRenderers.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/tool/render/TaskRenderers.kt @@ -221,6 +221,11 @@ internal object ProcessRenderer : ToolRenderer { return if (procId.isNotEmpty()) "$action: $procId" else action } + override fun pendingDetail(call: ToolCall): String { + val action = ToolJson.firstString(call.args, listOf("action")) + return "⏳ ${action.ifEmpty { "Process operation" }} in progress" + } + override fun detail(call: ToolCall): String { val result = call.result val error = ToolJson.firstString(result, listOf("error")) 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 5f79c483..70a8f0cf 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 @@ -980,7 +980,12 @@ class EventParserTest { mapOf( "type" to "tool.progress", "session_id" to "sess-123", - "payload" to mapOf("name" to "web_search", "preview" to "downloading content..."), + "payload" to + mapOf( + "tool_id" to "call-progress", + "name" to "web_search", + "preview" to "downloading content...", + ), ), ) val event = EventParser.parse(response) @@ -989,6 +994,7 @@ class EventParserTest { assertEquals("web_search", toolProgress.name) assertEquals("downloading content...", toolProgress.preview) assertEquals("sess-123", toolProgress.sessionId) + assertEquals("call-progress", toolProgress.toolId) } @Test @@ -1004,7 +1010,7 @@ class EventParserTest { mapOf( "type" to "tool.generating", "session_id" to "sess-123", - "payload" to mapOf("name" to "code_writer"), + "payload" to mapOf("tool_id" to "call-generating", "name" to "code_writer"), ), ) val event = EventParser.parse(response) @@ -1012,6 +1018,7 @@ class EventParserTest { val toolGenerating = event as WsEvent.ToolGenerating assertEquals("code_writer", toolGenerating.name) assertEquals("sess-123", toolGenerating.sessionId) + assertEquals("call-generating", toolGenerating.toolId) } @Test diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt index 7faa6810..2b9543c1 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt @@ -509,6 +509,103 @@ class ChatWsEventReducerTest { assertTrue(result.state.todos[1].isInProgress) } + @Test + fun testToolLifecycle_sameNameUsesToolIdForProgressAndCompletion() { + val state = ChatUiState(currentSessionId = "session-1") + val first = + ChatWsEventReducer.reduce( + state, + StreamingState(), + WsEvent.ToolStart( + name = "process_manage", + data = mapOf("tool_id" to "call-1", "action" to "wait"), + sessionId = "session-1", + ), + "session-1", + ) + val second = + ChatWsEventReducer.reduce( + first.state, + first.streamingState, + WsEvent.ToolStart( + name = "process_manage", + data = mapOf("tool_id" to "call-2", "action" to "wait"), + sessionId = "session-1", + ), + "session-1", + ) + + val progress = + ChatWsEventReducer.reduce( + second.state, + second.streamingState, + WsEvent.ToolProgress( + name = "process_manage", + preview = "call-1 is still waiting", + sessionId = "session-1", + toolId = "call-1", + ), + "session-1", + ) + assertEquals("call-1 is still waiting", progress.state.messages[0].progressPreview) + assertNull(progress.state.messages[1].progressPreview) + + val completed = + ChatWsEventReducer.reduce( + progress.state, + progress.streamingState, + WsEvent.ToolComplete( + name = "process_manage", + data = mapOf("tool_id" to "call-1", "output" to "finished"), + sessionId = "session-1", + ), + "session-1", + ) + assertEquals(ToolStatus.COMPLETED, completed.state.messages[0].toolStatus) + assertEquals(ToolStatus.RUNNING, completed.state.messages[1].toolStatus) + } + + @Test + fun testToolOutputRisk_sameNameUsesToolId() { + val state = ChatUiState(currentSessionId = "session-1") + val first = + ChatWsEventReducer.reduce( + state, + StreamingState(), + WsEvent.ToolStart("terminal", mapOf("tool_id" to "call-1"), "session-1"), + "session-1", + ) + val second = + ChatWsEventReducer.reduce( + first.state, + first.streamingState, + WsEvent.ToolStart("terminal", mapOf("tool_id" to "call-2"), "session-1"), + "session-1", + ) + + val result = + ChatWsEventReducer.reduce( + second.state, + second.streamingState, + WsEvent.ToolOutputRisk( + toolId = "call-1", + name = "terminal", + risk = "high", + findings = listOf("secret"), + sessionId = "session-1", + ), + "session-1", + ) + + assertEquals( + "high", + result.state.messages[0] + .toolOutputRiskData + ?.risk, + ) + assertNull(result.state.messages[1].toolOutputRiskData) + } + @Test fun testHydrateTodosFromMessages_parsesStoredToolMessage() { val todoMessage = diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilderTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilderTest.kt index 7b408a4f..58274d0b 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilderTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/tool/ToolViewBuilderTest.kt @@ -281,6 +281,21 @@ class ToolViewBuilderTest { assertTrue(view.detail.contains("sleep 100")) } + @Test + fun `running process_manage shows pending detail instead of completion`() { + val view = + build( + "process_manage", + """{"action":"wait","session_id":"proc-1"}""", + """{"tool_id":"call-1","name":"process_manage","args":{"action":"wait"}}""", + running = true, + ) + + assertEquals(ToolViewStatus.RUNNING, view.status) + assertEquals("⏳ wait in progress", view.detail) + assertFalse(view.detail.contains("done")) + } + @Test fun `todo_list alias activates TodoRenderer`() { val view =