mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore: assign amsg_ IDs to agent messages (#29750)
## Why The `ItemIds` path fills in missing IDs before response items are persisted and emitted as raw item events. `ResponseItem::AgentMessage` is part of that same response-item stream, but it was skipped by the missing-ID repair path, leaving agent messages without stable item IDs while messages and tool items received generated IDs. Agent messages recorded through `InterAgentCommunication` also need the generated ID to survive rollout persistence and resume. Otherwise clients can observe an `amsg_` ID for the live raw response item, then see that same persisted agent message lose its item ID after restart. ## What changed - Assign missing `ResponseItem::AgentMessage` IDs with the `amsg_` prefix. - Persist the generated item ID on `InterAgentCommunication` and replay it back into the reconstructed `ResponseItem::AgentMessage` on resume. - Keep the persisted ID out of the model-visible inter-agent message envelope. - Keep `CompactionTrigger` and `Other` skipped because they do not get generated item IDs. - Update session/protocol tests for agent-message ID assignment and resume preservation. ## Manual Testing Run the local dev build using `just c --enable item_ids` to ensure this code is exercised: https://github.com/openai/codex/blob/322e33512b2d38d38d705e2ef692a8aca50decac/codex-rs/core/src/session/mod.rs#L2713-L2715 In the `.jsonl` file, I saw entries like: ```json { "timestamp": "2026-06-24T00:44:03.098Z", "type": "inter_agent_communication", "payload": { "id": "amsg_019ef715-849a-7a50-becc-ce63c6a9c994", ``` ## Test plan - `just test -p codex-core record_inter_agent_communication_preserves_item_id_in_rollout_and_resume` - `just test -p codex-core record_inter_agent_communication_sets_turn_id_in_rollout_and_resume` - `just test -p codex-protocol inter_agent_communication_response_input_item_preserves_commentary_phase`
This commit is contained in:
committed by
GitHub
Unverified
parent
322e33512b
commit
97dce078c5
@@ -2740,9 +2740,8 @@ impl Session {
|
||||
ResponseItem::WebSearchCall { .. } => "ws",
|
||||
ResponseItem::ImageGenerationCall { .. } => "ig",
|
||||
ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } => "cmp",
|
||||
ResponseItem::AgentMessage { .. }
|
||||
| ResponseItem::CompactionTrigger { .. }
|
||||
| ResponseItem::Other => continue,
|
||||
ResponseItem::AgentMessage { .. } => "amsg",
|
||||
ResponseItem::CompactionTrigger { .. } | ResponseItem::Other => continue,
|
||||
};
|
||||
item.set_id(Some(format!("{prefix}_{}", Uuid::now_v7())));
|
||||
}
|
||||
@@ -2831,6 +2830,7 @@ impl Session {
|
||||
std::slice::from_ref(&response_item),
|
||||
);
|
||||
let items = items.as_ref();
|
||||
communication.id = items.first().and_then(ResponseItem::id).map(str::to_string);
|
||||
{
|
||||
let mut state = self.state.lock().await;
|
||||
state.record_items(
|
||||
|
||||
@@ -210,7 +210,7 @@ fn user_message(text: &str) -> ResponseItem {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assign_missing_response_item_ids_skips_agent_messages() {
|
||||
fn assign_missing_response_item_ids_assigns_agent_message_ids() {
|
||||
let items = Cow::Owned(vec![
|
||||
ResponseItem::AgentMessage {
|
||||
id: None,
|
||||
@@ -226,7 +226,7 @@ fn assign_missing_response_item_ids_skips_agent_messages() {
|
||||
|
||||
let items = Session::assign_missing_response_item_ids(items);
|
||||
|
||||
assert_eq!(items[0].id(), None);
|
||||
assert!(items[0].id().is_some_and(|id| id.starts_with("amsg_")));
|
||||
assert!(items[1].id().is_some_and(|id| id.starts_with("msg_")));
|
||||
}
|
||||
|
||||
@@ -1777,6 +1777,75 @@ async fn record_inter_agent_communication_sets_turn_id_in_rollout_and_resume() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn record_inter_agent_communication_preserves_item_id_in_rollout_and_resume() {
|
||||
let (mut session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
)
|
||||
.await;
|
||||
let rollout_path =
|
||||
attach_thread_persistence(Arc::get_mut(&mut session).expect("unique session")).await;
|
||||
let communication = InterAgentCommunication::new(
|
||||
AgentPath::root().join("worker").expect("worker path"),
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"child done".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
);
|
||||
|
||||
session
|
||||
.record_inter_agent_communication(&turn_context, communication)
|
||||
.await;
|
||||
|
||||
let live_history = session.clone_history().await;
|
||||
let [live_item] = live_history.raw_items() else {
|
||||
panic!("expected exactly one live history item");
|
||||
};
|
||||
let live_item_id = live_item
|
||||
.id()
|
||||
.expect("live agent message should have an item id")
|
||||
.to_string();
|
||||
assert!(live_item_id.starts_with("amsg_"));
|
||||
|
||||
session.flush_rollout().await.expect("rollout should flush");
|
||||
let InitialHistory::Resumed(resumed) = RolloutRecorder::get_rollout_history(&rollout_path)
|
||||
.await
|
||||
.expect("read rollout history")
|
||||
else {
|
||||
panic!("expected resumed rollout history");
|
||||
};
|
||||
let persisted_communication = resumed.history.iter().find_map(|item| match item {
|
||||
RolloutItem::InterAgentCommunication(communication) => Some(communication),
|
||||
_ => None,
|
||||
});
|
||||
assert_eq!(
|
||||
persisted_communication.and_then(|communication| communication.id.as_deref()),
|
||||
Some(live_item_id.as_str())
|
||||
);
|
||||
|
||||
let (resumed_session, _resumed_turn_context, _rx) =
|
||||
make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
)
|
||||
.await;
|
||||
resumed_session
|
||||
.record_initial_history(InitialHistory::Resumed(resumed))
|
||||
.await;
|
||||
let resumed_history = resumed_session.clone_history().await;
|
||||
let [resumed_item] = resumed_history.raw_items() else {
|
||||
panic!("expected exactly one resumed history item");
|
||||
};
|
||||
assert_eq!(resumed_item.id(), Some(live_item_id.as_str()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn prepares_image_failures_before_history_insertion() {
|
||||
let (session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||
|
||||
@@ -692,6 +692,9 @@ impl From<Vec<UserInput>> for Op {
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)]
|
||||
pub struct InterAgentCommunication {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub id: Option<String>,
|
||||
pub author: AgentPath,
|
||||
pub recipient: AgentPath,
|
||||
#[serde(default)]
|
||||
@@ -715,6 +718,7 @@ impl InterAgentCommunication {
|
||||
trigger_turn: bool,
|
||||
) -> Self {
|
||||
Self {
|
||||
id: None,
|
||||
author,
|
||||
recipient,
|
||||
other_recipients,
|
||||
@@ -733,6 +737,7 @@ impl InterAgentCommunication {
|
||||
trigger_turn: bool,
|
||||
) -> Self {
|
||||
Self {
|
||||
id: None,
|
||||
author,
|
||||
recipient,
|
||||
other_recipients,
|
||||
@@ -752,6 +757,7 @@ impl InterAgentCommunication {
|
||||
|
||||
pub fn to_response_input_item(&self) -> ResponseInputItem {
|
||||
let mut communication = self.clone();
|
||||
communication.id = None;
|
||||
communication.internal_chat_message_metadata_passthrough = None;
|
||||
ResponseInputItem::Message {
|
||||
role: "assistant".to_string(),
|
||||
@@ -787,7 +793,7 @@ impl InterAgentCommunication {
|
||||
}],
|
||||
};
|
||||
ResponseItem::AgentMessage {
|
||||
id: None,
|
||||
id: self.id.clone(),
|
||||
author: self.author.to_string(),
|
||||
recipient: self.recipient.to_string(),
|
||||
content,
|
||||
@@ -4396,6 +4402,7 @@ mod tests {
|
||||
#[test]
|
||||
fn inter_agent_communication_response_input_item_preserves_commentary_phase() {
|
||||
let mut communication = InterAgentCommunication {
|
||||
id: Some("amsg_1".to_string()),
|
||||
author: AgentPath::root(),
|
||||
recipient: AgentPath::root().join("reviewer").expect("recipient path"),
|
||||
other_recipients: vec![AgentPath::root().join("worker").expect("recipient path")],
|
||||
@@ -4406,6 +4413,7 @@ mod tests {
|
||||
};
|
||||
communication.set_turn_id_if_missing("turn-1");
|
||||
let mut serialized_communication = communication.clone();
|
||||
serialized_communication.id = None;
|
||||
serialized_communication.internal_chat_message_metadata_passthrough = None;
|
||||
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user