Support multiple plugin agent roots

This commit is contained in:
charlesgong-openai
2026-07-07 22:54:38 -04:00
parent 63d51ea592
commit d713f7cd6c
8 changed files with 110 additions and 54 deletions
+1 -3
View File
@@ -980,9 +980,7 @@ fn plugin_agent_roots(
manifest_paths: &PluginManifestPaths,
) -> Vec<AbsolutePathBuf> {
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
+44 -39
View File
@@ -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]
+6 -3
View File
@@ -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<String, AgentRoleConfig>,
plugin_agent_roots: Vec<PluginAgentRoot>,
startup_warnings: &mut Vec<String>,
) -> std::io::Result<BTreeMap<String, AgentRoleConfig>> {
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()
),
),
+15 -1
View File
@@ -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::<String>::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(),
+4 -5
View File
@@ -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
+18
View File
@@ -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(())
}
@@ -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.
@@ -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],