mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Sanitize forked child history (#16709)
- Keep only parent system/developer/user messages plus assistant final-answer messages in forked child history. - Strip parent tool/reasoning items and remove the unmatched synthetic spawn output.
This commit is contained in:
committed by
GitHub
Unverified
parent
3d8cdac797
commit
567d2603b8
@@ -18,7 +18,7 @@ use codex_protocol::AgentPath;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::Result as CodexResult;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
@@ -39,7 +39,6 @@ use tokio::sync::watch;
|
||||
use tracing::warn;
|
||||
|
||||
const AGENT_NAMES: &str = include_str!("agent_names.txt");
|
||||
const FORKED_SPAWN_AGENT_OUTPUT_MESSAGE: &str = "You are the newly spawned agent. The prior conversation history was forked from your parent agent. Treat the next user message as your new task, and use the forked history only as background context.";
|
||||
const ROOT_LAST_TASK_MESSAGE: &str = "Main thread";
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
@@ -93,6 +92,36 @@ fn agent_nickname_candidates(
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn keep_forked_rollout_item(item: &RolloutItem) -> bool {
|
||||
match item {
|
||||
RolloutItem::ResponseItem(ResponseItem::Message { role, phase, .. }) => match role.as_str()
|
||||
{
|
||||
"system" | "developer" | "user" => true,
|
||||
"assistant" => *phase == Some(MessagePhase::FinalAnswer),
|
||||
_ => false,
|
||||
},
|
||||
RolloutItem::ResponseItem(
|
||||
ResponseItem::Reasoning { .. }
|
||||
| ResponseItem::LocalShellCall { .. }
|
||||
| ResponseItem::FunctionCall { .. }
|
||||
| ResponseItem::ToolSearchCall { .. }
|
||||
| ResponseItem::FunctionCallOutput { .. }
|
||||
| ResponseItem::CustomToolCall { .. }
|
||||
| ResponseItem::CustomToolCallOutput { .. }
|
||||
| ResponseItem::ToolSearchOutput { .. }
|
||||
| ResponseItem::WebSearchCall { .. }
|
||||
| ResponseItem::ImageGenerationCall { .. }
|
||||
| ResponseItem::GhostSnapshot { .. }
|
||||
| ResponseItem::Compaction { .. }
|
||||
| ResponseItem::Other,
|
||||
) => false,
|
||||
RolloutItem::Compacted(_)
|
||||
| RolloutItem::EventMsg(_)
|
||||
| RolloutItem::SessionMeta(_)
|
||||
| RolloutItem::TurnContext(_) => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Control-plane handle for multi-agent operations.
|
||||
/// `AgentControl` is held by each session (via `SessionServices`). It provides capability to
|
||||
/// spawn new agents and the inter-agent communication layer.
|
||||
@@ -260,11 +289,11 @@ impl AgentControl {
|
||||
inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
|
||||
inherited_exec_policy: Option<Arc<crate::exec_policy::ExecPolicyManager>>,
|
||||
) -> CodexResult<crate::thread_manager::NewThread> {
|
||||
let Some(call_id) = options.fork_parent_spawn_call_id.as_deref() else {
|
||||
if options.fork_parent_spawn_call_id.is_none() {
|
||||
return Err(CodexErr::Fatal(
|
||||
"spawn_agent fork requires a parent spawn call id".to_string(),
|
||||
));
|
||||
};
|
||||
}
|
||||
let Some(fork_mode) = options.fork_mode.as_ref() else {
|
||||
return Err(CodexErr::Fatal(
|
||||
"spawn_agent fork requires a fork mode".to_string(),
|
||||
@@ -313,16 +342,7 @@ impl AgentControl {
|
||||
forked_rollout_items =
|
||||
truncate_rollout_to_last_n_fork_turns(&forked_rollout_items, *last_n_turns);
|
||||
}
|
||||
|
||||
let mut output =
|
||||
FunctionCallOutputPayload::from_text(FORKED_SPAWN_AGENT_OUTPUT_MESSAGE.to_string());
|
||||
output.success = Some(true);
|
||||
forked_rollout_items.push(RolloutItem::ResponseItem(
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
output,
|
||||
},
|
||||
));
|
||||
forked_rollout_items.retain(keep_forked_rollout_item);
|
||||
|
||||
state
|
||||
.fork_thread_with_source(
|
||||
|
||||
@@ -14,6 +14,7 @@ use codex_login::CodexAuth;
|
||||
use codex_protocol::AgentPath;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::ErrorEvent;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -62,6 +63,28 @@ fn text_input(text: &str) -> Op {
|
||||
.into()
|
||||
}
|
||||
|
||||
fn assistant_message(text: &str, phase: Option<MessagePhase>) -> ResponseItem {
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "assistant".to_string(),
|
||||
content: vec![ContentItem::OutputText {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase,
|
||||
}
|
||||
}
|
||||
|
||||
fn spawn_agent_call(call_id: &str) -> ResponseItem {
|
||||
ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "spawn_agent".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: call_id.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
struct AgentControlHarness {
|
||||
_home: TempDir,
|
||||
config: Config,
|
||||
@@ -561,7 +584,7 @@ async fn spawn_agent_creates_thread_and_sends_prompt() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_can_fork_parent_thread_history() {
|
||||
async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let (parent_thread_id, parent_thread) = harness.start_thread().await;
|
||||
parent_thread
|
||||
@@ -569,17 +592,32 @@ async fn spawn_agent_can_fork_parent_thread_history() {
|
||||
.await;
|
||||
let turn_context = parent_thread.codex.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-history".to_string();
|
||||
let parent_spawn_call = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "spawn_agent".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: parent_spawn_call_id.clone(),
|
||||
};
|
||||
let trigger_message = InterAgentCommunication::new(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
Vec::new(),
|
||||
"parent trigger message".to_string(),
|
||||
/*trigger_turn*/ true,
|
||||
);
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.record_conversation_items(turn_context.as_ref(), &[parent_spawn_call])
|
||||
.record_conversation_items(
|
||||
turn_context.as_ref(),
|
||||
&[
|
||||
assistant_message("parent commentary", Some(MessagePhase::Commentary)),
|
||||
assistant_message("parent final answer", Some(MessagePhase::FinalAnswer)),
|
||||
assistant_message("parent unknown phase", /*phase*/ None),
|
||||
ResponseItem::Reasoning {
|
||||
id: "parent-reasoning".to_string(),
|
||||
summary: Vec::new(),
|
||||
content: None,
|
||||
encrypted_content: None,
|
||||
},
|
||||
trigger_message.to_response_input_item().into(),
|
||||
spawn_agent_call(&parent_spawn_call_id),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
@@ -601,7 +639,7 @@ async fn spawn_agent_can_fork_parent_thread_history() {
|
||||
agent_role: None,
|
||||
})),
|
||||
SpawnAgentOptions {
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id),
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id.clone()),
|
||||
fork_mode: Some(SpawnAgentForkMode::FullHistory),
|
||||
},
|
||||
)
|
||||
@@ -616,10 +654,23 @@ async fn spawn_agent_can_fork_parent_thread_history() {
|
||||
.expect("child thread should be registered");
|
||||
assert_ne!(child_thread_id, parent_thread_id);
|
||||
let history = child_thread.codex.session.clone_history().await;
|
||||
assert!(history_contains_text(
|
||||
let expected_history = [
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "parent seed context".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
assistant_message("parent final answer", Some(MessagePhase::FinalAnswer)),
|
||||
];
|
||||
assert_eq!(
|
||||
history.raw_items(),
|
||||
"parent seed context"
|
||||
));
|
||||
&expected_history,
|
||||
"forked child history should keep only parent user messages and assistant final answers"
|
||||
);
|
||||
|
||||
let expected = (
|
||||
child_thread_id,
|
||||
@@ -649,102 +700,22 @@ async fn spawn_agent_can_fork_parent_thread_history() {
|
||||
.expect("parent shutdown should submit");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_fork_injects_output_for_parent_spawn_call() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let (parent_thread_id, parent_thread) = harness.start_thread().await;
|
||||
let turn_context = parent_thread.codex.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-1".to_string();
|
||||
let parent_spawn_call = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "spawn_agent".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: parent_spawn_call_id.clone(),
|
||||
};
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.record_conversation_items(turn_context.as_ref(), &[parent_spawn_call])
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.ensure_rollout_materialized()
|
||||
.await;
|
||||
parent_thread.codex.session.flush_rollout().await;
|
||||
|
||||
let child_thread_id = harness
|
||||
.control
|
||||
.spawn_agent_with_metadata(
|
||||
harness.config.clone(),
|
||||
text_input("child task"),
|
||||
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
})),
|
||||
SpawnAgentOptions {
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id.clone()),
|
||||
fork_mode: Some(SpawnAgentForkMode::FullHistory),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("forked spawn should succeed")
|
||||
.thread_id;
|
||||
|
||||
let child_thread = harness
|
||||
.manager
|
||||
.get_thread(child_thread_id)
|
||||
.await
|
||||
.expect("child thread should be registered");
|
||||
let history = child_thread.codex.session.clone_history().await;
|
||||
let injected_output = history.raw_items().iter().find_map(|item| match item {
|
||||
ResponseItem::FunctionCallOutput { call_id, output }
|
||||
if call_id == &parent_spawn_call_id =>
|
||||
{
|
||||
Some(output)
|
||||
}
|
||||
_ => None,
|
||||
});
|
||||
let injected_output =
|
||||
injected_output.expect("forked child should contain synthetic tool output");
|
||||
assert_eq!(
|
||||
injected_output.text_content(),
|
||||
Some(FORKED_SPAWN_AGENT_OUTPUT_MESSAGE)
|
||||
);
|
||||
assert_eq!(injected_output.success, Some(true));
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
.shutdown_live_agent(child_thread_id)
|
||||
.await
|
||||
.expect("child shutdown should submit");
|
||||
let _ = parent_thread
|
||||
.submit(Op::Shutdown {})
|
||||
.await
|
||||
.expect("parent shutdown should submit");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_fork_flushes_parent_rollout_before_loading_history() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let (parent_thread_id, parent_thread) = harness.start_thread().await;
|
||||
let turn_context = parent_thread.codex.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-unflushed".to_string();
|
||||
let parent_spawn_call = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "spawn_agent".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: parent_spawn_call_id.clone(),
|
||||
};
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.record_conversation_items(turn_context.as_ref(), &[parent_spawn_call])
|
||||
.record_conversation_items(
|
||||
turn_context.as_ref(),
|
||||
&[
|
||||
assistant_message("unflushed final answer", Some(MessagePhase::FinalAnswer)),
|
||||
spawn_agent_call(&parent_spawn_call_id),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let child_thread_id = harness
|
||||
@@ -774,28 +745,10 @@ async fn spawn_agent_fork_flushes_parent_rollout_before_loading_history() {
|
||||
.await
|
||||
.expect("child thread should be registered");
|
||||
let history = child_thread.codex.session.clone_history().await;
|
||||
|
||||
let mut parent_call_index = None;
|
||||
let mut injected_output_index = None;
|
||||
for (idx, item) in history.raw_items().iter().enumerate() {
|
||||
match item {
|
||||
ResponseItem::FunctionCall { call_id, .. } if call_id == &parent_spawn_call_id => {
|
||||
parent_call_index = Some(idx);
|
||||
}
|
||||
ResponseItem::FunctionCallOutput { call_id, .. }
|
||||
if call_id == &parent_spawn_call_id =>
|
||||
{
|
||||
injected_output_index = Some(idx);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
let parent_call_index =
|
||||
parent_call_index.expect("forked child should include the parent spawn_agent call");
|
||||
let injected_output_index = injected_output_index
|
||||
.expect("forked child should include synthetic output for the parent spawn_agent call");
|
||||
assert!(parent_call_index < injected_output_index);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "unflushed final answer"),
|
||||
"forked child history should include unflushed assistant final answers after flushing the parent rollout"
|
||||
);
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
@@ -849,23 +802,18 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() {
|
||||
&[triggered_communication.to_response_input_item().into()],
|
||||
)
|
||||
.await;
|
||||
|
||||
parent_thread
|
||||
.inject_user_message_without_turn("current parent task".to_string())
|
||||
.await;
|
||||
let spawn_turn_context = parent_thread.codex.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-last-n".to_string();
|
||||
let parent_spawn_call = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "spawn_agent".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: parent_spawn_call_id.clone(),
|
||||
};
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.record_conversation_items(spawn_turn_context.as_ref(), &[parent_spawn_call])
|
||||
.record_conversation_items(
|
||||
spawn_turn_context.as_ref(),
|
||||
&[spawn_agent_call(&parent_spawn_call_id)],
|
||||
)
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
@@ -887,7 +835,7 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() {
|
||||
agent_role: None,
|
||||
})),
|
||||
SpawnAgentOptions {
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id),
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id.clone()),
|
||||
fork_mode: Some(SpawnAgentForkMode::LastNTurns(2)),
|
||||
},
|
||||
)
|
||||
@@ -902,22 +850,22 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() {
|
||||
.expect("child thread should be registered");
|
||||
let history = child_thread.codex.session.clone_history().await;
|
||||
|
||||
assert!(!history_contains_text(
|
||||
history.raw_items(),
|
||||
"old parent context"
|
||||
));
|
||||
assert!(!history_contains_text(
|
||||
history.raw_items(),
|
||||
"queued message"
|
||||
));
|
||||
assert!(history_contains_text(
|
||||
history.raw_items(),
|
||||
"triggered context"
|
||||
));
|
||||
assert!(history_contains_text(
|
||||
history.raw_items(),
|
||||
"current parent task"
|
||||
));
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "old parent context"),
|
||||
"forked child history should drop parent context outside the requested last-N turn window"
|
||||
);
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "queued message"),
|
||||
"forked child history should drop queued inter-agent messages outside the requested last-N turn window"
|
||||
);
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "triggered context"),
|
||||
"forked child history should filter assistant inter-agent messages even when they fall inside the requested last-N turn window"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "current parent task"),
|
||||
"forked child history should keep the parent user message from the requested last-N turn window"
|
||||
);
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
|
||||
@@ -25,7 +25,6 @@ use tokio::time::sleep;
|
||||
use wiremock::MockServer;
|
||||
|
||||
const SPAWN_CALL_ID: &str = "spawn-call-1";
|
||||
const FORKED_SPAWN_AGENT_OUTPUT_MESSAGE: &str = "You are the newly spawned agent. The prior conversation history was forked from your parent agent. Treat the next user message as your new task, and use the forked history only as background context.";
|
||||
const TURN_0_FORK_PROMPT: &str = "seed fork context";
|
||||
const TURN_1_PROMPT: &str = "spawn a child and continue";
|
||||
const TURN_2_NO_WAIT_PROMPT: &str = "follow up without wait";
|
||||
@@ -372,8 +371,7 @@ async fn spawned_child_receives_forked_parent_context() -> Result<()> {
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.find(|request| {
|
||||
body_contains(request, CHILD_PROMPT)
|
||||
&& body_contains(request, FORKED_SPAWN_AGENT_OUTPUT_MESSAGE)
|
||||
body_contains(request, CHILD_PROMPT) && !body_contains(request, SPAWN_CALL_ID)
|
||||
})
|
||||
{
|
||||
break request;
|
||||
@@ -384,30 +382,7 @@ async fn spawned_child_receives_forked_parent_context() -> Result<()> {
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
};
|
||||
assert!(body_contains(&child_request, TURN_0_FORK_PROMPT));
|
||||
assert!(body_contains(&child_request, "seeded"));
|
||||
|
||||
let child_body = child_request
|
||||
.body_json::<serde_json::Value>()
|
||||
.expect("forked child request body should be json");
|
||||
let function_call_output = child_body["input"]
|
||||
.as_array()
|
||||
.and_then(|items| {
|
||||
items.iter().find(|item| {
|
||||
item["type"].as_str() == Some("function_call_output")
|
||||
&& item["call_id"].as_str() == Some(SPAWN_CALL_ID)
|
||||
})
|
||||
})
|
||||
.unwrap_or_else(|| panic!("expected forked child request to include spawn_agent output"));
|
||||
let (content, success) = match &function_call_output["output"] {
|
||||
serde_json::Value::String(text) => (Some(text.as_str()), None),
|
||||
serde_json::Value::Object(output) => (
|
||||
output.get("content").and_then(serde_json::Value::as_str),
|
||||
output.get("success").and_then(serde_json::Value::as_bool),
|
||||
),
|
||||
_ => (None, None),
|
||||
};
|
||||
assert_eq!(content, Some(FORKED_SPAWN_AGENT_OUTPUT_MESSAGE));
|
||||
assert_ne!(success, Some(false));
|
||||
assert!(!body_contains(&child_request, SPAWN_CALL_ID));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user