From d713f7cd6cef708b86b8136fec75d6dce115e013 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Sat, 20 Jun 2026 13:15:07 -0700 Subject: [PATCH] Support multiple plugin agent roots --- codex-rs/core-plugins/src/loader.rs | 4 +- codex-rs/core-plugins/src/loader_tests.rs | 83 ++++++++++--------- codex-rs/core/src/config/agent_roles.rs | 9 +- codex-rs/core/src/config/config_tests.rs | 16 +++- codex-rs/core/src/session/turn_context.rs | 9 +- codex-rs/core/tests/suite/plugins.rs | 18 ++++ .../references/plugin-json-spec.md | 4 +- .../plugin-creator/scripts/validate_plugin.py | 21 ++++- 8 files changed, 110 insertions(+), 54 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 635b027fbf..08da1b745a 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -980,9 +980,7 @@ fn plugin_agent_roots( manifest_paths: &PluginManifestPaths, ) -> Vec { let mut paths = default_agent_roots(plugin_root); - if let Some(path) = &manifest_paths.agents { - paths.push(path.clone()); - } + paths.extend(manifest_paths.agents.iter().cloned()); paths.sort_unstable(); paths.dedup(); paths diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index a7c54c272d..3da97b012e 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -70,13 +70,9 @@ fn configured_plugins_from_stack_merges_user_layers() { } #[tokio::test] -async fn load_plugins_discovers_default_and_manifest_agent_roots() { +async fn load_plugins_discovers_agent_roots_from_string_and_list_manifest_paths() { let temp_dir = TempDir::new().expect("tempdir"); let plugin_root = temp_dir.path().join("plugins/cache/test/agented/local"); - write_file( - &plugin_root.join(".codex-plugin/plugin.json"), - r#"{"name":"agented","agents":"./extra-agents"}"#, - ); write_file( &plugin_root.join("agents/researcher.toml"), r#"name = "researcher" @@ -106,41 +102,50 @@ enabled = true .expect("valid config layer stack"); let store = PluginStore::new(temp_dir.path().to_path_buf()); - let plugins = load_plugins_from_layer_stack( - &stack, - HashMap::new(), - &store, - Some(Product::Codex), - /*prefer_remote_curated_conflicts*/ false, - ) - .await; + for agents_json in [ + r#""./extra-agents""#, + r#"["./extra-agents", "./agents", "./extra-agents"]"#, + ] { + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + &format!(r#"{{"name":"agented","agents":{agents_json}}}"#), + ); + let plugins = load_plugins_from_layer_stack( + &stack, + HashMap::new(), + &store, + Some(Product::Codex), + /*prefer_remote_curated_conflicts*/ false, + ) + .await; - let plugin = plugins - .iter() - .find(|plugin| plugin.config_name == "agented@test") - .expect("agented plugin should load"); - assert_eq!( - plugin.agent_roots, - vec![ - plugin_root.join("agents").abs(), - plugin_root.join("extra-agents").abs(), - ] - ); - assert_eq!( - PluginLoadOutcome::from_plugins(plugins).effective_plugin_agent_roots(), - vec![ - PluginAgentRoot { - path: plugin_root.join("agents").abs(), - plugin_id: "agented@test".to_string(), - plugin_root: plugin_root.abs(), - }, - PluginAgentRoot { - path: plugin_root.join("extra-agents").abs(), - plugin_id: "agented@test".to_string(), - plugin_root: plugin_root.abs(), - }, - ] - ); + let plugin = plugins + .iter() + .find(|plugin| plugin.config_name == "agented@test") + .expect("agented plugin should load"); + assert_eq!( + plugin.agent_roots, + vec![ + plugin_root.join("agents").abs(), + plugin_root.join("extra-agents").abs(), + ] + ); + assert_eq!( + PluginLoadOutcome::from_plugins(plugins).effective_plugin_agent_roots(), + vec![ + PluginAgentRoot { + path: plugin_root.join("agents").abs(), + plugin_id: "agented@test".to_string(), + plugin_root: plugin_root.abs(), + }, + PluginAgentRoot { + path: plugin_root.join("extra-agents").abs(), + plugin_id: "agented@test".to_string(), + plugin_root: plugin_root.abs(), + }, + ] + ); + } } #[tokio::test] diff --git a/codex-rs/core/src/config/agent_roles.rs b/codex-rs/core/src/config/agent_roles.rs index 54243e335c..289447a8e4 100644 --- a/codex-rs/core/src/config/agent_roles.rs +++ b/codex-rs/core/src/config/agent_roles.rs @@ -117,12 +117,15 @@ pub(crate) async fn load_agent_roles( Ok(roles) } -pub(crate) async fn load_plugin_agent_roles( +/// Returns the configured agent roles plus all valid plugin roles. +/// +/// Configured roles keep precedence when a plugin produces the same fully-qualified role name. +pub(crate) async fn load_agent_roles_with_plugins( fs: &dyn ExecutorFileSystem, + mut roles: BTreeMap, plugin_agent_roots: Vec, startup_warnings: &mut Vec, ) -> std::io::Result> { - let mut roles = BTreeMap::new(); for plugin_agent_root in plugin_agent_roots { let discovered_roles = discover_agent_roles_in_dir( fs, @@ -157,7 +160,7 @@ pub(crate) async fn load_plugin_agent_roles( std::io::Error::new( std::io::ErrorKind::InvalidInput, format!( - "duplicate plugin agent role name `{namespaced_role_name}` discovered in {}", + "duplicate agent role name `{namespaced_role_name}` discovered in plugin agents root {}", plugin_agent_root.path.as_path().display() ), ), diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 8e192b1d1a..06f5c414a2 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -7491,8 +7491,16 @@ model = "gpt-5.2" .await?; let mut startup_warnings = Vec::new(); - let roles = agent_roles::load_plugin_agent_roles( + let roles = agent_roles::load_agent_roles_with_plugins( LOCAL_FS.as_ref(), + BTreeMap::from([( + "personal".to_string(), + AgentRoleConfig { + description: Some("Personal role".to_string()), + config_file: None, + nickname_candidates: None, + }, + )]), vec![PluginAgentRoot { path: agents_dir.abs(), plugin_id: "fallback@test".to_string(), @@ -7503,6 +7511,12 @@ model = "gpt-5.2" .await?; assert_eq!(startup_warnings, Vec::::new()); + assert_eq!( + roles + .get("personal") + .and_then(|role| role.description.as_deref()), + Some("Personal role") + ); assert_eq!( roles.get("sample-plugin:researcher").map(|role| ( role.description.as_deref(), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 07a3cab3e5..6f65b99063 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -722,17 +722,16 @@ impl Session { .plugins_for_config(&plugins_input) .await; let mut plugin_agent_role_warnings = Vec::new(); - match crate::config::agent_roles::load_plugin_agent_roles( + match crate::config::agent_roles::load_agent_roles_with_plugins( LOCAL_FS.as_ref(), + per_turn_config.agent_roles.clone(), plugin_outcome.effective_plugin_agent_roots(), &mut plugin_agent_role_warnings, ) .await { - Ok(plugin_agent_roles) => { - for (role_name, role) in plugin_agent_roles { - per_turn_config.agent_roles.entry(role_name).or_insert(role); - } + Ok(agent_roles) => { + per_turn_config.agent_roles = agent_roles; } Err(err) => { plugin_agent_role_warnings diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index f05d252e1b..c6afd11567 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -93,6 +93,19 @@ developer_instructions = "Research the sample data carefully" .expect("write plugin agent role"); } +fn write_personal_agent_role(home: &TempDir) { + let agents_dir = home.path().join("agents"); + std::fs::create_dir_all(&agents_dir).expect("create personal agents dir"); + std::fs::write( + agents_dir.join("personal.toml"), + r#"name = "personal" +description = "Inspect personal data" +developer_instructions = "Inspect personal data carefully" +"#, + ) + .expect("write personal agent role"); +} + fn write_plugin_mcp_plugin(home: &TempDir, command: &str) { let plugin_root = write_sample_plugin_manifest_and_config(home); std::fs::write( @@ -312,6 +325,7 @@ async fn plugin_agent_roles_are_available_to_spawn_agent() -> Result<()> { let codex_home = Arc::new(TempDir::new()?); write_plugin_agent_plugin(codex_home.as_ref()); + write_personal_agent_role(codex_home.as_ref()); let mut builder = test_codex().with_home(codex_home).with_config(|config| { config .features @@ -332,6 +346,10 @@ async fn plugin_agent_roles_are_available_to_spawn_agent() -> Result<()> { agent_type_description.contains("sample:researcher: {\nResearch sample data\n}"), "expected plugin agent role in spawn_agent description: {agent_type_description:?}" ); + assert!( + agent_type_description.contains("personal: {\nInspect personal data\n}"), + "expected personal agent role in spawn_agent description: {agent_type_description:?}" + ); Ok(()) } diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md b/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md index 9566131f19..b20f6ac7a1 100644 --- a/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md +++ b/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md @@ -62,7 +62,7 @@ - `repository` (`string`): Source code URL. - `license` (`string`): License identifier (for example `MIT`, `Apache-2.0`). - `keywords` (`array` of `string`): Search/discovery tags. -- `agents` (`string`): Relative path to plugin-bundled agent role TOML files. +- `agents` (`string` or array of `string`): Relative directory path or paths containing plugin-bundled agent role TOML files. - `skills` (`string`): Relative path to skill directories/files. - `hooks` (`string`): Hook config path. - `mcpServers` (`string` or `object`): MCP config path, or an object whose keys are MCP server names and whose values are MCP server config objects. @@ -115,7 +115,7 @@ Or as an object directly in `plugin.json`: ### Path conventions and defaults -- Path values should be relative and begin with `./`. +- Path values, including each entry in an `agents` array, should be relative and begin with `./`. - `agents`, `skills`, `hooks`, and string-valued `mcpServers` are supplemented on top of default component discovery; they do not replace defaults. - Custom path values must follow the plugin root convention and naming/namespacing rules. - This repo’s scaffold writes `.codex-plugin/plugin.json`; treat that as the manifest location this skill generates. diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py b/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py index 200339c5be..5caf1ff412 100644 --- a/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py +++ b/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py @@ -126,7 +126,7 @@ def validate_manifest_shape( validate_optional_https_url(author, "url", errors, prefix="author") validate_optional_contract_path(manifest, "skills", "skills", errors) - validate_optional_contract_path(manifest, "agents", "agents", errors) + validate_optional_contract_paths(manifest, "agents", "agents", errors) validate_optional_contract_path(manifest, "apps", ".app.json", errors) validate_manifest_mcp_servers(plugin_root, manifest, errors) @@ -284,6 +284,25 @@ def validate_optional_contract_path( errors.append(f"plugin.json field `{key}` must resolve to `{expected}`") +def validate_optional_contract_paths( + payload: dict[str, Any], + key: str, + expected: str, + errors: list[str], +) -> None: + value = payload.get(key) + if value is None: + return + values = [value] if isinstance(value, str) else value + if not isinstance(values, list) or not values or not all( + isinstance(item, str) for item in values + ): + errors.append(f"plugin.json field `{key}` must be a string path or array of string paths") + return + if any(normalize_contract_path(item) != expected for item in values): + errors.append(f"plugin.json field `{key}` must resolve to `{expected}`") + + def validate_manifest_mcp_servers( plugin_root: Path, manifest: dict[str, Any],