mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
e12dd73b7d
## Summary
Allow plugin manifests to declare `skills` as either a single path
string or an array of path strings in the core plugin loader.
## Why
Some plugin packages need to expose skills from more than one directory.
Before this change, `plugin.json` only accepted a single string for
`skills`, so manifests like this were ignored as an invalid `skills`
shape:
```json
{
"skills": ["./skills/abc", "./skills/edk"]
}
```
This keeps the existing single-string form working while adding support
for the list form. The final scope is intentionally limited to the core
plugin manifest/load path for `skills`; `apps`, file-backed
`mcpServers`, and the bundled plugin-creator assets are unchanged in
this PR.
## What changed
- Parse `skills` as either a string or an array of strings in
`plugin.json`.
- Store resolved skill paths as a list in `PluginManifestPaths`.
- Load manifest-declared skill roots in addition to the default
`./skills` root.
- Deduplicate exact duplicate skill roots before loading.
- Rely on existing skill-loader dedupe by canonical `SKILL.md` path for
overlapping roots such as `./skills` plus `./skills/abc`.
- Update plugin manifest tests to cover:
- single string `skills`
- list of string `skills`
- duplicate skill roots
- `./skills` as a manifest path
- explicit child roots like `./skills/abc` and `./skills/edk`
- overlapping-root dedupe
## Validation
- `just test -p codex-plugin`
- `just test -p codex-core-plugins`
- `just test -p codex-mcp-extension`
- `git diff --check`
131 lines
4.3 KiB
Rust
131 lines
4.3 KiB
Rust
use super::PluginResourceLocator;
|
|
use super::ResolvedPlugin;
|
|
use super::ResolvedPluginError;
|
|
use crate::manifest::PluginManifest;
|
|
use crate::manifest::PluginManifestHooks;
|
|
use crate::manifest::PluginManifestInterface;
|
|
use crate::manifest::PluginManifestMcpServers;
|
|
use crate::manifest::PluginManifestPaths;
|
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
|
use pretty_assertions::assert_eq;
|
|
|
|
fn absolute(path: impl AsRef<std::path::Path>) -> AbsolutePathBuf {
|
|
AbsolutePathBuf::from_absolute_path_checked(path.as_ref()).expect("absolute test path")
|
|
}
|
|
|
|
fn resource(environment_id: &str, path: AbsolutePathBuf) -> PluginResourceLocator {
|
|
PluginResourceLocator::Environment {
|
|
environment_id: environment_id.to_string(),
|
|
path,
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn environment_descriptor_binds_every_manifest_resource() {
|
|
let root = absolute(std::env::current_dir().expect("cwd").join("plugin-root"));
|
|
let manifest_path = root.join(".codex-plugin/plugin.json");
|
|
let skills = root.join("skills");
|
|
let mcp_servers = root.join(".mcp.json");
|
|
let apps = root.join(".app.json");
|
|
let hooks = root.join("hooks/hooks.json");
|
|
let composer_icon = root.join("assets/composer.svg");
|
|
let logo = root.join("assets/logo.svg");
|
|
let screenshot = root.join("assets/screenshot.png");
|
|
let manifest = PluginManifest {
|
|
name: "demo".to_string(),
|
|
version: None,
|
|
description: None,
|
|
keywords: Vec::new(),
|
|
paths: PluginManifestPaths {
|
|
skills: vec![skills.clone()],
|
|
mcp_servers: Some(PluginManifestMcpServers::Path(mcp_servers.clone())),
|
|
apps: Some(apps.clone()),
|
|
hooks: Some(PluginManifestHooks::Paths(vec![hooks.clone()])),
|
|
},
|
|
interface: Some(PluginManifestInterface {
|
|
composer_icon: Some(composer_icon.clone()),
|
|
logo: Some(logo.clone()),
|
|
screenshots: vec![screenshot.clone()],
|
|
..PluginManifestInterface::default()
|
|
}),
|
|
};
|
|
|
|
let plugin = ResolvedPlugin::from_environment(
|
|
"selected-demo".to_string(),
|
|
"executor-1".to_string(),
|
|
root,
|
|
manifest_path.clone(),
|
|
manifest,
|
|
)
|
|
.expect("valid descriptor");
|
|
|
|
assert_eq!(
|
|
plugin.manifest_path(),
|
|
&resource("executor-1", manifest_path)
|
|
);
|
|
assert_eq!(
|
|
plugin.manifest(),
|
|
&PluginManifest {
|
|
name: "demo".to_string(),
|
|
version: None,
|
|
description: None,
|
|
keywords: Vec::new(),
|
|
paths: PluginManifestPaths {
|
|
skills: vec![resource("executor-1", skills)],
|
|
mcp_servers: Some(PluginManifestMcpServers::Path(resource(
|
|
"executor-1",
|
|
mcp_servers,
|
|
))),
|
|
apps: Some(resource("executor-1", apps)),
|
|
hooks: Some(PluginManifestHooks::Paths(vec![resource(
|
|
"executor-1",
|
|
hooks,
|
|
)])),
|
|
},
|
|
interface: Some(PluginManifestInterface {
|
|
composer_icon: Some(resource("executor-1", composer_icon)),
|
|
logo: Some(resource("executor-1", logo)),
|
|
screenshots: vec![resource("executor-1", screenshot)],
|
|
..PluginManifestInterface::default()
|
|
}),
|
|
}
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn environment_descriptor_rejects_resources_outside_package_root() {
|
|
let cwd = std::env::current_dir().expect("cwd");
|
|
let root = absolute(cwd.join("plugin-root"));
|
|
let outside = absolute(cwd.join("outside/.mcp.json"));
|
|
let manifest = PluginManifest {
|
|
name: "demo".to_string(),
|
|
version: None,
|
|
description: None,
|
|
keywords: Vec::new(),
|
|
paths: PluginManifestPaths {
|
|
skills: Vec::new(),
|
|
mcp_servers: Some(PluginManifestMcpServers::Path(outside.clone())),
|
|
apps: None,
|
|
hooks: None,
|
|
},
|
|
interface: None,
|
|
};
|
|
|
|
let err = ResolvedPlugin::from_environment(
|
|
"selected-demo".to_string(),
|
|
"executor-1".to_string(),
|
|
root.clone(),
|
|
root.join(".codex-plugin/plugin.json"),
|
|
manifest,
|
|
)
|
|
.expect_err("outside resource should fail");
|
|
|
|
assert_eq!(
|
|
err,
|
|
ResolvedPluginError::ResourceOutsideRoot {
|
|
root,
|
|
path: outside,
|
|
}
|
|
);
|
|
}
|