Keep executor plugin MCP paths URI-native - #29628
Merged
Merged
Conversation
anp-oai
approved these changes
Jun 23, 2026
| .join("plugin-root") | ||
| } | ||
|
|
||
| fn plugin_root_uri(plugin_root: &Path) -> PathUri { |
This was referenced Jun 23, 2026
jif-oai
added a commit
that referenced
this pull request
Jun 23, 2026
## Why
Plugin manifests use the same schema whether the package lives on the
host or in an executor. Only the path representation differs: host
callers need native `Path` inputs and `AbsolutePathBuf` outputs, while
executor callers need `PathUri` throughout.
Maintaining separate parsing or resolver implementations would duplicate
the manifest rules and allow them to drift. This PR instead makes
URI-native resolution the single parsing path and keeps host conversion
at the boundary.
## What changed
- Make `parse_plugin_manifest_uri` the shared manifest parser and
resolve every path-bearing field as `PathUri`.
- Keep the existing host entrypoint as a thin adapter: convert its
native root and manifest path to `PathUri`, run the shared parser, then
map resources back to `AbsolutePathBuf`.
- Expose `PluginManifest::try_map_resources` so callers can convert the
generic resource type without duplicating manifest construction.
- Resolve relative manifest paths using the root URI's convention:
backslashes are separators for Windows roots and ordinary filename
characters for POSIX roots.
- Apply lexical containment after URI resolution, rejecting absolute
paths and parent traversal outside the plugin root.
- Make encoded backslashes fail containment only for Windows URIs;
encoded `/` remains unsafe for every convention.
- Use a host-native synthetic root for marketplace fallback manifests so
the host adapter also works on Windows.
```text
host Path --------> PathUri --\
+--> one manifest parser --> PluginManifest<PathUri>
executor PathUri -------------/
host result: PluginManifest<PathUri> --> PluginManifest<AbsolutePathBuf>
```
Existing host manifest behavior is preserved; #28918 is the first
executor consumer.
## Verification
- `just test -p codex-utils-path-uri`
- `just test -p codex-plugin`
- `just test -p codex-core-plugins`
## Stack
1. #29614 — add lexical `PathUri` containment.
2. **This PR** — share URI-native manifest path resolution.
3. #28918 — keep selected plugin roots and resources URI-native.
4. #29626 — load executor skills without host path conversion.
5. #29628 — resolve executor MCP working directories without host path
conversion.
jif-oai
added a commit
that referenced
this pull request
Jun 23, 2026
## Why Selected capability roots belong to the executor filesystem, not the app-server host. Converting their path strings into the host's native `Path` breaks whenever the two machines use different path conventions, such as a Windows executor behind a Unix app-server. This PR establishes `PathUri` as the selected-plugin boundary so the executor remains authoritative for its paths. ## What changed - Require `selectedCapabilityRoots[].location.path` to be a canonical `file:` URI and deserialize it directly as `PathUri`; native path strings are rejected. - Update the app-server schema, generated TypeScript, examples, and request coverage for the URI contract. - Keep selected roots, resolved plugin locations, manifest paths, and manifest resources as `PathUri`. - Inspect and read plugin roots and manifests only through the selected environment's `ExecutorFileSystem`. - Parse executor manifests with the shared URI-native parser from #29620 instead of projecting them onto the host filesystem. - Enforce resource containment lexically and preserve the root URI's POSIX or Windows path convention. - Cover foreign Windows plugin roots and URI-native manifest resources. ```text thread/start selectedCapabilityRoots[].location.path = "file:///C:/plugins/demo" | PathUri v ExecutorFileSystem | +--> plugin.json +--> manifest resources ``` This PR stops at the shared selected-plugin representation. The next two PRs remove the remaining host-path projections in the skill and MCP consumers. ## Stack 1. #29614 — add lexical `PathUri` containment. 2. #29620 — share URI-native manifest path resolution. 3. **This PR** — keep selected plugin roots and resources URI-native. 4. #29626 — load executor skills without host path conversion. 5. #29628 — resolve executor MCP working directories without host path conversion.
jif-oai
force-pushed
the
jif/uri-native-executor-skills
branch
from
June 23, 2026 21:55
4f503c4 to
4ed0594
Compare
jif-oai
added a commit
that referenced
this pull request
Jun 23, 2026
## 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.
jif-oai
force-pushed
the
jif/uri-native-executor-mcp
branch
from
June 24, 2026 06:58
74d37b4 to
2eb6307
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Executor-owned plugin roots are
PathUri, but MCP config normalization still converts them into a nativePathusing the app-server host's rules. Relativecwdvalues can therefore resolve against the wrong filesystem when host and executor path conventions differ.This PR keeps executor MCP paths URI-native until the selected environment launches the server, while retaining the existing host parser behavior.
What changed
Pathand executor-PathUrientrypoints.cwdto the plugin root and resolve relative working directories with the root URI's path convention.file:URIs only when they remain within the selected plugin root.codex doctordoes not probe executor-owned stdio commands or foreign working directories on the host.No new provider or filesystem abstraction is introduced.
Stack
PathUricontainment.