From c32c445f1cb7542f5bd69f6ffed5ab3159e188c9 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Wed, 11 Mar 2026 11:22:25 -0700 Subject: [PATCH] Clarify locked role settings in spawn prompt (#14283) - tell agents when a role pins model or reasoning effort so they know those settings are not changeable - add prompt-builder coverage for the locked-setting notes --- codex-rs/core/src/agent/role.rs | 92 +++++++++++++++++- .../tests/suite/subagent_notifications.rs | 93 +++++++++++++++++++ 2 files changed, 180 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 878f8fc84..23b60583e 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -180,17 +180,49 @@ pub(crate) mod spawn_tool_spec { } format!( - r#"Optional type name for the new agent. If omitted, `{DEFAULT_ROLE_NAME}` is used. -Available roles: -{} - "#, + "Optional type name for the new agent. If omitted, `{DEFAULT_ROLE_NAME}` is used.\nAvailable roles:\n{}", formatted_roles.join("\n"), ) } fn format_role(name: &str, declaration: &AgentRoleConfig) -> String { if let Some(description) = &declaration.description { - format!("{name}: {{\n{description}\n}}") + let locked_settings_note = declaration + .config_file + .as_ref() + .and_then(|config_file| { + built_in::config_file_contents(config_file) + .map(str::to_owned) + .or_else(|| std::fs::read_to_string(config_file).ok()) + }) + .and_then(|contents| toml::from_str::(&contents).ok()) + .map(|role_toml| { + let model = role_toml + .get("model") + .and_then(TomlValue::as_str); + let reasoning_effort = role_toml + .get("model_reasoning_effort") + .and_then(TomlValue::as_str); + + match (model, reasoning_effort) { + (Some(model), Some(reasoning_effort)) => format!( + "\n- This role's model is set to `{model}` and its reasoning effort is set to `{reasoning_effort}`. These settings cannot be changed." + ), + (Some(model), None) => { + format!( + "\n- This role's model is set to `{model}` and cannot be changed." + ) + } + (None, Some(reasoning_effort)) => { + format!( + "\n- This role's reasoning effort is set to `{reasoning_effort}` and cannot be changed." + ) + } + (None, None) => String::new(), + } + }) + .unwrap_or_default(); + format!("{name}: {{\n{description}{locked_settings_note}\n}}") } else { format!("{name}: no description") } @@ -901,6 +933,56 @@ enabled = false assert!(user_index < built_in_index); } + #[test] + fn spawn_tool_spec_marks_role_locked_model_and_reasoning_effort() { + let tempdir = TempDir::new().expect("create temp dir"); + let role_path = tempdir.path().join("researcher.toml"); + fs::write( + &role_path, + "developer_instructions = \"Research carefully\"\nmodel = \"gpt-5\"\nmodel_reasoning_effort = \"high\"\n", + ) + .expect("write role config"); + let user_defined_roles = BTreeMap::from([( + "researcher".to_string(), + AgentRoleConfig { + description: Some("Research carefully.".to_string()), + config_file: Some(role_path), + nickname_candidates: None, + }, + )]); + + let spec = spawn_tool_spec::build(&user_defined_roles); + + assert!(spec.contains( + "Research carefully.\n- This role's model is set to `gpt-5` and its reasoning effort is set to `high`. These settings cannot be changed." + )); + } + + #[test] + fn spawn_tool_spec_marks_role_locked_reasoning_effort_only() { + let tempdir = TempDir::new().expect("create temp dir"); + let role_path = tempdir.path().join("reviewer.toml"); + fs::write( + &role_path, + "developer_instructions = \"Review carefully\"\nmodel_reasoning_effort = \"medium\"\n", + ) + .expect("write role config"); + let user_defined_roles = BTreeMap::from([( + "reviewer".to_string(), + AgentRoleConfig { + description: Some("Review carefully.".to_string()), + config_file: Some(role_path), + nickname_candidates: None, + }, + )]); + + let spec = spawn_tool_spec::build(&user_defined_roles); + + assert!(spec.contains( + "Review carefully.\n- This role's reasoning effort is set to `medium` and cannot be changed." + )); + } + #[test] fn built_in_config_file_contents_resolves_explorer_only() { assert_eq!( diff --git a/codex-rs/core/tests/suite/subagent_notifications.rs b/codex-rs/core/tests/suite/subagent_notifications.rs index b56f84d30..895997579 100644 --- a/codex-rs/core/tests/suite/subagent_notifications.rs +++ b/codex-rs/core/tests/suite/subagent_notifications.rs @@ -63,6 +63,44 @@ fn has_subagent_notification(req: &ResponsesRequest) -> bool { .any(|text| text.contains("")) } +fn tool_parameter_description( + req: &ResponsesRequest, + tool_name: &str, + parameter_name: &str, +) -> Option { + req.body_json() + .get("tools") + .and_then(serde_json::Value::as_array) + .and_then(|tools| { + tools.iter().find_map(|tool| { + if tool.get("name").and_then(serde_json::Value::as_str) == Some(tool_name) { + tool.get("parameters") + .and_then(|parameters| parameters.get("properties")) + .and_then(|properties| properties.get(parameter_name)) + .and_then(|parameter| parameter.get("description")) + .and_then(serde_json::Value::as_str) + .map(str::to_owned) + } else { + None + } + }) + }) +} + +fn role_block(description: &str, role_name: &str) -> Option { + let role_header = format!("{role_name}: {{"); + let mut lines = description.lines().skip_while(|line| *line != role_header); + let first_line = lines.next()?; + let mut block = vec![first_line]; + for line in lines { + if line.ends_with(": {") { + break; + } + block.push(line); + } + Some(block.join("\n")) +} + async fn wait_for_spawned_thread_id(test: &TestCodex) -> Result { let deadline = Instant::now() + Duration::from_secs(2); loop { @@ -435,3 +473,58 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() -> 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(())); + + let server = start_mock_server().await; + let resp_mock = mount_sse_once_match( + &server, + |req: &wiremock::Request| body_contains(req, TURN_1_PROMPT), + sse(vec![ + ev_response_created("resp-turn1-1"), + ev_assistant_message("msg-turn1-1", "done"), + ev_completed("resp-turn1-1"), + ]), + ) + .await; + + let mut builder = test_codex().with_config(|config| { + config + .features + .enable(Feature::Collab) + .expect("test config should allow feature update"); + let role_path = config.codex_home.join("custom-role.toml"); + std::fs::write( + &role_path, + format!( + "developer_instructions = \"Stay focused\"\nmodel = \"{ROLE_MODEL}\"\nmodel_reasoning_effort = \"{ROLE_REASONING_EFFORT}\"\n", + ), + ) + .expect("write role config"); + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: Some("Custom role".to_string()), + config_file: Some(role_path), + nickname_candidates: None, + }, + ); + }); + let test = builder.build(&server).await?; + + test.submit_turn(TURN_1_PROMPT).await?; + + let request = resp_mock.single_request(); + let agent_type_description = tool_parameter_description(&request, "spawn_agent", "agent_type") + .expect("spawn_agent agent_type description"); + let custom_role_description = + role_block(&agent_type_description, "custom").expect("custom role description"); + assert_eq!( + custom_role_description, + "custom: {\nCustom role\n- This role's model is set to `gpt-5.1-codex-max` and its reasoning effort is set to `high`. These settings cannot be changed.\n}" + ); + + Ok(()) +}