mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Preserve context baselines for full-history agent forks (#23352)
## Why Full-history agent forks should continue from the same prompt prefix as the parent. Dropping the stored `TurnContext` baseline forced the child to rebuild startup context on its first turn, which can duplicate developer instructions and also loses the cache continuity that a full-history fork is supposed to preserve. Truncated forks are different: once we keep only the last N turns, the original prompt prefix is no longer intact, so the child must establish a fresh context baseline. ## What changed - Preserve `RolloutItem::TurnContext` when forking with `SpawnAgentForkMode::FullHistory`, and keep dropping it for truncated forks: https://github.com/openai/codex/blob/4090717d94c1fc7f33c9bd122be133a0c5752052/codex-rs/core/src/agent/control.rs#L98-L126 and https://github.com/openai/codex/blob/4090717d94c1fc7f33c9bd122be133a0c5752052/codex-rs/core/src/agent/control.rs#L399-L401 - Remove the special-case MultiAgentV2 usage-hint filtering path. Full-history fork now preserves the cached developer prefix instead of trying to reconstruct part of it. - Extend the fork coverage to assert both sides of the contract: full-history forks keep the parent reference baseline, while last-N forks rebuild context after truncation: https://github.com/openai/codex/blob/4090717d94c1fc7f33c9bd122be133a0c5752052/codex-rs/core/src/agent/control_tests.rs#L603-L759 and https://github.com/openai/codex/blob/4090717d94c1fc7f33c9bd122be133a0c5752052/codex-rs/core/src/agent/control_tests.rs#L854-L977 ## Verification - `cargo test -p codex-core spawn_agent_can_fork_parent_thread_history_with_sanitized_items -- --nocapture` - `RUST_MIN_STACK=16777216 cargo test -p codex-core spawn_agent_fork_last_n_turns_keeps_only_recent_turns -- --nocapture`
This commit is contained in:
committed by
GitHub
Unverified
parent
3009e23644
commit
826b2182ed
@@ -96,7 +96,7 @@ fn agent_nickname_candidates(
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn keep_forked_rollout_item(item: &RolloutItem) -> bool {
|
||||
fn keep_forked_rollout_item(item: &RolloutItem, preserve_reference_context_item: bool) -> bool {
|
||||
match item {
|
||||
RolloutItem::ResponseItem(ResponseItem::Message { role, phase, .. }) => match role.as_str()
|
||||
{
|
||||
@@ -120,13 +120,30 @@ fn keep_forked_rollout_item(item: &RolloutItem) -> bool {
|
||||
| ResponseItem::ContextCompaction { .. }
|
||||
| ResponseItem::Other,
|
||||
) => false,
|
||||
// A forked child gets its own runtime config, including spawned-agent
|
||||
// instructions, so it must establish a fresh context diff baseline.
|
||||
RolloutItem::TurnContext(_) => false,
|
||||
// Full-history forks preserve the cached prompt prefix and can keep diffing
|
||||
// from the parent's durable baseline. Truncated forks drop part of that prompt,
|
||||
// so they must rebuild context on their first child turn.
|
||||
RolloutItem::TurnContext(_) => preserve_reference_context_item,
|
||||
RolloutItem::Compacted(_) | RolloutItem::EventMsg(_) | RolloutItem::SessionMeta(_) => true,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_multi_agent_v2_usage_hint_message(item: &ResponseItem, usage_hint_texts: &[String]) -> bool {
|
||||
let ResponseItem::Message { role, content, .. } = item else {
|
||||
return false;
|
||||
};
|
||||
if role != "developer" {
|
||||
return false;
|
||||
}
|
||||
let [ContentItem::InputText { text }] = content.as_slice() else {
|
||||
return false;
|
||||
};
|
||||
|
||||
usage_hint_texts
|
||||
.iter()
|
||||
.any(|usage_hint_text| usage_hint_text == text)
|
||||
}
|
||||
|
||||
/// 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.
|
||||
@@ -396,16 +413,26 @@ impl AgentControl {
|
||||
forked_rollout_items =
|
||||
truncate_rollout_to_last_n_fork_turns(&forked_rollout_items, *last_n_turns);
|
||||
}
|
||||
// MultiAgentV2 root/subagent usage hints are injected as standalone developer
|
||||
// messages at thread start. When forking history, drop hints from the parent
|
||||
// so the child gets a fresh hint that matches its own session source/config.
|
||||
let multi_agent_v2_usage_hint_texts_to_filter: Vec<String> =
|
||||
if let Some(parent_thread) = parent_thread.as_ref() {
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.configured_multi_agent_v2_usage_hint_texts()
|
||||
.await
|
||||
if parent_thread.enabled(Feature::MultiAgentV2) {
|
||||
let parent_config = parent_thread.codex.session.get_config().await;
|
||||
[
|
||||
parent_config
|
||||
.multi_agent_v2
|
||||
.root_agent_usage_hint_text
|
||||
.clone(),
|
||||
parent_config
|
||||
.multi_agent_v2
|
||||
.subagent_usage_hint_text
|
||||
.clone(),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.collect()
|
||||
} else {
|
||||
Vec::new()
|
||||
}
|
||||
} else if config.features.enabled(Feature::MultiAgentV2) {
|
||||
[
|
||||
config.multi_agent_v2.root_agent_usage_hint_text.clone(),
|
||||
@@ -417,19 +444,41 @@ impl AgentControl {
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let preserve_reference_context_item = matches!(fork_mode, SpawnAgentForkMode::FullHistory);
|
||||
forked_rollout_items.retain(|item| {
|
||||
if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = item
|
||||
&& role == "developer"
|
||||
&& let [ContentItem::InputText { text }] = content.as_slice()
|
||||
&& multi_agent_v2_usage_hint_texts_to_filter
|
||||
.iter()
|
||||
.any(|usage_hint_text| usage_hint_text == text)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
keep_forked_rollout_item(item)
|
||||
keep_forked_rollout_item(item, preserve_reference_context_item)
|
||||
&& !matches!(
|
||||
item,
|
||||
RolloutItem::ResponseItem(response_item)
|
||||
if is_multi_agent_v2_usage_hint_message(
|
||||
response_item,
|
||||
&multi_agent_v2_usage_hint_texts_to_filter,
|
||||
)
|
||||
)
|
||||
});
|
||||
for item in &mut forked_rollout_items {
|
||||
if let RolloutItem::Compacted(compacted) = item
|
||||
&& let Some(replacement_history) = compacted.replacement_history.as_mut()
|
||||
{
|
||||
replacement_history.retain(|response_item| {
|
||||
!is_multi_agent_v2_usage_hint_message(
|
||||
response_item,
|
||||
&multi_agent_v2_usage_hint_texts_to_filter,
|
||||
)
|
||||
});
|
||||
}
|
||||
}
|
||||
if preserve_reference_context_item
|
||||
&& config.features.enabled(Feature::MultiAgentV2)
|
||||
&& let Some(subagent_usage_hint_text) =
|
||||
config.multi_agent_v2.subagent_usage_hint_text.clone()
|
||||
&& let Some(subagent_usage_hint_message) =
|
||||
crate::context_manager::updates::build_developer_update_item(vec![
|
||||
subagent_usage_hint_text,
|
||||
])
|
||||
{
|
||||
forked_rollout_items.push(RolloutItem::ResponseItem(subagent_usage_hint_message));
|
||||
}
|
||||
|
||||
state
|
||||
.fork_thread_with_source(
|
||||
|
||||
@@ -17,6 +17,7 @@ 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::CompactedItem;
|
||||
use codex_protocol::protocol::ErrorEvent;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
@@ -677,6 +678,14 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let parent_reference_context_item = turn_context.to_turn_context_item();
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.persist_rollout_items(&[RolloutItem::TurnContext(
|
||||
parent_reference_context_item.clone(),
|
||||
)])
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
@@ -728,11 +737,26 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
phase: None,
|
||||
},
|
||||
assistant_message("parent final answer", Some(MessagePhase::FinalAnswer)),
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "Child subagent guidance.".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
assert_eq!(
|
||||
history.raw_items(),
|
||||
&expected_history,
|
||||
"forked child history should keep only parent user messages and assistant final answers"
|
||||
"full-history forked child history should replace parent usage hints with the child subagent hint while filtering non-final assistant/tool chatter"
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::to_value(child_thread.codex.session.reference_context_item().await)
|
||||
.expect("serialize child reference context item"),
|
||||
serde_json::to_value(Some(parent_reference_context_item))
|
||||
.expect("serialize expected reference context item"),
|
||||
"full-history forked child should preserve the parent diff baseline"
|
||||
);
|
||||
|
||||
let expected = (
|
||||
@@ -766,6 +790,124 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
.expect("parent shutdown should submit");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let mut parent_config = harness.config.clone();
|
||||
let _ = parent_config.features.enable(Feature::MultiAgentV2);
|
||||
parent_config.multi_agent_v2.root_agent_usage_hint_text =
|
||||
Some("Parent root guidance.".to_string());
|
||||
parent_config.multi_agent_v2.subagent_usage_hint_text =
|
||||
Some("Parent subagent guidance.".to_string());
|
||||
let mut child_config = harness.config.clone();
|
||||
let _ = child_config.features.enable(Feature::MultiAgentV2);
|
||||
child_config.multi_agent_v2.root_agent_usage_hint_text =
|
||||
Some("Child root guidance.".to_string());
|
||||
child_config.multi_agent_v2.subagent_usage_hint_text =
|
||||
Some("Child subagent guidance.".to_string());
|
||||
let new_thread = harness
|
||||
.manager
|
||||
.start_thread(parent_config)
|
||||
.await
|
||||
.expect("start parent thread");
|
||||
let parent_thread_id = new_thread.thread_id;
|
||||
let parent_thread = new_thread.thread;
|
||||
let turn_context = parent_thread.codex.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-compacted-usage-hints".to_string();
|
||||
let replacement_history = vec![
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "compacted parent summary".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "Parent root guidance.".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.persist_rollout_items(&[
|
||||
RolloutItem::Compacted(CompactedItem {
|
||||
message: String::new(),
|
||||
replacement_history: Some(replacement_history),
|
||||
}),
|
||||
RolloutItem::TurnContext(turn_context.to_turn_context_item()),
|
||||
RolloutItem::ResponseItem(spawn_agent_call(&parent_spawn_call_id)),
|
||||
])
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.ensure_rollout_materialized()
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.flush_rollout()
|
||||
.await
|
||||
.expect("parent rollout should flush");
|
||||
|
||||
let child_thread_id = harness
|
||||
.control
|
||||
.spawn_agent_with_metadata(
|
||||
child_config,
|
||||
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),
|
||||
fork_mode: Some(SpawnAgentForkMode::FullHistory),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("forked spawn should sanitize compacted usage hints")
|
||||
.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;
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "compacted parent summary"),
|
||||
"forked child history should retain compacted non-hint content"
|
||||
);
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "Parent root guidance."),
|
||||
"forked child history should strip stale parent hints from compacted replacement history"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "Child subagent guidance."),
|
||||
"full-history forked child should add the child subagent hint after compacted-history sanitization"
|
||||
);
|
||||
|
||||
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;
|
||||
@@ -882,6 +1024,13 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() {
|
||||
&[spawn_agent_call(&parent_spawn_call_id)],
|
||||
)
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.persist_rollout_items(&[RolloutItem::TurnContext(
|
||||
spawn_turn_context.to_turn_context_item(),
|
||||
)])
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
@@ -939,6 +1088,116 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() {
|
||||
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"
|
||||
);
|
||||
assert!(
|
||||
child_thread
|
||||
.codex
|
||||
.session
|
||||
.reference_context_item()
|
||||
.await
|
||||
.is_none(),
|
||||
"last-N forked child should rebuild context after truncating the cached prefix"
|
||||
);
|
||||
|
||||
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_last_n_turns_strips_parent_usage_hints() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let mut parent_config = harness.config.clone();
|
||||
let _ = parent_config.features.enable(Feature::MultiAgentV2);
|
||||
parent_config.multi_agent_v2.root_agent_usage_hint_text =
|
||||
Some("Parent root guidance.".to_string());
|
||||
let mut child_config = harness.config.clone();
|
||||
let _ = child_config.features.enable(Feature::MultiAgentV2);
|
||||
child_config.multi_agent_v2.subagent_usage_hint_text =
|
||||
Some("Child subagent guidance.".to_string());
|
||||
let new_thread = harness
|
||||
.manager
|
||||
.start_thread(parent_config)
|
||||
.await
|
||||
.expect("start parent thread");
|
||||
let parent_thread_id = new_thread.thread_id;
|
||||
let parent_thread = new_thread.thread;
|
||||
parent_thread
|
||||
.inject_user_message_without_turn("parent task".to_string())
|
||||
.await;
|
||||
let turn_context = parent_thread.codex.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-last-n-usage-hints".to_string();
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.record_conversation_items(
|
||||
turn_context.as_ref(),
|
||||
&[
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "Parent root guidance.".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
},
|
||||
spawn_agent_call(&parent_spawn_call_id),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.ensure_rollout_materialized()
|
||||
.await;
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.flush_rollout()
|
||||
.await
|
||||
.expect("parent rollout should flush");
|
||||
|
||||
let child_thread_id = harness
|
||||
.control
|
||||
.spawn_agent_with_metadata(
|
||||
child_config,
|
||||
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),
|
||||
fork_mode: Some(SpawnAgentForkMode::LastNTurns(2)),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("bounded forked spawn should sanitize parent usage hints")
|
||||
.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;
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "parent task"),
|
||||
"bounded fork should retain the requested recent parent turn"
|
||||
);
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "Parent root guidance."),
|
||||
"bounded fork should strip stale parent root hints before the child rebuilds startup context"
|
||||
);
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
|
||||
@@ -884,22 +884,6 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn configured_multi_agent_v2_usage_hint_texts(&self) -> Vec<String> {
|
||||
if !self.features.enabled(Feature::MultiAgentV2) {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let state = self.state.lock().await;
|
||||
let config = &state.session_configuration.original_config_do_not_use;
|
||||
[
|
||||
config.multi_agent_v2.root_agent_usage_hint_text.clone(),
|
||||
config.multi_agent_v2.subagent_usage_hint_text.clone(),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn managed_network_proxy_active_for_permission_profile(
|
||||
permission_profile: &PermissionProfile,
|
||||
) -> bool {
|
||||
|
||||
@@ -22,7 +22,6 @@ use codex_config::loader::project_trust_key;
|
||||
use codex_config::types::ToolSuggestDisabledTool;
|
||||
|
||||
use codex_features::Feature;
|
||||
use codex_features::Features;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_models_manager::bundled_models_response;
|
||||
@@ -6745,40 +6744,6 @@ async fn build_initial_context_omits_multi_agent_v2_usage_hints_when_feature_dis
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_multi_agent_v2_usage_hint_texts_use_effective_enabled_feature_state() {
|
||||
let (mut session, _turn_context) =
|
||||
make_multi_agent_v2_usage_hint_test_session(/*enable_multi_agent_v2*/ false).await;
|
||||
let mut effective_features = Features::with_defaults();
|
||||
effective_features.enable(Feature::MultiAgentV2);
|
||||
Arc::get_mut(&mut session)
|
||||
.expect("session should not be shared")
|
||||
.features = effective_features.into();
|
||||
|
||||
let hint_texts = session.configured_multi_agent_v2_usage_hint_texts().await;
|
||||
|
||||
assert_eq!(
|
||||
hint_texts,
|
||||
vec![
|
||||
"Root guidance.".to_string(),
|
||||
"Subagent guidance.".to_string()
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_multi_agent_v2_usage_hint_texts_omit_effectively_disabled_feature() {
|
||||
let (mut session, _turn_context) =
|
||||
make_multi_agent_v2_usage_hint_test_session(/*enable_multi_agent_v2*/ true).await;
|
||||
Arc::get_mut(&mut session)
|
||||
.expect("session should not be shared")
|
||||
.features = Features::with_defaults().into();
|
||||
|
||||
let hint_texts = session.configured_multi_agent_v2_usage_hint_texts().await;
|
||||
|
||||
assert_eq!(hint_texts, Vec::<String>::new());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_omits_default_image_save_location_with_image_history() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
||||
Reference in New Issue
Block a user