Commit Graph

8 Commits

  • Cache plugin namespace during executor skill discovery (#29831)
    ## Why
    
    Executor skill discovery runs before the remote skills catalog is
    available. For a remote environment, each `ExecutorFileSystem` operation
    becomes an exec-server RPC.
    
    Previously, every discovered `SKILL.md` independently resolved its
    plugin namespace by walking its ancestors and probing both supported
    manifest locations. In the common `plugin/skills/<skill>/SKILL.md`
    layout, that repeats 8 RPCs per skill even though every skill under the
    plugin root uses the same namespace. These lookups happen while skills
    are parsed, so their cost grows linearly with the skill count and adds
    directly to first-turn latency.
    
    A selected capability root can also contain standalone skills, multiple
    sibling plugins, nested plugins, or symlinked directories. The
    optimization therefore needs to retain the nearest-ancestor namespace
    for each skill rather than assuming the selected root represents exactly
    one plugin.
    
    ## What changed
    
    - record plugin-root candidates from directory entries already returned
    during skill discovery
    - prune candidates that are not ancestors of any discovered `SKILL.md`
    before reading manifests
    - resolve each relevant plugin root once, with one fallback lookup per
    canonical traversal root for symlinked directories
    - select the nearest cached plugin namespace for each discovered skill
    - avoid namespace lookup entirely when the root contains no skills
    
    No additional directory traversal is required. Namespace work now scales
    with the number of plugin roots that contain discovered skills, rather
    than the total number of skills or unrelated sibling plugins. Standalone
    and nested-plugin names keep their previous behavior.
    
    ## Benchmarks
    
    I used a temporary counting `ExecutorFileSystem` around the real local
    filesystem. Each filesystem operation was counted as one remote RPC and
    given 1 ms of injected latency. Each variant ran three times; times
    below are medians.
    
    ### One plugin with 100 skills
    
    | Operation | Before | After | Delta |
    | --- | ---: | ---: | ---: |
    | `get_metadata` | 1,002 | 303 | -699 |
    | `read_file` | 200 | 101 | -99 |
    | `read_directory` | 102 | 102 | 0 |
    | **Total filesystem RPCs** | **1,304** | **506** | **-798 (-61.2%)** |
    | **Median load time** | **2.890 s** | **0.997 s** | **2.90× faster** |
    
    The namespace-specific work drops from 800 RPCs to 2 in this layout.
    
    ### Multiple plugins under one selected root
    
    These runs compare the correct pre-optimization implementation with the
    final nearest-plugin-root cache. The total plugin skill count stays at
    100 while the number of plugin roots changes.
    
    | Layout | Before RPCs | After RPCs | Reduction | Before | After |
    Speedup |
    | --- | ---: | ---: | ---: | ---: | ---: | ---: |
    | 2 plugins × 50 skills | 1,312 | 530 | 59.6% | 1,819 ms | 711 ms |
    2.56× |
    | 10 plugins × 10 skills | 1,344 | 578 | 57.0% | 1,850 ms | 778 ms |
    2.38× |
    | 50 plugins × 2 skills | 1,504 | 818 | 45.6% | 2,094 ms | 1,086 ms |
    1.93× |
    | 10 plugins × 10 skills + 10 standalone skills | 1,596 | 630 | 60.5% |
    2,209 ms | 860 ms | 2.57× |
    
    The remaining cost grows with the number of relevant plugin manifests.
    Each relevant manifest is read once instead of once per skill, while
    sibling plugins with no discovered skills are not read. Absolute latency
    savings depend on the executor's real RPC latency.
    
    ## Tests
    
    - `just test -p codex-core-skills` (109 passed across the library and
    integration-test binaries)
    - one integration test covers standalone, outer-plugin, nested-plugin,
    and unused sibling-plugin layouts, and asserts the exact set of
    manifests read
  • Load executor skills without host path conversion (#29626)
    ## Why
    
    After #28918, selected skill roots are `PathUri`, but the executor skill
    provider still converts them to the app-server host's `AbsolutePathBuf`.
    A foreign Windows root therefore cannot be discovered by a Unix host,
    and the inverse has the same problem.
    
    This PR keeps executor skill discovery and reads on the filesystem that
    owns the selected root while reusing the existing skill rules.
    
    ## What changed
    
    - Generalize the existing skill traversal to operate on `PathUri`
    through `ExecutorFileSystem`, preserving its depth, directory, symlink,
    and sibling-metadata concurrency behavior.
    - Add a small environment skill loader that reuses the shared discovery,
    frontmatter validation, dependency parsing, product policy, and
    prompt-visibility rules.
    - Keep the environment id and entrypoint `PathUri` in the skill catalog,
    then route `skills.read` back through the same environment filesystem.
    - Preserve the executor's path convention when deriving catalog handles,
    including literal backslashes in POSIX filenames.
    - Resolve plugin namespaces from nearby manifests through URI-native
    filesystem reads.
    - Cover foreign Windows roots, executor-owned reads, namespaces,
    metadata, policy, and path identity.
    
    ```text
    selected root (PathUri)
            |
            v
    shared discovery over ExecutorFileSystem
            |
            v
    environment-bound catalog entry --skills.read--> same ExecutorFileSystem
    ```
    
    No second filesystem abstraction or duplicate traversal implementation
    is introduced.
    
    ## Stack
    
    1. #29614 — add lexical `PathUri` containment.
    2. #29620 — share URI-native manifest path resolution.
    3. #28918 — keep selected plugin roots and resources URI-native.
    4. **This PR** — load executor skills without host path conversion.
    5. #29628 — resolve executor MCP working directories without host path
    conversion.
  • [codex] make PathUri::from_abs_path infallible (#27976)
    ## Why
    
    `PathUri::from_abs_path` can fail for absolute paths that do not have a
    normal `file:` URI representation, forcing filesystem call sites to
    handle a conversion error even though the original path can be preserved
    losslessly.
    
    ## What
    
    Make `from_abs_path` infallible and migrate its callers. Unrepresentable
    paths use `file:///%00/bad/path/<base64>`, encoding Unix bytes or
    Windows UTF-16LE; `to_abs_path` validates and decodes that fallback. The
    leading encoded null reserves a namespace that cannot collide with a
    real Unix or Windows path, and fallback URIs remain opaque to lexical
    path operations.
    
    ## Validation
    
    Added path-URI coverage for Unix null and non-UTF-8 paths, Windows
    device/verbatim and non-Unicode paths, serialization, malformed
    fallbacks, opaque lexical operations, invalid native payloads, and
    literal `/bad/path` collision resistance.
  • 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.
  • [codex] migrate ExecutorFileSystem paths to PathUri (#27424)
    ## Why
    
    We're moving exec-server to use PathUri for its internal path
    representations.
    
    ## What
    
    Move `ExecutorFileSystem` APIs to use `PathUri` instead of
    `AbsolutePathBuf`. Future changes will convert higher-level parts of
    exec-server.
  • feat: Handle alternate plugin manifest paths (#18182)
    Load plugin manifests through a shared discoverable-path helper so
    manifest reads, installs, and skill names all see the same alternate
    manifest location.
  • Make skill loading filesystem-aware (#17720)
    Migrates skill loading to support reading repo skills from the remote
    environment.
  • Extract codex-utils-plugins crate (#15746)
    ## Summary
    - extract shared plugin path and manifest helpers into
    codex-utils-plugins
    - update codex-core to consume the utility crate
    
    ## Testing
    - CI
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>