From 71c1bdded98df7d458c03659bd009027fb0211d2 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Tue, 14 Jul 2026 12:01:08 +0300 Subject: [PATCH] fix(#585): render terminal tool output via real backend payload keys (#595) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backend terminal tool (tools/terminal_tool.py) returns a single merged 'output' string plus 'exit_code' and 'error' — never 'stdout'/'stderr'. ChatBubble was reading the non-existent 'stdout' key, so command output was silently dropped and only the Duration footer showed. - Parse 'output' as canonical; keep legacy 'stdout'/'stderr' fallback for backward compat. - Route terminal output into mainOutput so it renders like other tool bubbles. - Surface non-zero exit codes as an error line when no 'error' string is set. - Remove dead 'stderr' render branch. - Update ToolBubbleParsingTest to assert the real payload shape (incl. the exact issue-585 'echo hellp' sample) and add regression cases. Co-authored-by: hy4ri --- .../m57/hermescontrol/ui/chat/ChatBubble.kt | 73 +++++++++++-------- .../ui/chat/ToolBubbleParsingTest.kt | 48 +++++++++++- 2 files changed, 88 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatBubble.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatBubble.kt index 09f0ae9f..430601c5 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatBubble.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatBubble.kt @@ -1643,31 +1643,40 @@ fun parseToolOutput( ) } - // Check if this looks like a terminal result (check dataSource for backward compat) + // Terminal result. The backend (tools/terminal_tool.py) emits a single + // `output` string (stdout+stderr merged, ANSI-stripped), plus `exit_code` + // and an `error` string on failure. Older payloads used `stdout`/`stderr` + // — accept both for backward compat, but `output` is canonical. + val hasOutput = dataSource.has("output") val hasStdout = dataSource.has("stdout") val hasStderr = dataSource.has("stderr") val hasExitCode = dataSource.has("exit_code") || dataSource.has("exitCode") - if (hasStdout || hasStderr || hasExitCode) { - val stdout = - dataSource - .get("stdout") - ?.takeIf { !it.isJsonNull } - ?.asString - ?.takeIf { it.isNotEmpty() } - val stderr = - dataSource - .get("stderr") - ?.takeIf { !it.isJsonNull } - ?.asString - ?.takeIf { it.isNotEmpty() } - val exitCode = (dataSource.get("exit_code") ?: dataSource.get("exitCode"))?.takeIf { !it.isJsonNull }?.asInt + if (hasOutput || hasStdout || hasStderr || hasExitCode) { + val terminalOutput = + ( + dataSource.get("output")?.takeIf { !it.isJsonNull }?.asString + ?: listOfNotNull( + dataSource.get("stdout")?.takeIf { !it.isJsonNull }?.asString?.takeIf { it.isNotEmpty() }, + dataSource.get("stderr")?.takeIf { !it.isJsonNull }?.asString?.takeIf { it.isNotEmpty() }, + ).joinToString("\n").takeIf { it.isNotEmpty() } + )?.takeIf { it.isNotEmpty() } + val exitCode = (dataSource.get("exit_code") ?: dataSource.get("exitCode"))?.toIntOrNull() val error = dataSource .get("error") ?.takeIf { !it.isJsonNull } ?.asString ?.takeIf { it.isNotEmpty() } + // Surface non-zero exit codes as an error line even when `error` is absent. + val effectiveError = + if (error != null) { + error + } else if (exitCode != null && exitCode != 0 && terminalOutput == null) { + "exit code $exitCode" + } else { + null + } val duration = obj.get("duration_s")?.takeIf { !it.isJsonNull }?.asDouble ParsedToolData( @@ -1675,11 +1684,12 @@ fun parseToolOutput( args = args, result = dataSource.entrySet().associate { it.key to it.value.toString() }, isTerminal = true, - stdout = stdout, - stderr = stderr, + stdout = terminalOutput, + stderr = null, exitCode = exitCode, - error = error, + error = effectiveError, summaryText = summaryText, + mainOutput = terminalOutput, durationSec = duration, isRunning = isRunning, ) @@ -1758,17 +1768,6 @@ private fun ExpandedToolContent( ), ) } - parsed.stderr?.let { - Text( - text = stringResource(R.string.chat_tool_stderr, it), - style = - MaterialTheme.typography.bodySmall.copy( - color = statusColors.error.copy(alpha = 0.9f), - fontFamily = FontFamily.Monospace, - fontSize = 11.sp, - ), - ) - } parsed.error?.let { Text( text = stringResource(R.string.chat_tool_execution_error, it), @@ -2319,3 +2318,19 @@ private fun JsonObject.entrySet(): Set> = entries private fun JsonObject.keySet(): Set = keys private fun JsonArray.size(): Int = size + +/** + * Parse a numeric/string JSON primitive into an Int, tolerating backend quirks + * (e.g. terminal `exit_code` arrives as a FLOAT `0.0` or a STRING). Returns null + * if missing, null, or non-numeric — callers must not throw, or the whole tool + * parse is caught and the bubble falls back to raw JSON. + */ +private fun JsonElement?.toIntOrNull(): Int? { + if (this == null || this is JsonNull) return null + return when (this) { + is JsonPrimitive -> { + content.toIntOrNull() ?: content.toDoubleOrNull()?.toInt() + } + else -> null + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ToolBubbleParsingTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ToolBubbleParsingTest.kt index 433fbebe..83545edc 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ToolBubbleParsingTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ToolBubbleParsingTest.kt @@ -11,25 +11,65 @@ class ToolBubbleParsingTest { // ── Terminal ────────────────────────────────────────────── @Test - fun testParseTerminal_withStdout() { + fun testParseTerminal_withOutput() { val json = """{ "tool_id": "call_001", "name": "terminal", + "args": {"command": "echo hellp"}, + "result": {"output": "hellp", "exit_code": 0, "error": null}, + "duration_s": 0.06 + }""" + val parsed = parseToolOutput(json, "terminal", false) + assertNotNull(parsed) + assertEquals("terminal", parsed!!.toolName) + assertTrue(parsed.isTerminal) + assertEquals("hellp", parsed.stdout) + assertEquals("hellp", parsed.mainOutput) + assertEquals(0, parsed.exitCode) + assertEquals(0.06, parsed.durationSec!!, 0.01) + assertEquals("$ echo hellp", parsed.summaryText) + } + + @Test + fun testParseTerminal_legacyStdout() { + val json = + """{ + "tool_id": "call_001b", + "name": "terminal", "args": {"command": "npm run build"}, "result": {"stdout": "Build succeeded!", "exit_code": 0}, "duration_s": 5.2 }""" val parsed = parseToolOutput(json, "terminal", false) assertNotNull(parsed) - assertEquals("terminal", parsed!!.toolName) - assertTrue(parsed.isTerminal) + assertTrue(parsed!!.isTerminal) assertEquals("Build succeeded!", parsed.stdout) + assertEquals("Build succeeded!", parsed.mainOutput) assertEquals(0, parsed.exitCode) assertEquals(5.2, parsed.durationSec!!, 0.01) assertEquals("$ npm run build", parsed.summaryText) } + @Test + fun testParseTerminal_floatExitCode_isNotDropped() { + // Backend emits exit_code as a FLOAT (0.0), not an int. asInt on a + // "0.0" primitive throws NumberFormatException, which previously + // nulled the whole parse -> raw JSON. Reproduces the live bug. + val json = + """{ + "tool_id": "call_001d", + "name": "terminal", + "args": {"command": "echo hi"}, + "result": {"output": "hi", "exit_code": 0.0, "error": null} + }""" + val parsed = parseToolOutput(json, "terminal", false) + assertNotNull("float exit_code must not null the parse", parsed) + assertTrue(parsed!!.isTerminal) + assertEquals("hi", parsed.stdout) + assertEquals(0, parsed.exitCode) + } + @Test fun testParseTerminal_withStderr() { val json = @@ -42,7 +82,7 @@ class ToolBubbleParsingTest { val parsed = parseToolOutput(json, "terminal", false) assertNotNull(parsed) assertTrue(parsed!!.isTerminal) - assertEquals("Permission denied", parsed.stderr) + assertEquals("Permission denied", parsed.stdout) assertEquals(1, parsed.exitCode) assertEquals("$ rm -rf /", parsed.summaryText) }