Skip to content

Support OAuth for HTTP MCP servers from selected executor plugins - #28529

Merged
jif-oai merged 1 commit into
mainfrom
jif/oauth-mco
Jun 25, 2026
Merged

Support OAuth for HTTP MCP servers from selected executor plugins#28529
jif-oai merged 1 commit into
mainfrom
jif/oauth-mco

Conversation

@jif-oai

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

Copy link
Copy Markdown
Contributor

Why

#28522 routes selected-plugin HTTP MCP traffic through the owning executor, but OAuth bootstrap and refresh still used host-local clients. Executor-only servers therefore cannot complete discovery or login through the same network boundary as the MCP connection.

What changed

  • adapt codex_exec_server::HttpClient to RMCP 1.8's OAuthHttpClient contract
  • let RMCP own discovery, dynamic registration, PKCE, token exchange, and refresh
  • route auth status, persisted-token startup, and app-server login through the server runtime while preserving the existing local discovery path
  • add optional threadId to mcpServer/oauth/login and echo it in the completion notification
  • implement RMCP's redirect policy and 1 MiB OAuth response limit over executor HTTP
  • cover selected-thread OAuth discovery and login through an executor-only route

Depends on #28522.

@jif-oai
jif-oai requested a review from a team as a code owner June 16, 2026 13:22
@jif-oai
jif-oai changed the base branch from main to jif/http-mcps June 16, 2026 13:23
@jif-oai jif-oai changed the title Jif/oauth mco Support OAuth for HTTP MCP servers from selected executor plugins Jun 16, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

💡 Codex Review

"url": format!("http://{http_addr}/mcp"),

P2 Badge Make executor HTTP MCP test fail on host fallback

This binds the HTTP MCP to loopback, which the host ReqwestHttpClient can also reach, so the test still passes if executor HTTP MCP falls back to host routing. Please assert an executor-only routing signal/endpoint so this integration test catches that regression. guidance



P2 Badge Cover dynamic client registration default path

This default branch is used when no oauth_client_id is configured, but the added authorization coverage only exercises Some(client_id). Please add a mock registration_endpoint test so dynamic registration and the returned credentials are verified. guidance



P3 Badge Split the OAuth plumbing into a smaller review

This non-mechanical OAuth plumbing changes 1022 lines across rmcp-client plus app-server/core/CLI wiring, exceeding the repo review-size guidance and making regressions hard to isolate. Please split rmcp-client OAuth client/tests from integration wiring. guidance


std::sync::Arc::new(ReqwestHttpClient),

P2 Badge Preserve no_proxy for local OAuth discovery

On macOS when the local CLI/status path has system proxy configuration, this default discovery client again builds reqwest without no_proxy(). The deleted discovery code explicitly used Client::builder().timeout(...).no_proxy() to avoid the system-configuration panic, so supports_oauth_login can now crash before reporting unsupported OAuth.


let http_client = runtime_context
.resolve_streamable_http_client(server_name, config)
.map_err(anyhow::Error::msg)?;

P2 Badge Short-circuit bearer auth before resolving runtime

When a streamable HTTP server is already authenticated via bearer_token_env_var or an Authorization header but its environment_id is unavailable to the status caller, this now fails while resolving the HTTP client before determine_streamable_http_auth_status can return BearerToken. For example, codex mcp list builds an EnvironmentManager::without_environments(), so those configured remote servers are reported as Unsupported even though no OAuth discovery request is needed.


"grant_types": ["authorization_code", "refresh_token"],

P2 Badge Do not require refresh-token grants during registration

When an MCP OAuth server supports authorization-code login but does not issue refresh tokens, this dynamic-registration request can be rejected because it advertises the refresh_token grant as required. The client already handles token responses without a refresh token, so default login for those servers should not fail during registration just because refresh is unavailable.

ℹ️ 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 force-pushed the jif/oauth-mco branch 2 times, most recently from d322f3e to 1e501af Compare June 23, 2026 15:48
Comment thread codex-rs/codex-mcp/src/runtime.rs
Base automatically changed from jif/http-mcps to main June 25, 2026 09:10
@jif-oai
jif-oai merged commit b215961 into main Jun 25, 2026
30 checks passed
@jif-oai
jif-oai deleted the jif/oauth-mco branch June 25, 2026 09:31
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 25, 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.

2 participants