Clarify subagent coordination cards

This commit is contained in:
iris committed 2026-09-13 02:48:21 -04:00
1 parent cad0cbcfbe
commit 898e6b92d0
5 files changed
+152 -21

No files matched your search

+7
View File
@@ -33,6 +33,13 @@ does not repeat the completed item's `delivery` field, so the translator
remembers that field from `item/started` and suppresses those deltas. Letting remembers that field from `item/started` and suppresses those deltas. Letting
one into the recipient's provisional assistant row makes its next completed one into the recipient's provisional assistant row makes its next completed
message replace the combined row, visibly erasing text that Codex still has. message replace the combined row, visibly erasing text that Codex still has.
The parent draws the initial `spawnAgent` as its ordinary `Task` card and
closes it when the matching `subAgentActivity.started` arrives. The remaining
collaboration calls remain visible as coordination -- waiting, messaging,
listing and lifecycle controls -- rather than being mistaken for generic task
output. Null optional fields and a bare `completed` status carry no information
and are omitted; their useful result is the child transcript, status or peer
message beside them.
## Storage ## Storage
@@ -68,12 +68,14 @@ private val SUBJECTS: Map<String, Pair<String, Language?>> =
private val DESCRIPTIONS = listOf("description", "prompt") private val DESCRIPTIONS = listOf("description", "prompt")
fun parseToolInput(tool: String, input: String): ToolInput { fun parseToolInput(tool: String, input: String): ToolInput {
if (input.trim() == "null") return ToolInput(null, null, null, null, emptyList())
val json = val json =
try { try {
JSONObject(input) JSONObject(input)
} catch (_: org.json.JSONException) { } catch (_: org.json.JSONException) {
// Not an object: older transcripts and some tools send a bare string. It is still the // Not an object: older transcripts and some tools send a bare string. It is still the
// input, so it is still shown. // input, so it is still shown. JSON null is the one exception: it means the call had
// no input, and drawing the word makes an absent value look like an instruction.
return ToolInput( return ToolInput(
null, null,
null, null,
@@ -85,17 +87,18 @@ fun parseToolInput(tool: String, input: String): ToolInput {
val (subjectKey, language) = SUBJECTS[tool] ?: (null to null) val (subjectKey, language) = SUBJECTS[tool] ?: (null to null)
val subject = val subject =
subjectKey subjectKey
?.let { json.optString(it) } ?.let { json.text(it) }
?.takeIf { it.isNotBlank() } ?.takeIf { it.isNotBlank() }
?.let { if (tool == "Bash") renderedBashScript(it) ?: it else it } ?.let { if (tool == "Bash") renderedBashScript(it) ?: it else it }
val description = DESCRIPTIONS.firstNotNullOfOrNull { val description = DESCRIPTIONS.firstNotNullOfOrNull {
json.optString(it).takeIf { v -> v.isNotBlank() } json.text(it)?.takeIf { value -> value.isNotBlank() }
} }
val timeout = json.optString("timeout").takeIf { it.isNotBlank() }?.let { formatMillisText(it) } val timeout = json.text("timeout")?.takeIf { it.isNotBlank() }?.let { formatMillisText(it) }
val rest = val rest =
json json
.keys() .keys()
.asSequence() .asSequence()
.filterNot(json::isNull)
.filter { it != subjectKey || subject == null } .filter { it != subjectKey || subject == null }
.filter { it !in DESCRIPTIONS || description == null } .filter { it !in DESCRIPTIONS || description == null }
.filter { it != "timeout" || timeout == null } .filter { it != "timeout" || timeout == null }
@@ -105,6 +108,9 @@ fun parseToolInput(tool: String, input: String): ToolInput {
return ToolInput(subject, language, description, timeout, rest) return ToolInput(subject, language, description, timeout, rest)
} }
private fun JSONObject.text(key: String): String? =
if (isNull(key)) null else optString(key).takeIf { it.isNotEmpty() }
/** /**
* Removes Codex's rendered Bash argv from old transcript rows. * Removes Codex's rendered Bash argv from old transcript rows.
* *
@@ -295,12 +295,14 @@ fun ToolCard(
shape: Shape = CardDefaults.shape, shape: Shape = CardDefaults.shape,
) { ) {
val parsed = remember(tool.tool, tool.input) { parseToolInput(tool.tool, tool.input) } val parsed = remember(tool.tool, tool.input) { parseToolInput(tool.tool, tool.input) }
val name = toolDisplayName(tool.tool)
val output = toolDisplayOutput(tool.tool, tool.output)
val deciding = tool.asks.any { it.answers.isEmpty() } val deciding = tool.asks.any { it.answers.isEmpty() }
val open = expanded || deciding val open = expanded || deciding
Card(Modifier.fillMaxWidth().clickable(onClick = onToggle), shape = shape) { Card(Modifier.fillMaxWidth().clickable(onClick = onToggle), shape = shape) {
Column(Modifier.padding(GROUP_INSET_LARGE)) { Column(Modifier.padding(GROUP_INSET_LARGE)) {
Row(verticalAlignment = Alignment.CenterVertically) { Row(verticalAlignment = Alignment.CenterVertically) {
Text(tool.tool, style = MaterialTheme.typography.titleSmall) Text(name, style = MaterialTheme.typography.titleSmall)
if (open) { if (open) {
Spacer(Modifier.weight(1f)) Spacer(Modifier.weight(1f))
parsed.timeout?.let { parsed.timeout?.let {
@@ -355,7 +357,7 @@ fun ToolCard(
if (tool.tool != ASK_USER_QUESTION) { if (tool.tool != ASK_USER_QUESTION) {
ToolInputView(tool.tool, tool.input, Modifier.padding(top = 4.dp)) ToolInputView(tool.tool, tool.input, Modifier.padding(top = 4.dp))
} }
if (tool.output.isNotEmpty()) { if (output.isNotEmpty()) {
Spacer(Modifier.height(8.dp)) Spacer(Modifier.height(8.dp))
Text("Output", style = MaterialTheme.typography.labelSmall) Text("Output", style = MaterialTheme.typography.labelSmall)
// What the tool printed, on the surface everything verbatim gets and in the // What the tool printed, on the surface everything verbatim gets and in the
@@ -367,7 +369,7 @@ fun ToolCard(
// often the whole of what a diff or a test run is saying. Remembered against // often the whole of what a diff or a test run is saying. Remembered against
// the text, so a card that is open through a scroll parses once. // the text, so a card that is open through a scroll parses once.
val palette = remember { ansiPalette() } val palette = remember { ansiPalette() }
val styled = remember(tool.output, palette) { ansiStyled(tool.output, palette) } val styled = remember(output, palette) { ansiStyled(output, palette) }
RawBlock(Modifier.padding(top = 2.dp)) { RawBlock(Modifier.padding(top = 2.dp)) {
Text( Text(
styled, styled,
@@ -391,6 +393,22 @@ fun ToolCard(
} }
} }
private val collaborationToolNames =
mapOf(
"Task" to "Spawn agent",
"TaskOutput" to "Wait for agents",
"SendMessage" to "Message agent",
"CloseAgent" to "Close agent",
"InterruptAgent" to "Interrupt agent",
"ListAgents" to "List agents",
"ResumeAgent" to "Resume agent",
)
internal fun toolDisplayName(tool: String): String = collaborationToolNames[tool] ?: tool
internal fun toolDisplayOutput(tool: String, output: String): String =
if (tool in collaborationToolNames && output == "completed") "" else output
/** /**
* The permission ask on the call it is about. * The permission ask on the call it is about.
* *
@@ -3,6 +3,7 @@ package com.example.aiapp
import kotlin.test.Test import kotlin.test.Test
import kotlin.test.assertEquals import kotlin.test.assertEquals
import kotlin.test.assertNull import kotlin.test.assertNull
import kotlin.test.assertTrue
class ToolInputTest { class ToolInputTest {
@Test @Test
@@ -26,4 +27,17 @@ class ToolInputTest {
assertNull(renderedBashScript("/usr/bin/bash -lc echo hello")) assertNull(renderedBashScript("/usr/bin/bash -lc echo hello"))
assertNull(renderedBashScript("/usr/bin/fish -lc 'echo hello'")) assertNull(renderedBashScript("/usr/bin/fish -lc 'echo hello'"))
} }
@Test
fun `missing optional input is not displayed as null`() {
assertTrue(parseToolInput("TaskOutput", "null").rest.isEmpty())
}
@Test
fun `collaboration calls say what they do and omit empty completion`() {
assertEquals("Spawn agent", toolDisplayName("Task"))
assertEquals("Wait for agents", toolDisplayName("TaskOutput"))
assertEquals("", toolDisplayOutput("TaskOutput", "completed"))
assertEquals("failed", toolDisplayOutput("TaskOutput", "failed"))
}
} }
+100 -14
View File
@@ -348,7 +348,15 @@ impl Translator {
if let Some(subagents) = &self.subagents { if let Some(subagents) = &self.subagents {
subagents.reopen(id); subagents.reopen(id);
} }
Vec::new() item.get("id")
.and_then(Value::as_str)
.map(|call| {
vec![Event::ToolEnd {
id: call.to_string(),
output: String::new(),
}]
})
.unwrap_or_default()
} }
Some("interacted") => { Some("interacted") => {
self.ensure_child(id, &title, None); self.ensure_child(id, &title, None);
@@ -591,14 +599,14 @@ fn tool(item: &Value) -> Option<(String, String, Value)> {
None => "Agent", None => "Agent",
}; };
let mut input = json!({}); let mut input = json!({});
if let Some(prompt) = item.get("prompt") { if let Some(prompt) = item.get("prompt").and_then(Value::as_str) {
input["prompt"] = prompt.clone(); input["prompt"] = Value::String(prompt.to_string());
} }
if let Some(model) = item.get("model") { if let Some(model) = item.get("model").and_then(Value::as_str) {
input["model"] = model.clone(); input["model"] = Value::String(model.to_string());
} }
if let Some(effort) = item.get("reasoningEffort") { if let Some(effort) = item.get("reasoningEffort").and_then(Value::as_str) {
input["reasoningEffort"] = effort.clone(); input["reasoningEffort"] = Value::String(effort.to_string());
} }
(name.to_string(), input) (name.to_string(), input)
} }
@@ -766,6 +774,11 @@ fn tool_output(item: &Value) -> String {
{ {
return String::new(); return String::new();
} }
Some("collabAgentToolCall")
if item.get("status").and_then(Value::as_str) == Some("completed") =>
{
return String::new();
}
Some("dynamicToolCall") => { Some("dynamicToolCall") => {
return content_text(item.get("contentItems"), "inputText"); return content_text(item.get("contentItems"), "inputText");
} }
@@ -1131,15 +1144,17 @@ mod tests {
vec![Event::ToolStart { vec![Event::ToolStart {
id: "spawn-1".to_string(), id: "spawn-1".to_string(),
tool: "Task".to_string(), tool: "Task".to_string(),
input: json!({"prompt": "audit the history", "model": null, "reasoningEffort": null}) input: json!({"prompt": "audit the history"})
}] }]
); );
assert!( assert_eq!(
translator translator.translate(&line(
.translate(&line( r#"{"method":"item/completed","params":{"threadId":"parent-thread","turnId":"turn-1","item":{"id":"spawn-1","type":"subAgentActivity","kind":"started","agentThreadId":"child-thread","agentPath":"/root/history_boundaries"}}}"#
r#"{"method":"item/completed","params":{"threadId":"parent-thread","turnId":"turn-1","item":{"id":"spawn-1","type":"subAgentActivity","kind":"started","agentThreadId":"child-thread","agentPath":"/root/history_boundaries"}}}"# )),
)) vec![Event::ToolEnd {
.is_empty() id: "spawn-1".to_string(),
output: String::new()
}]
); );
let rows = subagents.list(true); let rows = subagents.list(true);
assert_eq!(rows.len(), 1); assert_eq!(rows.len(), 1);
@@ -1203,6 +1218,77 @@ mod tests {
); );
} }
#[test]
fn codex_collaboration_coordination_has_clean_parent_tool_cards() {
let mut translator = Translator::default();
for (tool, name) in [
("wait", "TaskOutput"),
("sendInput", "SendMessage"),
("sendMessage", "SendMessage"),
("followupTask", "SendMessage"),
("closeAgent", "CloseAgent"),
("interruptAgent", "InterruptAgent"),
("listAgents", "ListAgents"),
("resumeAgent", "ResumeAgent"),
] {
let started = json!({
"method": "item/started",
"params": {
"threadId": "parent-thread",
"turnId": "turn-1",
"item": {
"id": format!("{tool}-1"),
"type": "collabAgentToolCall",
"tool": tool,
"status": "inProgress",
"senderThreadId": "parent-thread",
"receiverThreadIds": [],
"agentsStates": {},
"prompt": null,
"model": null,
"reasoningEffort": null
}
}
});
let completed = json!({
"method": "item/completed",
"params": {
"threadId": "parent-thread",
"turnId": "turn-1",
"item": {
"id": format!("{tool}-1"),
"type": "collabAgentToolCall",
"tool": tool,
"status": "completed",
"senderThreadId": "parent-thread",
"receiverThreadIds": [],
"agentsStates": {},
"prompt": null,
"model": null,
"reasoningEffort": null
}
}
});
assert_eq!(
translator.translate(&started),
vec![Event::ToolStart {
id: format!("{tool}-1"),
tool: name.to_string(),
input: json!({})
}],
"{tool} start"
);
assert_eq!(
translator.translate(&completed),
vec![Event::ToolEnd {
id: format!("{tool}-1"),
output: String::new()
}],
"{tool} completion"
);
}
}
#[test] #[test]
fn delivered_agent_messages_are_peer_messages_not_the_parents_reply() { fn delivered_agent_messages_are_peer_messages_not_the_parents_reply() {
let event = final_item(&json!({ let event = final_item(&json!({