From 1da59ad257115cc5734e6781d5ac0b4f08a6e064 Mon Sep 17 00:00:00 2001 From: willwang-openai Date: Thu, 13 Aug 2026 22:23:22 +0000 Subject: [PATCH] Support per-server MCP OAuth callback ports (#38448) ## What changed - Add `oauth.callback_port` to MCP server configuration and preserve it when serializing configuration edits. - Accept `oauth.callbackPort` from plugin MCP declarations and skill dependency metadata. - Prefer the server-specific callback port over `mcp_oauth_callback_port` for CLI login, app-server, plugin installation, executor, and skill dependency OAuth flows. ## Testing - Cover configuration parsing, serialization, and fallback behavior. - Verify plugin, executor, and skill OAuth registrations use their configured callback ports instead of the global port. GitOrigin-RevId: 7f65e5e7869358307e49779f1b75e8672b607736 --- .../src/request_processors/mcp_processor.rs | 2 +- .../src/request_processors/plugins.rs | 2 +- .../app-server/tests/suite/v2/executor_mcp.rs | 19 +++++-- .../tests/suite/v2/plugin_install.rs | 43 ++++++++++++-- codex-rs/cli/src/mcp_cmd.rs | 3 +- codex-rs/codex-mcp/src/plugin_config.rs | 10 ++-- codex-rs/codex-mcp/src/plugin_config_tests.rs | 1 + codex-rs/config/src/mcp_types.rs | 16 ++++++ codex-rs/config/src/mcp_types_tests.rs | 30 ++++++++++ codex-rs/core-plugins/src/manager_tests.rs | 1 + codex-rs/core/config.schema.json | 6 ++ codex-rs/core/src/config/config_tests.rs | 1 + .../core/src/config/edit/document_helpers.rs | 18 ++++-- codex-rs/core/src/config/edit_tests.rs | 2 + codex-rs/core/src/mcp_skill_dependencies.rs | 13 ++++- .../suite/mcp_startup_refresh_http_proxy.rs | 56 ++++++++++++++++--- .../character_routing_card_tests.rs | 1 + .../routing_card_lexical_tests.rs | 3 + .../skills/src/loader/environment_tests.rs | 1 + codex-rs/ext/skills/src/loader/host_tests.rs | 3 + codex-rs/ext/skills/src/loader/metadata.rs | 10 ++++ codex-rs/skills/src/model.rs | 1 + 22 files changed, 205 insertions(+), 37 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 4deaecb904..69c2c285ef 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -210,7 +210,7 @@ impl McpRequestProcessor { client_registration, server.oauth_resource.as_deref(), timeout_secs, - mcp_config.mcp_oauth_callback_port, + server.oauth_callback_port(mcp_config.mcp_oauth_callback_port), mcp_config.mcp_oauth_callback_url.as_deref(), http_client, redirect_mode, diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 1185812a88..221205e6c0 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1980,7 +1980,7 @@ impl PluginRequestProcessor { let store_mode = config.mcp_oauth_credentials_store_mode; let keyring_backend_kind = config.auth_keyring_backend_kind(); - let callback_port = config.mcp_oauth_callback_port; + let callback_port = server.oauth_callback_port(config.mcp_oauth_callback_port); let callback_url = config.mcp_oauth_callback_url.clone(); let outgoing = Arc::clone(&self.outgoing); let notification_name = name.clone(); diff --git a/codex-rs/app-server/tests/suite/v2/executor_mcp.rs b/codex-rs/app-server/tests/suite/v2/executor_mcp.rs index 73c4eb3747..3817c8c9dc 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_mcp.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_mcp.rs @@ -150,11 +150,17 @@ async fn selected_executor_plugin_exposes_its_mcps_only_to_that_thread() -> Resu let http_server_handle = tokio::spawn(async move { let _ = axum::serve(http_listener, http_router).await; }); + let plugin_callback_listener = TcpListener::bind("127.0.0.1:0").await?; + let plugin_callback_port = plugin_callback_listener.local_addr()?.port(); + let global_callback_listener = TcpListener::bind("127.0.0.1:0").await?; + let global_callback_port = global_callback_listener.local_addr()?.port(); + drop(plugin_callback_listener); let codex_home = TempDir::new()?; + let root_config = format!( + "compact_prompt = \"compact\"\nmodel_auto_compact_token_limit = 1024\nmcp_oauth_credentials_store = \"file\"\nmcp_oauth_callback_port = {global_callback_port}" + ); MockResponsesConfig::new(&responses_server.uri()) - .with_root_config( - "compact_prompt = \"compact\"\nmodel_auto_compact_token_limit = 1024\nmcp_oauth_credentials_store = \"file\"", - ) + .with_root_config(&root_config) .with_provider_config("supports_websockets = false") .write(codex_home.path())?; let executor_config: codex_config::types::McpServerConfig = serde_json::from_value(json!({ @@ -221,12 +227,16 @@ HTTP_PROXY = {http_proxy} (OAUTH_MCP_SERVER_NAME): { "url": EXECUTOR_OAUTH_MCP_URL, "environment_id": "local", + "oauth": {"callbackPort": plugin_callback_port}, "startup_timeout_sec": 10, }, (PRE_REGISTERED_OAUTH_MCP_SERVER_NAME): { "url": EXECUTOR_OAUTH_MCP_URL, "environment_id": "local", - "oauth": {"clientId": "configured-client"}, + "oauth": { + "clientId": "configured-client", + "callbackPort": plugin_callback_port, + }, "startup_timeout_sec": 10, } } @@ -362,6 +372,7 @@ startup_timeout_sec = 10 json!([redirect_uri.clone()]) ); let mut callback_url = reqwest::Url::parse(&redirect_uri)?; + assert_eq!(callback_url.port(), Some(plugin_callback_port)); callback_url .query_pairs_mut() .append_pair("code", "executor-test-code") diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 26ce236f29..d08790eea0 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -1818,10 +1818,16 @@ async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Resu .mount(&proxy) .await; + let plugin_callback_listener = TcpListener::bind("127.0.0.1:0").await?; + let plugin_callback_port = plugin_callback_listener.local_addr()?.port(); + let global_callback_listener = TcpListener::bind("127.0.0.1:0").await?; + let global_callback_port = global_callback_listener.local_addr()?.port(); + drop(plugin_callback_listener); + let codex_home = TempDir::new()?; std::fs::write( codex_home.path().join("config.toml"), - "[features]\nplugins = true\n", + format!("mcp_oauth_callback_port = {global_callback_port}\n\n[features]\nplugins = true\n"), )?; let repo_root = TempDir::new()?; write_plugin_marketplace( @@ -1833,7 +1839,18 @@ async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Resu /*auth_policy*/ None, )?; write_plugin_source(repo_root.path(), "sample-plugin", &[])?; - write_plugin_mcp_config(repo_root.path(), "sample-plugin", resource_url)?; + std::fs::write( + repo_root.path().join("sample-plugin/.mcp.json"), + serde_json::to_vec_pretty(&json!({ + "mcpServers": { + "sample-mcp": { + "type": "http", + "url": format!("{resource_url}/mcp"), + "oauth": {"callbackPort": plugin_callback_port}, + } + } + }))?, + )?; let marketplace_path = AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; @@ -1871,13 +1888,27 @@ async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Resu ) .await?; - let resource_metadata_requested = proxy - .received_requests() - .await - .unwrap_or_default() + let requests = proxy.received_requests().await.unwrap_or_default(); + let resource_metadata_requested = requests .iter() .any(|request| request.url.path() == "/oauth-resource"); assert!(resource_metadata_requested); + + let registration_request = requests + .iter() + .find(|request| request.url.path() == "/oauth/register") + .expect("OAuth client registration request"); + let registration: serde_json::Value = serde_json::from_slice(®istration_request.body)?; + let redirect_uri: Uri = registration["redirect_uris"][0] + .as_str() + .expect("OAuth client registration redirect URI") + .parse()?; + assert_eq!( + redirect_uri + .authority() + .and_then(axum::http::uri::Authority::port_u16), + Some(plugin_callback_port) + ); Ok(()) } diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 3a8d71e9c6..57be665d97 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -429,6 +429,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re .clone() .map(|client_id| McpServerOAuthConfig { client_id: Some(client_id), + callback_port: None, }), oauth_resource: oauth_resource.clone(), tools: HashMap::new(), @@ -593,7 +594,7 @@ async fn run_login(config: &Config, login_args: LoginArgs) -> Result<()> { server.oauth_client_id(), client_registration, server.oauth_resource.as_deref(), - config.mcp_oauth_callback_port, + server.oauth_callback_port(config.mcp_oauth_callback_port), config.mcp_oauth_callback_url.as_deref(), http_client, ) diff --git a/codex-rs/codex-mcp/src/plugin_config.rs b/codex-rs/codex-mcp/src/plugin_config.rs index 949419decb..eadc12de54 100644 --- a/codex-rs/codex-mcp/src/plugin_config.rs +++ b/codex-rs/codex-mcp/src/plugin_config.rs @@ -266,12 +266,10 @@ fn normalize_plugin_mcp_server_value( } if let Some(JsonValue::Object(mut oauth)) = object.remove("oauth") { - if oauth.remove("callbackPort").is_some() { - let plugin_display = source.display(); - warn!( - plugin = %plugin_display, - "plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings" - ); + if let Some(callback_port) = oauth.remove("callbackPort") { + oauth + .entry("callback_port".to_string()) + .or_insert(callback_port); } if let Some(client_id) = oauth.remove("clientId") { diff --git a/codex-rs/codex-mcp/src/plugin_config_tests.rs b/codex-rs/codex-mcp/src/plugin_config_tests.rs index c218b873c4..c2c50b277f 100644 --- a/codex-rs/codex-mcp/src/plugin_config_tests.rs +++ b/codex-rs/codex-mcp/src/plugin_config_tests.rs @@ -582,6 +582,7 @@ fn declared_placement_preserves_local_plugin_normalization() { scopes: None, oauth: Some(McpServerOAuthConfig { client_id: Some("client-id".to_string()), + callback_port: Some(9876), }), oauth_resource: None, tools: HashMap::new(), diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index f5e7821944..b914633e26 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -145,6 +145,10 @@ pub struct McpServerOAuthConfig { /// Explicit OAuth client identifier to present during authorization and token exchange. #[serde(default, skip_serializing_if = "Option::is_none")] pub client_id: Option, + + /// Fixed callback port that takes precedence over Codex's global OAuth callback port. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub callback_port: Option, } /// Authentication flow Codex attempts after resolving an HTTP MCP server's @@ -272,6 +276,18 @@ impl McpServerConfig { .as_ref() .and_then(|oauth| oauth.client_id.as_deref()) } + + pub fn oauth_callback_port(&self, global_callback_port: Option) -> Option { + let callback_port = self.oauth.as_ref().and_then(|oauth| oauth.callback_port); + if let Some(callback_port) = callback_port { + tracing::info!( + callback_port, + ?global_callback_port, + "using plugin-specific MCP OAuth callback port instead of the global callback port" + ); + } + callback_port.or(global_callback_port) + } } /// Raw MCP config shape used for deserialization and supported-field JSON diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index ecc4c54e2b..d1b0754b20 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -364,6 +364,7 @@ fn deserialize_streamable_http_server_config_with_oauth_client_id() { [oauth] client_id = "eci-prd-pub-codex-123" + callback_port = 9876 "#, ) .expect("should deserialize http config with oauth client id"); @@ -372,10 +373,39 @@ fn deserialize_streamable_http_server_config_with_oauth_client_id() { cfg.oauth, Some(McpServerOAuthConfig { client_id: Some("eci-prd-pub-codex-123".to_string()), + callback_port: Some(9876), }) ); } +#[test] +fn oauth_callback_port_prefers_server_port_over_global_port() { + let cfg: McpServerConfig = toml::from_str( + r#" + url = "https://example.com/mcp" + + [oauth] + callback_port = 9876 + "#, + ) + .expect("should deserialize http config with oauth callback port"); + + assert_eq!(cfg.oauth_callback_port(Some(4321)), Some(9876)); +} + +#[test] +fn oauth_callback_port_falls_back_to_global_port() { + let cfg: McpServerConfig = toml::from_str( + r#" + url = "https://example.com/mcp" + "#, + ) + .expect("should deserialize http config without oauth callback port"); + + assert_eq!(cfg.oauth_callback_port(Some(4321)), Some(4321)); + assert_eq!(cfg.oauth_callback_port(/*global_callback_port*/ None), None); +} + #[test] fn deserialize_server_config_with_tool_filters() { let cfg: McpServerConfig = toml::from_str( diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 9dbada513f..cc9d205e86 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -969,6 +969,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { scopes: None, oauth: Some(McpServerOAuthConfig { client_id: Some("client-id".to_string()), + callback_port: Some(3118), }), oauth_resource: None, tools: HashMap::new(), diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 19292d34c5..28e84d8caf 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1592,6 +1592,12 @@ "additionalProperties": false, "description": "OAuth client settings used when Codex launches an MCP OAuth flow.", "properties": { + "callback_port": { + "description": "Fixed callback port that takes precedence over Codex's global OAuth callback port.", + "format": "uint16", + "minimum": 0.0, + "type": "integer" + }, "client_id": { "description": "Explicit OAuth client identifier to present during authorization and token exchange.", "type": "string" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index da3555d01c..133d98fd3d 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -7449,6 +7449,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh scopes: None, oauth: Some(McpServerOAuthConfig { client_id: Some("eci-prd-pub-codex-123".to_string()), + callback_port: None, }), oauth_resource: Some("https://resource.example.com".to_string()), tools: HashMap::new(), diff --git a/codex-rs/core/src/config/edit/document_helpers.rs b/codex-rs/core/src/config/edit/document_helpers.rs index fd43c18c7a..769341dce8 100644 --- a/codex-rs/core/src/config/edit/document_helpers.rs +++ b/codex-rs/core/src/config/edit/document_helpers.rs @@ -147,14 +147,20 @@ fn serialize_mcp_server_table(config: &McpServerConfig) -> TomlTable { { entry["scopes"] = array_from_iter(scopes.iter().cloned()); } - if let Some(oauth) = &config.oauth - && let Some(client_id) = &oauth.client_id - && !client_id.is_empty() - { + if let Some(oauth) = &config.oauth { let mut oauth_table = TomlTable::new(); oauth_table.set_implicit(false); - oauth_table["client_id"] = value(client_id.clone()); - entry["oauth"] = TomlItem::Table(oauth_table); + if let Some(client_id) = &oauth.client_id + && !client_id.is_empty() + { + oauth_table["client_id"] = value(client_id.clone()); + } + if let Some(callback_port) = oauth.callback_port { + oauth_table["callback_port"] = value(i64::from(callback_port)); + } + if !oauth_table.is_empty() { + entry["oauth"] = TomlItem::Table(oauth_table); + } } if let Some(resource) = &config.oauth_resource && !resource.is_empty() diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 754dd3ed11..605a6a9f2a 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -1056,6 +1056,7 @@ fn blocking_replace_mcp_servers_round_trips() { scopes: None, oauth: Some(McpServerOAuthConfig { client_id: Some("eci-prd-pub-codex-123".to_string()), + callback_port: Some(9876), }), oauth_resource: Some("https://resource.example.com".to_string()), tools: HashMap::new(), @@ -1084,6 +1085,7 @@ Z-Header = \"z\" [mcp_servers.http.oauth] client_id = \"eci-prd-pub-codex-123\" +callback_port = 9876 [mcp_servers.stdio] command = \"cmd\" diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 6194f2d693..61c6ee64a6 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use std::sync::Arc; use codex_config::McpServerConfig; +use codex_config::McpServerOAuthConfig; use codex_config::McpServerTransportConfig; use codex_config::load_global_mcp_servers; use codex_login::default_client::is_first_party_originator; @@ -174,6 +175,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( ); let oauth_client_id = server_config.oauth_client_id(); let oauth_credential_name = server_config.oauth_credential_name(&name); + let callback_port = server_config.oauth_callback_port(config.mcp_oauth_callback_port); let first_attempt = perform_oauth_login( oauth_credential_name.as_ref(), &oauth_config.url, @@ -185,7 +187,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_client_id, McpOAuthClientRegistration::Auto, server_config.oauth_resource.as_deref(), - config.mcp_oauth_callback_port, + callback_port, config.mcp_oauth_callback_url.as_deref(), Arc::clone(&http_client), ) @@ -204,7 +206,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_client_id, McpOAuthClientRegistration::Auto, server_config.oauth_resource.as_deref(), - config.mcp_oauth_callback_port, + callback_port, config.mcp_oauth_callback_url.as_deref(), Arc::clone(&http_client), ) @@ -405,7 +407,12 @@ fn mcp_dependency_to_server_config( enabled_tools: None, disabled_tools: None, scopes: None, - oauth: None, + oauth: dependency + .oauth_callback_port + .map(|callback_port| McpServerOAuthConfig { + client_id: None, + callback_port: Some(callback_port), + }), oauth_resource: None, tools: HashMap::new(), }); diff --git a/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs b/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs index 90cbbf8247..cb74895e94 100644 --- a/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs +++ b/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs @@ -5,6 +5,7 @@ use std::time::Duration; use anyhow::Result; use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_config::McpServerConfig; +use codex_config::McpServerOAuthConfig; use codex_config::McpServerTransportConfig; use codex_exec_server::CreateDirectoryOptions; use codex_features::Feature; @@ -25,7 +26,9 @@ use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use tokio::net::TcpListener; use tokio::process::Command; +use url::Url; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -33,6 +36,8 @@ use wiremock::matchers::method; use wiremock::matchers::path; const PROXY_TEST_SUBPROCESS_ENV_VAR: &str = "CODEX_MCP_HTTP_PROXY_TEST_SUBPROCESS"; +const SKILL_CALLBACK_PORT_ENV_VAR: &str = "CODEX_MCP_SKILL_CALLBACK_PORT"; +const SKILL_GLOBAL_CALLBACK_PORT_ENV_VAR: &str = "CODEX_MCP_SKILL_GLOBAL_CALLBACK_PORT"; const TEST_NAME: &str = "suite::mcp_startup_refresh_http_proxy::local_mcp_startup_and_refresh_use_configured_http_client"; const SKILL_TEST_NAME: &str = "suite::mcp_startup_refresh_http_proxy::skill_mcp_dependency_oauth_uses_configured_http_client"; @@ -215,6 +220,12 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> .mount(&proxy) .await; + let skill_callback_listener = TcpListener::bind("127.0.0.1:0").await?; + let skill_callback_port = skill_callback_listener.local_addr()?.port(); + let global_callback_listener = TcpListener::bind("127.0.0.1:0").await?; + let global_callback_port = global_callback_listener.local_addr()?.port(); + drop(skill_callback_listener); + let mut command = Command::new(std::env::current_exe()?); command.arg("--exact").arg(SKILL_TEST_NAME); for &key in codex_network_proxy::PROXY_ENV_KEYS { @@ -222,6 +233,11 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> } command .env(PROXY_TEST_SUBPROCESS_ENV_VAR, "1") + .env(SKILL_CALLBACK_PORT_ENV_VAR, skill_callback_port.to_string()) + .env( + SKILL_GLOBAL_CALLBACK_PORT_ENV_VAR, + global_callback_port.to_string(), + ) .env("HTTP_PROXY", proxy.uri()) .env("http_proxy", proxy.uri()) .env("NO_PROXY", codex_network_proxy::DEFAULT_NO_PROXY_VALUE) @@ -238,22 +254,36 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr), ); - assert!( - requests.iter().any(|request| { + let registration_request = requests + .iter() + .find(|request| { request.method == "POST" && request.url.path() == "/oauth/register" - }), - "skill dependency OAuth registration should use the configured proxy: {requests:#?}" - ); + }) + .unwrap_or_else(|| { + panic!( + "skill dependency OAuth registration should use the configured proxy: {requests:#?}" + ) + }); + let registration: Value = serde_json::from_slice(®istration_request.body)?; + let redirect_uri = Url::parse( + registration["redirect_uris"][0] + .as_str() + .expect("OAuth client registration redirect URI"), + )?; + assert_eq!(redirect_uri.port(), Some(skill_callback_port)); return Ok(()); } + let skill_callback_port = std::env::var(SKILL_CALLBACK_PORT_ENV_VAR)?.parse::()?; + let global_callback_port = std::env::var(SKILL_GLOBAL_CALLBACK_PORT_ENV_VAR)?.parse::()?; let responses_server = responses::start_mock_server().await; let mut builder = test_codex() - .with_config(|config| { + .with_config(move |config| { config .features .enable(Feature::SkillMcpDependencyInstall) .expect("test config should allow skill MCP dependency installation"); + config.mcp_oauth_callback_port = Some(global_callback_port); if cfg!(target_os = "linux") { config .features @@ -262,7 +292,7 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> config.respect_system_proxy = true; } }) - .with_workspace_setup(|cwd, fs| async move { + .with_workspace_setup(move |cwd, fs| async move { let skill_dir = cwd.join(".agents/skills/proxy-skill"); let agents_dir = skill_dir.join("agents"); fs.create_directory( @@ -278,7 +308,7 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> ) .await?; let metadata = format!( - "dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n" + "dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n oauth:\n callbackPort: {skill_callback_port}\n" ); fs.write_file( &PathUri::from_host_native_path(agents_dir.join("openai.yaml"))?, @@ -340,6 +370,14 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> .await; let servers = codex_config::load_global_mcp_servers(&fixture.config.codex_home).await?; - assert!(servers.contains_key(SERVER_NAME)); + assert_eq!( + servers + .get(SERVER_NAME) + .and_then(|server| server.oauth.as_ref()), + Some(&McpServerOAuthConfig { + client_id: None, + callback_port: Some(skill_callback_port), + }) + ); Ok(()) } diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/character_routing_card_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/character_routing_card_tests.rs index 697b46da96..3091a0b52c 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/character_routing_card_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/character_routing_card_tests.rs @@ -76,6 +76,7 @@ fn connector_dependencies_make_generic_skills_discoverable() { transport: None, command: None, url: None, + oauth_callback_port: None, }], }; let documents = [ diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs index b8010703aa..dca9636c70 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs @@ -14,6 +14,7 @@ fn dependency_names_outrank_description_only_matches() { transport: None, command: None, url: None, + oauth_callback_port: None, }], }; let documents = [ @@ -80,6 +81,7 @@ fn dependency_records_are_bounded() { transport: None, command: None, url: None, + oauth_callback_port: None, }) .collect::>(); tools.push(SkillToolDependency { @@ -89,6 +91,7 @@ fn dependency_records_are_bounded() { transport: None, command: None, url: None, + oauth_callback_port: None, }); let dependencies = SkillDependencies { tools }; let document = SkillSelectionDocument { diff --git a/codex-rs/ext/skills/src/loader/environment_tests.rs b/codex-rs/ext/skills/src/loader/environment_tests.rs index 59305a3e93..816be7011e 100644 --- a/codex-rs/ext/skills/src/loader/environment_tests.rs +++ b/codex-rs/ext/skills/src/loader/environment_tests.rs @@ -68,6 +68,7 @@ policy: transport: Some("stdio".to_string()), command: Some("deploy-mcp".to_string()), url: None, + oauth_callback_port: None, }], }), policy: Some(SkillPolicy { diff --git a/codex-rs/ext/skills/src/loader/host_tests.rs b/codex-rs/ext/skills/src/loader/host_tests.rs index ddaa00442e..74db74687d 100644 --- a/codex-rs/ext/skills/src/loader/host_tests.rs +++ b/codex-rs/ext/skills/src/loader/host_tests.rs @@ -117,6 +117,8 @@ async fn loads_host_frontmatter_dependencies_and_policy() { - type: mcp value: demo-tool description: Demo tool + oauth: + callbackPort: 3118 policy: allow_implicit_invocation: false products: [codex, CHATGPT, atlas] @@ -141,6 +143,7 @@ policy: transport: None, command: None, url: None, + oauth_callback_port: Some(3118), }], }), policy: Some(SkillPolicy { diff --git a/codex-rs/ext/skills/src/loader/metadata.rs b/codex-rs/ext/skills/src/loader/metadata.rs index bb6c17c61f..030106c32e 100644 --- a/codex-rs/ext/skills/src/loader/metadata.rs +++ b/codex-rs/ext/skills/src/loader/metadata.rs @@ -62,6 +62,14 @@ struct DependencyTool { transport: Option, command: Option, url: Option, + oauth: Option, +} + +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct DependencyOAuth { + #[serde(alias = "callback_port")] + callback_port: Option, } pub(super) async fn load_host_skill_metadata( @@ -186,6 +194,7 @@ fn resolve_dependency_tool(tool: DependencyTool) -> Option "dependencies.tools.command", ); let url = resolve_str(tool.url, MAX_DEPENDENCY_URL_LEN, "dependencies.tools.url"); + let oauth_callback_port = tool.oauth.and_then(|oauth| oauth.callback_port); Some(SkillToolDependency { r#type, @@ -194,6 +203,7 @@ fn resolve_dependency_tool(tool: DependencyTool) -> Option transport, command, url, + oauth_callback_port, }) } diff --git a/codex-rs/skills/src/model.rs b/codex-rs/skills/src/model.rs index e656c4267e..a47ce1dd96 100644 --- a/codex-rs/skills/src/model.rs +++ b/codex-rs/skills/src/model.rs @@ -90,6 +90,7 @@ pub struct SkillToolDependency { pub transport: Option, pub command: Option, pub url: Option, + pub oauth_callback_port: Option, } fn matches_product_restriction(