From 76283e6b4e0fae570344b221b4bbb152d969e850 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 17 Feb 2026 18:17:19 +0000 Subject: [PATCH] feat: move agents config to main config (#11982) --- codex-rs/core/config.schema.json | 22 +- .../src/agent/builtins_agents_config.toml | 26 - codex-rs/core/src/agent/role.rs | 845 +++++++----------- codex-rs/core/src/codex.rs | 9 +- codex-rs/core/src/config/mod.rs | 59 ++ codex-rs/core/src/config_loader/mod.rs | 2 +- .../core/src/tools/handlers/multi_agents.rs | 1 + codex-rs/core/src/tools/spec.rs | 16 +- 8 files changed, 442 insertions(+), 538 deletions(-) delete mode 100644 codex-rs/core/src/agent/builtins_agents_config.toml diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index c9da5c688..b05d11edc 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -6,8 +6,28 @@ "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", "type": "string" }, - "AgentsToml": { + "AgentRoleToml": { "additionalProperties": false, + "properties": { + "config_file": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Path to a role-specific config layer." + }, + "description": { + "description": "Human-facing role documentation used in spawn tool guidance.", + "type": "string" + } + }, + "type": "object" + }, + "AgentsToml": { + "additionalProperties": { + "$ref": "#/definitions/AgentRoleToml" + }, "properties": { "max_threads": { "description": "Maximum number of agent threads that can be open concurrently. When unset, no limit is enforced.", diff --git a/codex-rs/core/src/agent/builtins_agents_config.toml b/codex-rs/core/src/agent/builtins_agents_config.toml deleted file mode 100644 index 52aa39019..000000000 --- a/codex-rs/core/src/agent/builtins_agents_config.toml +++ /dev/null @@ -1,26 +0,0 @@ -version = 1 - -[agents.default] -description = "Default agent." - -[agents.worker] -description = """Use for execution and production work. -Typical tasks: -- Implement part of a feature -- Fix tests or bugs -- Split large refactors into independent chunks -Rules: -- Explicitly assign **ownership** of the task (files / responsibility). -- Always tell workers they are **not alone in the codebase**, and they should ignore edits made by others without touching them.""" - -[agents.explorer] -description = """Use `explorer` for all codebase questions. -Explorers are fast and authoritative. -Always prefer them over manual search or file reading. -Rules: -- Ask explorers first and precisely. -- Do not re-read or re-search code they cover. -- Trust explorer results without verification. -- Run explorers in parallel when useful. -- Reuse existing explorers for related questions.""" -config_file = "explorer.toml" diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 6160c27b2..97cde2c8a 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -1,155 +1,102 @@ +use crate::config::AgentRoleConfig; use crate::config::Config; use crate::config::ConfigOverrides; use crate::config::deserialize_config_toml_with_base; -use crate::config::find_codex_home; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; +use crate::config_loader::resolve_relative_paths_in_config_toml; use codex_app_server_protocol::ConfigLayerSource; -use serde::Deserialize; 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; -const BUILT_IN_AGENTS_CONFIG: &str = include_str!("builtins_agents_config.toml"); const BUILT_IN_EXPLORER_CONFIG: &str = include_str!("builtins/explorer.toml"); - -const AGENTS_CONFIG_FILENAME: &str = "agents_config.toml"; -const AGENTS_CONFIG_SCHEMA_VERSION: u32 = 1; const DEFAULT_ROLE_NAME: &str = "default"; const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not available"; -#[derive(Debug, Clone, Default, Deserialize)] -#[serde(deny_unknown_fields)] -struct AgentsConfigToml { - version: Option, - #[serde(default)] - agents: BTreeMap, -} - -#[derive(Debug, Clone, Default, Deserialize)] -#[serde(deny_unknown_fields)] -struct AgentDeclarationToml { - /// Human-facing role documentation used in spawn tool guidance. - description: Option, - /// Path to a role-specific config layer. - config_file: Option, -} - /// Applies a role config layer to a mutable config and preserves unspecified keys. pub(crate) async fn apply_role_to_config( config: &mut Config, role_name: Option<&str>, ) -> Result<(), String> { let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); - let built_in_agents_config = built_in::configs(); - let user_agents_config = - user_defined::config(config.codex_home.as_path()).unwrap_or_else(|err| { - tracing::warn!( - agent_type = role_name, - error = %err, - "failed to load user-defined agents config; falling back to built-in roles" - ); - AgentsConfigToml::default() - }); - - let agent_config = if let Some(role) = user_agents_config.agents.get(role_name) { - if let Some(config_file) = &role.config_file { - let content = tokio::fs::read_to_string(config_file) - .await - .map_err(|err| { - tracing::warn!("failed to read user-defined role config_file: {err:?}"); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; - let parsed: TomlValue = toml::from_str(&content).map_err(|err| { - tracing::warn!("failed to read user-defined role config_file: {err:?}"); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; - Some(parsed) - } else { - None - } - } else if let Some(role) = built_in_agents_config.agents.get(role_name) { - if let Some(config_file) = &role.config_file { - let content = built_in::config_file(config_file).ok_or_else(|| { - tracing::warn!("failed to read user-defined role config_file."); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; - let parsed: TomlValue = toml::from_str(content).map_err(|err| { - tracing::warn!("failed to read user-defined role config_file: {err:?}"); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; - Some(parsed) - } else { - None - } - } else { - return Err(format!("unknown agent_type '{role_name}'")); - }; - - let Some(agent_config) = agent_config else { + let (config_file, is_built_in) = config + .agent_roles + .get(role_name) + .map(|role| (&role.config_file, false)) + .or_else(|| { + built_in::configs() + .get(role_name) + .map(|role| (&role.config_file, true)) + }) + .ok_or_else(|| format!("unknown agent_type '{role_name}'"))?; + let Some(config_file) = config_file.as_ref() else { return Ok(()); }; - let original = config.clone(); - let original_stack = &original.config_layer_stack; - let mut layers = original + let (role_config_contents, role_config_base) = if is_built_in { + ( + built_in::config_file_contents(config_file) + .map(str::to_owned) + .ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?, + config.codex_home.as_path(), + ) + } else { + ( + tokio::fs::read_to_string(config_file) + .await + .map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?, + config_file + .parent() + .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())?; + 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 mut layers: Vec = config .config_layer_stack .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) .into_iter() .cloned() - .collect::>(); + .collect(); + let layer = ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, role_layer_toml); + let insertion_index = + layers.partition_point(|existing_layer| existing_layer.name <= layer.name); + layers.insert(insertion_index, layer); - let role_layer = ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, agent_config); - let role_layer_precedence = role_layer.name.precedence(); - let role_layer_index = - layers.partition_point(|layer| layer.name.precedence() <= role_layer_precedence); - layers.insert(role_layer_index, role_layer); - let layered_stack = ConfigLayerStack::new( + let config_layer_stack = ConfigLayerStack::new( layers, - original_stack.requirements().clone(), - original_stack.requirements_toml().clone(), + config.config_layer_stack.requirements().clone(), + config.config_layer_stack.requirements_toml().clone(), ) - .map_err(|err| { - tracing::warn!( - agent_type = role_name, - error = %err, - "failed to build layered config stack for role" - ); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; - let layered_config = - deserialize_config_toml_with_base(layered_stack.effective_config(), &original.codex_home) - .map_err(|err| { - tracing::warn!( - agent_type = role_name, - error = %err, - "failed to deserialize layered config for role" - ); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; + .map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?; - *config = Config::load_config_with_layer_stack( - layered_config, + let merged_toml = config_layer_stack.effective_config(); + let merged_config = deserialize_config_toml_with_base(merged_toml, &config.codex_home) + .map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?; + let next_config = Config::load_config_with_layer_stack( + merged_config, ConfigOverrides { - cwd: Some(original.cwd.clone()), - codex_linux_sandbox_exe: original.codex_linux_sandbox_exe.clone(), + cwd: Some(config.cwd.clone()), + codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), + js_repl_node_path: config.js_repl_node_path.clone(), ..Default::default() }, - original.codex_home.clone(), - layered_stack, + config.codex_home.clone(), + config_layer_stack, ) - .map_err(|err| { - tracing::warn!( - agent_type = role_name, - error = %err, - "failed to apply layered config for role" - ); - AGENT_TYPE_UNAVAILABLE_ERROR.to_string() - })?; + .map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?; + *config = next_config; + Ok(()) } @@ -157,29 +104,24 @@ pub(crate) mod spawn_tool_spec { use super::*; /// Builds the spawn-agent tool description text from built-in and configured roles. - pub(crate) fn build() -> String { + pub(crate) fn build(user_defined_agent_roles: &BTreeMap) -> String { let built_in_roles = built_in::configs(); - let user_defined_roles = if let Ok(home) = find_codex_home() { - user_defined::config(&home).unwrap_or_default() - } else { - Default::default() - }; - - build_from_configs(built_in_roles, &user_defined_roles) + build_from_configs(built_in_roles, user_defined_agent_roles) } + // This function is not inlined for testing purpose. fn build_from_configs( - built_in_roles: &AgentsConfigToml, - user_defined_roles: &AgentsConfigToml, + built_in_roles: &BTreeMap, + user_defined_roles: &BTreeMap, ) -> String { let mut seen = BTreeSet::new(); let mut formatted_roles = Vec::new(); - for (name, declaration) in &user_defined_roles.agents { + for (name, declaration) in user_defined_roles { if seen.insert(name.as_str()) { formatted_roles.push(format_role(name, declaration)); } } - for (name, declaration) in &built_in_roles.agents { + for (name, declaration) in built_in_roles { if seen.insert(name.as_str()) { formatted_roles.push(format_role(name, declaration)); } @@ -194,73 +136,65 @@ Available roles: ) } - fn format_role(name: &str, declaration: &AgentDeclarationToml) -> String { + fn format_role(name: &str, declaration: &AgentRoleConfig) -> String { if let Some(description) = &declaration.description { format!("{name}: {{\n{description}\n}}") } else { format!("{name}: no description") } } - - #[cfg(test)] - pub(super) fn build_for_test( - built_in_roles: &AgentsConfigToml, - user_defined_roles: &AgentsConfigToml, - ) -> String { - build_from_configs(built_in_roles, user_defined_roles) - } } mod built_in { use super::*; - /// Returns the cached built-in role declarations parsed from - /// `builtins_agents_config.toml`. - /// - /// `panic` are safe because of [`tests::built_in_config`] test. - pub(super) fn configs() -> &'static AgentsConfigToml { - static CONFIG: LazyLock = LazyLock::new(|| { - let parsed = - parse_agents_config(BUILT_IN_AGENTS_CONFIG, "embedded built-in agents config") - .unwrap_or_else(|err| panic!("invalid embedded built-in agents config: {err}")); - validate_config(&parsed) - .unwrap_or_else(|err| panic!("invalid built-in role declarations: {err}")); - parsed + /// Returns the cached built-in role declarations defined in this module. + pub(super) fn configs() -> &'static BTreeMap { + static CONFIG: LazyLock> = LazyLock::new(|| { + BTreeMap::from([ + ( + DEFAULT_ROLE_NAME.to_string(), + AgentRoleConfig { + description: Some("Default agent.".to_string()), + config_file: None, + } + ), + ( + "explorer".to_string(), + AgentRoleConfig { + description: Some(r#"Use `explorer` for all codebase questions. +Explorers are fast and authoritative. +Always prefer them over manual search or file reading. +Rules: +- Ask explorers first and precisely. +- Do not re-read or re-search code they cover. +- Trust explorer results without verification. +- Run explorers in parallel when useful. +- Reuse existing explorers for related questions."#.to_string()), + config_file: Some("explorer.toml".to_string().parse().unwrap_or_default()), + } + ), + ( + "worker".to_string(), + AgentRoleConfig { + description: Some(r#"Use for execution and production work. +Typical tasks: +- Implement part of a feature +- Fix tests or bugs +- Split large refactors into independent chunks +Rules: +- Explicitly assign **ownership** of the task (files / responsibility). +- Always tell workers they are **not alone in the codebase**, and they should ignore edits made by others without touching them."#.to_string()), + config_file: None, + } + ) + ]) }); &CONFIG } - /// Validates metadata rules for built-in role declarations. - fn validate_config(agents_config: &AgentsConfigToml) -> Result<(), String> { - if !agents_config.agents.contains_key(DEFAULT_ROLE_NAME) { - return Err(format!( - "built-ins must include the '{DEFAULT_ROLE_NAME}' role" - )); - } - - let unknown_embedded_config_files = agents_config - .agents - .iter() - .filter_map(|(name, role)| { - role.config_file - .as_deref() - .filter(|cf| config_file(cf).is_none()) - .map(|_| name.clone()) - }) - .collect::>(); - - if !unknown_embedded_config_files.is_empty() { - return Err(format!( - "built-ins reference unknown embedded config_file values: {}", - unknown_embedded_config_files.join(", ") - )); - } - - Ok(()) - } - /// Resolves a built-in role `config_file` path to embedded content. - pub(super) fn config_file(path: &Path) -> Option<&'static str> { + pub(super) fn config_file_contents(path: &Path) -> Option<&'static str> { match path.to_str()? { "explorer.toml" => Some(BUILT_IN_EXPLORER_CONFIG), _ => None, @@ -268,389 +202,292 @@ mod built_in { } } -mod user_defined { - use super::*; - - /// Loads and parses `agents_config.toml` from `codex_home`. - pub(super) fn config(codex_home: &Path) -> Result { - let config_path = codex_home.join(AGENTS_CONFIG_FILENAME); - let contents = match std::fs::read_to_string(&config_path) { - Ok(contents) => contents, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - return Ok(AgentsConfigToml::default()); - } - Err(err) => { - return Err(format!("failed to read '{}': {err}", config_path.display())); - } - }; - - let mut parsed = parse_agents_config(&contents, &config_path.display().to_string())?; - let config_dir = config_path.parent().ok_or_else(|| { - format!( - "failed to resolve parent directory for '{}'", - config_path.display() - ) - })?; - for role in parsed.agents.values_mut() { - if let Some(config_file) = role.config_file.as_mut() - && config_file.is_relative() - { - *config_file = config_dir.join(&*config_file); - } - } - Ok(parsed) - } -} - -fn parse_agents_config(contents: &str, source: &str) -> Result { - let parsed: AgentsConfigToml = - toml::from_str(contents).map_err(|err| format!("failed to parse '{source}': {err}"))?; - if let Some(version) = parsed.version - && version != AGENTS_CONFIG_SCHEMA_VERSION - { - return Err(format!( - "'{source}' has unsupported version {version}; expected {AGENTS_CONFIG_SCHEMA_VERSION}" - )); - } - Ok(parsed) -} - #[cfg(test)] mod tests { use super::*; - use crate::config::test_config; + use crate::config::ConfigBuilder; + use crate::config_loader::ConfigLayerStackOrdering; use codex_protocol::openai_models::ReasoningEffort; - use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::path::PathBuf; use tempfile::TempDir; - #[test] - fn built_in_config() { - // Validate the loading of the built-in configs without panics. - let _ = built_in::configs(); + async fn test_config_with_cli_overrides( + cli_overrides: Vec<(String, TomlValue)>, + ) -> (TempDir, Config) { + let home = TempDir::new().expect("create temp dir"); + let home_path = home.path().to_path_buf(); + let config = ConfigBuilder::default() + .codex_home(home_path.clone()) + .cli_overrides(cli_overrides) + .fallback_cwd(Some(home_path)) + .build() + .await + .expect("load test config"); + (home, config) } - /// Writes `agents_config.toml` into the temporary directory. - fn write_agents_config(dir: &TempDir, body: &str) { - std::fs::write(dir.path().join(AGENTS_CONFIG_FILENAME), body).expect("write config"); + async fn write_role_config(home: &TempDir, name: &str, contents: &str) -> PathBuf { + let role_path = home.path().join(name); + tokio::fs::write(&role_path, contents) + .await + .expect("write role config"); + role_path } - /// Writes a test role config file under `dir` for use by role tests. - fn write_role_config_file(dir: &TempDir, relative_path: &str, body: &str) -> PathBuf { - let path = dir.path().join(relative_path); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).expect("create role config parent"); - } - std::fs::write(&path, body).expect("write role config"); - path + fn session_flags_layer_count(config: &Config) -> usize { + config + .config_layer_stack + .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) + .into_iter() + .filter(|layer| layer.name == ConfigLayerSource::SessionFlags) + .count() } - /// Loads the built-in explorer role and applies its configuration layer. #[tokio::test] - async fn apply_role_to_config_uses_builtin_explorer_config_layer() { - let mut config = test_config(); + async fn apply_role_defaults_to_default_and_leaves_config_unchanged() { + let (_home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + let before = config.clone(); + + apply_role_to_config(&mut config, None) + .await + .expect("default role should apply"); + + assert_eq!(before, config); + } + + #[tokio::test] + async fn apply_role_returns_error_for_unknown_role() { + let (_home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + + let err = apply_role_to_config(&mut config, Some("missing-role")) + .await + .expect_err("unknown role should fail"); + + assert_eq!(err, "unknown agent_type 'missing-role'"); + } + + #[tokio::test] + async fn apply_explorer_role_sets_model_and_adds_session_flags_layer() { + let (_home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + let before_layers = session_flags_layer_count(&config); apply_role_to_config(&mut config, Some("explorer")) .await - .expect("apply explorer role"); + .expect("explorer role should apply"); - assert_eq!(config.model, Some("gpt-5.1-codex-mini".to_string())); + assert_eq!(config.model.as_deref(), Some("gpt-5.1-codex-mini")); assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::Medium)); + assert_eq!(session_flags_layer_count(&config), before_layers + 1); } #[tokio::test] - async fn apply_role_to_config_falls_back_to_builtins_when_user_config_is_invalid() { - let dir = TempDir::new().expect("tempdir"); - write_agents_config( - &dir, - r#" -[agents.explorer -description = "broken" -"#, + async fn apply_role_returns_unavailable_for_missing_user_role_file() { + let (_home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(PathBuf::from("/path/does/not/exist.toml")), + }, ); - let mut config = test_config(); - config.codex_home = dir.path().to_path_buf(); - - apply_role_to_config(&mut config, Some("explorer")) + let err = apply_role_to_config(&mut config, Some("custom")) .await - .expect("apply explorer role"); - - assert_eq!(config.model, Some("gpt-5.1-codex-mini".to_string())); - assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::Medium)); - } - - /// Applies a custom user role config loaded from disk. - #[tokio::test] - async fn apply_role_to_config_supports_custom_role_config_file() { - let dir = TempDir::new().expect("tempdir"); - let planner_path = write_role_config_file( - &dir, - "agents/planner.toml", - r#" -model = "gpt-5.1-codex" -sandbox_mode = "read-only" -"#, - ); - write_agents_config( - &dir, - &format!( - "[agents.planner]\ndescription = \"Planning-focused role.\"\nconfig_file = {planner_path:?}\n" - ), - ); - - let mut config = test_config(); - config.codex_home = dir.path().to_path_buf(); - apply_role_to_config(&mut config, Some("planner")) - .await - .expect("apply planner role"); - - assert_eq!(config.model, Some("gpt-5.1-codex".to_string())); - assert_eq!( - config.permissions.sandbox_policy.get(), - &crate::protocol::SandboxPolicy::new_read_only_policy() - ); - } - - /// Resolves relative config_file paths from the agents_config.toml directory. - #[tokio::test] - async fn apply_role_to_config_supports_relative_custom_role_config_file() { - let dir = TempDir::new().expect("tempdir"); - write_role_config_file( - &dir, - "agents/planner.toml", - r#" -model = "gpt-5.1-codex" -sandbox_mode = "read-only" -"#, - ); - write_agents_config( - &dir, - r#" -[agents.planner] -description = "Planning-focused role." -config_file = "agents/planner.toml" -"#, - ); - - let mut config = test_config(); - config.codex_home = dir.path().to_path_buf(); - apply_role_to_config(&mut config, Some("planner")) - .await - .expect("apply planner role"); - - assert_eq!(config.model, Some("gpt-5.1-codex".to_string())); - assert_eq!( - config.permissions.sandbox_policy.get(), - &crate::protocol::SandboxPolicy::new_read_only_policy() - ); - } - - #[tokio::test] - async fn apply_role_to_config_reports_unknown_agent_type() { - let mut config = test_config(); - - let err = apply_role_to_config(&mut config, Some("missing")) - .await - .expect_err("missing role should fail"); - - assert_eq!(err, "unknown agent_type 'missing'"); - } - - #[tokio::test] - async fn apply_role_to_config_reports_unavailable_agent_type() { - let dir = TempDir::new().expect("tempdir"); - write_agents_config( - &dir, - r#" -[agents.planner] -config_file = "agents/does-not-exist.toml" -"#, - ); - - let mut config = test_config(); - config.codex_home = dir.path().to_path_buf(); - let err = apply_role_to_config(&mut config, Some("planner")) - .await - .expect_err("missing config file should fail"); + .expect_err("missing role file should fail"); assert_eq!(err, AGENT_TYPE_UNAVAILABLE_ERROR); } - /// Lets a user config file override a built-in role config file. #[tokio::test] - async fn apply_role_to_config_lets_user_override_builtin_config_file() { - let dir = TempDir::new().expect("tempdir"); - let custom_explorer_path = write_role_config_file( - &dir, - "agents/custom_explorer.toml", - r#" -model = "gpt-5.1-codex" -model_reasoning_effort = "high" -"#, - ); - write_agents_config( - &dir, - &format!("[agents.explorer]\nconfig_file = {custom_explorer_path:?}\n"), + async fn apply_role_returns_unavailable_for_invalid_user_role_toml() { + let (home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + let role_path = write_role_config(&home, "invalid-role.toml", "model = [").await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(role_path), + }, ); - let mut config = test_config(); - config.codex_home = dir.path().to_path_buf(); - apply_role_to_config(&mut config, Some("explorer")) + let err = apply_role_to_config(&mut config, Some("custom")) .await - .expect("apply explorer role"); + .expect_err("invalid role file should fail"); - assert_eq!(config.model, Some("gpt-5.1-codex".to_string())); + assert_eq!(err, AGENT_TYPE_UNAVAILABLE_ERROR); + } + + #[tokio::test] + async fn apply_role_preserves_unspecified_keys() { + let (home, mut config) = test_config_with_cli_overrides(vec![( + "model".to_string(), + TomlValue::String("base-model".to_string()), + )]) + .await; + let role_path = write_role_config( + &home, + "effort-only.toml", + "model_reasoning_effort = \"high\"", + ) + .await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(role_path), + }, + ); + + apply_role_to_config(&mut config, Some("custom")) + .await + .expect("custom role should apply"); + + assert_eq!(config.model.as_deref(), Some("base-model")); assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::High)); } - /// Applies MCP server settings from a role config file. #[tokio::test] - async fn apply_role_to_config_applies_mcp_servers_config_file_layer() { - let dir = TempDir::new().expect("tempdir"); - let tester_path = write_role_config_file( - &dir, - "agents/tester.toml", - r#" -[mcp_servers.docs] -command = "echo" -enabled_tools = ["search"] + #[cfg(not(windows))] + async fn apply_role_does_not_materialize_default_sandbox_workspace_write_fields() { + use codex_protocol::protocol::SandboxPolicy; + let (home, mut config) = test_config_with_cli_overrides(vec![ + ( + "sandbox_mode".to_string(), + TomlValue::String("workspace-write".to_string()), + ), + ( + "sandbox_workspace_write.network_access".to_string(), + TomlValue::Boolean(true), + ), + ]) + .await; + let role_path = write_role_config( + &home, + "sandbox-role.toml", + r#"[sandbox_workspace_write] +writable_roots = ["./sandbox-root"] "#, - ); - write_agents_config( - &dir, - &format!("[agents.tester]\nconfig_file = {tester_path:?}\n"), - ); - - let mut config = test_config(); - config.codex_home = dir.path().to_path_buf(); - apply_role_to_config(&mut config, Some("tester")) - .await - .expect("apply tester role"); - - let mcp_servers = config.mcp_servers.get(); - assert_eq!( - mcp_servers - .get("docs") - .and_then(|server| server.enabled_tools.clone()), - Some(vec!["search".to_string()]) - ); - } - - /// Inserts a role SessionFlags layer in precedence order when legacy managed - /// layers are already present. - #[tokio::test] - async fn apply_role_to_config_keeps_layer_ordering_with_legacy_managed_layers() { - let mut config = test_config(); - let dir = TempDir::new().expect("tempdir"); - let managed_path = dir.path().join("managed_config.toml"); - std::fs::write(&managed_path, "").expect("write managed config"); - let managed_file = AbsolutePathBuf::try_from(managed_path).expect("managed file"); - config.config_layer_stack = ConfigLayerStack::new( - vec![ConfigLayerEntry::new( - ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }, - TomlValue::Table(toml::map::Map::new()), - )], - config.config_layer_stack.requirements().clone(), - config.config_layer_stack.requirements_toml().clone(), ) - .expect("build initial stack"); + .await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(role_path), + }, + ); - apply_role_to_config(&mut config, Some("explorer")) + apply_role_to_config(&mut config, Some("custom")) .await - .expect("apply explorer role"); + .expect("custom role should apply"); - let layers = config + let role_layer = config .config_layer_stack - .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true); - assert!(matches!( - layers.first().map(|layer| &layer.name), - Some(ConfigLayerSource::SessionFlags) - )); - assert!(matches!( - layers.last().map(|layer| &layer.name), - Some(ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. }) - )); + .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) + .into_iter() + .rfind(|layer| layer.name == ConfigLayerSource::SessionFlags) + .expect("expected a session flags layer"); + let sandbox_workspace_write = role_layer + .config + .get("sandbox_workspace_write") + .and_then(TomlValue::as_table) + .expect("role layer should include sandbox_workspace_write"); + assert_eq!( + sandbox_workspace_write.contains_key("network_access"), + false + ); + assert_eq!( + sandbox_workspace_write.contains_key("exclude_tmpdir_env_var"), + false + ); + assert_eq!( + sandbox_workspace_write.contains_key("exclude_slash_tmp"), + false + ); + + match &*config.permissions.sandbox_policy { + SandboxPolicy::WorkspaceWrite { network_access, .. } => { + assert_eq!(*network_access, true); + } + other => panic!("expected workspace-write sandbox policy, got {other:?}"), + } + } + + #[tokio::test] + async fn apply_role_takes_precedence_over_existing_session_flags_for_same_key() { + let (home, mut config) = test_config_with_cli_overrides(vec![( + "model".to_string(), + TomlValue::String("cli-model".to_string()), + )]) + .await; + let before_layers = session_flags_layer_count(&config); + let role_path = write_role_config(&home, "model-role.toml", "model = \"role-model\"").await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(role_path), + }, + ); + + apply_role_to_config(&mut config, Some("custom")) + .await + .expect("custom role should apply"); + + assert_eq!(config.model.as_deref(), Some("role-model")); + assert_eq!(session_flags_layer_count(&config), before_layers + 1); } #[test] - fn spawn_tool_spec_build_dedups_and_prefers_user_defined_roles() { - let built_in_roles = parse_agents_config( - r#" -[agents.default] -description = "Built-in default." + fn spawn_tool_spec_build_deduplicates_user_defined_built_in_roles() { + let user_defined_roles = BTreeMap::from([ + ( + "explorer".to_string(), + AgentRoleConfig { + description: Some("user override".to_string()), + config_file: None, + }, + ), + ("researcher".to_string(), AgentRoleConfig::default()), + ]); -[agents.explorer] -description = "Built-in explorer." -"#, - "built-in test roles", - ) - .expect("parse built-in roles"); - let user_defined_roles = parse_agents_config( - r#" -[agents.explorer] -description = "User explorer." -"#, - "user-defined test roles", - ) - .expect("parse user roles"); + let spec = spawn_tool_spec::build(&user_defined_roles); - let spec = spawn_tool_spec::build_for_test(&built_in_roles, &user_defined_roles); - - assert_eq!(spec.matches("explorer:").count(), 1); - assert!(spec.contains("explorer: {\nUser explorer.\n}")); - assert!(!spec.contains("Built-in explorer.")); + assert!(spec.contains("researcher: no description")); + assert!(spec.contains("explorer: {\nuser override\n}")); + assert!(spec.contains("default: {\nDefault agent.\n}")); + assert!(!spec.contains("Explorers are fast and authoritative.")); } #[test] - fn spawn_tool_spec_build_lists_user_defined_roles_first() { - let built_in_roles = parse_agents_config( - r#" -[agents.default] -description = "Built-in default." + fn spawn_tool_spec_lists_user_defined_roles_before_built_ins() { + let user_defined_roles = BTreeMap::from([( + "aaa".to_string(), + AgentRoleConfig { + description: Some("first".to_string()), + config_file: None, + }, + )]); -[agents.worker] -description = "Built-in worker." -"#, - "built-in test roles", - ) - .expect("parse built-in roles"); - let user_defined_roles = parse_agents_config( - r#" -[agents.planner] -description = "User planner." -"#, - "user-defined test roles", - ) - .expect("parse user roles"); + let spec = spawn_tool_spec::build(&user_defined_roles); + let user_index = spec.find("aaa: {\nfirst\n}").expect("find user role"); + let built_in_index = spec + .find("default: {\nDefault agent.\n}") + .expect("find built-in role"); - let spec = spawn_tool_spec::build_for_test(&built_in_roles, &user_defined_roles); - - let planner_pos = spec.find("planner:").expect("planner role is present"); - let default_pos = spec.find("default:").expect("default role is present"); - assert!(planner_pos < default_pos); + assert!(user_index < built_in_index); } #[test] - fn spawn_tool_spec_build_formats_missing_description() { - let built_in_roles = parse_agents_config( - r#" -[agents.default] -description = "Built-in default." -"#, - "built-in test roles", - ) - .expect("parse built-in roles"); - let user_defined_roles = parse_agents_config( - r#" -[agents.planner] -"#, - "user-defined test roles", - ) - .expect("parse user roles"); - - let spec = spawn_tool_spec::build_for_test(&built_in_roles, &user_defined_roles); - - assert!(spec.contains("planner: no description")); + fn built_in_config_file_contents_resolves_explorer_only() { + assert_eq!( + built_in::config_file_contents(Path::new("explorer.toml")), + Some(BUILT_IN_EXPLORER_CONFIG) + ); + assert_eq!( + built_in::config_file_contents(Path::new("missing.toml")), + None + ); } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 34f0d9f87..5eb80de2e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -608,7 +608,8 @@ impl TurnContext { model_info: &model_info, features: &features, web_search_mode: self.tools_config.web_search_mode, - }); + }) + .with_agent_roles(config.agent_roles.clone()); Self { sub_id: self.sub_id.clone(), @@ -942,7 +943,8 @@ impl Session { model_info: &model_info, features: &per_turn_config.features, web_search_mode: Some(per_turn_config.web_search_mode.value()), - }); + }) + .with_agent_roles(per_turn_config.agent_roles.clone()); let cwd = session_configuration.cwd.clone(); let turn_metadata_state = Arc::new(TurnMetadataState::new( @@ -4025,7 +4027,8 @@ async fn spawn_review_thread( model_info: &review_model_info, features: &review_features, web_search_mode: Some(review_web_search_mode), - }); + }) + .with_agent_roles(config.agent_roles.clone()); let review_prompt = resolved.prompt.clone(); let provider = parent_turn_context.provider.clone(); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index b83a17592..e69f57737 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -298,6 +298,9 @@ pub struct Config { /// Maximum number of agent threads that can be open concurrently. pub agent_max_threads: Option, + /// User-defined role declarations keyed by role name. + pub agent_roles: BTreeMap, + /// Memories subsystem settings. pub memories: MemoriesConfig, @@ -1148,6 +1151,35 @@ pub struct AgentsToml { /// When unset, no limit is enforced. #[schemars(range(min = 1))] pub max_threads: Option, + + /// User-defined role declarations keyed by role name. + /// + /// Example: + /// ```toml + /// [agents.researcher] + /// description = "Research-focused role." + /// config_file = "./agents/researcher.toml" + /// ``` + #[serde(default, flatten)] + pub roles: BTreeMap, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct AgentRoleConfig { + /// Human-facing role documentation used in spawn tool guidance. + pub description: Option, + /// Path to a role-specific config layer. + pub config_file: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct AgentRoleToml { + /// Human-facing role documentation used in spawn tool guidance. + pub description: Option, + + /// Path to a role-specific config layer. + pub config_file: Option, } impl From for Tools { @@ -1590,6 +1622,28 @@ impl Config { "agents.max_threads must be at least 1", )); } + let agent_roles = cfg + .agents + .as_ref() + .map(|agents| { + agents + .roles + .iter() + .map(|(name, role)| { + ( + name.clone(), + AgentRoleConfig { + description: role.description.clone(), + config_file: role + .config_file + .as_ref() + .map(AbsolutePathBuf::to_path_buf), + }, + ) + }) + .collect() + }) + .unwrap_or_default(); let ghost_snapshot = { let mut config = GhostSnapshotConfig::default(); @@ -1787,6 +1841,7 @@ impl Config { .collect(), tool_output_token_limit: cfg.tool_output_token_limit, agent_max_threads, + agent_roles, memories: cfg.memories.unwrap_or_default().into(), codex_home, log_dir, @@ -4115,6 +4170,7 @@ model_verbosity = "high" project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, + agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), @@ -4226,6 +4282,7 @@ model_verbosity = "high" project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, + agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), @@ -4335,6 +4392,7 @@ model_verbosity = "high" project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, + agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), @@ -4430,6 +4488,7 @@ model_verbosity = "high" project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, + agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 841ef7004..f30a627c4 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -688,7 +688,7 @@ async fn project_trust_context( /// /// This ensures that multiple config layers can be merged together correctly /// even if they were loaded from different directories. -fn resolve_relative_paths_in_config_toml( +pub(crate) fn resolve_relative_paths_in_config_toml( value_from_config_toml: TomlValue, base_dir: &Path, ) -> io::Result { diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index c5a0f59ae..f005c3f3c 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -1036,6 +1036,7 @@ mod tests { .expect("spawned agent thread should exist") .config_snapshot() .await; + assert_eq!(snapshot.model, "gpt-5.1-codex-mini"); assert_eq!(snapshot.approval_policy, AskForApproval::Never); } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index a897e5a8f..6ea7edd00 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -2,6 +2,7 @@ use crate::client_common::tools::FreeformTool; use crate::client_common::tools::FreeformToolFormat; use crate::client_common::tools::ResponsesApiTool; use crate::client_common::tools::ToolSpec; +use crate::config::AgentRoleConfig; use crate::features::Feature; use crate::features::Features; use crate::mcp_connection_manager::ToolInfo; @@ -36,6 +37,7 @@ pub(crate) struct ToolsConfig { pub shell_type: ConfigShellToolType, pub apply_patch_tool_type: Option, pub web_search_mode: Option, + pub agent_roles: BTreeMap, pub search_tool: bool, pub js_repl_enabled: bool, pub js_repl_tools_only: bool, @@ -94,6 +96,7 @@ impl ToolsConfig { shell_type, apply_patch_tool_type, web_search_mode: *web_search_mode, + agent_roles: BTreeMap::new(), search_tool: include_search_tool, js_repl_enabled: include_js_repl, js_repl_tools_only: include_js_repl_tools_only, @@ -102,6 +105,11 @@ impl ToolsConfig { experimental_supported_tools: model_info.experimental_supported_tools.clone(), } } + + pub fn with_agent_roles(mut self, agent_roles: BTreeMap) -> Self { + self.agent_roles = agent_roles; + self + } } pub(crate) fn filter_tools_for_model(tools: Vec, config: &ToolsConfig) -> Vec { @@ -522,7 +530,7 @@ fn create_collab_input_items_schema() -> JsonSchema { } } -fn create_spawn_agent_tool() -> ToolSpec { +fn create_spawn_agent_tool(config: &ToolsConfig) -> ToolSpec { let properties = BTreeMap::from([ ( "message".to_string(), @@ -537,7 +545,9 @@ fn create_spawn_agent_tool() -> ToolSpec { ( "agent_type".to_string(), JsonSchema::String { - description: Some(crate::agent::role::spawn_tool_spec::build()), + description: Some(crate::agent::role::spawn_tool_spec::build( + &config.agent_roles, + )), }, ), ]); @@ -1564,7 +1574,7 @@ pub(crate) fn build_specs( if config.collab_tools { let multi_agent_handler = Arc::new(MultiAgentHandler); - builder.push_spec(create_spawn_agent_tool()); + builder.push_spec(create_spawn_agent_tool(config)); builder.push_spec(create_send_input_tool()); builder.push_spec(create_resume_agent_tool()); builder.push_spec(create_wait_tool());