From cad0cbcfbe4f0e1587fff713ee06ab3bf8d79e50 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 13 Sep 2026 01:05:35 -0400 Subject: [PATCH] Keep subagent delivery out of assistant text --- SUBAGENTS.md | 6 ++- server/src/session/codex/translate.rs | 76 +++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/SUBAGENTS.md b/SUBAGENTS.md index 91ac4bd..6c31562 100644 --- a/SUBAGENTS.md +++ b/SUBAGENTS.md @@ -28,7 +28,11 @@ items name the child thread and its lifecycle, and `collabAgentToolCall` items carry the spawn prompt. The Codex translator routes a non-root `threadId` exactly as Claude routes a `parent_tool_use_id`. The child thread id is the subagent id on disk. An asynchronously delivered `agentMessage` is -a `PeerMessage`, not assistant text from the recipient. +a `PeerMessage`, not assistant text from the recipient. Its delta notification +does not repeat the completed item's `delivery` field, so the translator +remembers that field from `item/started` and suppresses those deltas. Letting +one into the recipient's provisional assistant row makes its next completed +message replace the combined row, visibly erasing text that Codex still has. ## Storage diff --git a/server/src/session/codex/translate.rs b/server/src/session/codex/translate.rs index b76cf88..929d6e6 100644 --- a/server/src/session/codex/translate.rs +++ b/server/src/session/codex/translate.rs @@ -4,7 +4,7 @@ //! therefore match only the records that have a useful common equivalent and //! ignore the rest; an added Codex item must not make a live session go deaf. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use serde_json::{Value, json}; @@ -21,6 +21,7 @@ pub(super) struct Translator { subagents: Option>, children: HashMap, prompts: HashMap, + async_messages: HashSet, in_turn: bool, } @@ -134,15 +135,43 @@ impl Translator { state: SessionStatus::Running, }] } - Some("item.started") | Some("item/started") => start_item(&body["item"]), + Some("item.started") | Some("item/started") => { + let item = &body["item"]; + if let Some(id) = item.get("id").and_then(Value::as_str) + && item.get("delivery").and_then(Value::as_str) == Some("async") + { + self.async_messages.insert(id.to_string()); + } + start_item(item) + } Some("item.updated") => update_item(&line["item"]), // The old `codex exec --json` dialect reports only the completed message. App-server // also reports deltas, but they are provisional: safety buffering can revise their // text before completion. Keep its completed copy as an append-only correction rather // than guessing that the two representations concatenate to the same answer. Some("item.completed") => complete_item(&body["item"], true), - Some("item/completed") => final_item(&body["item"]), + Some("item/completed") => { + let item = &body["item"]; + let events = final_item(item); + if let Some(id) = item.get("id").and_then(Value::as_str) { + self.async_messages.remove(id); + } + events + } Some("item/agentMessage/delta") => { + if body + .get("itemId") + .and_then(Value::as_str) + .is_some_and(|id| self.async_messages.contains(id)) + { + // The delta notification does not repeat `delivery`. + // Appending an asynchronously delivered peer report as + // assistant text merges it into the recipient's current + // reply; the next completed assistant item then replaces + // that whole row. The item/started record did identify it, + // so leave its text to the PeerMessage emitted at completion. + return Vec::new(); + } let Some(delta) = body.get("delta").and_then(Value::as_str) else { return Vec::new(); }; @@ -1192,6 +1221,47 @@ mod tests { ); } + #[test] + fn delivered_agent_message_deltas_do_not_replace_the_recipients_text() { + let mut translator = Translator::default(); + + assert!( + translator + .translate(&line( + r#"{"method":"item/started","params":{"threadId":"parent-thread","turnId":"turn-1","item":{"id":"delivery-1","type":"agentMessage","text":"","phase":null,"delivery":"async"}}}"# + )) + .is_empty() + ); + assert!( + translator + .translate(&line( + r#"{"method":"item/agentMessage/delta","params":{"threadId":"parent-thread","turnId":"turn-1","itemId":"delivery-1","delta":"a child report"}}"# + )) + .is_empty() + ); + assert_eq!( + translator.translate(&line( + r#"{"method":"item/completed","params":{"threadId":"parent-thread","turnId":"turn-1","item":{"id":"delivery-1","type":"agentMessage","text":"a child report","phase":null,"delivery":"async"}}}"# + )), + vec![Event::PeerMessage { + from: "another agent".to_string(), + text: "a child report".to_string(), + turn_start: None + }] + ); + + // Removing the completed id is the path out for this state: a later + // item reusing it is not silently suppressed. + assert_eq!( + translator.translate(&line( + r#"{"method":"item/agentMessage/delta","params":{"threadId":"parent-thread","turnId":"turn-2","itemId":"delivery-1","delta":"ordinary text"}}"# + )), + vec![Event::AssistantText { + delta: "ordinary text".to_string() + }] + ); + } + #[test] fn an_adopted_turn_does_not_go_idle_when_its_last_child_finishes() { let dir = tempfile::tempdir().expect("tempdir");