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 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
47258bd8b0
commit
71c1bdded9
2 files changed
+88
-33
No files matched your search
@@ -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 hasStdout = dataSource.has("stdout")
|
||||||
val hasStderr = dataSource.has("stderr")
|
val hasStderr = dataSource.has("stderr")
|
||||||
val hasExitCode = dataSource.has("exit_code") || dataSource.has("exitCode")
|
val hasExitCode = dataSource.has("exit_code") || dataSource.has("exitCode")
|
||||||
|
|
||||||
if (hasStdout || hasStderr || hasExitCode) {
|
if (hasOutput || hasStdout || hasStderr || hasExitCode) {
|
||||||
val stdout =
|
val terminalOutput =
|
||||||
dataSource
|
(
|
||||||
.get("stdout")
|
dataSource.get("output")?.takeIf { !it.isJsonNull }?.asString
|
||||||
?.takeIf { !it.isJsonNull }
|
?: listOfNotNull(
|
||||||
?.asString
|
dataSource.get("stdout")?.takeIf { !it.isJsonNull }?.asString?.takeIf { it.isNotEmpty() },
|
||||||
?.takeIf { it.isNotEmpty() }
|
dataSource.get("stderr")?.takeIf { !it.isJsonNull }?.asString?.takeIf { it.isNotEmpty() },
|
||||||
val stderr =
|
).joinToString("\n").takeIf { it.isNotEmpty() }
|
||||||
dataSource
|
)?.takeIf { it.isNotEmpty() }
|
||||||
.get("stderr")
|
val exitCode = (dataSource.get("exit_code") ?: dataSource.get("exitCode"))?.toIntOrNull()
|
||||||
?.takeIf { !it.isJsonNull }
|
|
||||||
?.asString
|
|
||||||
?.takeIf { it.isNotEmpty() }
|
|
||||||
val exitCode = (dataSource.get("exit_code") ?: dataSource.get("exitCode"))?.takeIf { !it.isJsonNull }?.asInt
|
|
||||||
val error =
|
val error =
|
||||||
dataSource
|
dataSource
|
||||||
.get("error")
|
.get("error")
|
||||||
?.takeIf { !it.isJsonNull }
|
?.takeIf { !it.isJsonNull }
|
||||||
?.asString
|
?.asString
|
||||||
?.takeIf { it.isNotEmpty() }
|
?.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
|
val duration = obj.get("duration_s")?.takeIf { !it.isJsonNull }?.asDouble
|
||||||
|
|
||||||
ParsedToolData(
|
ParsedToolData(
|
||||||
@@ -1675,11 +1684,12 @@ fun parseToolOutput(
|
|||||||
args = args,
|
args = args,
|
||||||
result = dataSource.entrySet().associate { it.key to it.value.toString() },
|
result = dataSource.entrySet().associate { it.key to it.value.toString() },
|
||||||
isTerminal = true,
|
isTerminal = true,
|
||||||
stdout = stdout,
|
stdout = terminalOutput,
|
||||||
stderr = stderr,
|
stderr = null,
|
||||||
exitCode = exitCode,
|
exitCode = exitCode,
|
||||||
error = error,
|
error = effectiveError,
|
||||||
summaryText = summaryText,
|
summaryText = summaryText,
|
||||||
|
mainOutput = terminalOutput,
|
||||||
durationSec = duration,
|
durationSec = duration,
|
||||||
isRunning = isRunning,
|
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 {
|
parsed.error?.let {
|
||||||
Text(
|
Text(
|
||||||
text = stringResource(R.string.chat_tool_execution_error, it),
|
text = stringResource(R.string.chat_tool_execution_error, it),
|
||||||
@@ -2319,3 +2318,19 @@ private fun JsonObject.entrySet(): Set<Map.Entry<String, JsonElement>> = entries
|
|||||||
private fun JsonObject.keySet(): Set<String> = keys
|
private fun JsonObject.keySet(): Set<String> = keys
|
||||||
|
|
||||||
private fun JsonArray.size(): Int = size
|
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
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -11,25 +11,65 @@ class ToolBubbleParsingTest {
|
|||||||
// ── Terminal ──────────────────────────────────────────────
|
// ── Terminal ──────────────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testParseTerminal_withStdout() {
|
fun testParseTerminal_withOutput() {
|
||||||
val json =
|
val json =
|
||||||
"""{
|
"""{
|
||||||
"tool_id": "call_001",
|
"tool_id": "call_001",
|
||||||
"name": "terminal",
|
"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"},
|
"args": {"command": "npm run build"},
|
||||||
"result": {"stdout": "Build succeeded!", "exit_code": 0},
|
"result": {"stdout": "Build succeeded!", "exit_code": 0},
|
||||||
"duration_s": 5.2
|
"duration_s": 5.2
|
||||||
}"""
|
}"""
|
||||||
val parsed = parseToolOutput(json, "terminal", false)
|
val parsed = parseToolOutput(json, "terminal", false)
|
||||||
assertNotNull(parsed)
|
assertNotNull(parsed)
|
||||||
assertEquals("terminal", parsed!!.toolName)
|
assertTrue(parsed!!.isTerminal)
|
||||||
assertTrue(parsed.isTerminal)
|
|
||||||
assertEquals("Build succeeded!", parsed.stdout)
|
assertEquals("Build succeeded!", parsed.stdout)
|
||||||
|
assertEquals("Build succeeded!", parsed.mainOutput)
|
||||||
assertEquals(0, parsed.exitCode)
|
assertEquals(0, parsed.exitCode)
|
||||||
assertEquals(5.2, parsed.durationSec!!, 0.01)
|
assertEquals(5.2, parsed.durationSec!!, 0.01)
|
||||||
assertEquals("$ npm run build", parsed.summaryText)
|
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
|
@Test
|
||||||
fun testParseTerminal_withStderr() {
|
fun testParseTerminal_withStderr() {
|
||||||
val json =
|
val json =
|
||||||
@@ -42,7 +82,7 @@ class ToolBubbleParsingTest {
|
|||||||
val parsed = parseToolOutput(json, "terminal", false)
|
val parsed = parseToolOutput(json, "terminal", false)
|
||||||
assertNotNull(parsed)
|
assertNotNull(parsed)
|
||||||
assertTrue(parsed!!.isTerminal)
|
assertTrue(parsed!!.isTerminal)
|
||||||
assertEquals("Permission denied", parsed.stderr)
|
assertEquals("Permission denied", parsed.stdout)
|
||||||
assertEquals(1, parsed.exitCode)
|
assertEquals(1, parsed.exitCode)
|
||||||
assertEquals("$ rm -rf /", parsed.summaryText)
|
assertEquals("$ rm -rf /", parsed.summaryText)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user