From 776246c3f5f931a72ebe41f52ef719e3b413371d Mon Sep 17 00:00:00 2001 From: friel-openai Date: Mon, 13 Apr 2026 08:28:40 -0700 Subject: [PATCH] Make forked agent spawns keep parent model config (#17247) ## Summary When a `spawn_agent` call does a full-history fork, keep the parent's effective agent type and model configuration instead of applying child role/model overrides. This is the minimal config-inheritance slice of #16055. Prompt-cache key inheritance and MCP tool-surface stability are split into follow-up PRs. ## Design - Reject `agent_type`, `model`, and `reasoning_effort` for v1 `fork_context` spawns. - Reject `agent_type`, `model`, and `reasoning_effort` for v2 `fork_turns = "all"` spawns. - Keep v2 partial-history forks (`fork_turns = "N"`) configurable; requested model/reasoning overrides and role config still apply there. - Keep non-forked spawn behavior unchanged. ## Tests - `cargo +1.93.1 test -p codex-core spawn_agent_fork_context --lib` - `cargo +1.93.1 test -p codex-core multi_agent_v2_spawn_fork_turns --lib` - `cargo +1.93.1 test -p codex-core multi_agent_v2_spawn_partial_fork_turns_allows_agent_type_override --lib` --- .../src/tools/handlers/multi_agents/spawn.rs | 35 ++- .../src/tools/handlers/multi_agents_common.rs | 17 +- .../src/tools/handlers/multi_agents_tests.rs | 241 ++++++++++++++++++ .../tools/handlers/multi_agents_v2/spawn.rs | 30 ++- 4 files changed, 297 insertions(+), 26 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 8e4bfb5b5..523b1ed35 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -2,11 +2,10 @@ use super::*; use crate::agent::control::SpawnAgentForkMode; use crate::agent::control::SpawnAgentOptions; use crate::agent::control::render_input_preview; -use crate::agent::role::DEFAULT_ROLE_NAME; -use crate::agent::role::apply_role_to_config; - use crate::agent::exceeds_thread_spawn_depth_limit; use crate::agent::next_thread_spawn_depth; +use crate::agent::role::DEFAULT_ROLE_NAME; +use crate::agent::role::apply_role_to_config; pub(crate) struct Handler; @@ -61,17 +60,25 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; - apply_requested_spawn_agent_model_overrides( - &session, - turn.as_ref(), - &mut config, - args.model.as_deref(), - args.reasoning_effort, - ) - .await?; - apply_role_to_config(&mut config, role_name) - .await - .map_err(FunctionCallError::RespondToModel)?; + if args.fork_context { + reject_full_fork_spawn_overrides( + role_name, + args.model.as_deref(), + args.reasoning_effort, + )?; + } else { + apply_requested_spawn_agent_model_overrides( + &session, + turn.as_ref(), + &mut config, + args.model.as_deref(), + args.reasoning_effort, + ) + .await?; + apply_role_to_config(&mut config, role_name) + .await + .map_err(FunctionCallError::RespondToModel)?; + } apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?; apply_spawn_agent_overrides(&mut config, child_depth); diff --git a/codex-rs/core/src/tools/handlers/multi_agents_common.rs b/codex-rs/core/src/tools/handlers/multi_agents_common.rs index 2078c229b..9c2740d48 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -225,7 +225,9 @@ fn build_agent_shared_config(turn: &TurnContext) -> Result Result, + model: Option<&str>, + reasoning_effort: Option, +) -> Result<(), FunctionCallError> { + if agent_type.is_some() || model.is_some() || reasoning_effort.is_some() { + return Err(FunctionCallError::RespondToModel( + "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + )); + } + Ok(()) +} + /// Copies runtime-only turn state onto a child config before it is handed to `AgentControl`. /// /// These values are chosen by the live turn rather than persisted config, so leaving them stale diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 733d99853..9eb0679bf 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -2,6 +2,7 @@ use super::*; use crate::CodexThread; use crate::ThreadManager; use crate::codex::make_session_and_context; +use crate::config::AgentRoleConfig; use crate::config::DEFAULT_AGENT_MAX_DEPTH; use crate::function_tool::FunctionCallError; use crate::session_prefix::format_subagent_notification_message; @@ -28,6 +29,7 @@ use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; +use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AgentStatus; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -89,6 +91,36 @@ fn thread_manager() -> ThreadManager { ) } +async fn install_role_with_model_override(turn: &mut TurnContext) -> String { + let role_name = "fork-context-role".to_string(); + tokio::fs::create_dir_all(&turn.config.codex_home) + .await + .expect("codex home should be created"); + let role_config_path = turn.config.codex_home.join("fork-context-role.toml"); + tokio::fs::write( + &role_config_path, + r#"model = "gpt-5-role-override" +model_provider = "ollama" +model_reasoning_effort = "minimal" +"#, + ) + .await + .expect("role config should be written"); + + let mut config = (*turn.config).clone(); + config.agent_roles.insert( + role_name.clone(), + AgentRoleConfig { + description: Some("Role with model overrides".to_string()), + config_file: Some(role_config_path), + nickname_candidates: None, + }, + ); + turn.config = Arc::new(config); + + role_name +} + fn history_contains_inter_agent_communication( history_items: &[ResponseItem], expected: &InterAgentCommunication, @@ -365,6 +397,215 @@ async fn spawn_agent_uses_explorer_role_and_preserves_approval_policy() { assert_eq!(snapshot.model_provider_id, "ollama"); } +#[tokio::test] +async fn spawn_agent_fork_context_rejects_agent_type_override() { + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_model_override(&mut turn).await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let err = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "agent_type": role_name, + "fork_context": true + })), + )) + .await + .expect_err("fork_context should reject agent_type overrides"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + ) + ); +} + +#[tokio::test] +async fn spawn_agent_fork_context_rejects_child_model_overrides() { + let (mut session, turn) = make_session_and_context().await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + + let err = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "model": "gpt-5-child-override", + "reasoning_effort": "low", + "fork_context": true + })), + )) + .await + .expect_err("forked spawn should reject child model overrides"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + ) + ); +} + +#[tokio::test] +async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() { + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_model_override(&mut turn).await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + let turn = TurnContext { + config: Arc::new(config), + ..turn + }; + + let err = SpawnAgentHandlerV2 + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "task_name": "fork_context_v2", + "agent_type": role_name, + "fork_turns": "all" + })), + )) + .await + .expect_err("fork_turns=all should reject agent_type overrides"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + ) + ); +} + +#[tokio::test] +async fn multi_agent_v2_spawn_fork_turns_rejects_child_model_overrides() { + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + turn.config = Arc::new(config); + + let err = SpawnAgentHandlerV2 + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "task_name": "fork_context_v2", + "model": "gpt-5-child-override", + "reasoning_effort": "low", + "fork_turns": "all" + })), + )) + .await + .expect_err("forked spawn should reject child model overrides"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + ) + ); +} + +#[tokio::test] +async fn multi_agent_v2_spawn_partial_fork_turns_allows_agent_type_override() { + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_model_override(&mut turn).await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + let turn = TurnContext { + config: Arc::new(config), + ..turn + }; + + let output = SpawnAgentHandlerV2 + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "task_name": "partial_fork", + "agent_type": role_name, + "fork_turns": "1" + })), + )) + .await + .expect("partial fork should allow agent_type overrides"); + let (content, _) = expect_text_output(output); + let result: serde_json::Value = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + assert_eq!(result["task_name"], "/root/partial_fork"); + let agent_id = manager + .captured_ops() + .into_iter() + .map(|(thread_id, _)| thread_id) + .find(|thread_id| *thread_id != root.thread_id) + .expect("spawned agent should receive an op"); + let snapshot = manager + .get_thread(agent_id) + .await + .expect("spawned agent thread should exist") + .config_snapshot() + .await; + + assert_eq!(snapshot.model, "gpt-5-role-override"); + assert_eq!(snapshot.model_provider_id, "ollama"); + assert_eq!(snapshot.reasoning_effort, Some(ReasoningEffort::Minimal)); +} + #[tokio::test] async fn spawn_agent_returns_agent_id_without_task_name() { let (mut session, turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 3c475e790..03287b20a 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -70,17 +70,25 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; - apply_requested_spawn_agent_model_overrides( - &session, - turn.as_ref(), - &mut config, - args.model.as_deref(), - args.reasoning_effort, - ) - .await?; - apply_role_to_config(&mut config, role_name) - .await - .map_err(FunctionCallError::RespondToModel)?; + if matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory)) { + reject_full_fork_spawn_overrides( + role_name, + args.model.as_deref(), + args.reasoning_effort, + )?; + } else { + apply_requested_spawn_agent_model_overrides( + &session, + turn.as_ref(), + &mut config, + args.model.as_deref(), + args.reasoning_effort, + ) + .await?; + apply_role_to_config(&mut config, role_name) + .await + .map_err(FunctionCallError::RespondToModel)?; + } apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?; apply_spawn_agent_overrides(&mut config, child_depth); config.developer_instructions = Some(