Commit Graph
12 Commits
Author SHA1 Message Date
jif e8eb60092a perf(skills): reuse walk inventory for host loading (#31566)
## Why

CCA thread startup loads repository skills through the primary remote
environment. This still used the older host skill loader, which
recursively issued directory and metadata requests over the exec-server
connection. On a high-latency connection, one skill scan could turn into
hundreds of round trips.

Executor-selected skill roots already avoid this pattern: they use one
bounded filesystem walk, reuse the returned inventory, and overlap
independent reads.

## What changed

- Extract the URI-native walk inventory into a shared private discovery
module.
- Use that discovery path for both environment-owned and host/repository
skill roots.
- Reuse the walk inventory to avoid per-skill metadata existence checks
when the inventory is complete.
- Canonicalize host skill identities and parse skill files with bounded
concurrency.
- Prune hidden host directories during traversal, preserving visible
aliases and walk limits.
- Cache namespace probes by ancestor and keep all remote namespace reads
under the same concurrency bound.
- Keep safe metadata probes for incomplete walks, file symlinks, and
case aliases.

## Dependency

#31570 is now merged. It provides the optional `pruneHiddenDirectories`
walk flag used here. That flag defaults to false and is omitted when
disabled.

No other exec-server API changes are introduced.
2026-07-09 09:54:24 +01:00
jif a52b35fcf6 fs: support pruning hidden directories during walks (#31570)
Why

Filtering hidden directories after a walk is too late: their descendants
consume traversal limits, and canonical directory deduplication can let
a hidden path claim a target before a visible symlink reaches it.

What this changes

- Add an optional pruneHiddenDirectories walk option. It defaults to
false and is omitted from the wire when disabled.
- Return hidden directory entries, but do not traverse them or add their
canonical identities to the visited set.
- Cover the visible-symlink-to-hidden-directory case through both local
and remote filesystem implementations.

This is the small filesystem prerequisite for #31566. Skill-specific
behavior remains in that PR.
2026-07-08 13:49:05 +01:00
Adam Perry @ OpenAI 58b66d39e4 perf(skills): resolve plugin namespaces per root (#31348)
## Why

Loading skills from a remote executor can add a lot to thread start time
when there are many skills. Previous changes added some concurrency for
the file reads themselves, but we're still bottlenecked on the initial
root path discovery.

Using the benchmark from
[#31295](https://github.com/openai/codex/pull/31295), this change
reduces the measured mean of loading 66 skills about 71%. Behavioral
coverage lands separately in
[#31369](https://github.com/openai/codex/pull/31369).

## What

- resolve the scanned root's inherited namespace once
- resolve discovered nested plugin roots once, retaining
nearest-valid-ancestor behavior
- pass an explicit resolved Plain / Plugin namespace into skill parsing
instead of probing per skill
- preserve explicitly provided plugin namespaces as the highest-priority
source
- reuse the same resolver for environment skills, deleting its duplicate
root-probe and ancestor-selection path

## Validation

- just test -p codex-core-skills namespace: 12 passed on both the parent
and optimized branches
2026-07-07 23:00:15 +00:00
Adam Perry @ OpenAI a86d525e4d core: trace executor skill discovery (#30318)
## Why

Make it easier to measure the performance of different parts of skill
loading.

## What

- Add spans for step-context capture, world-state construction, executor
catalog snapshot/root loading, and environment skill loading.
- Record the discovered environment skill count.
- Trace outbound exec-server requests with client kind and RPC method
fields.
- Update trace propagation tests to assert that requests keep the parent
trace id while creating their own child span.
2026-07-06 18:44:08 +00:00
jif a938d5f607 Overlap executor skill reads with namespace discovery (#30225)
## Why

Environment skill discovery needs two independent pieces of information:

- plugin namespaces from `plugin.json` files; and
- skill metadata from each `SKILL.md` file.

Today these happen in sequence. Codex waits for every plugin namespace
lookup to finish before it starts reading any skill files. On a remote
executor, that creates an avoidable network-latency barrier.

```text
before: walk -> namespace lookups -> skill reads -> build catalog
after:  walk -> namespace lookups ─┐
             -> skill reads ───────┴-> build catalog
```

## What changes

- Read and parse skill files without waiting for plugin namespace
discovery.
- Resolve root and nested plugin namespaces concurrently.
- Join both results only when constructing the final qualified skill
names.
- Keep the existing 64-skill concurrency bound, output ordering,
warnings, metadata behavior, and namespace rules.

## Testing

The regression test makes plugin manifest lookup wait until a `SKILL.md`
read has started. The old serialized pipeline would time out; the new
pipeline completes and still returns the correctly namespaced skill.

`just test -p codex-core-skills` passes all 111 tests.

## Out of scope

This does not add an exec-server endpoint, batch filesystem calls, or
reduce the number of files transferred. A frontmatter-only read or
server-side skill catalog can remain a separate follow-up if benchmarks
show that transferred bytes are the next bottleneck.
2026-06-26 18:37:59 +00:00
jif 8ebf71ec25 Reuse walk inventory for environment skill metadata (#30145)
## Why

Environment skill discovery already asks the executor to run one
`fs/walk`. That response contains every regular file path found under
the selected root, including any `agents/openai.yaml` files.

Today Core keeps the discovered `SKILL.md` paths but discards the rest
of that file inventory. It then sends one `fs/getMetadata` request per
skill just to ask whether `agents/openai.yaml` exists. A root with 66
skills and no metadata therefore pays for 66 unnecessary network round
trips.

## What changes

- Keep the `fs/walk` file and directory inventory for the duration of
the scan.
- Associate each discovered `SKILL.md` with metadata that is known
present, known absent, or still requires a fallback probe.
- Read a known `agents/openai.yaml` directly instead of statting it
first.
- Skip the metadata request entirely when a complete walk shows that the
skill has no `agents` directory.
- Read a known `SKILL.md` and `agents/openai.yaml` concurrently.
- Keep parsing and validation in `core-skills`.

The inventory is scan-local. This does not add another cache or change
cache lifetime.

## Network impact

For a complete scan of 66 valid skills with no `agents/openai.yaml`, and
one root `.codex-plugin/plugin.json`:

| Operation | Current | After this PR |
| --- | ---: | ---: |
| `fs/walk` | 1 | 1 |
| Read `SKILL.md` | 66 | 66 |
| Stat `agents/openai.yaml` | 66 | 0 |
| Read `agents/openai.yaml` | 0 | 0 |
| Stat plugin manifest | 1 | 1 |
| Read plugin manifest | 1 | 1 |
| **Total executor RPCs** | **135** | **69** |

This removes exactly 66 request/response exchanges from the common cold
scan. Warm scans remain at zero discovery RPCs because the thread-level
executor catalog cache is unchanged.

When metadata exists, each file still requires one read. This PR removes
only the preceding existence check; it does not batch file contents into
a new RPC.

## Correctness fallbacks

Absence is trusted only when the walk is complete and the metadata
directory was not present. Core keeps the existing `getMetadata`
fallback when:

- the walk was truncated;
- the walk reported an error; or
- an `agents` directory was observed but `openai.yaml` was not, which
preserves support for file symlinks and traversal boundaries.

## Deliberate scope

This PR changes only the environment skill loader and its existing
filesystem-call regression coverage. It does not:

- change `fs/walk` or any exec-server protocol;
- add `readFiles` or a skills-list endpoint;
- change thread caching;
- change local skill discovery;
- change exec-server request concurrency; or
- optimize plugin-manifest lookup.

The plugin-manifest stat is intentionally left in place, which is why
this PR reaches 69 calls rather than the broader 68-call estimate. That
lookup has separate alternate-path, ancestor, and symlink semantics and
should not be mixed into this change.
2026-06-26 01:47:00 +01:00
Adam Perry @ OpenAI 5579792b3b Parallelize environment skill loading (#29990)
## Why

Avoid a request waterfall for loading lots of skills at once by hiding
latency in concurrent tasks.

## What changed

Poll the per-skill parse futures concurrently with an order-preserving
stream capped at 64 in-flight loads. Results retain discovery order, and
the existing filtering, warnings, and final catalog sorting are
unchanged.
2026-06-25 10:02:07 +01:00
jif 96d8e34712 Follow directory symlinks in filesystem walks (#29844)
Stack 3 of 3. Stacked on #29842.

## What changes

Adds an opt-in `followDirectorySymlinks` setting to `fs/walk`.

When enabled, the walk follows directory symlinks but continues to
ignore symlinked files. Canonical directory identities prevent symlink
cycles, while normal paths keep their existing spelling.

Environment skill discovery enables the setting so symlinked skill
directories continue to work with the new single-RPC scan.
2026-06-24 20:52:36 +01:00
jif 8a6a34be75 Fix environment skill discovery after merge (#29887)
## Why

The merge of #29831 with the new `fs/walk` environment discovery path
left three `SkillFileDiscovery` initializers without the new namespace
fields. This makes `codex-core-skills` fail to compile and breaks CI for
every PR based on current `main`.

## What changed

- collect plugin roots from the directory entries already returned by
`fs/walk`
- keep the selected root as the namespace fallback
- initialize empty discovery results with empty namespace sets

This preserves the bounded `fs/walk` implementation while restoring the
namespace caching added by #29831.
2026-06-24 19:08:39 +01:00
jif 390b73133b 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
2026-06-24 17:14:34 +01:00
jif 69b76e9d07 Use fs/walk for environment skill discovery (#29842)
Stack 2 of 3. Base: #29841. Follow-up: #29844.

## What changes

Environment skill discovery currently walks remote filesystems through
repeated `readDirectory` and `getMetadata` calls. This switches that
scan to the bounded `fs/walk` operation from the base PR.

```text
Before: readDirectory(root) -> getMetadata(...) -> readDirectory(child) -> ...
After:  fs/walk(root, limits) -> filter the result for SKILL.md
```

This makes environment skill discovery one RPC while preserving
traversal warnings and the existing depth and directory limits. The scan
also has an explicit entry limit. The follow-up restores
directory-symlink traversal.
2026-06-24 16:32:35 +01:00
jif 220f5b76b2 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.
2026-06-23 23:26:06 +01:00