clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 14:45:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Reject role-locked spawn model overrides

Browse Source
This commit is contained in:
Ahmed Ibrahim
2026-03-10 17:15:12 -07:00
parent 16daab66d9
commit 5a16ac4ed1
4 changed files with 507 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

273
codex-rs/core/src/agent/role.rs
View File

@@ -19,6 +19,7 @@ use codex_app_server_protocol::ConfigLayerSource;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::path::Path;
use std::path::PathBuf;
use std::sync::LazyLock;
use toml::Value as TomlValue;
@@ -26,6 +27,12 @@ use toml::Value as TomlValue;
pub const DEFAULT_ROLE_NAME: &str = "default";
const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not available";
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) struct RoleSettingsLocks {
pub(crate) model: Option<String>,
pub(crate) reasoning_effort: Option<String>,
}
/// Applies a named role layer to `config` while preserving caller-owned model selection.
///
/// The role layer is inserted at session-flag precedence so it can override persisted config, but
@@ -39,46 +46,13 @@ pub(crate) async fn apply_role_to_config(
role_name: Option<&str>,
) -> Result<(), String> {
let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME);
let is_built_in = !config.agent_roles.contains_key(role_name);
let (config_file, is_built_in) = resolve_role_config(config, role_name)
.map(|role| (&role.config_file, is_built_in))
.ok_or_else(|| format!("unknown agent_type '{role_name}'"))?;
let Some(config_file) = config_file.as_ref() else {
let Some(role) = resolve_role_config(config, role_name) else {
return Err(format!("unknown agent_type '{role_name}'"));
};
if role.config_file.is_none() {
return Ok(());
};
let (role_config_toml, role_config_base) = if is_built_in {
let role_config_contents = built_in::config_file_contents(config_file)
.map(str::to_owned)
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml: TomlValue = toml::from_str(&role_config_contents)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
(role_config_toml, config.codex_home.as_path())
} else {
let role_config_contents = tokio::fs::read_to_string(config_file)
.await
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml = parse_agent_role_file_contents(
&role_config_contents,
config_file,
config_file
.parent()
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?,
Some(role_name),
)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?
.config;
(
role_config_toml,
config_file
.parent()
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?,
)
};
deserialize_config_toml_with_base(role_config_toml.clone(), role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_layer_toml = resolve_relative_paths_in_config_toml(role_config_toml, role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
}
let role_layer_toml = load_role_layer_toml(config, role_name).await?;
let role_selects_provider = role_layer_toml.get("model_provider").is_some();
let role_selects_profile = role_layer_toml.get("profile").is_some();
let role_updates_active_profile_provider = config
@@ -142,6 +116,25 @@ pub(crate) async fn apply_role_to_config(
Ok(())
}
pub(crate) async fn role_settings_locks(
config: &Config,
role_name: Option<&str>,
) -> Result<RoleSettingsLocks, String> {
let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME);
let role_layer_toml = load_role_layer_toml(config, role_name).await?;
Ok(RoleSettingsLocks {
model: role_layer_toml
.get("model")
.and_then(TomlValue::as_str)
.map(str::to_string),
reasoning_effort: role_layer_toml
.get("model_reasoning_effort")
.and_then(TomlValue::as_str)
.map(str::to_string),
})
}
pub(crate) fn resolve_role_config<'a>(
config: &'a Config,
role_name: &str,
@@ -152,6 +145,60 @@ pub(crate) fn resolve_role_config<'a>(
.or_else(|| built_in::configs().get(role_name))
}
async fn load_role_layer_toml(config: &Config, role_name: &str) -> Result<TomlValue, String> {
let is_built_in = !config.agent_roles.contains_key(role_name);
let (config_file, is_built_in) = resolve_role_config(config, role_name)
.map(|role| (&role.config_file, is_built_in))
.ok_or_else(|| format!("unknown agent_type '{role_name}'"))?;
let Some(config_file) = config_file.as_ref() else {
return Ok(TomlValue::Table(Default::default()));
};
let (role_config_toml, role_config_base) = read_role_config_source(
config.codex_home.as_path(),
config_file,
is_built_in,
role_name,
)
.await?;
deserialize_config_toml_with_base(role_config_toml.clone(), role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
resolve_relative_paths_in_config_toml(role_config_toml, role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())
}
async fn read_role_config_source<'a>(
codex_home: &'a Path,
config_file: &'a PathBuf,
is_built_in: bool,
role_name: &str,
) -> Result<(TomlValue, &'a Path), String> {
if is_built_in {
let role_config_contents = built_in::config_file_contents(config_file)
.map(str::to_owned)
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml: TomlValue = toml::from_str(&role_config_contents)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
Ok((role_config_toml, codex_home))
} else {
let role_config_base = config_file
.parent()
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_contents = tokio::fs::read_to_string(config_file)
.await
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml = parse_agent_role_file_contents(
&role_config_contents,
config_file,
role_config_base,
Some(role_name),
)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?
.config;
Ok((role_config_toml, role_config_base))
}
}
pub(crate) mod spawn_tool_spec {
use super::*;
@@ -170,12 +217,12 @@ pub(crate) mod spawn_tool_spec {
let mut formatted_roles = Vec::new();
for (name, declaration) in user_defined_roles {
if seen.insert(name.as_str()) {
formatted_roles.push(format_role(name, declaration));
formatted_roles.push(format_role(name, declaration, false));
}
}
for (name, declaration) in built_in_roles {
if seen.insert(name.as_str()) {
formatted_roles.push(format_role(name, declaration));
formatted_roles.push(format_role(name, declaration, true));
}
}
@@ -188,13 +235,92 @@ Available roles:
)
}
fn format_role(name: &str, declaration: &AgentRoleConfig) -> String {
fn format_role(name: &str, declaration: &AgentRoleConfig, is_built_in: bool) -> String {
let locks = declaration
.config_file
.as_ref()
.and_then(|config_file| {
locks_from_role_config_path(name, config_file, is_built_in).ok()
})
.filter(|locks| locks.model.is_some() || locks.reasoning_effort.is_some());
let lock_lines = locks
.as_ref()
.map(render_lock_lines)
.filter(|lines| !lines.is_empty());
if let Some(description) = &declaration.description {
format!("{name}: {{\n{description}\n}}")
match lock_lines {
Some(lock_lines) => format!("{name}: {{\n{description}\n{lock_lines}\n}}"),
None => format!("{name}: {{\n{description}\n}}"),
}
} else {
format!("{name}: no description")
match lock_lines {
Some(lock_lines) => format!("{name}: {{\n{lock_lines}\n}}"),
None => format!("{name}: no description"),
}
}
}
fn locks_from_role_config_path(
role_name: &str,
config_file: &Path,
is_built_in: bool,
) -> Result<RoleSettingsLocks, String> {
let (role_config_toml, role_config_base) = if is_built_in {
let role_config_contents = built_in::config_file_contents(config_file)
.map(str::to_owned)
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml: TomlValue = toml::from_str(&role_config_contents)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
(role_config_toml, Path::new("."))
} else {
let role_config_base = config_file
.parent()
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_contents = std::fs::read_to_string(config_file)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml = parse_agent_role_file_contents(
&role_config_contents,
config_file,
role_config_base,
Some(role_name),
)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?
.config;
(role_config_toml, role_config_base)
};
deserialize_config_toml_with_base(role_config_toml.clone(), role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_layer_toml =
resolve_relative_paths_in_config_toml(role_config_toml, role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
Ok(RoleSettingsLocks {
model: role_layer_toml
.get("model")
.and_then(TomlValue::as_str)
.map(str::to_string),
reasoning_effort: role_layer_toml
.get("model_reasoning_effort")
.and_then(TomlValue::as_str)
.map(str::to_string),
})
}
fn render_lock_lines(locks: &RoleSettingsLocks) -> String {
let mut lines = Vec::new();
if let Some(model) = locks.model.as_deref() {
lines.push(format!(
"- This role sets model to `{model}`. This is set per this role and cannot be changed."
));
}
if let Some(reasoning_effort) = locks.reasoning_effort.as_deref() {
lines.push(format!(
"- This role sets reasoning effort to `{reasoning_effort}`. This is set per this role and cannot be changed."
));
}
lines.join("\n")
}
}
mod built_in {
@@ -901,6 +1027,65 @@ enabled = false
assert!(user_index < built_in_index);
}
#[tokio::test]
async fn role_settings_locks_reads_locked_model_and_reasoning_effort() {
let (home, mut config) = test_config_with_cli_overrides(Vec::new()).await;
let role_path = write_role_config(
&home,
"locked-role.toml",
"model = \"gpt-5.1\"\nmodel_reasoning_effort = \"high\"\n",
)
.await;
config.agent_roles.insert(
"custom".to_string(),
AgentRoleConfig {
description: None,
config_file: Some(role_path),
nickname_candidates: None,
},
);
let locks = role_settings_locks(&config, Some("custom"))
.await
.expect("role locks should load");
assert_eq!(
locks,
RoleSettingsLocks {
model: Some("gpt-5.1".to_string()),
reasoning_effort: Some("high".to_string()),
}
);
}
#[test]
fn spawn_tool_spec_includes_lock_notices_for_user_defined_roles() {
let temp_dir = tempfile::tempdir().expect("create temp dir");
let role_path = temp_dir.path().join("custom-role.toml");
std::fs::write(
&role_path,
"model = \"gpt-5.1\"\nmodel_reasoning_effort = \"high\"\n",
)
.expect("write role config");
let user_defined_roles = BTreeMap::from([(
"custom".to_string(),
AgentRoleConfig {
description: None,
config_file: Some(role_path),
nickname_candidates: None,
},
)]);
let spec = spawn_tool_spec::build(&user_defined_roles);
assert!(spec.contains(
"This role sets model to `gpt-5.1`. This is set per this role and cannot be changed."
));
assert!(spec.contains(
"This role sets reasoning effort to `high`. This is set per this role and cannot be changed."
));
}
#[test]
fn built_in_config_file_contents_resolves_explorer_only() {
assert_eq!(

55
codex-rs/core/src/tools/handlers/multi_agents.rs
View File

@@ -105,7 +105,9 @@ mod spawn {
use super::*;
use crate::agent::control::SpawnAgentOptions;
use crate::agent::role::DEFAULT_ROLE_NAME;
use crate::agent::role::RoleSettingsLocks;
use crate::agent::role::apply_role_to_config;
use crate::agent::role::role_settings_locks;
use crate::agent::exceeds_thread_spawn_depth_limit;
use crate::agent::next_thread_spawn_depth;
@@ -140,6 +142,9 @@ mod spawn {
.as_deref()
.map(str::trim)
.filter(|role| !role.is_empty());
let mut config =
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
reject_role_locked_spawn_overrides(&config, role_name, &args).await?;
let input_items = parse_collab_input(args.message, args.items)?;
let prompt = input_preview(&input_items);
let session_source = turn.session_source.clone();
@@ -161,8 +166,6 @@ mod spawn {
.into(),
)
.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(),
@@ -241,6 +244,54 @@ mod spawn {
Ok(FunctionToolOutput::from_text(content, Some(true)))
}
async fn reject_role_locked_spawn_overrides(
config: &Config,
role_name: Option<&str>,
args: &SpawnAgentArgs,
) -> Result<(), FunctionCallError> {
let Some(role_name) = role_name else {
return Ok(());
};
let locks = role_settings_locks(config, Some(role_name))
.await
.map_err(FunctionCallError::RespondToModel)?;
let rejection = build_locked_override_rejection(role_name, &locks, args);
if let Some(message) = rejection {
return Err(FunctionCallError::RespondToModel(message));
}
Ok(())
}
fn build_locked_override_rejection(
role_name: &str,
locks: &RoleSettingsLocks,
args: &SpawnAgentArgs,
) -> Option<String> {
let mut locked_fields = Vec::new();
if let Some(model) = locks.model.as_deref()
&& args.model.is_some()
{
locked_fields.push(format!(
"`model` is set to `{model}` per the `{role_name}` role and cannot be changed; omit `model` from this spawn_agent call."
));
}
if let Some(reasoning_effort) = locks.reasoning_effort.as_deref()
&& args.reasoning_effort.is_some()
{
locked_fields.push(format!(
"`reasoning_effort` is set to `{reasoning_effort}` per the `{role_name}` role and cannot be changed; omit `reasoning_effort` from this spawn_agent call."
));
}
if locked_fields.is_empty() {
None
} else {
Some(locked_fields.join(" "))
}
}
}
mod send_input {

81
codex-rs/core/tests/suite/collaboration_instructions.rs
View File

@@ -1,4 +1,6 @@
use anyhow::Result;
use codex_core::config::AgentRoleConfig;
use codex_core::features::Feature;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Settings;
@@ -17,6 +19,7 @@ use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use serde_json::Value;
use std::fs;
fn collab_mode_with_mode_and_instructions(
mode: ModeKind,
@@ -63,6 +66,25 @@ fn count_exact(texts: &[String], target: &str) -> usize {
texts.iter().filter(|text| text.as_str() == target).count()
}
fn spawn_agent_type_description(body: &Value) -> Option<&str> {
body.get("tools")?.as_array()?.iter().find_map(|tool| {
let name = tool
.get("name")
.and_then(Value::as_str)
.or_else(|| tool.get("function")?.get("name")?.as_str());
if name == Some("spawn_agent") {
tool.get("parameters")
.or_else(|| tool.get("function")?.get("parameters"))
.and_then(|parameters| parameters.get("properties"))
.and_then(|properties| properties.get("agent_type"))
.and_then(|agent_type| agent_type.get("description"))
.and_then(Value::as_str)
} else {
None
}
})
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn no_collaboration_instructions_by_default() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -770,3 +792,62 @@ async fn empty_collaboration_instructions_are_ignored() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn spawn_agent_tool_description_mentions_role_locked_model_and_reasoning() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let req = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
)
.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("custom-role.toml");
fs::write(
&role_path,
"model = \"gpt-5.1\"\nmodel_reasoning_effort = \"high\"\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,
},
);
})
.build(&server)
.await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "hello".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let request_body = req.single_request().body_json();
let description = spawn_agent_type_description(&request_body)
.expect("spawn_agent agent_type description should exist");
assert!(description.contains(
"This role sets model to `gpt-5.1`. This is set per this role and cannot be changed."
));
assert!(description.contains(
"This role sets reasoning effort to `high`. This is set per this role and cannot be changed."
));
Ok(())
}

147
codex-rs/core/tests/suite/subagent_notifications.rs
View File

@@ -18,6 +18,7 @@ use core_test_support::skip_if_no_network;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use std::time::Duration;
use tokio::time::Instant;
@@ -63,6 +64,17 @@ fn has_subagent_notification(req: &ResponsesRequest) -> bool {
.any(|text| text.contains("<subagent_notification>"))
}
fn call_output_text(req: &ResponsesRequest, call_id: &str) -> String {
let raw = req.function_call_output(call_id);
assert_eq!(
raw.get("call_id").and_then(Value::as_str),
Some(call_id),
"mismatched call_id in function_call_output"
);
req.function_call_output_text(call_id)
.unwrap_or_else(|| panic!("function_call_output text present"))
}
async fn wait_for_spawned_thread_id(test: &TestCodex) -> Result<String> {
let deadline = Instant::now() + Duration::from_secs(2);
loop {
@@ -395,7 +407,7 @@ async fn spawn_agent_requested_model_and_reasoning_override_inherited_settings_w
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() -> Result<()> {
async fn spawn_agent_role_sets_model_and_reasoning_without_explicit_override() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -404,8 +416,6 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() ->
json!({
"message": CHILD_PROMPT,
"agent_type": "custom",
"model": REQUESTED_MODEL,
"reasoning_effort": REQUESTED_REASONING_EFFORT,
}),
|builder| {
builder.with_config(|config| {
@@ -435,3 +445,134 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() ->
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn spawn_agent_rejects_requested_model_when_role_locks_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",
"model": REQUESTED_MODEL,
}))?;
mount_sse_once_match(
&server,
|req: &wiremock::Request| body_contains(req, TURN_1_PROMPT),
sse(vec![
ev_response_created("resp-turn1-1"),
ev_function_call(SPAWN_CALL_ID, "spawn_agent", &spawn_args),
ev_completed("resp-turn1-1"),
]),
)
.await;
let followup = mount_sse_once_match(
&server,
|req: &wiremock::Request| body_contains(req, SPAWN_CALL_ID),
sse(vec![
ev_response_created("resp-turn1-2"),
ev_assistant_message("msg-turn1-2", "parent done"),
ev_completed("resp-turn1-2"),
]),
)
.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!("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),
nickname_candidates: None,
},
);
});
let test = builder.build(&server).await?;
test.submit_turn(TURN_1_PROMPT).await?;
let output = call_output_text(&followup.single_request(), SPAWN_CALL_ID);
assert!(output.contains(
&format!(
"`model` is set to `{ROLE_MODEL}` per the `custom` role and cannot be changed; omit `model` from this spawn_agent call."
)
));
assert_eq!(test.thread_manager.list_thread_ids().await.len(), 1);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn spawn_agent_rejects_requested_reasoning_effort_when_role_locks_effort() -> 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",
"reasoning_effort": REQUESTED_REASONING_EFFORT,
}))?;
mount_sse_once_match(
&server,
|req: &wiremock::Request| body_contains(req, TURN_1_PROMPT),
sse(vec![
ev_response_created("resp-turn1-1"),
ev_function_call(SPAWN_CALL_ID, "spawn_agent", &spawn_args),
ev_completed("resp-turn1-1"),
]),
)
.await;
let followup = mount_sse_once_match(
&server,
|req: &wiremock::Request| body_contains(req, SPAWN_CALL_ID),
sse(vec![
ev_response_created("resp-turn1-2"),
ev_assistant_message("msg-turn1-2", "parent done"),
ev_completed("resp-turn1-2"),
]),
)
.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!("model_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 output = call_output_text(&followup.single_request(), SPAWN_CALL_ID);
assert!(output.contains(
&format!(
"`reasoning_effort` is set to `{ROLE_REASONING_EFFORT}` per the `custom` role and cannot be changed; omit `reasoning_effort` from this spawn_agent call."
)
));
assert_eq!(test.thread_manager.list_thread_ids().await.len(), 1);
Ok(())
}