Skip to content

Extract shared plugin MCP config parsing#27863

Merged
jif-oai merged 2 commits into
mainfrom
jif/executor-plugin-mcp-parser
Jun 12, 2026
Merged

Extract shared plugin MCP config parsing#27863
jif-oai merged 2 commits into
mainfrom
jif/executor-plugin-mcp-parser

Conversation

@jif-oai

@jif-oai jif-oai commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Why

We want a thread-selected plugin to eventually expose stdio MCP servers that run on the executor owning that plugin.

The existing plugin MCP parser lived inside core-plugins and was coupled to the host filesystem loader. Reusing it from an executor provider would either duplicate MCP normalization or make the plugin package layer own MCP runtime semantics. This PR creates the shared MCP-owned boundary first.

In simple terms:

plugin .mcp.json
        |
        v
shared parser in codex-mcp
        |
        +-- Declared placement: preserve current local-plugin behavior
        |
        +-- Environment placement: produce config bound to one executor

This builds on the authority-bound plugin descriptors from #27692. It intentionally does not discover, register, or launch executor MCP servers yet.

What changed

  • Moved plugin MCP file parsing and normalization from core-plugins into codex-mcp.
  • Kept support for both existing file shapes: a top-level server map and an object containing mcpServers.
  • Kept per-server failure isolation: one invalid server does not discard valid siblings, while malformed top-level JSON still fails the whole file.
  • Updated the existing local plugin loader to use Declared placement, preserving its current transport, OAuth, relative cwd, and error behavior.
  • Added Environment placement for the next stacked PR:
    • the selected environment ID overrides anything declared by the plugin;
    • missing stdio cwd defaults to the plugin root;
    • relative cwd is resolved beneath the plugin root and cannot traverse outside it;
    • bare or source-less environment-variable references resolve on a non-local executor;
    • explicit orchestrator environment-variable forwarding is rejected for executor-owned plugins.

User impact

None in this PR. Existing local plugin MCP loading follows the same behavior through the shared parser. The executor placement mode is not connected to thread startup until the follow-up registration PR.

Assumptions

  • A selected capability root's environment is authoritative. A plugin cannot redirect its stdio process to the orchestrator or another executor.
  • Relative working directories belong under the plugin package root. Explicit absolute working directories remain valid within the owning environment.
  • For a non-local executor, unqualified environment-variable names refer to that executor. Reading an orchestrator variable requires an explicit contract and is rejected for now.
  • Parsing only produces normalized McpServerConfig values. Process startup remains owned by the existing MCP runtime and connection manager.

Follow-ups

  1. Add the executor MCP provider and catalog registration: read the selected plugin's MCP config through the same executor filesystem, support stdio only, freeze the result per active thread, apply managed policy, and resolve name collisions as discovered plugin < selected plugin < explicit config.
  2. Install that provider in app-server and add an end-to-end test proving thread/start.selectedCapabilityRoots launches and calls the MCP tool on the selected executor, preserves the frozen registration across refresh, and does not expose it to an unselected thread.
  3. After the initial executor-stdio vertical, define resume/fork/environment-replacement semantics, executor HTTP placement, warning delivery, common MCP tool-context bounds, and move remaining MCP source composition above core.

Verification

  • cargo check -p codex-mcp -p codex-core-plugins --tests
  • just bazel-lock-check
  • Added focused parser coverage for legacy local normalization, executor authority, working-directory handling, and environment-variable sourcing.

@jif-oai jif-oai changed the title [codex] Extract shared plugin MCP config parsing Extract shared plugin MCP config parsing Jun 12, 2026
@jif-oai jif-oai marked this pull request as ready for review June 12, 2026 12:40
@jif-oai

jif-oai commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 4157b14280

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jif-oai

jif-oai commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c491faf47

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/codex-mcp/src/plugin_config.rs
@jif-oai jif-oai merged commit 17b9f48 into main Jun 12, 2026
28 of 30 checks passed
@jif-oai

jif-oai commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

CI red is unrelated

@jif-oai jif-oai deleted the jif/executor-plugin-mcp-parser branch June 12, 2026 13:10
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant