fix(#1173): correct skill_manage result rendering (#1177)

This commit is contained in:
M57 authored and GitHub committed 2026-09-17 18:36:27 +03:00
1 parent e0688535dd
commit a567c91132
2 files changed
+75 -6

No files matched your search

@@ -105,14 +105,12 @@ internal object SkillManageRenderer : ToolRenderer {
val msg = ToolJson.firstString(call.result, listOf("message"))
val failed = ToolJson.boolFalse(call.result?.get("success"))
val succeeded = ToolJson.boolTrue(call.result?.get("success"))
// Pinned quirk from the original engine: the branch order is inverted
// (a non-false `success` renders the failure line). Preserved verbatim
// by the characterization suite; fix upstream deliberately if desired.
return when {
!failed -> "❌ Skill operation failed"
msg.isNotEmpty() -> "✅ $msg"
else -> "✅ Skill operation done"
failed -> "❌ ${msg.ifEmpty { "Skill operation failed" }}"
succeeded -> "✅ ${msg.ifEmpty { "Skill operation done" }}"
else -> "❌ ${msg.ifEmpty { "Skill operation failed" }}"
}
}
}
@@ -1,5 +1,6 @@
package com.m57.hermescontrol.ui.chat.tool
import com.m57.hermescontrol.ui.chat.tool.render.SkillManageRenderer
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonElement
import kotlinx.serialization.json.JsonPrimitive
@@ -655,6 +656,76 @@ class ToolViewCharacterizationTest {
assertEquals("${"x".repeat(500)}\n... [120 more chars]", view.detail)
}
@Test
fun `skill_manage success renders a success detail and status`() {
val view =
build(
"skill_manage",
"""{"action":"create","name":"summarize"}""",
"""{"success":true,"message":"Skill 'summarize' created."}""",
)
assertEquals(ToolViewStatus.SUCCESS, view.status)
assertEquals("✅ Skill 'summarize' created.", view.detail)
}
@Test
fun `skill_manage explicit failure renders a failure detail`() {
val call =
ToolCall.of(
"skill_manage",
Json.parseToJsonElement("""{"action":"create","name":"summarize"}"""),
Json.parseToJsonElement("""{"success":false,"message":"Permission denied"}"""),
)
assertEquals("❌ Permission denied", SkillManageRenderer.detail(call))
}
@Test
fun `skill_manage failure status and detail agree`() {
val view =
build(
"skill_manage",
"""{"action":"create","name":"summarize"}""",
"""{"success":false,"message":"Permission denied"}""",
)
assertEquals(ToolViewStatus.ERROR, view.status)
assertTrue(view.detail.contains("❌ Permission denied"))
assertTrue(!view.detail.contains("✅"))
}
@Test
fun `skill_manage error field remains the failure detail`() {
val call =
ToolCall.of(
"skill_manage",
Json.parseToJsonElement("""{"action":"create","name":"summarize"}"""),
Json.parseToJsonElement("""{"success":false,"error":"Permission denied"}"""),
)
assertEquals("❌ Permission denied", SkillManageRenderer.detail(call))
}
@Test
fun `skill_manage missing or invalid success fails closed`() {
val missing =
ToolCall.of(
"skill_manage",
null,
Json.parseToJsonElement("""{"message":"unexpected payload"}"""),
)
val invalid =
ToolCall.of(
"skill_manage",
null,
Json.parseToJsonElement("""{"success":"true","message":"unexpected payload"}"""),
)
assertEquals("❌ unexpected payload", SkillManageRenderer.detail(missing))
assertEquals("❌ unexpected payload", SkillManageRenderer.detail(invalid))
}
@Test
fun `tool_search shows match count and entries`() {
val view =