Commit Graph
4 Commits
Author SHA1 Message Date
charlesgong-openaiandGitHub e12dd73b7d [codex] Support plugin manifest path lists (#28790)
## 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`
2026-06-17 21:33:53 -07:00
charlesgong-openaiandGitHub 1883dedc0e [codex] Support object-valued plugin MCP manifests (#28580)
## Summary
This fixes plugin manifest parsing for MCP servers declared as an object
directly in `plugin.json`.

Before this change, Codex modeled `mcpServers` as only a string path,
for example:

```json
{
  "name": "counter-sample",
  "version": "1.1.1",
  "mcpServers": "./.mcp.json"
}
```

Some migrated plugins instead provide the server map directly in the
manifest:

```json
{
  "name": "counter-sample",
  "version": "1.1.1",
  "description": "Plugin that declares MCP servers in the manifest",
  "mcpServers": {
    "counter": {
      "type": "http",
      "url": "https://sample.example/counter/mcp"
    }
  }
}
```

That object form previously failed during install/load with an error
like:

```text
failed to parse plugin manifest: invalid type: map, expected a string
```

## What changed
- Add a manifest representation for `mcpServers` as either
`Path(Resource)` or `Object(map)`.
- Parse `plugin.json` `mcpServers` as either a string path or an object.
- Route object-valued MCP server maps through the existing plugin MCP
config parser instead of adding a second parser.
- Apply existing per-plugin MCP server policy to object-valued MCP
servers the same way as file-backed MCP servers.
- Include object-valued MCP server names in plugin telemetry/capability
metadata.
- Support object-valued MCP config for executor plugins without
requiring a `.mcp.json` filesystem read.
- Update the bundled plugin-creator validator and `plugin-json-spec.md`
so generated-plugin validation accepts the same object-valued shape.

## Compatibility
Existing plugin manifests that use `"mcpServers": "./.mcp.json"`
continue to work. Plugins can now also use the object shape shown above.

## Tests
Added coverage for the new manifest attribute shape at the install,
normal load, telemetry, and executor-provider layers:

- `install_accepts_manifest_mcp_server_objects`
- `load_plugins_loads_manifest_mcp_server_objects`
- `plugin_telemetry_metadata_uses_manifest_mcp_server_objects`
- `reads_manifest_object_config_without_executor_file_system_access`

Also smoke-tested the plugin-creator validator against both supported
forms:

- `mcpServers` as a direct object in `plugin.json`
- `mcpServers` as `"./.mcp.json"` with a companion `.mcp.json`

## Validation
- `just test -p codex-plugin`
- `just test -p codex-core-plugins`
- `just test -p codex-mcp-extension`
- `just bazel-lock-update`
- `just bazel-lock-check`
- `just fmt`
- `git diff --check`
- Focused rename/object-form rerun: `just test -p codex-core-plugins
manager::tests::load_plugins_loads_manifest_mcp_server_objects
manager::tests::plugin_telemetry_metadata_uses_manifest_mcp_server_objects
store::tests::install_accepts_manifest_mcp_server_objects`
- Focused executor rerun: `just test -p codex-mcp-extension
executor_plugin::provider::tests::reads_manifest_object_config_without_executor_file_system_access`
- `python3
codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py
/private/tmp/codex-validator-object`
- `python3
codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py
/private/tmp/codex-validator-path`
2026-06-16 19:22:57 -07:00
jifandGitHub b3c423e475 Discover stdio MCP servers from selected executor plugins (#27870)
## Why

**In short:** this PR discovers MCP registrations by reading a selected
plugin's `.mcp.json` on its executor. #27884 then resolves those
registrations in the shared catalog.

`thread/start.selectedCapabilityRoots` can select a plugin root owned by
an executor, and Codex can resolve that package through the executor
filesystem. MCP declarations inside the selected plugin are still
ignored.

This PR adds the source-specific discovery layer on top of the
selected-plugin catalog boundary in #27884:

```text
selected capability root
        |
        v
resolve the plugin through its executor filesystem
        |
        v
read and normalize its MCP config through the same filesystem
        |
        v
contribute stdio registrations bound to that environment ID
```

The existing MCP launcher and connection manager remain unchanged. MCP
config parsing is shared with local plugins through #27863.

## What changed

- Added an executor plugin MCP provider in the MCP extension.
- Retained only the exact filesystem capability used for package
resolution and reused it for the selected plugin's MCP config, with no
host-filesystem fallback or unrelated process/HTTP authority.
- Read either the manifest-declared MCP config or the default
`.mcp.json`; a missing default file means the plugin has no MCP servers.
- Accepted stdio servers only for this first vertical. Executor-owned
HTTP declarations are skipped with a warning until their placement
semantics are defined.
- Normalized stdio registrations with the owning environment's stable
logical ID and plugin-root working directory.
- Resolved environment-variable names on the owning executor and
rejected explicit local forwarding for non-local plugins.
- Froze discovered declarations once per active thread runtime, then
applied current managed plugin and MCP requirements when contributing
them.
- Carried the selected root ID, display name, and selection order into
the catalog contribution defined by #27884.

## Behavior and scope

There is intentionally no production behavior change yet. This PR
provides the executor provider and contribution boundary, but app-server
does not install it in this change. Existing local plugin MCP loading is
unchanged, and no MCP process is launched by this PR alone.

## Assumptions

- The selected root ID is the plugin policy identity; the manifest
display name is presentation metadata.
- An environment ID is a stable logical authority. Reconnection or
replacement under the same ID does not change ownership.
- Selected plugin packages and their manifests are trusted inputs.
- The selected package and MCP discovery snapshot remain frozen for the
active thread runtime.

## Follow-up

The next PR installs this contributor in app-server and adds an
end-to-end test proving that a selected plugin MCP tool launches on its
owning executor, can be called by the model, survives an explicit MCP
refresh, and is invisible when its root was not selected.

Resume, fork, environment removal or ID changes, dynamic catalog reload,
and executor-owned HTTP MCP placement remain separate lifecycle
decisions.

## Verification

Focused tests cover executor-only filesystem reads, missing and
malformed config, stdio filtering and normalization, managed
requirements, package attribution, and selection order. CI owns
execution of the test suite.
2026-06-15 11:52:05 +02:00
jifandGitHub 267eacfca2 Add executor-owned plugin resolution (#27692)
## Why

CCA can select a capability root that lives in an executor environment,
but
Codex only had a host-filesystem plugin loader. Before selected executor
plugins can contribute MCP servers, we need a small package boundary
that can
answer:

> Does this selected root contain a plugin, and if so, what does its
manifest
> declare?

The answer must come from the selected environment's filesystem. A
failed
executor lookup must never fall back to the orchestrator filesystem.

## What this changes

This PR introduces:

```rust
PluginProvider::resolve(root)
    -> Result<Option<ResolvedPlugin>, Error>
```

`ExecutorPluginProvider` resolves one `SelectedCapabilityRoot` through
its
exact `environment_id`. It checks the recognized manifest locations,
reads the
manifest through that environment's `ExecutorFileSystem`, and returns an
inert
`ResolvedPlugin` containing:

- the opaque selected-root ID;
- the environment-bound plugin root;
- the authority-bound manifest resource;
- parsed metadata and authority-bound component locators.

Descriptor construction rejects manifest or component paths outside the
selected package root, so consumers cannot accidentally lose the package
boundary when they receive a resolved plugin.

If the root has no plugin manifest, resolution returns `None`, allowing
the
caller to treat it as a standalone capability such as a skill.

```text
selected root: repo -> env-1:/workspace/repo
                         |
                         | env-1 filesystem only
                         v
             .codex-plugin/plugin.json
                         |
                         v
        ResolvedPlugin { authority, root, manifest }
```

The existing host loader and the new executor provider now share the
same
manifest parser. Existing `codex-core-plugins::manifest` type paths
remain
available through re-exports, so host behavior and callers are
unchanged.

## Scope

This is intentionally a non-user-visible package-resolution PR. It does
not:

- parse or register plugin MCP server configurations;
- activate skills, connectors, hooks, or MCP servers;
- change app-server wiring;
- introduce host fallback, caching, or lifecycle behavior.

#27670 has merged, and this PR is now based directly on `main`. Together
with
the resolved MCP catalog from #27634, it establishes the inputs needed
for the
executor stdio MCP vertical without changing the existing MCP runtime.

## Follow-up

The next PR will consume `ResolvedPlugin`, read its declared/default MCP
config
through the same executor filesystem, bind supported stdio servers to
that
environment, and feed those registrations into the resolved MCP catalog.
An
app-server E2E will prove that selecting an executor plugin exposes and
invokes
its tool on the owning executor.

Resume/fork semantics, dynamic environment replacement, and non-stdio
placement remain separate lifecycle decisions.

## Validation

- `just fmt`
- `cargo check --tests -p codex-plugin -p codex-core-plugins`
- `just bazel-lock-check`
- `git diff --check`

Test targets were compiled but not executed locally; CI will run the
test and
Clippy suites.
2026-06-12 13:37:33 +02:00