mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Validate reasoning effort after applying spawn roles (#33656)
## Why Agent roles can override the model or reasoning effort after the initial `spawn_agent` configuration is validated, leaving an unsupported combination unchecked. ## What changed - Validate the final reasoning effort against model metadata whenever a role changes the spawned agent's model settings. - Preserve independently configured subagent defaults when a role does not override them. - Apply the same role validation in both multi-agent spawn implementations. ## Testing Added coverage for independent model and reasoning defaults, instruction-only roles, and rejection of a reasoning effort unsupported by a role-selected model. GitOrigin-RevId: 1885314e9380f778603f067fc04a6225ee963428
This commit is contained in:
@@ -5,7 +5,6 @@ use crate::agent::control::render_input_preview;
|
||||
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;
|
||||
use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions;
|
||||
use crate::tools::handlers::multi_agents_spec::create_spawn_agent_tool_v1;
|
||||
use codex_tools::ToolSpec;
|
||||
@@ -103,9 +102,7 @@ async fn handle_spawn_agent(
|
||||
)
|
||||
.await?;
|
||||
if !args.fork_context {
|
||||
apply_role_to_config(&mut config, role_name)
|
||||
.await
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
apply_spawn_agent_role(&session, &mut config, role_name).await?;
|
||||
}
|
||||
apply_spawn_agent_service_tier(
|
||||
&session,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::agent::role::apply_role_to_config;
|
||||
use crate::config::Config;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::config::HARD_MAX_MULTI_AGENT_V2_TIMEOUT_MS;
|
||||
@@ -353,6 +354,46 @@ pub(crate) async fn apply_spawn_agent_service_tier(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn apply_spawn_agent_role(
|
||||
session: &Session,
|
||||
config: &mut Config,
|
||||
role_name: Option<&str>,
|
||||
) -> Result<(), FunctionCallError> {
|
||||
let previous_model = config.model.clone();
|
||||
let previous_reasoning_effort = config.model_reasoning_effort.clone();
|
||||
apply_role_to_config(config, role_name)
|
||||
.await
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
if config.model == previous_model && config.model_reasoning_effort == previous_reasoning_effort
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(reasoning_effort) = config.model_reasoning_effort.clone() else {
|
||||
return Ok(());
|
||||
};
|
||||
let model = config.model.clone().ok_or_else(|| {
|
||||
FunctionCallError::RespondToModel(
|
||||
"spawn_agent could not resolve the child model for reasoning effort validation"
|
||||
.to_string(),
|
||||
)
|
||||
})?;
|
||||
let model_info = session
|
||||
.services
|
||||
.models_manager
|
||||
.get_model_info(&model, &config.to_models_manager_config())
|
||||
.await;
|
||||
if model_info.used_fallback_model_metadata {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
validate_spawn_agent_reasoning_effort(
|
||||
&model,
|
||||
&model_info.supported_reasoning_levels,
|
||||
&reasoning_effort,
|
||||
)
|
||||
}
|
||||
|
||||
fn find_spawn_agent_model_name(
|
||||
available_models: &[ModelPreset],
|
||||
requested_model: &str,
|
||||
|
||||
@@ -3,7 +3,6 @@ use crate::agent::control::SpawnAgentForkMode;
|
||||
use crate::agent::control::SpawnAgentOptions;
|
||||
use crate::agent::next_thread_spawn_depth;
|
||||
use crate::agent::role::DEFAULT_ROLE_NAME;
|
||||
use crate::agent::role::apply_role_to_config;
|
||||
use crate::agent_communication::AgentCommunicationContext;
|
||||
use crate::agent_communication::AgentCommunicationKind;
|
||||
use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions;
|
||||
@@ -77,9 +76,7 @@ async fn handle_spawn_agent(
|
||||
)
|
||||
.await?;
|
||||
if !is_full_history_fork {
|
||||
apply_role_to_config(&mut config, role_name)
|
||||
.await
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
apply_spawn_agent_role(&session, &mut config, role_name).await?;
|
||||
}
|
||||
apply_spawn_agent_service_tier(
|
||||
&session,
|
||||
|
||||
@@ -471,7 +471,7 @@ async fn setup_turn_one_with_custom_spawned_child(
|
||||
config.model = Some(INHERITED_MODEL.to_string());
|
||||
config.model_reasoning_effort = Some(INHERITED_REASONING_EFFORT);
|
||||
}));
|
||||
let test = builder.build(server).await?;
|
||||
let test = builder.build_with_auto_env(server).await?;
|
||||
test.submit_turn(TURN_1_PROMPT).await?;
|
||||
if child_response_delay.is_none() && wait_for_parent_notification {
|
||||
let _ = wait_for_requests(&child_request_log).await?;
|
||||
@@ -1162,6 +1162,47 @@ async fn spawn_agent_uses_configured_subagent_defaults() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(
|
||||
Some(REQUESTED_MODEL),
|
||||
None,
|
||||
REQUESTED_MODEL,
|
||||
Some(ReasoningEffort::Medium);
|
||||
"model only"
|
||||
)]
|
||||
#[test_case(
|
||||
None,
|
||||
Some(REQUESTED_REASONING_EFFORT),
|
||||
INHERITED_MODEL,
|
||||
Some(REQUESTED_REASONING_EFFORT);
|
||||
"reasoning effort only"
|
||||
)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawn_agent_uses_independent_configured_subagent_defaults(
|
||||
default_model: Option<&str>,
|
||||
default_reasoning_effort: Option<ReasoningEffort>,
|
||||
expected_model: &str,
|
||||
expected_reasoning_effort: Option<ReasoningEffort>,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let default_model = default_model.map(str::to_string);
|
||||
let child_snapshot =
|
||||
spawn_child_and_capture_snapshot(&server, json!({ "message": CHILD_PROMPT }), |builder| {
|
||||
builder.with_config(move |config| {
|
||||
config.agent_default_subagent_model = default_model;
|
||||
config.agent_default_subagent_reasoning_effort = default_reasoning_effort;
|
||||
})
|
||||
})
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
(child_snapshot.model, child_snapshot.reasoning_effort),
|
||||
(expected_model.to_string(), expected_reasoning_effort)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(true, false; "unsupported child")]
|
||||
#[test_case(false, true; "supported child")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -1719,6 +1760,119 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawn_agent_preserves_configured_defaults_through_unrelated_role() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let child_snapshot = spawn_child_and_capture_snapshot(
|
||||
&server,
|
||||
json!({
|
||||
"message": CHILD_PROMPT,
|
||||
"agent_type": "custom",
|
||||
}),
|
||||
|builder| {
|
||||
builder.with_config(|config| {
|
||||
let role_path = config.codex_home.join("instructions-only-role.toml");
|
||||
std::fs::write(&role_path, "developer_instructions = \"Stay focused\"\n")
|
||||
.expect("write role config");
|
||||
config.agent_roles.insert(
|
||||
"custom".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: Some("Custom role".to_string()),
|
||||
config_file: Some(role_path.to_path_buf()),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
);
|
||||
config.agent_default_subagent_model = Some(REQUESTED_MODEL.to_string());
|
||||
config.agent_default_subagent_reasoning_effort = Some(REQUESTED_REASONING_EFFORT);
|
||||
})
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
(child_snapshot.model, child_snapshot.reasoning_effort),
|
||||
(
|
||||
REQUESTED_MODEL.to_string(),
|
||||
Some(REQUESTED_REASONING_EFFORT)
|
||||
)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawn_agent_rejects_reasoning_effort_unsupported_by_role_model() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let spawn_args = serde_json::to_string(&json!({
|
||||
"message": CHILD_PROMPT,
|
||||
"agent_type": "custom",
|
||||
}))?;
|
||||
mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| body_contains(request, TURN_1_PROMPT),
|
||||
sse(vec![
|
||||
ev_response_created("resp-turn1-1"),
|
||||
ev_function_call_with_namespace(
|
||||
SPAWN_CALL_ID,
|
||||
MULTI_AGENT_V1_NAMESPACE,
|
||||
"spawn_agent",
|
||||
&spawn_args,
|
||||
),
|
||||
ev_completed("resp-turn1-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let tool_output = mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| body_contains(request, SPAWN_CALL_ID),
|
||||
sse(vec![
|
||||
ev_response_created("resp-turn1-2"),
|
||||
ev_completed("resp-turn1-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let test = test_codex()
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::Collab)
|
||||
.expect("test config should allow feature update");
|
||||
let role_path = config.codex_home.join("model-only-role.toml");
|
||||
std::fs::write(&role_path, format!("model = \"{ROLE_MODEL}\"\n"))
|
||||
.expect("write role config");
|
||||
config.agent_roles.insert(
|
||||
"custom".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: Some("Custom role".to_string()),
|
||||
config_file: Some(role_path.to_path_buf()),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
);
|
||||
config.agent_default_subagent_model = Some("gpt-5.6-sol".to_string());
|
||||
config.agent_default_subagent_reasoning_effort = Some(ReasoningEffort::Ultra);
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
|
||||
test.submit_turn(TURN_1_PROMPT).await?;
|
||||
|
||||
let (output, _) = tool_output
|
||||
.single_request()
|
||||
.function_call_output_content_and_success(SPAWN_CALL_ID)
|
||||
.expect("spawn_agent output");
|
||||
assert_eq!(
|
||||
output.as_deref(),
|
||||
Some(
|
||||
"Reasoning effort `ultra` is not supported for model `gpt-5.4`. Supported reasoning efforts: low, medium, high, xhigh"
|
||||
)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawn_agent_tool_description_mentions_role_locked_settings() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user