diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index c811600717..2f23a8b636 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -340,6 +340,18 @@ Attachment creation and deletion requests using the same thread ID are serialize # Thread plugin settings +Plugin summaries returned by `plugin/list`, `plugin/installed`, `plugin/read`, +and `plugin/share/list` report effective `enabled` state. When +`plugins._default.enabled` is configured, an explicit +`plugins."@".enabled` overrides that default; +a plugin disabled at its source remains disabled. Omitting the default preserves +existing source and account enablement. Other plugin settings, including remembered +tool approvals, inherit the activation default unless `enabled` is explicitly set. +`plugin/search` retains its discovery behavior: results report `enabled: false` +while preserving their installation state. +Remote `plugin/install` still installs a policy-disabled package, but skips MCP and +Apps authentication setup without writing an `enabled` override. + `thread/settings/update` and `turn/start` accept `disabledPluginIds`, a list of `PluginSummary.id` values from `plugin/list`, in the `@` format. A supplied list replaces the selection; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 90f5146ebe..12a6e3f1fc 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -1609,7 +1609,8 @@ impl MessageProcessor { self.catalog_processor.skills_config_write(params).await } ClientRequest::PluginInstall { params, .. } => { - self.plugin_processor.plugin_install(params).await + // Keep installation and auth setup state off the shared request dispatcher stack. + Box::pin(self.plugin_processor.plugin_install(params)).await } ClientRequest::PluginUninstall { params, .. } => { self.plugin_processor.plugin_uninstall(params).await diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 5d6e8fbaf4..b5adda30bb 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -8,6 +8,7 @@ use codex_app_server_protocol::PluginAvailability; use codex_app_server_protocol::PluginSharePrincipalRole; use codex_app_server_protocol::PluginShareTargetRole; use codex_config::types::McpServerConfig; +use codex_config::types::PluginsConfigToml; use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core_plugins::PluginListBackgroundTaskOptions; use codex_core_plugins::PluginMarketplaceContext; @@ -599,6 +600,7 @@ impl PluginRequestProcessor { let use_remote_global_catalog = include_global_remote && auth_mode.is_some_and(DomainAuthMode::uses_codex_backend); let remote_plugin_service_config = remote_plugin_service_config(&config); + let plugin_policy = &config.plugins; let remote_catalog_cache_mode = if force_refetch { RemotePluginCatalogCacheMode::ForceRefetch } else { @@ -687,7 +689,10 @@ impl PluginRequestProcessor { .map(|outcome| outcome.marketplace) { Ok(Some(remote_marketplace)) => { - data.push(remote_marketplace_to_info(remote_marketplace)); + data.push(remote_marketplace_to_info( + remote_marketplace, + plugin_policy, + )); } Ok(None) => {} Err(RemotePluginCatalogError::UnsupportedAuthMode) => {} @@ -737,7 +742,7 @@ impl PluginRequestProcessor { for remote_marketplace in outcome .marketplaces .into_iter() - .map(remote_marketplace_to_info) + .map(|marketplace| remote_marketplace_to_info(marketplace, plugin_policy)) { data.push(remote_marketplace); } @@ -965,6 +970,7 @@ impl PluginRequestProcessor { visible_marketplaces: &[&str], auth: Option<&CodexAuth>, ) -> Vec { + let plugin_policy = &plugins_input.plugins; let remote_marketplaces = if let Some(remote_marketplaces) = plugins_manager .build_remote_installed_plugin_marketplaces_from_cache(visible_marketplaces) { @@ -983,7 +989,7 @@ impl PluginRequestProcessor { match remote_marketplaces { Ok(remote_marketplaces) => remote_marketplaces .into_iter() - .map(remote_marketplace_to_info) + .map(|marketplace| remote_marketplace_to_info(marketplace, plugin_policy)) .collect(), Err( RemotePluginCatalogError::AuthRequired @@ -1208,7 +1214,8 @@ impl PluginRequestProcessor { &app_category_by_id, ) .await; - remote_plugin_detail_to_info(remote_detail, app_summaries) + let plugin_policy = &config.plugins; + remote_plugin_detail_to_info(remote_detail, app_summaries, plugin_policy) } }; @@ -1373,6 +1380,7 @@ impl PluginRequestProcessor { ) -> Result { let (config, auth) = self.load_plugin_share_config_and_auth().await?; let remote_plugin_service_config = remote_plugin_service_config(&config); + let plugin_policy = &config.plugins; let data = codex_core_plugins::remote::list_remote_plugin_shares( &remote_plugin_service_config, auth.as_ref(), @@ -1386,7 +1394,7 @@ impl PluginRequestProcessor { summary, local_plugin_path, } = summary; - let plugin = remote_plugin_summary_to_info(summary); + let plugin = remote_plugin_summary_to_info(summary, plugin_policy); PluginShareListItem { plugin, local_plugin_path, @@ -1664,6 +1672,30 @@ impl PluginRequestProcessor { self.analytics_events_client .track_plugin_installed(plugin_metadata); + let config = match self.load_latest_config(/*fallback_cwd*/ None).await { + Ok(mut reloaded_config) => { + // Keep the policy captured with this installation's auth snapshot. + reloaded_config.application_network_policy = config.application_network_policy; + reloaded_config + } + Err(err) => { + warn!( + "failed to reload config after remote plugin install, skipping auth setup: {err:?}" + ); + return Ok(PluginInstallResponse { + auth_policy: remote_detail.summary.auth_policy, + apps_needing_auth: Vec::new(), + }); + } + }; + + if !config.plugins.allows_plugin(&result.plugin_id.as_key()) { + return Ok(PluginInstallResponse { + auth_policy: remote_detail.summary.auth_policy, + apps_needing_auth: Vec::new(), + }); + } + let plugin_mcp_servers = load_configured_plugin_mcp_servers( result.installed_path.as_path(), auth.as_ref().map(CodexAuth::auth_mode), @@ -2174,7 +2206,10 @@ fn plugin_app_category_by_id_from_value(value: &serde_json::Value) -> HashMap PluginMarketplaceEntry { +fn remote_marketplace_to_info( + marketplace: RemoteMarketplace, + plugin_policy: &PluginsConfigToml, +) -> PluginMarketplaceEntry { PluginMarketplaceEntry { name: marketplace.name, path: None, @@ -2184,12 +2219,16 @@ fn remote_marketplace_to_info(marketplace: RemoteMarketplace) -> PluginMarketpla plugins: marketplace .plugins .into_iter() - .map(remote_plugin_summary_to_info) + .map(|summary| remote_plugin_summary_to_info(summary, plugin_policy)) .collect(), } } -fn remote_plugin_summary_to_info(summary: RemoteCatalogPluginSummary) -> PluginSummary { +fn remote_plugin_summary_to_info( + summary: RemoteCatalogPluginSummary, + plugin_policy: &PluginsConfigToml, +) -> PluginSummary { + let enabled = summary.enabled && plugin_policy.allows_plugin(&summary.id); PluginSummary { id: summary.id, remote_plugin_id: Some(summary.remote_plugin_id), @@ -2204,7 +2243,7 @@ fn remote_plugin_summary_to_info(summary: RemoteCatalogPluginSummary) -> PluginS installed_at: summary .installed_at .map(|installed_at| installed_at.timestamp()), - enabled: summary.enabled, + enabled, install_policy: summary.install_policy, install_policy_source: summary.install_policy_source, must_show_installation_interstitial: summary.must_show_installation_interstitial, @@ -2258,7 +2297,9 @@ fn remote_plugin_share_discoverability_to_info( fn remote_plugin_detail_to_info( detail: RemoteCatalogPluginDetail, apps: Vec, + plugin_policy: &PluginsConfigToml, ) -> PluginDetail { + let summary = remote_plugin_summary_to_info(detail.summary, plugin_policy); let app_templates = detail .app_templates .into_iter() @@ -2294,8 +2335,8 @@ fn remote_plugin_detail_to_info( enabled: skill.enabled, }) .collect::>(); - let onboarding_skill = if detail.summary.enabled - && detail.summary.availability == PluginAvailability::Available + let onboarding_skill = if summary.enabled + && summary.availability == PluginAvailability::Available && let Some(name) = detail.onboarding_skill_name.as_ref() { skills @@ -2309,7 +2350,7 @@ fn remote_plugin_detail_to_info( PluginDetail { marketplace_name: detail.marketplace_name, marketplace_path: None, - summary: remote_plugin_summary_to_info(detail.summary), + summary, share_url: detail.share_url, description: detail.description, skills, diff --git a/codex-rs/app-server/src/request_processors/plugins/search.rs b/codex-rs/app-server/src/request_processors/plugins/search.rs index 6cc25e4b8b..b3d3de93c7 100644 --- a/codex-rs/app-server/src/request_processors/plugins/search.rs +++ b/codex-rs/app-server/src/request_processors/plugins/search.rs @@ -94,6 +94,7 @@ impl PluginRequestProcessor { next_cursor = page.next_page_token; remote_results.reserve(page.plugins.len()); + let plugin_policy = &config.plugins; for plugin in page.plugins { let plugin_id = PluginId::parse(&plugin.id).map_err(|err| { internal_error(format!("invalid remote plugin search result id: {err}")) @@ -114,7 +115,7 @@ impl PluginRequestProcessor { continue; } remote_results.push(plugin_search_result( - remote_plugin_summary_to_info(plugin), + remote_plugin_summary_to_info(plugin, plugin_policy), plugin_id.marketplace_name, /*marketplace_path*/ None, )); 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 3450633f47..3d48cb3f4c 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -2111,8 +2111,17 @@ connectors = true Ok(()) } +#[test_case("", None, true; "legacy activation")] +#[test_case("[plugins._default]\nenabled = false", None, false; "default disabled")] +#[test_case("[plugins._default]\nenabled = false\n[plugins.\"linear@openai-curated-remote\"]\nenabled = true", None, true; "explicitly enabled overrides default")] +#[test_case("", Some("[plugins._default]\nenabled = false"), false; "default disabled during install")] +#[test_case("", Some("[plugins._default]\nenabled = \"invalid\""), false; "invalid policy during install skips auth")] #[tokio::test] -async fn plugin_install_starts_remote_mcp_oauth_for_install_response_only_app() -> Result<()> { +async fn plugin_install_remote_auth_setup_respects_activation_policy( + plugin_config: &str, + plugin_config_after_install: Option<&str>, + expected_auth: bool, +) -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; let oauth_server = MockServer::start().await; @@ -2123,9 +2132,36 @@ async fn plugin_install_starts_remote_mcp_oauth_for_install_response_only_app() ) .await; configure_remote_plugin_with_apps_test(codex_home.path(), &server)?; + let config_path = codex_home.path().join("config.toml"); + let original_config = std::fs::read_to_string(&config_path)?; + let config_contents = format!("{original_config}\n{plugin_config}\n"); + std::fs::write(&config_path, &config_contents)?; mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await; mount_empty_remote_installed_plugins(&server).await; - mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await; + let updated_config = plugin_config_after_install + .map(|plugin_config| format!("{original_config}\n{plugin_config}\n")); + let expected_config = updated_config.as_ref().unwrap_or(&config_contents).clone(); + let install_config_path = config_path.clone(); + Mock::given(method("POST")) + .and(path(format!( + "/backend-api/ps/plugins/{REMOTE_PLUGIN_ID}/install" + ))) + .and(query_param("includeAppsNeedingAuth", "true")) + .and(header("authorization", "Bearer chatgpt-token")) + .and(header("chatgpt-account-id", "account-123")) + .respond_with(move |_request: &Request| { + if let Some(updated_config) = &updated_config { + std::fs::write(&install_config_path, updated_config) + .expect("install responder should update config"); + } + ResponseTemplate::new(/*s*/ 200).set_body_json(json!({ + "id": REMOTE_PLUGIN_ID, + "enabled": true, + "app_ids_needing_auth": ["alpha"], + })) + }) + .mount(&server) + .await; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -2141,16 +2177,44 @@ async fn plugin_install_starts_remote_mcp_oauth_for_install_response_only_app() response, PluginInstallResponse { auth_policy: PluginAuthPolicy::OnUse, - apps_needing_auth: vec![AppSummary { - id: "alpha".to_string(), - name: "alpha".to_string(), - description: None, - install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), - category: None, - }], + apps_needing_auth: if expected_auth { + vec![AppSummary { + id: "alpha".to_string(), + name: "alpha".to_string(), + description: None, + install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), + category: None, + }] + } else { + Vec::new() + }, } ); - assert!(oauth_discovery_request_count(&oauth_server).await > 0); + if expected_auth { + assert!(oauth_discovery_request_count(&oauth_server).await > 0); + } else { + assert!( + oauth_server + .received_requests() + .await + .unwrap_or_default() + .is_empty() + ); + } + wait_for_remote_plugin_request_count( + &server, + "POST", + &format!("/ps/plugins/{REMOTE_PLUGIN_ID}/install"), + /*expected_count*/ 1, + ) + .await?; + assert!( + codex_home + .path() + .join("plugins/cache/openai-curated-remote/linear/1.2.3/.codex-plugin/plugin.json") + .is_file() + ); + assert_eq!(std::fs::read_to_string(config_path)?, expected_config); Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index dd200094ce..075ab81845 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -279,9 +279,13 @@ enabled = true Ok(()) } +#[test_case("", true; "without default")] +#[test_case("[plugins._default]\nenabled = false", false; "default disabled")] #[tokio::test] -async fn plugin_installed_prefers_remote_curated_conflicts_when_remote_plugin_enabled() -> Result<()> -{ +async fn plugin_installed_prefers_remote_curated_conflicts_when_remote_plugin_enabled( + plugin_config: &str, + expected_remote_enabled: bool, +) -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; write_openai_curated_marketplace(codex_home.path(), &["linear", "calendar"])?; @@ -301,6 +305,8 @@ enabled = true [plugins."calendar@openai-curated"] enabled = true + +{plugin_config} "#, server.uri() ), @@ -356,9 +362,9 @@ enabled = true local_marketplace .plugins .iter() - .map(|plugin| plugin.id.clone()) + .map(|plugin| (plugin.id.clone(), plugin.enabled)) .collect::>(), - vec!["calendar@openai-curated".to_string()] + vec![("calendar@openai-curated".to_string(), true)] ); let remote_marketplace = response .marketplaces @@ -372,6 +378,7 @@ enabled = true .map(|plugin| { ( plugin.id.clone(), + plugin.enabled, plugin.install_policy_source, plugin.must_show_installation_interstitial, ) @@ -380,11 +387,13 @@ enabled = true vec![ ( "linear@openai-curated-remote".to_string(), + expected_remote_enabled, Some(PluginInstallPolicySource::WorkspaceSetting), Some(false), ), ( "remote-only@openai-curated-remote".to_string(), + expected_remote_enabled, Some(PluginInstallPolicySource::WorkspaceSetting), Some(false), ), @@ -1332,8 +1341,15 @@ async fn plugin_catalogs_skip_invalid_project_config_and_report_cwd_error() -> R Ok(()) } +#[test_case("", "enabled = true", true; "legacy explicit enabled")] +#[test_case("[plugins._default]\nenabled = false", "mcp_servers.example.tools.search.approval_mode = 'approve'", false; "default disabled approval-only entry")] +#[test_case("[plugins._default]\nenabled = false", "enabled = true", true; "explicitly enabled overrides default")] #[tokio::test] -async fn plugin_list_includes_install_and_enabled_state_from_config() -> Result<()> { +async fn plugin_list_includes_install_and_enabled_state_from_config( + plugin_default: &str, + plugin_settings: &str, + expected_enabled: bool, +) -> Result<()> { let codex_home = TempDir::new()?; let repo_root = TempDir::new()?; std::fs::create_dir_all(repo_root.path().join(".git"))?; @@ -1374,15 +1390,19 @@ async fn plugin_list_includes_install_and_enabled_state_from_config() -> Result< )?; std::fs::write( codex_home.path().join("config.toml"), - r#"[features] + format!( + r#"[features] plugins = true +{plugin_default} + [plugins."enabled-plugin@codex-curated"] -enabled = true +{plugin_settings} [plugins."disabled-plugin@codex-curated"] enabled = false "#, + ), )?; let mut mcp = TestAppServer::builder() @@ -1427,7 +1447,7 @@ enabled = false assert_eq!(marketplace.plugins[0].id, "enabled-plugin@codex-curated"); assert_eq!(marketplace.plugins[0].name, "enabled-plugin"); assert_eq!(marketplace.plugins[0].installed, true); - assert_eq!(marketplace.plugins[0].enabled, true); + assert_eq!(marketplace.plugins[0].enabled, expected_enabled); assert_eq!( marketplace.plugins[0].install_policy, PluginInstallPolicy::Available @@ -1463,6 +1483,35 @@ enabled = false marketplace.plugins[2].auth_policy, PluginAuthPolicy::OnInstall ); + + let request_id = mcp + .send_plugin_installed_request(PluginInstalledParams { + cwds: Some(vec![AbsolutePathBuf::try_from(repo_root.path())?]), + install_suggestion_plugin_names: None, + }) + .await?; + let response: PluginInstalledResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + let installed = response + .marketplaces + .into_iter() + .find(|entry| entry.path == marketplace.path) + .expect("expected installed repo marketplace entry"); + assert_eq!( + installed + .plugins + .into_iter() + .map(|plugin| (plugin.id, plugin.installed, plugin.enabled)) + .collect::>(), + vec![ + ( + "enabled-plugin@codex-curated".to_string(), + true, + expected_enabled, + ), + ("disabled-plugin@codex-curated".to_string(), true, false), + ] + ); Ok(()) } @@ -2088,14 +2137,20 @@ enum ProjectPluginConfiguration { Invalid, } -#[test_case(ProjectPluginConfiguration::None, None; "without project override")] -#[test_case(ProjectPluginConfiguration::Disabled, None; "project disables local plugins")] -#[test_case(ProjectPluginConfiguration::Invalid, None; "invalid project preserves remote plugins")] -#[test_case(ProjectPluginConfiguration::None, Some("tpp"); "configured product sku")] +#[test_case(ProjectPluginConfiguration::None, None, "", true, true; "without project override")] +#[test_case(ProjectPluginConfiguration::Disabled, None, "", true, true; "project disables local plugins")] +#[test_case(ProjectPluginConfiguration::Invalid, None, "", true, true; "invalid project preserves remote plugins")] +#[test_case(ProjectPluginConfiguration::None, Some("tpp"), "", true, true; "configured product sku")] +#[test_case(ProjectPluginConfiguration::None, None, "[plugins._default]\nenabled = false", true, false; "default disabled")] +#[test_case(ProjectPluginConfiguration::None, None, "[plugins._default]\nenabled = false\n[plugins.\"linear@openai-curated-remote\"]\nenabled = true", true, true; "explicitly enabled")] +#[test_case(ProjectPluginConfiguration::None, None, "[plugins._default]\nenabled = false\n[plugins.\"linear@openai-curated-remote\"]\nenabled = true", false, false; "source disabled")] #[tokio::test] async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled( project_configuration: ProjectPluginConfiguration, product_sku: Option<&str>, + plugin_config: &str, + source_enabled: bool, + expected_enabled: bool, ) -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; @@ -2103,6 +2158,9 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled( codex_home.path(), &format!("{}/backend-api/", server.uri()), )?; + let config_path = codex_home.path().join("config.toml"); + let config = std::fs::read_to_string(&config_path)?; + std::fs::write(config_path, format!("{config}\n{plugin_config}\n"))?; write_chatgpt_auth( codex_home.path(), ChatGptAuthFixture::new("chatgpt-token") @@ -2210,6 +2268,8 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled( } }"#; + let mut global_installed_body: serde_json::Value = serde_json::from_str(global_installed_body)?; + global_installed_body["plugins"][0]["enabled"] = serde_json::json!(source_enabled); let expected_product_sku = product_sku.unwrap_or("codex"); Mock::given(method("GET")) .and(path("/backend-api/ps/plugins/list")) @@ -2237,7 +2297,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled( .and(header("authorization", "Bearer chatgpt-token")) .and(header("chatgpt-account-id", "account-123")) .and(header("oai-product-sku", expected_product_sku)) - .respond_with(ResponseTemplate::new(200).set_body_string(global_installed_body)) + .respond_with(ResponseTemplate::new(200).set_body_json(global_installed_body)) .mount(&server) .await; Mock::given(method("GET")) @@ -2329,7 +2389,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled( remote_marketplace.plugins[0].installed_at, Some(1_767_312_000) ); - assert_eq!(remote_marketplace.plugins[0].enabled, true); + assert_eq!(remote_marketplace.plugins[0].enabled, expected_enabled); assert_eq!( remote_marketplace.plugins[0].install_policy_source, Some(PluginInstallPolicySource::ImplicitCanonicalApp) diff --git a/codex-rs/app-server/tests/suite/v2/plugin_read.rs b/codex-rs/app-server/tests/suite/v2/plugin_read.rs index b4edaa061f..f553cfe8e6 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_read.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_read.rs @@ -61,16 +61,21 @@ use wiremock::matchers::query_param; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); -#[test_case(Some("skills/setup/SKILL.md"), true, true, true; "enabled matching skill")] -#[test_case(Some("skills/setup/SKILL.md"), false, true, false; "disabled plugin")] -#[test_case(Some("skills/setup/SKILL.md"), true, false, false; "disabled skill")] -#[test_case(None, true, true, false; "missing declaration")] -#[test_case(Some("skills/missing/SKILL.md"), true, true, false; "unmatched declaration")] +#[test_case(Some("skills/setup/SKILL.md"), "enabled = true", true, "", true, true; "enabled matching skill")] +#[test_case(Some("skills/setup/SKILL.md"), "enabled = false", true, "", false, false; "disabled plugin")] +#[test_case(Some("skills/setup/SKILL.md"), "enabled = true", false, "", true, false; "disabled skill")] +#[test_case(None, "enabled = true", true, "", true, false; "missing declaration")] +#[test_case(Some("skills/missing/SKILL.md"), "enabled = true", true, "", true, false; "unmatched declaration")] +#[test_case(Some("skills/setup/SKILL.md"), "", true, "", true, true; "legacy empty entry")] +#[test_case(Some("skills/setup/SKILL.md"), "", true, "[plugins._default]\nenabled = false", false, false; "default disabled empty entry")] +#[test_case(Some("skills/setup/SKILL.md"), "enabled = true", true, "[plugins._default]\nenabled = false", true, true; "explicitly enabled overrides default")] #[tokio::test] async fn plugin_read_selects_local_onboarding_skill( onboarding_path: Option<&str>, - plugin_enabled: bool, + plugin_settings: &str, skill_enabled: bool, + plugin_default: &str, + expected_plugin_enabled: bool, expect_onboarding: bool, ) -> Result<()> { let codex_home = TempDir::new()?; @@ -106,8 +111,10 @@ async fn plugin_read_selects_local_onboarding_skill( r#"[features] plugins = true +{plugin_default} + [plugins."demo-plugin@codex-curated"] -enabled = {plugin_enabled} +{plugin_settings} [[skills.config]] name = "demo-plugin:setup" @@ -146,7 +153,7 @@ enabled = {skill_enabled} ), enabled: skill_enabled, }; - assert_eq!(response.plugin.summary.enabled, plugin_enabled); + assert_eq!(response.plugin.summary.enabled, expected_plugin_enabled); assert!(response.plugin.skills.contains(&setup_skill)); assert_eq!( response.plugin.onboarding_skill, @@ -155,18 +162,20 @@ enabled = {skill_enabled} Ok(()) } -#[test_case(Some("setup"), true, true, "AVAILABLE", true; "enabled matching skill")] -#[test_case(Some("setup"), false, true, "AVAILABLE", false; "disabled plugin")] -#[test_case(Some("setup"), true, false, "AVAILABLE", false; "disabled skill")] -#[test_case(None, true, true, "AVAILABLE", false; "missing declaration")] -#[test_case(Some("missing"), true, true, "AVAILABLE", false; "unmatched declaration")] -#[test_case(Some("setup"), true, true, "DISABLED_BY_ADMIN", false; "unavailable plugin")] +#[test_case(Some("setup"), true, true, "AVAILABLE", None, true; "enabled matching skill")] +#[test_case(Some("setup"), false, true, "AVAILABLE", None, false; "disabled plugin")] +#[test_case(Some("setup"), true, false, "AVAILABLE", None, false; "disabled skill")] +#[test_case(None, true, true, "AVAILABLE", None, false; "missing declaration")] +#[test_case(Some("missing"), true, true, "AVAILABLE", None, false; "unmatched declaration")] +#[test_case(Some("setup"), true, true, "DISABLED_BY_ADMIN", None, false; "unavailable plugin")] +#[test_case(Some("setup"), true, true, "AVAILABLE", Some(false), false; "default disabled")] #[tokio::test] async fn plugin_read_selects_remote_onboarding_skill( onboarding_name: Option<&str>, plugin_enabled: bool, skill_enabled: bool, status: &str, + default_enabled: Option, expect_onboarding: bool, ) -> Result<()> { let codex_home = TempDir::new()?; @@ -223,8 +232,11 @@ async fn plugin_read_selects_remote_onboarding_skill( }))) .mount(&server) .await; - let mut mcp = TestAppServer::builder() - .with_codex_home(codex_home.path()) + let mut builder = TestAppServer::builder().with_codex_home(codex_home.path()); + if let Some(enabled) = default_enabled { + builder = builder.with_args(&["-c", &format!("plugins._default.enabled={enabled}")]); + } + let mut mcp = builder .without_auto_env() .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; @@ -246,7 +258,10 @@ async fn plugin_read_selects_remote_onboarding_skill( path: None, enabled: skill_enabled, }; - assert_eq!(response.plugin.summary.enabled, plugin_enabled); + assert_eq!( + response.plugin.summary.enabled, + plugin_enabled && default_enabled.unwrap_or(true) + ); assert!(response.plugin.skills.contains(&setup_skill)); assert_eq!( response.plugin.onboarding_skill, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index a0aa1fc230..c41c66e062 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -156,6 +156,7 @@ pub struct McpConfig { pub permission_profile: PermissionProfile, /// Configuration layers used to evaluate Apps tool policy and reviewer selection. pub config_layer_stack: ConfigLayerStack, + pub plugins: codex_config::types::PluginsConfigToml, /// Default reviewer used when an Apps tool has no reviewer override. pub approvals_reviewer: ApprovalsReviewer, /// Working directories for the exact environment handles used by this runtime. diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 3ca349f047..79eeefb4e7 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -101,6 +101,7 @@ pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig { approval_policy: Constrained::allow_any(AskForApproval::OnRequest), permission_profile: PermissionProfile::default(), config_layer_stack: codex_config::ConfigLayerStack::default(), + plugins: Default::default(), approvals_reviewer: codex_config::types::ApprovalsReviewer::default(), environment_cwds: HashMap::new(), environment_use_mxc: HashMap::new(), diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index d74aeff37a..1ff5cf929a 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -24,7 +24,7 @@ use crate::types::MemoriesToml; use crate::types::Notice; use crate::types::OAuthCredentialsStoreMode; use crate::types::OtelConfigToml; -use crate::types::PluginConfig; +use crate::types::PluginsConfigToml; use crate::types::SandboxWorkspaceWrite; use crate::types::ShellEnvironmentPolicyToml; use crate::types::SkillsConfig; @@ -496,9 +496,9 @@ pub struct ConfigToml { /// Lifecycle hooks configured inline in TOML plus user-level overrides. pub hooks: Option, - /// User-level plugin config entries keyed by plugin name. + /// Default and per-plugin settings keyed by exact `@` IDs. #[serde(default)] - pub plugins: HashMap, + pub plugins: PluginsConfigToml, /// User-level marketplace entries keyed by marketplace name. #[serde(default)] diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index ab2a0aaec6..15f74c9e71 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -1034,11 +1034,49 @@ pub use crate::skills_config::BundledSkillsConfig; pub use crate::skills_config::SkillConfig; pub use crate::skills_config::SkillsConfig; +/// Default activation settings for installed plugins. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct PluginsDefaultConfig { + #[serde(default = "default_enabled")] + pub enabled: bool, +} + +/// Plugin activation and capability settings loaded from configuration layers. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct PluginsConfigToml { + /// Defaults for installed plugin activation. Omission preserves source enablement. + /// Explicit plugin enablement settings override this default, but cannot enable + /// an installation disabled by its source. + #[serde(default, rename = "_default", skip_serializing_if = "Option::is_none")] + pub default: Option, + + #[serde(default, flatten)] + pub plugins: HashMap, +} + +impl PluginsConfigToml { + /// Whether configuration permits a source-enabled plugin to remain active. + /// Without `_default`, preserve legacy source and account enablement. + /// Otherwise, an explicit enablement setting overrides the configured default. + pub fn allows_plugin(&self, plugin_id: &str) -> bool { + let Some(default) = &self.default else { + return true; + }; + self.plugins + .get(plugin_id) + .and_then(|plugin| plugin.enabled) + .unwrap_or(default.enabled) + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct PluginConfig { - #[serde(default = "default_enabled")] - pub enabled: bool, + /// Explicit activation override. Omission inherits the installed-plugin default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub enabled: Option, /// Per-MCP-server policy overlays for MCP servers contributed by this plugin. #[serde(default, skip_serializing_if = "HashMap::is_empty")] diff --git a/codex-rs/config/src/types_tests.rs b/codex-rs/config/src/types_tests.rs index 5a9b3d81d9..5b5303b6d9 100644 --- a/codex-rs/config/src/types_tests.rs +++ b/codex-rs/config/src/types_tests.rs @@ -1,6 +1,60 @@ use super::*; use pretty_assertions::assert_eq; +#[test] +fn plugin_activation_inherits_defaults_without_explicit_enablement() { + for default_enabled in [None, Some(false), Some(true)] { + let default_config = default_enabled.map_or_else(String::new, |enabled| { + format!("[_default]\nenabled = {enabled}\n") + }); + let config: PluginsConfigToml = toml::from_str(&format!( + r#"{default_config} +["empty@market"] +["mcp@market".mcp_servers.example] +enabled = false +["approval@market".mcp_servers.example] +default_tools_approval_mode = "approve" +["enabled@market"] +enabled = true +["disabled@market"] +enabled = false +"# + )) + .expect("plugin activation config"); + assert_eq!( + [ + "empty@market", + "mcp@market", + "approval@market", + "enabled@market", + "disabled@market" + ] + .map(|plugin_id| config.plugins[plugin_id].enabled), + [None, None, None, Some(true), Some(false)], + ); + assert_eq!( + [ + "missing@market", + "empty@market", + "mcp@market", + "approval@market", + "enabled@market", + "disabled@market" + ] + .map(|plugin_id| config.allows_plugin(plugin_id)), + match default_enabled { + None => [true, true, true, true, true, true], + Some(false) => [false, false, false, false, true, false], + Some(true) => [true, true, true, true, true, false], + }, + ); + let round_trip: PluginsConfigToml = + toml::from_str(&toml::to_string(&config).expect("serialize plugin config")) + .expect("deserialize plugin config"); + assert_eq!(round_trip, config); + } +} + #[test] fn mouse_scroll_speed_accepts_integer_and_fractional_multipliers() { for (value, expected) in [("1", 1.0), ("0.5", 0.5), ("3.0", 3.0)] { diff --git a/codex-rs/core-plugins/src/executor_hooks.rs b/codex-rs/core-plugins/src/executor_hooks.rs index b848d97502..abc59ab80b 100644 --- a/codex-rs/core-plugins/src/executor_hooks.rs +++ b/codex-rs/core-plugins/src/executor_hooks.rs @@ -2,6 +2,7 @@ use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_config::HookHandlerConfig; +use codex_config::types::PluginsConfigToml; use codex_exec_server::ExecutorCapabilityDiscoverySnapshot; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY; @@ -28,11 +29,15 @@ use crate::manifest::parse_plugin_manifest_uri; /// hooks from the bundled cleanup allowlist and the remote Browser plugin. pub fn executor_plugin_hook_sources<'a>( snapshot: &ExecutorCapabilityDiscoverySnapshot, + plugins_config: &PluginsConfigToml, lookup_enabled_tool: impl Fn(&str, &str) -> Option<&'a ToolInfo>, ) -> Vec { let mut sources = Vec::new(); for entry in snapshot.roots() { + if !plugins_config.allows_plugin(&entry.selected_root.id) { + continue; + } let Ok(plugin_id) = PluginId::parse(&entry.selected_root.id) else { continue; }; diff --git a/codex-rs/core-plugins/src/executor_hooks_tests.rs b/codex-rs/core-plugins/src/executor_hooks_tests.rs index b0e5f8c35a..fab975d655 100644 --- a/codex-rs/core-plugins/src/executor_hooks_tests.rs +++ b/codex-rs/core-plugins/src/executor_hooks_tests.rs @@ -130,7 +130,8 @@ fn discovers_allowlisted_executor_plugin_hook_sources() { manifest, ); - let sources = executor_plugin_hook_sources(&snapshot, |_, _| None); + let sources = + executor_plugin_hook_sources(&snapshot, &PluginsConfigToml::default(), |_, _| None); let mut interrupt_source = expected_source(/*index*/ 1); interrupt_source.hooks.interrupt = std::mem::take(&mut interrupt_source.hooks.stop); let mut stop_and_interrupt_source = expected_source(/*index*/ 2); @@ -144,6 +145,22 @@ fn discovers_allowlisted_executor_plugin_hook_sources() { stop_and_interrupt_source, ] ); + for (config, expected) in [ + ( + "[_default]\nenabled = false\n[\"computer-use@other-marketplace\"]\nenabled = true", + Vec::new(), + ), + ( + "[_default]\nenabled = false\n[\"computer-use@openai-bundled\"]\nenabled = true", + sources, + ), + ] { + let config = toml::from_str(config).expect("plugin policy"); + assert_eq!( + executor_plugin_hook_sources(&snapshot, &config, |_, _| None), + expected + ); + } } #[test] @@ -165,7 +182,7 @@ fn discovers_unified_computer_use_cleanup_hooks() { *server = "cua_repl".to_string(); assert_eq!( - executor_plugin_hook_sources(&snapshot, |_, _| None), + executor_plugin_hook_sources(&snapshot, &PluginsConfigToml::default(), |_, _| None), vec![expected] ); } @@ -202,7 +219,7 @@ fn filters_mixed_handlers_without_rewriting_allowed_groups() { ); assert_eq!( - executor_plugin_hook_sources(&snapshot, |_, _| None), + executor_plugin_hook_sources(&snapshot, &PluginsConfigToml::default(), |_, _| None), vec![expected] ); } @@ -234,7 +251,7 @@ fn preserves_allowlisted_executor_plugin_hook_options() { expected.hooks.interrupt = expected.hooks.stop.clone(); assert_eq!( - executor_plugin_hook_sources(&snapshot, |_, _| None), + executor_plugin_hook_sources(&snapshot, &PluginsConfigToml::default(), |_, _| None), vec![expected] ); } @@ -378,11 +395,15 @@ fn resolves_apps_hook_metadata_from_the_registered_connector() { })) .expect("listed tool") }); - let actual = executor_plugin_hook_sources(&snapshot, |server, tool| { - tool_info - .as_ref() - .filter(|info| info.server_name == server && info.tool.name == tool) - }); + let actual = executor_plugin_hook_sources( + &snapshot, + &PluginsConfigToml::default(), + |server, tool| { + tool_info + .as_ref() + .filter(|info| info.server_name == server && info.tool.name == tool) + }, + ); let expected = if admitted { vec![expected.clone()] } else { @@ -454,7 +475,7 @@ fn ignores_unallowlisted_executor_plugin_hooks() { ); assert_eq!( - executor_plugin_hook_sources(&snapshot, |_, _| None), + executor_plugin_hook_sources(&snapshot, &PluginsConfigToml::default(), |_, _| None), Vec::::new(), "{name}" ); @@ -474,7 +495,7 @@ fn ignores_file_backed_executor_plugin_hooks() { ); assert_eq!( - executor_plugin_hook_sources(&file_backed, |_, _| None), + executor_plugin_hook_sources(&file_backed, &PluginsConfigToml::default(), |_, _| None), Vec::::new() ); } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index d0a9e06622..f0f9a12a77 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -30,6 +30,7 @@ use codex_config::types::McpServerConfig; use codex_config::types::McpServerTransportConfig; use codex_config::types::PluginConfig; use codex_config::types::PluginMcpServerConfig; +use codex_config::types::PluginsConfigToml; use codex_connectors::parse_plugin_app_config; use codex_connectors::parse_plugin_app_config_value; use codex_mcp::parse_agent_plugin_mcp_config; @@ -54,6 +55,7 @@ use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::SkillDiscoveryMode; use codex_utils_plugins::find_plugin_manifest_path; use codex_utils_plugins::migrated_command_skills_root; +use serde::Deserialize; use serde_json::Value as JsonValue; use std::collections::HashMap; use std::collections::HashSet; @@ -131,8 +133,11 @@ pub(crate) fn log_plugin_load_errors(plugins: &[LoadedPlugin]) /// Load configured plugins without applying auth-dependent runtime policies. #[instrument(level = "trace", skip_all)] +// Keep validated policy with the existing snapshot inputs instead of reparsing layers. +#[allow(clippy::too_many_arguments)] pub(crate) async fn load_plugins_from_layer_stack( config_layer_stack: &ConfigLayerStack, + plugin_policy: &codex_config::types::PluginsConfigToml, remote_installed_plugins_snapshot: RemoteInstalledPluginsSnapshot, store: &PluginStore, plugin_skill_snapshots: Option<&SkillRootSnapshots>, @@ -147,6 +152,7 @@ pub(crate) async fn load_plugins_from_layer_stack( } = remote_installed_plugins_snapshot; load_plugins_from_layer_stack_with_scope( config_layer_stack, + plugin_policy, extra_plugins, store, remote_global_catalog_active, @@ -163,17 +169,22 @@ pub(crate) async fn load_plugins_from_layer_stack( async fn load_plugins_from_layer_stack_with_scope( config_layer_stack: &ConfigLayerStack, + plugin_policy: &codex_config::types::PluginsConfigToml, extra_plugins: HashMap, store: &PluginStore, remote_global_catalog_active: bool, scope: PluginLoadScope<'_>, ) -> Vec> { - let configured_plugins = merge_configured_plugins_with_remote_installed( + let mut configured_plugins = merge_configured_plugins_with_remote_installed( configured_plugins_from_stack(config_layer_stack, store.codex_home().as_path()), extra_plugins, store, remote_global_catalog_active, ); + for (plugin_id, plugin) in &mut configured_plugins { + plugin.enabled = + Some(plugin.enabled.unwrap_or(true) && plugin_policy.allows_plugin(plugin_id)); + } let mut configured_plugins: Vec<_> = configured_plugins.into_iter().collect(); configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); @@ -202,6 +213,7 @@ async fn load_plugins_from_layer_stack_with_scope( /// Load hooks from enabled plugins without loading their skills, MCP servers, or apps. pub async fn load_plugin_hooks_from_layer_stack( config_layer_stack: &ConfigLayerStack, + plugin_policy: &codex_config::types::PluginsConfigToml, extra_plugins: HashMap, store: &PluginStore, target_curated_marketplace: TargetCuratedMarketplace, @@ -209,6 +221,7 @@ pub async fn load_plugin_hooks_from_layer_stack( ) -> PluginHookLoadOutcome { let mut plugins = load_plugins_from_layer_stack_with_scope( config_layer_stack, + plugin_policy, extra_plugins, store, remote_global_catalog_active, @@ -365,7 +378,7 @@ pub fn remote_installed_plugins_to_config( Some(( plugin_id.as_key(), PluginConfig { - enabled: plugin.enabled, + enabled: Some(plugin.enabled), mcp_servers: HashMap::new(), }, )) @@ -736,8 +749,8 @@ fn configured_plugins_from_config_value( let Some(plugins_value) = user_config.get("plugins") else { return HashMap::new(); }; - match plugins_value.clone().try_into() { - Ok(plugins) => plugins, + match PluginsConfigToml::deserialize(plugins_value.clone()) { + Ok(plugins) => plugins.plugins, Err(err) => { warn!("invalid plugins config: {err}"); HashMap::new() @@ -846,7 +859,7 @@ async fn load_plugin( plugin_namespace: None, manifest_description: None, root, - enabled: plugin.enabled, + enabled: plugin.enabled.unwrap_or(true), skill_roots: Vec::new(), skill_discovery_mode: SkillDiscoveryMode::Recursive, disabled_skill_paths: HashSet::new(), @@ -858,7 +871,7 @@ async fn load_plugin( error: None, }; - if !plugin.enabled { + if !loaded_plugin.enabled { return loaded_plugin; } @@ -1421,16 +1434,6 @@ pub async fn load_configured_plugin_mcp_servers( load_plugin_mcp_servers_with_policy(plugin_root, auth_mode, plugin_policy).await } -/// Resolves effective per-plugin MCP policies without validating opaque selected-root IDs. -pub fn configured_plugin_mcp_server_policies( - config_layer_stack: &ConfigLayerStack, -) -> HashMap> { - configured_plugins_from_config_value(&config_layer_stack.effective_config()) - .into_iter() - .map(|(plugin_id, plugin)| (plugin_id, plugin.mcp_servers)) - .collect() -} - /// Applies user policy without widening the selected plugin's declared restrictions. pub fn apply_configured_plugin_mcp_server_policies( policies: &HashMap, diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index e997825b0c..605879bea8 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -7,6 +7,7 @@ use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerSource; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; +use codex_config::config_toml::ConfigToml; use codex_plugin::PluginId; use codex_utils_plugins::AGENT_PLUGIN_SCHEMA_URI; use pretty_assertions::assert_eq; @@ -278,6 +279,11 @@ async fn installed_agent_plugin_uses_isolated_data_root_for_stdio_mcp() { let plugins = load_plugins_from_layer_stack( &stack, + &stack + .effective_config() + .try_into::() + .unwrap() + .plugins, RemoteInstalledPluginsSnapshot::default(), &store, /*plugin_skill_snapshots*/ None, @@ -370,29 +376,30 @@ fn configured_plugins_from_stack_merges_enabled_effective_layers() { ( "base".to_string(), PluginConfig { - enabled: true, + enabled: Some(true), mcp_servers: HashMap::new(), }, ), ( "profile".to_string(), PluginConfig { - enabled: true, + enabled: Some(true), mcp_servers: project_mcp_servers.clone(), }, ), ( "system".to_string(), PluginConfig { - enabled: true, + enabled: Some(true), mcp_servers: HashMap::new(), }, ), ]) ); + let config: ConfigToml = stack.effective_config().try_into().expect("plugin config"); assert_eq!( - configured_plugin_mcp_server_policies(&stack).get("profile"), - Some(&project_mcp_servers) + config.plugins.plugins["profile"].mcp_servers, + project_mcp_servers ); } @@ -420,8 +427,8 @@ async fn legacy_ema_policy_disables_installed_and_selected_plugin_servers() { ConfigRequirementsToml::default(), ) .expect("legacy policy stack"); - let policies = configured_plugin_mcp_server_policies(&stack); - let policy = policies.get("plugin@market").expect("plugin policy"); + let config: ConfigToml = stack.effective_config().try_into().expect("plugin config"); + let policy = &config.plugins.plugins["plugin@market"].mcp_servers; let installed = load_plugin_mcp_servers_with_policy( &plugin_root, /*auth_mode*/ None, @@ -536,6 +543,11 @@ enabled = true let full = load_plugins_from_layer_stack( &stack, + &stack + .effective_config() + .try_into::() + .unwrap() + .plugins, RemoteInstalledPluginsSnapshot::default(), &store, /*plugin_skill_snapshots*/ None, @@ -546,6 +558,11 @@ enabled = true .await; let hooks_only = load_plugins_from_layer_stack_with_scope( &stack, + &stack + .effective_config() + .try_into::() + .unwrap() + .plugins, HashMap::new(), &store, /*remote_global_catalog_active*/ false, @@ -587,6 +604,78 @@ enabled = true assert!(hooks_only_valid.skill_roots.is_empty()); assert!(hooks_only_valid.mcp_servers.is_empty()); assert!(hooks_only_valid.apps.is_empty()); + + for (policy, active, extra_plugins) in [ + ( + "[plugins._default]\nenabled = false\n[plugins.\"valid@test\"]\nenabled = true", + true, + // Typed policy must also veto enabled entries from the original source map. + configured_plugins_from_stack(&stack, temp_dir.path()), + ), + ( + "[plugins.\"valid@test\".mcp_servers.example]\nenabled = true", + true, + HashMap::new(), + ), + ( + "[plugins._default]\nenabled = false\n[plugins.\"valid@test\".mcp_servers.example]\ndefault_tools_approval_mode = 'approve'", + false, + HashMap::new(), + ), + ] { + let stack = ConfigLayerStack::default() + .with_user_config( + &user_config_path(&temp_dir, "inherited.toml"), + toml::from_str(policy).expect("plugin policy"), + ) + .expect("valid inherited config"); + let policy = stack + .effective_config() + .try_into::() + .expect("plugin config") + .plugins; + let plugins = load_plugins_from_layer_stack( + &stack, + &policy, + RemoteInstalledPluginsSnapshot::default(), + &store, + /*plugin_skill_snapshots*/ None, + Some(Product::Codex), + /*remote_global_catalog_active*/ false, + test_skill_root_loader().as_ref(), + ) + .await; + let plugin = &plugins[0]; + assert_eq!( + ( + plugin.is_active(), + !plugin.skill_roots.is_empty(), + !plugin.mcp_servers.is_empty(), + !plugin.apps.is_empty() + ), + (active, active, active, active), + ); + let hooks = load_plugin_hooks_from_layer_stack( + &stack, + &policy, + extra_plugins, + &store, + TargetCuratedMarketplace::OpenAi, + /*remote_global_catalog_active*/ false, + ) + .await; + assert_eq!( + hooks, + PluginHookLoadOutcome { + hook_sources: if active { + hooks_only_valid.hook_sources.clone() + } else { + Vec::new() + }, + hook_load_warnings: Vec::new(), + }, + ); + } } #[test] diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 07eb179f45..d4411b50da 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -95,6 +95,7 @@ use codex_config::clear_user_plugin; use codex_config::set_user_plugin_enabled; use codex_config::skill_config_rules_from_stack; use codex_config::types::PluginConfig; +use codex_config::types::PluginsConfigToml; use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverableType; use codex_connectors::ConnectorSnapshot; @@ -157,6 +158,7 @@ type EffectivePluginsChangedCallback = Arc Self { Self { config_layer_stack, + plugins, model_provider_id, plugins_enabled, remote_plugin_enabled, @@ -633,6 +639,7 @@ impl LoadedPluginsCache { #[derive(Clone, PartialEq, Eq)] struct PluginLoadCacheKey { configured_plugins: HashMap, + plugin_policy: PluginsConfigToml, skill_config_rules: SkillConfigRules, remote_global_catalog_active: bool, auth_identity: Option, @@ -650,6 +657,7 @@ impl PluginLoadCacheKey { &config.config_layer_stack, codex_home, ), + plugin_policy: config.plugins.clone(), skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack), remote_global_catalog_active, // Local curated loads are auth-independent; only remote snapshots vary by account. @@ -882,6 +890,7 @@ impl PluginsManager { let load_started = Instant::now(); let plugins = load_plugins_from_layer_stack( &config.config_layer_stack, + &config.plugins, self.remote_installed_plugins_snapshot(), &self.store, Some(&plugin_skill_snapshots), @@ -1003,6 +1012,7 @@ impl PluginsManager { let target_curated_marketplace = target_curated_marketplace(self.auth_mode()); load_plugin_hooks_from_layer_stack( config_layer_stack, + &config.plugins, self.remote_installed_plugin_configs(), &self.store, target_curated_marketplace, @@ -3577,7 +3587,10 @@ impl PluginsManager { .collect::>(); let enabled = configured_plugins .into_iter() - .filter_map(|(plugin_key, plugin)| plugin.enabled.then_some(plugin_key)) + .filter_map(|(plugin_key, plugin)| { + (plugin.enabled.unwrap_or(true) && config.plugins.allows_plugin(&plugin_key)) + .then_some(plugin_key) + }) .collect::>(); ConfiguredPluginStates { installed, enabled } } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 27ec9eb0ca..6ba94a6738 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -122,6 +122,9 @@ fn plugins_config_input_with_requirements( ) -> PluginsConfigInput { PluginsConfigInput::new( config_layer_stack_with_requirements(codex_home, user_config, requirements), + toml::from_str::(user_config) + .unwrap() + .plugins, String::new(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ false, @@ -163,6 +166,7 @@ fn curated_repo_sync_stays_deferred_for_remote_chatgpt_catalog() { let tmp = TempDir::new().unwrap(); let config = PluginsConfigInput::new( unrestricted_config_layer_stack(), + Default::default(), "openai".to_string(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ true, @@ -2987,6 +2991,11 @@ async fn plugin_cache_reuses_effective_configurations() { let config = |session_config| { PluginsConfigInput::new( stack(session_config), + stack(session_config) + .effective_config() + .try_into::() + .unwrap() + .plugins, String::new(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ false, @@ -3037,6 +3046,113 @@ enabled = false"#, ); } +#[tokio::test] +async fn plugin_defaults_resolve_remote_enablement_and_separate_cache_entries() { + let codex_home = TempDir::new().unwrap(); + let cache_root = codex_home + .path() + .join("plugins/cache/openai-curated-remote"); + for name in ["sample", "other"] { + write_plugin(&cache_root, &format!("{name}/local"), name); + write_file( + &cache_root.join(format!("{name}/local/.app.json")), + &format!(r#"{{"apps":{{"{name}":{{"id":"connector_{name}"}}}}}}"#), + ); + } + let config_path = codex_home.path().join(CONFIG_TOML_FILE); + let manager = test_plugins_manager_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + manager.write_remote_installed_plugins_cache(vec![ + remote_installed_plugin("sample"), + remote_installed_plugin("other"), + ]); + + let assert_active_plugins = |loaded: &PluginLoadOutcome, expected_names: Vec<&str>| { + assert_eq!( + loaded + .capability_summaries() + .iter() + .map(|summary| summary.config_name.as_str()) + .collect::>(), + expected_names + ); + assert_eq!( + loaded.effective_plugin_skill_roots().len(), + expected_names.len() + ); + assert_eq!(loaded.effective_apps().len(), expected_names.len()); + assert!(loaded.plugins().iter().all(|plugin| plugin.error.is_none())); + }; + + let sample = "sample@openai-curated-remote"; + let other = "other@openai-curated-remote"; + for (policy, expected_names) in [ + ("", vec![other, sample]), + ("[plugins._default]\nenabled = false", Vec::new()), + ( + "[plugins._default]\nenabled = false\n[plugins.\"sample@openai-curated-remote\"]\nenabled = true", + vec![sample], + ), + ( + "[plugins._default]\nenabled = true\n[plugins.\"sample@openai-curated-remote\"]\nenabled = false", + vec![other], + ), + ( + "[plugins._default]\nenabled = false\n[plugins.\"sample@openai-curated-remote\".mcp_servers.example]\ndefault_tools_approval_mode = 'approve'", + Vec::new(), + ), + // Without the new default, legacy remote account enablement still wins. + ( + "[plugins.\"sample@openai-curated-remote\"]\nenabled = false", + vec![other, sample], + ), + ] { + write_file(&config_path, policy); + let config = load_config(codex_home.path(), codex_home.path()).await; + let loaded = manager.plugins_for_config(&config).await; + assert_active_plugins(&loaded, expected_names); + } + + // Marketplace filtering removes these raw entries from the configured map, + // but remote discovery still supplies the installed bundle. Its raw policy + // must distinguish cache entries as host enablement changes. + for enabled in [true, false] { + let mut restricted_config = plugins_config_input_with_requirements( + codex_home.path(), + &format!( + "[plugins._default]\nenabled = false\n[plugins.\"sample@openai-curated-remote\"]\nenabled = {enabled}" + ), + "[marketplaces]\nrestrict_to_allowed_sources = true", + ); + restricted_config.remote_plugin_enabled = true; + assert!( + configured_plugins_from_stack( + &restricted_config.config_layer_stack, + codex_home.path(), + ) + .is_empty() + ); + let loaded = manager.plugins_for_config(&restricted_config).await; + let expected = if enabled { vec![sample] } else { Vec::new() }; + assert_active_plugins(&loaded, expected); + } + + let mut disabled_plugin = remote_installed_plugin("sample"); + disabled_plugin.enabled = false; + disabled_plugin.availability = codex_app_server_protocol::PluginAvailability::DisabledByAdmin; + manager.write_remote_installed_plugins_cache(vec![disabled_plugin]); + write_file( + &config_path, + "[plugins._default]\nenabled = false\n[plugins.\"sample@openai-curated-remote\"]\nenabled = true", + ); + let config = load_config(codex_home.path(), codex_home.path()).await; + let loaded = manager.plugins_for_config(&config).await; + assert_active_plugins(&loaded, Vec::new()); +} + #[tokio::test] async fn skill_roots_resolve_remote_plugin_identity_from_authoritative_source() { let mut duplicate_plugin = remote_installed_plugin("sample"); @@ -3177,10 +3293,11 @@ fn loaded_plugins_cache_evicts_least_recently_used_configuration() { configured_plugins: HashMap::from([( format!("plugin-{index}@test"), PluginConfig { - enabled: true, + enabled: Some(true), mcp_servers: HashMap::new(), }, )]), + plugin_policy: PluginsConfigToml::default(), skill_config_rules: SkillConfigRules::default(), remote_global_catalog_active: false, auth_identity: None, @@ -3225,6 +3342,7 @@ fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() { let manager = test_plugins_manager(codex_home.path().to_path_buf()); let cache_key = PluginLoadCacheKey { configured_plugins: HashMap::new(), + plugin_policy: PluginsConfigToml::default(), skill_config_rules: SkillConfigRules::default(), remote_global_catalog_active: false, auth_identity: None, @@ -3252,6 +3370,7 @@ async fn plugins_for_config_discards_in_flight_load_after_account_change() { ); let config = PluginsConfigInput::new( unrestricted_config_layer_stack(), + Default::default(), String::new(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ true, @@ -7010,6 +7129,11 @@ async fn load_plugins_uses_project_config_files() { let plugins = load_plugins_from_layer_stack( &stack, + &stack + .effective_config() + .try_into::() + .unwrap() + .plugins, crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot::default(), &PluginStore::new(codex_home.path().to_path_buf()), /*plugin_skill_snapshots*/ None, diff --git a/codex-rs/core-plugins/src/marketplace_policy.rs b/codex-rs/core-plugins/src/marketplace_policy.rs index d5907554fb..e4d7630302 100644 --- a/codex-rs/core-plugins/src/marketplace_policy.rs +++ b/codex-rs/core-plugins/src/marketplace_policy.rs @@ -21,10 +21,12 @@ use codex_config::RequirementSource; use codex_config::types::MarketplaceConfig; use codex_config::types::MarketplaceSourceType; use codex_config::types::PluginConfig; +use codex_config::types::PluginsConfigToml; use codex_plugin::PluginId; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path::paths_match_after_normalization; use regex::Regex; +use serde::Deserialize; use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; @@ -236,6 +238,9 @@ pub(crate) fn policy_filtered_plugin_config( .and_then(toml::Value::as_table_mut) { plugins.retain(|plugin_key, _| { + if plugin_key == "_default" { + return true; + } let Ok(plugin_id) = PluginId::parse(plugin_key) else { return !policy.is_restricted(); }; @@ -301,8 +306,8 @@ pub(crate) fn configured_plugins_from_stack( let Some(plugins_value) = effective_config.get("plugins") else { return HashMap::new(); }; - match plugins_value.clone().try_into() { - Ok(plugins) => plugins, + match PluginsConfigToml::deserialize(plugins_value.clone()) { + Ok(plugins) => plugins.plugins, Err(err) => { tracing::warn!("invalid plugins config: {err}"); HashMap::new() diff --git a/codex-rs/core-plugins/src/marketplace_policy/curated_skill_tests.rs b/codex-rs/core-plugins/src/marketplace_policy/curated_skill_tests.rs index 958ad664e1..780163ef27 100644 --- a/codex-rs/core-plugins/src/marketplace_policy/curated_skill_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_policy/curated_skill_tests.rs @@ -82,7 +82,12 @@ async fn curated_git_requirements_control_plugin_skills() -> Result<()> { ) .await?; let config = PluginsConfigInput::new( - config_layer_stack, + config_layer_stack.clone(), + config_layer_stack + .effective_config() + .try_into::() + .unwrap() + .plugins, "openai".to_string(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ false, diff --git a/codex-rs/core-plugins/src/plugins_config_input_tests.rs b/codex-rs/core-plugins/src/plugins_config_input_tests.rs index 58c7afb6f4..69cadf4510 100644 --- a/codex-rs/core-plugins/src/plugins_config_input_tests.rs +++ b/codex-rs/core-plugins/src/plugins_config_input_tests.rs @@ -18,6 +18,7 @@ use wiremock::matchers::path; fn repeated_service_configs_and_early_clones_share_one_lazy_pool() { let input = PluginsConfigInput::new( ConfigLayerStack::default(), + Default::default(), "openai".to_string(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ true, @@ -38,6 +39,7 @@ fn repeated_service_configs_and_early_clones_share_one_lazy_pool() { .collect(); let new_input = PluginsConfigInput::new( input.config_layer_stack.clone(), + input.plugins.clone(), input.model_provider_id.clone(), input.plugins_enabled, input.remote_plugin_enabled, @@ -85,6 +87,7 @@ async fn reused_pool_uses_current_endpoint_product_and_authentication() { } let mut input = PluginsConfigInput::new( ConfigLayerStack::default(), + Default::default(), "openai".to_string(), /*plugins_enabled*/ true, /*remote_plugin_enabled*/ true, diff --git a/codex-rs/core-plugins/src/test_support.rs b/codex-rs/core-plugins/src/test_support.rs index 6b06e32cc1..3bb6041153 100644 --- a/codex-rs/core-plugins/src/test_support.rs +++ b/codex-rs/core-plugins/src/test_support.rs @@ -501,6 +501,11 @@ pub(crate) async fn load_plugins_config(codex_home: &Path, cwd: &Path) -> Plugin .to_string(); PluginsConfigInput::new( config_layer_stack, + effective_config + .clone() + .try_into::() + .unwrap() + .plugins, model_provider_id, feature_enabled(&effective_config, "plugins", /*default_enabled*/ true), feature_enabled( diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index eee26647fa..bdc8201f5d 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3626,7 +3626,7 @@ "additionalProperties": false, "properties": { "enabled": { - "default": true, + "description": "Explicit activation override. Omission inherits the installed-plugin default.", "type": "boolean" }, "mcp_servers": { @@ -3680,6 +3680,34 @@ }, "type": "object" }, + "PluginsConfigToml": { + "additionalProperties": { + "$ref": "#/definitions/PluginConfig" + }, + "description": "Plugin activation and capability settings loaded from configuration layers.", + "properties": { + "_default": { + "allOf": [ + { + "$ref": "#/definitions/PluginsDefaultConfig" + } + ], + "description": "Defaults for installed plugin activation. Omission preserves source enablement. Explicit plugin enablement settings override this default, but cannot enable an installation disabled by its source." + } + }, + "type": "object" + }, + "PluginsDefaultConfig": { + "additionalProperties": false, + "description": "Default activation settings for installed plugins.", + "properties": { + "enabled": { + "default": true, + "type": "boolean" + } + }, + "type": "object" + }, "ProfileTui": { "additionalProperties": false, "description": "TUI settings supported inside a named profile.", @@ -7802,12 +7830,13 @@ "$ref": "#/definitions/ReasoningEffort" }, "plugins": { - "additionalProperties": { - "$ref": "#/definitions/PluginConfig" - }, + "allOf": [ + { + "$ref": "#/definitions/PluginsConfigToml" + } + ], "default": {}, - "description": "User-level plugin config entries keyed by plugin name.", - "type": "object" + "description": "Default and per-plugin settings keyed by exact `@` IDs." }, "profile": { "description": "Profile to use from the `profiles` map.", diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e055866797..99c0176fc0 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -616,6 +616,9 @@ pub struct Config { /// requirements). pub config_layer_stack: ConfigLayerStack, + /// Plugin settings resolved with this snapshot, before capability admission. + pub plugins: codex_config::types::PluginsConfigToml, + /// Warnings collected during config load that should be shown on startup. pub startup_warnings: Vec, @@ -1741,6 +1744,7 @@ impl Config { pub fn plugins_config_input(&self) -> PluginsConfigInput { PluginsConfigInput::new( self.config_layer_stack.clone(), + self.plugins.clone(), self.model_provider_id.clone(), self.features.enabled(Feature::Plugins), self.features.enabled(Feature::RemotePlugin), @@ -1865,6 +1869,7 @@ impl Config { approval_policy: self.permissions.approval_policy.clone(), permission_profile: self.permissions.permission_profile().clone(), config_layer_stack: self.config_layer_stack.clone(), + plugins: self.plugins.clone(), approvals_reviewer: self.approvals_reviewer, environment_cwds: HashMap::new(), environment_use_mxc: HashMap::new(), @@ -4313,6 +4318,7 @@ impl Config { .map_err(std::io::Error::from)?; let otel = otel::resolve_config(cfg.otel.unwrap_or_default(), &mut startup_warnings); let config = Self { + plugins: cfg.plugins, prefer_mxc, model, daybreak_enabled: cfg.daybreak.unwrap_or(false), diff --git a/codex-rs/core/src/config/runtime_refresh.rs b/codex-rs/core/src/config/runtime_refresh.rs index abcf4fbcae..05b017376a 100644 --- a/codex-rs/core/src/config/runtime_refresh.rs +++ b/codex-rs/core/src/config/runtime_refresh.rs @@ -31,6 +31,7 @@ pub(crate) enum RuntimeConfigRefresh { #[serde(default)] struct McpRefreshToml { features: Option, + plugins: codex_config::types::PluginsConfigToml, } impl Config { @@ -166,6 +167,7 @@ impl Config { config.tool_suggest = resolve_tool_suggest_config_from_layer_stack(&layers); } config.config_layer_stack = layers; + config.plugins = cfg.plugins; if enterprise_retired { config.disable_mcp_enterprise_auth(); } diff --git a/codex-rs/core/src/config/runtime_refresh_tests.rs b/codex-rs/core/src/config/runtime_refresh_tests.rs index c52c89348c..698d794867 100644 --- a/codex-rs/core/src/config/runtime_refresh_tests.rs +++ b/codex-rs/core/src/config/runtime_refresh_tests.rs @@ -71,6 +71,60 @@ async fn refresh_honors_fresh_managed_denial(scope: RuntimeConfigRefresh) { assert_eq!(refreshed.model, current.model); } +#[tokio::test] +async fn plugin_refresh_resolves_only_the_final_layer_combination() { + let (_home, base) = base_config().await; + let snapshot = |user: &str, project: &str| { + let mut config = base.clone(); + config.config_layer_stack = codex_config::ConfigLayerStack::new( + vec![ + codex_config::ConfigLayerEntry::new( + ConfigLayerSource::User { + file: base.codex_home.join(CONFIG_TOML_FILE), + profile: None, + }, + toml::from_str(user).unwrap(), + ), + codex_config::ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: base.codex_home.join(".codex"), + }, + toml::from_str(project).unwrap(), + ), + ], + Default::default(), + Default::default(), + ) + .unwrap(); + config.plugins = config + .config_layer_stack + .effective_config() + .try_into::() + .unwrap() + .plugins; + config + }; + let current = snapshot("[plugins._default]\nenabled = false", ""); + let incoming = snapshot( + "[plugins._default]\nenabled = false\n[plugins.\"example@marketplace\"]\nenabled = 'invalid'", + "[plugins.\"example@marketplace\"]\nenabled = false", + ); + for (current, incoming, scope) in [ + (¤t, &incoming, RuntimeConfigRefresh::User), + (¤t, &incoming, RuntimeConfigRefresh::UserFiles), + (&incoming, ¤t, RuntimeConfigRefresh::Mcp), + ] { + assert!(current.resolve_runtime_refresh(incoming, scope).is_err()); + assert!(!current.plugins.allows_plugin("example@marketplace")); + } + let corrected = snapshot("[plugins._default]\nenabled = true", ""); + let refreshed = current + .resolve_runtime_refresh(&corrected, RuntimeConfigRefresh::User) + .unwrap(); + assert_eq!(refreshed.plugins, corrected.plugins); + assert!(refreshed.plugins.allows_plugin("example@marketplace")); +} + #[tokio::test] async fn refresh_honors_disabled_enterprise_host_load_result() { let (_home, base) = base_config().await; diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 9a4c86b11e..4cbb06f81f 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -332,9 +332,9 @@ fn executor_hook_sources_for_step(step_context: &StepContext) -> Vec, + plugins_config: &PluginsConfigToml, disabled_plugin_ids: &[String], ) -> SelectedPluginSnapshot { let roots = context @@ -205,12 +207,13 @@ impl McpManager { selected_root_id, plugin_id, .. - } in contributor.selected_plugins(context).await + } in contributor.selected_plugins(context, plugins_config).await { if !roots.iter().any(|root| root.id == selected_root_id) { continue; } if !context.config().features.enabled(Feature::Plugins) + || !plugins_config.allows_plugin(&plugin_id) || disabled_plugin_ids.contains(&plugin_id) { selected.disabled_plugin_roots.push(selected_root_id); @@ -233,6 +236,7 @@ impl McpManager { disabled_plugin_ids: &[String], ) -> McpRuntimeProjection { let config = context.config(); + let plugins_config = &config.plugins; let mut selected_plugin_connector_sources = Vec::new(); let mut selected_plugin_registrations = Vec::new(); let mut selected_plugins = Vec::new(); @@ -249,8 +253,12 @@ impl McpManager { selected_root_id, plugin_id, mcp, - } in contributor.selected_plugins(context).await + } in contributor.selected_plugins(context, plugins_config).await { + if !plugins_config.allows_plugin(&plugin_id) { + disabled_plugin_roots.push(selected_root_id); + continue; + } let selection_order = plugin_selection_order; plugin_selection_order += 1; let disabled = disabled_plugin_ids.contains(&plugin_id); diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 5931da9100..5f4af05235 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -155,8 +155,8 @@ async fn maybe_install_mcp_dependencies( } let (_, runtime_context) = sess.runtime_mcp_config_and_context(config).await; - for (name, server_config) in added { - let http_client = match runtime_context.resolve_http_client(&name, &server_config) { + for (name, server_config) in &added { + let http_client = match runtime_context.resolve_http_client(name, server_config) { Ok(http_client) => http_client, Err(err) => { warn!("failed to resolve MCP dependency runtime for {name}: {err}"); @@ -190,7 +190,7 @@ async fn maybe_install_mcp_dependencies( oauth_config.discovered_scopes.clone(), ); let oauth_client_config = server_config.oauth.as_ref(); - let oauth_credential_name = server_config.oauth_credential_name(&name); + 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(), @@ -238,18 +238,7 @@ async fn maybe_install_mcp_dependencies( } } - let mut refresh_config = config.clone(); - let mut configured_servers = config.mcp_servers.get().clone(); - for (name, server_config) in &servers { - configured_servers - .entry(name.clone()) - .or_insert_with(|| server_config.clone()); - } - if let Err(err) = refresh_config.mcp_servers.set(configured_servers) { - warn!("failed to refresh MCP dependencies for mentioned skills: {err}"); - return; - } - sess.refresh_mcp_servers_now(turn_context, &refresh_config, elicitation_reviewer) + sess.refresh_mcp_servers_now(added.into_iter().collect(), elicitation_reviewer) .await; } diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index ddd2377ede..1dc06e3d00 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -2621,10 +2621,15 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_serve } #[tokio::test] -async fn maybe_persist_mcp_tool_approval_writes_plugin_mcp_policy() { +async fn maybe_persist_mcp_tool_approval_preserves_plugin_default_activation() { let (session, turn_context) = make_session_and_context().await; let codex_home = session.codex_home().await; - std::fs::create_dir_all(&codex_home).expect("create codex home"); + write_sample_plugin_mcp(codex_home.as_path()); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + "[features]\nplugins = true\n", + ) + .expect("seed config"); let key = McpToolApprovalKey { server: "sample".to_string(), plugin_id: Some("sample@test".to_string()), @@ -2638,6 +2643,7 @@ async fn maybe_persist_mcp_tool_approval_writes_plugin_mcp_policy() { let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); let parsed: ConfigToml = toml::from_str(&contents).expect("parse config"); let tool = parsed + .plugins .plugins .get("sample@test") .and_then(|plugin| plugin.mcp_servers.get("sample")) @@ -2652,7 +2658,41 @@ async fn maybe_persist_mcp_tool_approval_writes_plugin_mcp_policy() { } ); assert!(contents.contains(r#"[plugins."sample@test".mcp_servers.sample.tools.search]"#)); + assert_eq!(parsed.plugins.plugins["sample@test"].enabled, None); assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); + + let config = session.get_config().await; + let servers = codex_mcp::configured_mcp_servers( + &config + .to_mcp_config(session.services.plugins_manager.as_ref()) + .await, + ); + assert_eq!(&servers["sample"].tools["search"], tool); + + ConfigEditsBuilder::for_config(&config) + .with_edits([ConfigEdit::SetPath { + segments: vec![ + "plugins".to_string(), + "_default".to_string(), + "enabled".to_string(), + ], + value: value(false), + }]) + .apply() + .await + .expect("disable plugins by default"); + session.reload_user_config_layer().await; + + let config = session.get_config().await; + assert_eq!(config.plugins.plugins, parsed.plugins.plugins); + assert!( + codex_mcp::configured_mcp_servers( + &config + .to_mcp_config(session.services.plugins_manager.as_ref()) + .await, + ) + .is_empty() + ); } #[tokio::test] diff --git a/codex-rs/core/src/session/config_refresh.rs b/codex-rs/core/src/session/config_refresh.rs index 0fe59df6c6..166c1f6045 100644 --- a/codex-rs/core/src/session/config_refresh.rs +++ b/codex-rs/core/src/session/config_refresh.rs @@ -172,9 +172,7 @@ impl Session { }; self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref()); self.schedule_mcp_prewarm(); - if !matches!(scope, RuntimeConfigRefresh::Mcp) { - self.refresh_hooks(config).await; - } + self.refresh_hooks(config).await; outcome } } diff --git a/codex-rs/core/src/session/config_refresh_tests.rs b/codex-rs/core/src/session/config_refresh_tests.rs index 72663109c3..927a286309 100644 --- a/codex-rs/core/src/session/config_refresh_tests.rs +++ b/codex-rs/core/src/session/config_refresh_tests.rs @@ -1,4 +1,4 @@ -//! File-reload integration regressions; pure authority resolution is tested with Config. +//! Runtime-refresh regressions; pure authority resolution is tested with Config. use super::*; use pretty_assertions::assert_eq; @@ -127,3 +127,78 @@ async fn reload_user_config_layer_resolves_paths_without_resetting_runtime_setti assert_eq!(config.notify, initial.notify); assert_eq!(runtime_servers(&session).await, original_servers); } + +#[tokio::test] +async fn hook_refresh_retries_current_plugin_policy_after_owner_replacement() -> anyhow::Result<()> +{ + let (session, _) = make_session_and_context().await; + let home = session.codex_home().await; + let file = home.join(CONFIG_TOML_FILE); + let source = tempfile::tempdir()?; + std::fs::create_dir_all(source.path().join(".codex-plugin"))?; + std::fs::write( + source.path().join(".codex-plugin/plugin.json"), + r#"{"name":"sample","hooks":{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"echo sample"}]}]}}}"#, + )?; + codex_core_plugins::store::PluginStore::new(home.to_path_buf()).install( + AbsolutePathBuf::try_from(source.path().to_path_buf())?, + codex_plugin::PluginId::parse("sample@test")?, + )?; + let user_config = "[features]\nplugins = true\nhooks = true\n[plugins.\"sample@test\"]\n"; + std::fs::write(&file, user_config)?; + let mut initial = load_latest_config_for_session(&session).await; + initial.bypass_hook_trust = true; + let initial = Arc::new(initial); + session + .state + .lock() + .await + .session_configuration + .original_config_do_not_use = Arc::clone(&initial); + session.refresh_hooks(Arc::clone(&initial)).await; + let request = codex_hooks::SessionStartRequest { + session_id: session.thread_id, + cwd: initial.cwd.clone(), + transcript_path: None, + model: "gpt-5.2".to_string(), + permission_mode: "default".to_string(), + target: codex_hooks::StartHookTarget::SessionStart { + source: codex_hooks::SessionStartSource::Startup, + }, + }; + assert_eq!(session.hooks().preview_session_start(&request).len(), 1); + + let mut incoming = initial.as_ref().clone(); + incoming.config_layer_stack = initial.config_layer_stack.with_user_config( + &file, + toml::from_str(&format!( + "{user_config}[plugins._default]\nenabled = false\n" + ))?, + )?; + let denied = Arc::new(initial.resolve_runtime_refresh(&incoming, RuntimeConfigRefresh::User)?); + // A concurrent refresh publishes its policy before its hook rebuild completes. + session + .state + .lock() + .await + .session_configuration + .original_config_do_not_use = Arc::clone(&denied); + assert_eq!(session.hooks().preview_session_start(&request).len(), 1); + + session.disable_mcp_enterprise_auth().await; + + let published = session.get_config().await; + assert!(!Arc::ptr_eq(&published, &denied)); + assert_eq!(published.plugins, denied.plugins); + assert_eq!(session.hooks().preview_session_start(&request).len(), 1); + + // Complete the pending deny rebuild after its owner was replaced. + session.refresh_hooks(denied).await; + assert!(Arc::ptr_eq(&session.get_config().await, &published)); + assert!(session.hooks().preview_session_start(&request).is_empty()); + + // Finishing a hook rebuild from the previous allow policy must not restore it. + session.refresh_hooks(initial).await; + assert!(session.hooks().preview_session_start(&request).is_empty()); + Ok(()) +} diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 6b652a3559..67e99d1c65 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -690,8 +690,7 @@ impl Session { pub(crate) async fn refresh_mcp_servers_now( &self, - turn_context: &TurnContext, - refresh_config: &Config, + added_servers: HashMap, elicitation_reviewer: Option, ) { let Ok(_refresh) = self.mcp_refresh.acquire().await else { @@ -699,24 +698,29 @@ impl Session { return; }; let auth = self.services.auth_manager.auth().await; - let disabled_plugin_ids = { + { let mut state = self.state.lock().await; let mut config = (*state.session_configuration.original_config_do_not_use).clone(); - config.mcp_servers = refresh_config.mcp_servers.clone(); + let mut servers = config.mcp_servers.get().clone(); + for (name, server) in added_servers { + servers.entry(name).or_insert(server); + } + if let Err(err) = config.mcp_servers.set(servers) { + warn!("failed to refresh MCP dependencies for mentioned skills: {err}"); + return; + } state.session_configuration.original_config_do_not_use = Arc::new(config); - state.active_disabled_plugin_ids.clone() - }; + } let ready_selected_capability_roots = self .services .mcp_runtime .current_ready_selected_capability_roots(); let environments = self.services.turn_environments.snapshot().await; let environment_selections = environments.all_selections(); - let mut desired = self.latest_mcp_desired_state(auth, environments).await; - desired.config = Arc::new(refresh_config.clone()); + let desired = self.latest_mcp_desired_state(auth, environments).await; let executor_capability_discovery = self .executor_capability_discovery_for_step( - refresh_config, + &desired.config, &ready_selected_capability_roots, &desired.environments, ) @@ -725,7 +729,7 @@ impl Session { .services .mcp_manager .runtime_config_for_step( - refresh_config, + &desired.config, &self.services.mcp_thread_init, &self.services.thread_extension_data, McpThreadIdentity { @@ -733,9 +737,9 @@ impl Session { .services .mcp_runtime .current_auth_matches(desired.auth.as_ref()), - session_source: &turn_context.session_source, - originator: &turn_context.originator, - disabled_plugin_ids: &disabled_plugin_ids, + session_source: &desired.session_source, + originator: &desired.originator, + disabled_plugin_ids: &desired.disabled_plugin_ids, environments: McpEnvironmentScope::Selected(&environment_selections), }, &ready_selected_capability_roots, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 2b5baf264c..f749eb7f53 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2167,27 +2167,31 @@ impl Session { state.session_configuration.provider.info().clone() } - pub(crate) async fn refresh_hooks(&self, config: Arc) { - let disabled_plugin_ids = self.state.lock().await.active_disabled_plugin_ids.clone(); - let environments = self.services.turn_environments.snapshot().await; - let hooks_config = build_hooks_config( - config.as_ref(), - self.services.plugins_manager.as_ref(), - environments.single_local_environment(), - &disabled_plugin_ids, - ) - .await; + pub(crate) async fn refresh_hooks(&self, mut config: Arc) { + loop { + let disabled_plugin_ids = self.state.lock().await.active_disabled_plugin_ids.clone(); + let environments = self.services.turn_environments.snapshot().await; + let hooks_config = build_hooks_config( + config.as_ref(), + self.services.plugins_manager.as_ref(), + environments.single_local_environment(), + &disabled_plugin_ids, + ) + .await; - let state = self.state.lock().await; - // A newer refresh may have updated the config while this hook build was in flight. - // Only publish hooks derived from the current config snapshot. - if Arc::ptr_eq( - &state.session_configuration.original_config_do_not_use, - &config, - ) && state.active_disabled_plugin_ids == disabled_plugin_ids - { - let hooks = self.hooks().reconfigured(hooks_config); - self.services.hooks.store(Arc::new(hooks)); + let state = self.state.lock().await; + if Arc::ptr_eq( + &state.session_configuration.original_config_do_not_use, + &config, + ) && state.active_disabled_plugin_ids == disabled_plugin_ids + { + let hooks = self.hooks().reconfigured(hooks_config); + self.services.hooks.store(Arc::new(hooks)); + return; + } + // Unrelated updates can replace the owner too. Rebuild current inputs + // so they cannot strand a pending plugin-policy refresh. + config = Arc::clone(&state.session_configuration.original_config_do_not_use); } } @@ -3875,6 +3879,7 @@ impl Session { ); // A step keeps the plugins from the environments it captured, even if shared MCP // moves on to another environment. Its skill tools and the model use this same copy. + // Policy follows this step's MCP binding, which may have refreshed mid-turn. let selected_plugins = self .services .mcp_manager @@ -3889,6 +3894,7 @@ impl Session { ) .with_session_source(&turn_context.session_source) .with_selected_environments(&environments.all_selections()), + &mcp.config().plugins, &turn_context.disabled_plugin_ids, ) .await; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 07739aa4cb..acbd1ac26c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1788,14 +1788,21 @@ async fn reload_user_config_layer_updates_effective_apps_config() { assert_eq!(app.destructive_enabled, Some(false)); } +#[test_case("[shell_environment_policy]\nexclude = [\"SECRET_*\", 17]"; "shell policy")] +#[test_case("[plugins._default]\nenabled = false\n[plugins.\"sample@openai-curated-remote\"]\nenabled = \"false\""; "plugin policy")] #[tokio::test] -async fn reload_user_config_layer_keeps_previous_config_for_malformed_shell_policy() { +async fn reload_user_config_layer_keeps_previous_config_for_malformed_policy( + malformed_policy: &str, +) { let (session, _turn_context) = make_session_and_context().await; let codex_home = session.codex_home().await; std::fs::create_dir_all(&codex_home).expect("create codex home"); let config_toml_path = codex_home.join(CONFIG_TOML_FILE); - std::fs::write(&config_toml_path, "[apps.calendar]\nenabled = false\n") - .expect("write valid user config"); + std::fs::write( + &config_toml_path, + "[apps.calendar]\nenabled = false\n[plugins._default]\nenabled = false\n", + ) + .expect("write valid user config"); session.reload_user_config_layer().await; let previous_config = session .get_config() @@ -1806,25 +1813,19 @@ async fn reload_user_config_layer_keeps_previous_config_for_malformed_shell_poli std::fs::write( &config_toml_path, - r#" -[apps.calendar] -enabled = true - -[shell_environment_policy] -exclude = ["SECRET_*", 17] -"#, + format!("[apps.calendar]\nenabled = true\n{malformed_policy}\n"), ) .expect("write malformed user config"); session.reload_user_config_layer().await; - let current_config = session - .get_config() - .await + let config = session.get_config().await; + let current_config = config .config_layer_stack .effective_user_config() .expect("current user config"); assert_eq!(current_config, previous_config); + assert!(!config.plugins.allows_plugin("sample@openai-curated-remote")); } #[tokio::test] @@ -10085,8 +10086,7 @@ async fn step_context_keeps_its_mcp_runtime_for_tools() -> anyhow::Result<()> { .capture_step_context(turn_context, &CancellationToken::new()) .await?; - let mut refresh_config = step_context.turn.config.as_ref().clone(); - refresh_config.mcp_servers.set(HashMap::from([( + let mut added_servers = HashMap::from([( "newer".to_string(), McpServerConfig { auth: Default::default(), @@ -10115,20 +10115,47 @@ async fn step_context_keeps_its_mcp_runtime_for_tools() -> anyhow::Result<()> { oauth_resource: None, tools: HashMap::new(), }, - )]))?; + )]); + let ordinary: McpServerConfig = serde_json::from_value(json!({ + "url": "https://ordinary.example/mcp", + "enabled": false, + }))?; + let mut current_config = session.get_config().await.as_ref().clone(); + current_config.plugins = toml::from_str("[_default]\nenabled = false\n")?; + current_config + .mcp_servers + .set(HashMap::from([("ordinary".to_string(), ordinary.clone())]))?; session - .refresh_mcp_servers_now( - step_context.turn.as_ref(), - &refresh_config, - /*elicitation_reviewer*/ None, - ) + .state + .lock() + .await + .session_configuration + .original_config_do_not_use = Arc::new(current_config.clone()); + let mut stale_ordinary = ordinary; + stale_ordinary.enabled = true; + added_servers.insert("ordinary".to_string(), stale_ordinary); + let mut expected_servers = codex_mcp::configured_mcp_servers(step_context.mcp.config()); + expected_servers.extend(current_config.mcp_servers.get().clone()); + expected_servers.insert("newer".to_string(), added_servers["newer"].clone()); + session + .refresh_mcp_servers_now(added_servers, /*elicitation_reviewer*/ None) .await; let next_step = session .capture_step_context(Arc::clone(&step_context.turn), &CancellationToken::new()) .await .expect("a fresh cancellation token cannot be cancelled"); - assert!(codex_mcp::configured_mcp_servers(next_step.mcp.config()).contains_key("newer")); + assert_eq!( + codex_mcp::configured_mcp_servers(next_step.mcp.config()), + expected_servers + ); + assert_eq!(next_step.mcp.config().plugins, current_config.plugins); + assert_eq!(session.get_config().await.plugins, current_config.plugins); + assert_eq!( + step_context.mcp.config().plugins, + step_context.turn.config.plugins + ); + assert_ne!(step_context.mcp.config().plugins, current_config.plugins); session.mark_mcp_runtime_dirty(); session.refresh_mcp_if_dirty().await; @@ -10138,7 +10165,11 @@ async fn step_context_keeps_its_mcp_runtime_for_tools() -> anyhow::Result<()> { .current_binding() .await .expect("refreshed runtime should be available"); - assert!(codex_mcp::configured_mcp_servers(current.config()).contains_key("newer")); + assert_eq!( + codex_mcp::configured_mcp_servers(current.config()), + expected_servers + ); + assert_eq!(current.config().plugins, current_config.plugins); let router = &step_context.tool_router; assert!( @@ -10150,6 +10181,33 @@ async fn step_context_keeps_its_mcp_runtime_for_tools() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn mcp_dependency_refresh_preserves_current_server_constraints() -> anyhow::Result<()> { + let (session, _) = make_session_and_context().await; + let mut config = session.get_config().await.as_ref().clone(); + config.mcp_servers = crate::config::Constrained::allow_only(config.mcp_servers.get().clone()); + let config = Arc::new(config); + session + .state + .lock() + .await + .session_configuration + .original_config_do_not_use = Arc::clone(&config); + + session + .refresh_mcp_servers_now( + HashMap::from([( + "dependency".to_string(), + serde_json::from_value(json!({"command": "missing-test-mcp-server"}))?, + )]), + /*elicitation_reviewer*/ None, + ) + .await; + + assert!(Arc::ptr_eq(&session.get_config().await, &config)); + Ok(()) +} + #[tokio::test] async fn spawn_task_does_not_update_previous_turn_settings_for_non_run_turn_tasks() { let (sess, tc, _rx) = make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 35944175a8..b6d3ab0b53 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1577,6 +1577,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() struct InitialDataRecorder { lifecycle_observed: Arc>>, mcp_observed: Arc>>, + mcp_loaded: Arc, } impl codex_extension_api::ThreadLifecycleContributor for InitialDataRecorder { @@ -1617,6 +1618,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() fn selected_plugins<'a>( &'a self, context: codex_extension_api::McpServerContributionContext<'a, Config>, + _plugins_config: &'a codex_config::types::PluginsConfigToml, ) -> codex_extension_api::ExtensionFuture<'a, Vec>> { Box::pin(async move { @@ -1652,16 +1654,19 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() let source_environment_id = environment_id.clone(); server.environment_id = source_environment_id.clone(); server.enabled = false; - let plugin_id = format!("plugin-{}", selected_root.id); + let selected_root_id = selected_root.id; + let plugin_id = format!("plugin-{selected_root_id}"); + let mcp_loaded = Arc::clone(&self.mcp_loaded); vec![codex_extension_api::SelectedPlugin { - selected_root_id: selected_root.id.clone(), + selected_root_id: selected_root_id.clone(), plugin_id: plugin_id.clone(), mcp: Box::pin(async move { + mcp_loaded.fetch_add(1, Ordering::Relaxed); codex_extension_api::SelectedPluginContribution { plugin_display_name: plugin_id, source_environment_id, - connector_ids: vec![format!("{}-connector", selected_root.id)], - servers: vec![(selected_root.id, server)], + connector_ids: vec![format!("{selected_root_id}-connector")], + servers: vec![(selected_root_id, server)], } }), }] @@ -1685,9 +1690,11 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() let lifecycle_observed = Arc::new(std::sync::Mutex::new(Vec::new())); let mcp_observed = Arc::new(std::sync::Mutex::new(Vec::new())); + let mcp_loaded = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let recorder = Arc::new(InitialDataRecorder { lifecycle_observed: Arc::clone(&lifecycle_observed), mcp_observed: Arc::clone(&mcp_observed), + mcp_loaded: Arc::clone(&mcp_loaded), }); let mut extensions = codex_extension_api::ExtensionRegistryBuilder::new(); extensions.thread_lifecycle_contributor(recorder.clone()); @@ -1859,7 +1866,29 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() .get("originator"), Some(&"codex_work_desktop".to_string()) ); - for disabled_plugin_ids in [vec!["plugin-selected-a".to_string()], vec![]] { + for (policy, disabled_plugin_ids, selected_enabled, expected_mcp_loads) in [ + ("", vec!["plugin-selected-a".to_string()], false, 1), + ("", vec![], true, 1), + ("[plugins._default]\nenabled = false", vec![], false, 0), + ( + "[plugins._default]\nenabled = false\n[plugins.plugin-selected-a]\nenabled = true", + vec![], + true, + 1, + ), + ] { + let mut config = config.clone(); + config.config_layer_stack = config + .config_layer_stack + .with_user_config( + &config.codex_home.join("config.toml").abs(), + toml::from_str(policy).expect("plugin policy"), + ) + .expect("plugin policy layers"); + config.plugins = toml::from_str::(policy) + .unwrap() + .plugins; + let loaded_before = mcp_loaded.load(Ordering::Relaxed); let projection = first_session .services .mcp_manager @@ -1882,7 +1911,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() .await; assert_eq!( projection.selected_plugins.disabled_plugin_roots, - if disabled_plugin_ids.is_empty() { + if selected_enabled { vec![] } else { vec!["selected-a".to_string()] @@ -1890,7 +1919,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() ); assert_eq!( selected_servers(&projection.config).contains_key("selected-a"), - disabled_plugin_ids.is_empty() + selected_enabled ); assert_eq!( projection @@ -1902,7 +1931,11 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() ); assert_eq!( projection.selected_plugins.plugins.len(), - usize::from(disabled_plugin_ids.is_empty()) + usize::from(selected_enabled) + ); + assert_eq!( + mcp_loaded.load(Ordering::Relaxed) - loaded_before, + expected_mcp_loads ); } } diff --git a/codex-rs/core/tests/suite/hooks_executor.rs b/codex-rs/core/tests/suite/hooks_executor.rs index 6ac3bac0d8..f033f7c64c 100644 --- a/codex-rs/core/tests/suite/hooks_executor.rs +++ b/codex-rs/core/tests/suite/hooks_executor.rs @@ -23,6 +23,8 @@ use codex_protocol::protocol::ThreadSource; use codex_protocol::protocol::TurnEnvironmentRequest; use codex_protocol::protocol::TurnEnvironmentRequests; use codex_protocol::protocol::TurnEnvironmentSelection; +use codex_protocol::request_user_input::RequestUserInputAnswer; +use codex_protocol::request_user_input::RequestUserInputResponse; use codex_protocol::user_input::UserInput; use codex_utils_path_uri::PathUri; use core_test_support::apps_test_server::AppsTestServer; @@ -31,6 +33,7 @@ use core_test_support::apps_test_server::recorded_apps_tool_calls; use core_test_support::responses::ResponseMock; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_response_sequence; use core_test_support::responses::sse; @@ -43,10 +46,12 @@ use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; +use core_test_support::wait_for_event_match; use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; use test_case::test_case; @@ -114,6 +119,81 @@ async fn thread_plugin_selection_disables_executor_hooks_without_disabling_their Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn executor_stop_hook_uses_plugin_policy_refreshed_during_turn() -> Result<()> { + skip_if_no_network!(Ok(())); + let fixture = executor_plugin_hook_fixture( + test_codex().with_config(|config| { + config + .features + .enable(Feature::DefaultModeRequestUserInput) + .expect("enable request_user_input"); + }), + &[("computer-use@openai-bundled", computer_use_hook_manifest())], + vec![ + sse_response(sse(vec![ + ev_function_call( + "pause", + "request_user_input", + r#"{"questions":[{"id":"continue","header":"Continue","question":"Continue?","options":[{"label":"Yes","description":"Finish."},{"label":"No","description":"Stop."}]}]}"#, + ), + ev_completed("paused-step"), + ])), + completed_turn_response("finished-turn"), + ], + ) + .await?; + fixture.attach().await?; + fixture + .test + .codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Ask before finishing this turn".to_string(), + text_elements: Vec::new(), + }])) + .await?; + let request = wait_for_event_match(&fixture.test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + + let current_config = fixture.test.codex.config().await; + let mut config = (*current_config).clone(); + config.config_layer_stack = config.config_layer_stack.with_user_config( + &config.codex_home.join("config.toml"), + toml::from_str("[plugins._default]\nenabled = false")?, + )?; + let _ = fixture + .test + .codex + .refresh_runtime_config(current_config, config) + .await; + fixture + .test + .codex + .submit(Op::UserInputAnswer { + id: request.turn_id, + response: RequestUserInputResponse { + answers: HashMap::from([( + "continue".to_string(), + RequestUserInputAnswer { + answers: vec!["Yes".to_string()], + }, + )]), + }, + }) + .await?; + wait_for_event(&fixture.test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert_eq!(fixture.responses.requests().len(), 2); + assert_eq!(fixture.calls().await?, Vec::::new()); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn executor_stop_hook_runs_after_attachment() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/hooks_plugin_policy_refresh.rs b/codex-rs/core/tests/suite/hooks_plugin_policy_refresh.rs new file mode 100644 index 0000000000..4021e96409 --- /dev/null +++ b/codex-rs/core/tests/suite/hooks_plugin_policy_refresh.rs @@ -0,0 +1,202 @@ +//! Native plugin hook admission must follow policy changes within an active turn. + +use std::collections::HashMap; +use std::fs; +use std::sync::Arc; + +use anyhow::Result; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerSource; +use codex_config::ConfigLayerStack; +use codex_core::ConfigRefreshOutcome; +use codex_core::TurnInputRequest; +use codex_features::Feature; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::request_user_input::RequestUserInputAnswer; +use codex_protocol::request_user_input::RequestUserInputResponse; +use codex_protocol::user_input::UserInput; +use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::apps_test_server::recorded_apps_tool_calls; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use core_test_support::wait_for_event_match; +use core_test_support::wait_for_mcp_server; +use pretty_assertions::assert_eq; +use serde_json::json; +use tempfile::TempDir; +use wiremock::Mock; +use wiremock::Request; +use wiremock::ResponseTemplate; +use wiremock::matchers::body_partial_json; +use wiremock::matchers::method; +use wiremock::matchers::path; + +use super::rmcp_client::remote_aware_environment_id; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn native_stop_hook_uses_plugin_policy_refreshed_during_turn() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let observer = AppsTestServer::mount(&server).await?; + Mock::given(method("POST")) + .and(path("/api/codex/ps/mcp")) + .and(body_partial_json(json!({ "method": "tools/call" }))) + .respond_with(|request: &Request| { + let request: serde_json::Value = + serde_json::from_slice(&request.body).expect("observer tool call"); + ResponseTemplate::new(/*s*/ 200).set_body_json(json!({ + "jsonrpc": "2.0", + "id": request["id"], + "result": { "content": [{ "type": "text", "text": "{}" }] }, + })) + }) + .with_priority(/*p*/ 1) + .mount(&server) + .await; + let home = Arc::new(TempDir::new()?); + let plugin_root = home.path().join("plugins/cache/test/example/local"); + fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + serde_json::to_vec(&json!({ + "name": "example", + "hooks": { "hooks": { "Stop": [{ "hooks": [{ + "type": "mcp_tool", + "server": "hook_observer", + "tool": "calendar_list_events", + }] }] } }, + }))?, + )?; + fs::write( + home.path().join("config.toml"), + "[features]\nplugins = true\nhooks = true\n\n[plugins.\"example@test\"]\n", + )?; + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_assistant_message("baseline", "done"), + ev_completed("baseline"), + ]), + sse(vec![ + ev_function_call( + "pause", + "request_user_input", + r#"{"questions":[{"id":"continue","header":"Continue","question":"Continue?","options":[{"label":"Yes","description":"Finish."},{"label":"No","description":"Stop."}]}]}"#, + ), + ev_completed("paused-step"), + ]), + sse(vec![ + ev_assistant_message("finished-turn", "done"), + ev_completed("finished-turn"), + ]), + ], + ) + .await; + let test = test_codex() + .with_home(home) + .with_config(move |config| { + config.bypass_hook_trust = true; + config + .features + .enable(Feature::DefaultModeRequestUserInput) + .expect("enable request_user_input"); + config + .mcp_servers + .set(HashMap::from([( + "hook_observer".to_string(), + serde_json::from_value(json!({ + "url": format!("{}/api/codex/ps/mcp", observer.chatgpt_base_url), + "environment_id": remote_aware_environment_id(), + })) + .expect("observer MCP configuration"), + )])) + .expect("configure independent hook observer"); + }) + .build_with_auto_env(&server) + .await?; + wait_for_mcp_server(&test.codex, "hook_observer").await?; + test.submit_text_turn("finish the enabled baseline turn") + .await?; + assert_eq!(recorded_apps_tool_calls(&server).await.len(), 1); + + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Ask before finishing this turn".to_string(), + text_elements: Vec::new(), + }])) + .await?; + let request = wait_for_event_match(&test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + let current_config = test.codex.config().await; + let mut next_config = (*current_config).clone(); + let mut layers = current_config + .config_layer_stack + .all_layers_low_to_high() + .cloned() + .collect::>(); + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::System { + file: current_config.codex_home.join("system-config.toml"), + }, + toml::from_str("[plugins._default]\nenabled = false")?, + )); + layers.sort_by_key(|layer| layer.name.precedence()); + next_config.config_layer_stack = ConfigLayerStack::new( + layers, + current_config.config_layer_stack.requirements().clone(), + current_config + .config_layer_stack + .requirements_toml() + .clone(), + ) + .expect("updated system layer"); + assert_eq!( + test.codex + .refresh_mcp_config(current_config, next_config) + .await, + ConfigRefreshOutcome::Published + ); + test.codex + .submit(Op::UserInputAnswer { + id: request.turn_id, + response: RequestUserInputResponse { + answers: HashMap::from([( + "continue".to_string(), + RequestUserInputAnswer { + answers: vec!["Yes".to_string()], + }, + )]), + }, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + assert_eq!(responses.requests().len(), 3); + assert_eq!(recorded_apps_tool_calls(&server).await.len(), 1); + + test.codex + .call_mcp_tool( + "hook_observer", + "calendar_list_events", + /*arguments*/ None, + /*meta*/ None, + ) + .await?; + assert_eq!(recorded_apps_tool_calls(&server).await.len(), 2); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mcp_executor_context_tests.rs b/codex-rs/core/tests/suite/mcp_executor_context_tests.rs index d780d5ec4f..fa5f079c3d 100644 --- a/codex-rs/core/tests/suite/mcp_executor_context_tests.rs +++ b/codex-rs/core/tests/suite/mcp_executor_context_tests.rs @@ -38,6 +38,7 @@ impl McpServerContributor for ExecutorSelectionRecorder { fn selected_plugins<'a>( &'a self, context: McpServerContributionContext<'a, Config>, + _plugins_config: &'a codex_config::types::PluginsConfigToml, ) -> ExtensionFuture<'a, Vec>> { Box::pin(async move { self.plugin_contexts diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 5d2cd461ad..5fba928d98 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -118,6 +118,7 @@ mod hooks; mod hooks_executor; #[cfg(not(target_os = "windows"))] mod hooks_mcp; +mod hooks_plugin_policy_refresh; mod image_rollout; mod injected_models_cache; #[cfg(not(target_os = "windows"))] diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 1a7f51d4ee..55c80c7108 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -716,7 +716,101 @@ async fn astra_kickoff_with_skills_plugins_and_remote_compaction() -> Result<()> } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> Result<()> { +async fn astra_plugin_policy_limits_visible_skills() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + write_scenario_capabilities(&home)?; + fs::rename( + home.path().join("plugins/cache/test"), + home.path().join("plugins/cache/openai-curated-remote"), + )?; + fs::write( + home.path().join("config.toml"), + r#"[features] +plugins = true +remote_plugin = true +[skills.bundled] +enabled = false +[plugins._default] +enabled = false +[plugins."calendar@openai-curated-remote"] +enabled = true +"#, + )?; + let installed_plugins = ["calendar", "notes"].map(|name| json!({ + "id": format!("plugins~Plugin_{name}"), + "name": name, + "scope": "GLOBAL", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { "version": "local", "display_name": name, "description": name, "interface": {} }, + "enabled": true, + })); + wiremock::Mock::given(wiremock::matchers::method("GET")) + .and(wiremock::matchers::path("/ps/plugins/installed")) + .respond_with(wiremock::ResponseTemplate::new(200).set_body_json(json!({ + "plugins": installed_plugins, + "pagination": { "next_page_token": null }, + }))) + .mount(&server) + .await; + let mock = mount_sse_sequence( + &server, + vec![sse(vec![ + ev_response_created("skills-response"), + ev_assistant_message("skills", "The calendar agenda skill is available."), + ev_completed("skills-response"), + ])], + ) + .await; + let chatgpt_base_url = server.uri(); + let test = test_codex() + .with_model("gpt-6-astra") + .with_home(home) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_extensions(skills_extensions()) + .with_config(move |config| { + configure_scenario_catalog(config); + config.chatgpt_base_url = chatgpt_base_url; + config.workspace_roots = vec![config.cwd.clone()]; + }) + .build_with_auto_env(&server) + .await?; + let auth = test.thread_manager.auth_manager().auth().await; + test.thread_manager + .plugins_manager() + .reconcile_remote_installed_plugins(&test.config.plugins_config_input(), auth.as_ref()) + .await?; + test.submit_turn("Plan a team kickoff. Which planning skills are available?") + .await?; + + let requests = mock.requests(); + insta::assert_snapshot!( + "astra_plugin_policy_limits_visible_skills", + context_snapshot::format_request_history_snapshot( + "Astra sees the permitted calendar plugin and standalone skills, but not the unlisted notes plugin.", + &requests, + &ContextSnapshotOptions::default().include_request_settings(), + ) + ); + Ok(()) +} + +#[derive(Clone, Copy)] +enum ExecutorPluginDisablement { + ThreadSelection, + ConfigRefresh, +} + +#[test_case::test_case(ExecutorPluginDisablement::ThreadSelection; "thread_selection")] +#[test_case::test_case(ExecutorPluginDisablement::ConfigRefresh; "config_refresh")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context( + disablement: ExecutorPluginDisablement, +) -> Result<()> { use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionFuture; use codex_extension_api::SelectedPluginSnapshot; @@ -736,9 +830,11 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> input: WorldStateContributionInput<'a>, ) -> ExtensionFuture<'a, Vec> { Box::pin(async move { - let first = self.first.lock().unwrap().take(); + let first = self.first.lock().expect("world state pause lock").take(); if let Some(first) = first { - first.send(input.turn_id.to_string()).unwrap(); + first + .send(input.turn_id.to_string()) + .expect("world state pause receiver"); self.resume.notified().await; } Vec::new() @@ -851,7 +947,10 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> let mut thread_extension_init = ExtensionDataInit::new(); thread_extension_init.insert(vec![selected_root( "plugin-skills", - PathUri::from_host_native_path(plugin_root)?, + PathUri::from_host_native_path(match disablement { + ExecutorPluginDisablement::ThreadSelection => plugin_root, + ExecutorPluginDisablement::ConfigRefresh => plugin_root.join("skills"), + })?, )]); let mut first_environment = test.executor_environment().selection().clone(); let mut environment_config = environment_config_for_selection(&test.config, &first_environment); @@ -867,7 +966,12 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> .start_thread(StartThreadOptions { environments: Some(vec![first_environment.into_request()]), thread_extension_init, - disabled_plugin_ids: Some(vec!["plugin-skills".to_string()]), + disabled_plugin_ids: match disablement { + ExecutorPluginDisablement::ThreadSelection => { + Some(vec!["plugin-skills".to_string()]) + } + ExecutorPluginDisablement::ConfigRefresh => None, + }, ..StartThreadOptions::new(test.config.clone()) }) .await? @@ -890,6 +994,20 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> }) .await?; assert_eq!(outcome.await?, TurnSettingsUpdateOutcome::Applied); + if matches!(disablement, ExecutorPluginDisablement::ConfigRefresh) { + let current_config = thread.config().await; + let mut next_config = (*current_config).clone(); + next_config.config_layer_stack = next_config.config_layer_stack.with_user_config( + &next_config.codex_home.join("config.toml"), + toml::from_str("[plugins._default]\nenabled = false")?, + )?; + assert_eq!( + thread + .refresh_runtime_config(current_config, next_config) + .await, + codex_core::ConfigRefreshOutcome::Published + ); + } // Refresh shared MCP for the new roots (there is no Apps server in this test), then // replace its plugin list before the next step captures it or needs another refresh. let _ = thread.refresh_codex_apps_tools().await; @@ -911,6 +1029,15 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> .expect("first step's skills.list output"); assert!(first_step_tool.contains("active-helper")); assert!(!first_step_tool.contains("next-helper")); + let plugin_was_enabled = matches!(disablement, ExecutorPluginDisablement::ConfigRefresh); + assert_eq!( + first_step_tool.contains("disabled-plugin-helper"), + plugin_was_enabled + ); + assert_eq!( + requests[0].body_contains_text("disabled-plugin-helper"), + plugin_was_enabled + ); let developer_texts = requests[1].message_input_texts("developer"); let latest_skills = developer_texts .iter() @@ -919,6 +1046,7 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> .expect("second step's skills section"); assert!(latest_skills.contains("next-helper")); assert!(!latest_skills.contains("active-helper")); + assert!(!latest_skills.contains("disabled-plugin-helper")); let mut bodies = requests .iter() .map(core_test_support::responses::ResponsesRequest::body_json) @@ -926,7 +1054,9 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> for body in &mut bodies { let input = body["input"].to_string(); assert!(!input.contains("retired-helper")); - assert!(!input.contains("disabled-plugin-helper")); + if !plugin_was_enabled { + assert!(!input.contains("disabled-plugin-helper")); + } // Normalize opaque skill locators before snapshot truncation and hashing. body["input"] = serde_json::from_str(&input.replace(&skill_files, "/"))?; // Cargo and Bazel can serialize the JSON inside the tool output in different key orders. @@ -940,10 +1070,20 @@ async fn astra_omits_disabled_executor_and_plugin_skills_from_model_context() -> } } let entries = bodies.iter().map(SnapshotEntry::body).collect::>(); - insta::assert_snapshot!( - "astra_disabled_executor_skills", - context_snapshot::format_context_snapshot( + let (snapshot_name, description) = match disablement { + ExecutorPluginDisablement::ThreadSelection => ( + "astra_disabled_executor_skills", "Astra keeps disabled skills hidden and uses each step's selected skills after the environment changes during preparation.", + ), + ExecutorPluginDisablement::ConfigRefresh => ( + "astra_executor_plugin_policy_refresh", + "Astra applies refreshed plugin policy to the next step while preserving earlier authorized skill context and tool output.", + ), + }; + insta::assert_snapshot!( + snapshot_name, + context_snapshot::format_context_snapshot( + description, &entries, &ContextSnapshotOptions::default().include_request_settings(), ) diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_executor_plugin_policy_refresh.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_executor_plugin_policy_refresh.snap new file mode 100644 index 0000000000..fe1ed50eec --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_executor_plugin_policy_refresh.snap @@ -0,0 +1,139 @@ +--- +source: core/tests/suite/scenarios.rs +expression: "context_snapshot::format_context_snapshot(description, &entries,\n&ContextSnapshotOptions::default().include_request_settings(),)" +--- +Scenario: Astra applies refreshed plugin policy to the next step while preserving earlier authorized skill context and tool output. + +## Window 1 +Settings: + include: ["reasoning.encrypted_content"] + model: "gpt-6-astra" + parallel_tool_calls: false + prompt_cache_key: "" + reasoning: {"context":"all_turns","effort":"low"} + store: false + stream: true + text: {"verbosity":"low"} + tool_choice: "auto" +-- request 1 (request) -- +00:additional_tools/developer (3; hash=5e8370540461db70): + - namespace/functions; hash=7d26e086c1dbba28 + - custom/exec + - function/wait + - function/request_user_input + - function/request_user_input_async + - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 + - function/sleep + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - function/followup_task + - function/interrupt_agent + - function/list_agents + - function/send_message + - function/spawn_agent + - function/wait_agent +01:message/developer: + You are Codex, an agent based on GPT-6. You and the user share one workspace, and your job is to collabo...them until their intended goal is completely handled. [hash=efa50f074efa5803] + + # When to ask the user for permission + + Use your best judgement given task context for when you really need user permission, like a competent co...ork without ending the turn to clarify with the user. [hash=ca7367e658b51388] + + User authorization and preferences persist across turns. Do not request permission again when the user h... any guidelines provided in skills or external files. [hash=911f91c92c72882c] + + + + - Skill naming: If a plugin contributes skills, those skill entries are prefixed with plugin_name: in the Skills list. + - MCP naming: Plugin-provided MCP tools keep standard MCP identifiers such as mcp__server__tool; use tool provenance to tell which plugin they come from. + - Trigger rules: If the user explicitly names a plugin, prefer capabilities associated with that plugin for that turn. + - Relationship to capabilities: Plugins are not invoked directly. Use their underlying skills, MCP tools, and app tools to help solve the task. + - Relevance: Determine what a plugin can help with from explicit user mention or from the plugin-associa..., MCP tools, and apps exposed elsewhere in this turn. [hash=620459a4e7b590f0] + - Missing/blocked: If the user requests a plugin that does not have relevant callable capabilities for t..., say so briefly and continue with the best fallback. [hash=746b0469a8f6cb3e] + +02:message/developer[3]: + [01] + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c] + # Escalation Requests + + Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb] + + - Pipes: | + - Logical operators: &&, || + + NEVER provide a prefix_rule if your command uses a heredoc or herestring. + + ### Examples + Good examples of prefixes: + - ["npm", "run", "dev"] + - ["gh", "pr", "check"] + - ["cargo", "test"] + + [02] # Collaboration Mode: Default + + You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. + + Your active mode changes only when new developer instructions with a different `...<...by themselves. Known mode names are Default and Plan. [hash=e75f6b12fc52d812] + + ## request_user_input availability + + Use the `request_user_input` tool only when it is listed in the available tools for this turn. + + Use the `request_user_input` tool only for optional questions where the answer would materially improve the quality of the work. + + If `request_user_input` returns no answers, continue with best judgment instead of asking again or treating the turn as blocked. + + Never use the `request_user_input` tool for permission requests or permission-related escalations. + + [03] + ## Skills + A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that ...locators can be expanded using the skill roots table. [hash=f00d6a4d3e192f06] + ### Skill roots + - `e0` = `skill://plugin-skills//plugin/skills` + - `e1` = `skill://workspace-skills//skills` + Read a skill package directly with `skills.read({"package":""})` to read its `SKILL.md`; root a...ackage is not provided, use `skills.list` to find it. [hash=25bf84ea2123d647] + ### Available skills + - disabled:disabled-plugin-helper: Use a disabled plugin workflow (executor package: e0/disabled-plugin-helper) + - active-helper: Use the current workflow (executor package: e1/active-helper) + +03:message/developer: + You are `/root`, the primary agent in a team of agents collaborating to fulfill the user's goals. + + At the start of your turn, you are the active agent. + You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. + All agents in the team, including the agents that you can assign tasks to, are equally intelligent and capable, and have access to the same set of tools. + + You can use `spawn_agent` to create a new agent, `followup_task` to give an existing agent a new task an...message to a running agent without triggering a turn. [hash=7d40ff8b58977202] + `send_message` calls may be read by a human, so ensure they are legible. Always put proper spaces between words and/or numbers. + + - All agents use the same current working directory. + - As a result, edits made by one agent are immediately visible to all other agents. + + When calling `wait_agent`, prefer longer waits (minutes) to avoid busy polling. + + There are 4 available concurrency slots, meaning that up to 4 agents can be active at once, including you. + + Full-history forks (`fork_turns` omitted or `"all"`) inherit the parent model and reasoning effort and d... so, set `fork_turns` to `"none"`. [hash=d5928243dc643bcc] +04:message/developer: + Any earlier instruction enabling proactive multi-agent delegation no longer applies. D...elegation, or parallel agent work. [hash=2b6e037067b4390c] +05:message/user: + + + + + + [hash=e8c1de60307d0a3d] + +06:message/user: + Which workflow skills are available? +-- request 2 (request) -- +07:function_call/skills.list:{"authority":{"kind":"executor"}} +08:function_call_output:{"next_cursor":null,"skills":[{"authority":{"id":"plugin-skills","kind":"executor"},"description":"Use a...//skills/active-helper"}],"warnings":[]} [hash=a640b563d0b76db9] +09:message/developer: + + ## Skills + A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that ...locators can be expanded using the skill roots table. [hash=f00d6a4d3e192f06] + ### Skill roots + - `e0` = `skill://other-skills//other-skills` + Read a skill package directly with `skills.read({"package":""})` to read its `SKILL.md`; root a...ackage is not provided, use `skills.list` to find it. [hash=25bf84ea2123d647] + ### Available skills + - next-helper: Use the next workflow (executor package: e0/next-helper) + diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_policy_limits_visible_skills.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_policy_limits_visible_skills.snap new file mode 100644 index 0000000000..e46343c0bf --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_policy_limits_visible_skills.snap @@ -0,0 +1,113 @@ +--- +source: core/tests/suite/scenarios.rs +expression: "context_snapshot::format_request_history_snapshot(\"Astra sees the permitted calendar plugin and standalone skills, but not the unlisted notes plugin.\",\n&requests, &ContextSnapshotOptions::default().include_request_settings(),)" +--- +Scenario: Astra sees the permitted calendar plugin and standalone skills, but not the unlisted notes plugin. + +## Window 1 +Settings: + include: ["reasoning.encrypted_content"] + model: "gpt-6-astra" + parallel_tool_calls: false + prompt_cache_key: "" + reasoning: {"context":"all_turns","effort":"low"} + store: false + stream: true + text: {"verbosity":"low"} + tool_choice: "auto" +-- request 1 (turn) -- +00:additional_tools/developer (3; hash=bf2e30605ff5dc17): + - namespace/functions; hash=fada4a476a3754dd + - custom/exec + - function/wait + - function/request_user_input + - function/request_user_input_async + - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 + - function/sleep + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - function/followup_task + - function/interrupt_agent + - function/list_agents + - function/send_message + - function/spawn_agent + - function/wait_agent +01:message/developer: + You are Codex, an agent based on GPT-6. You and the user share one workspace, and your job is to collabo...them until their intended goal is completely handled. [hash=efa50f074efa5803] + + # When to ask the user for permission + + Use your best judgement given task context for when you really need user permission, like a competent co...ork without ending the turn to clarify with the user. [hash=ca7367e658b51388] + + User authorization and preferences persist across turns. Do not request permission again when the user h... any guidelines provided in skills or external files. [hash=911f91c92c72882c] + + + + - Skill naming: If a plugin contributes skills, those skill entries are prefixed with plugin_name: in the Skills list. + - MCP naming: Plugin-provided MCP tools keep standard MCP identifiers such as mcp__server__tool; use tool provenance to tell which plugin they come from. + - Trigger rules: If the user explicitly names a plugin, prefer capabilities associated with that plugin for that turn. + - Relationship to capabilities: Plugins are not invoked directly. Use their underlying skills, MCP tools, and app tools to help solve the task. + - Relevance: Determine what a plugin can help with from explicit user mention or from the plugin-associa..., MCP tools, and apps exposed elsewhere in this turn. [hash=620459a4e7b590f0] + - Missing/blocked: If the user requests a plugin that does not have relevant callable capabilities for t..., say so briefly and continue with the best fallback. [hash=746b0469a8f6cb3e] + +02:message/developer[3]: + [01] + ## Skills + A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list ...ed into an absolute path using the skill roots table. [hash=ca27c3957f42502b] + ### Skill roots + - `r0` = `` + - `r1` = `/openai-curated-remote` + ### Available skills + - calendar:agenda: Plan a team agenda (file: r1/calendar/local/skills/agenda/SKILL.md) + - final-check: Review a final brief (file: r0/final-check/SKILL.md) + - outline: Draft a project outline (file: r0/outline/SKILL.md) + + [02] + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`...ll commands are permitted. Network access is enabled. [hash=c134061cebb94a1b] + Approval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected. + + [03] # Collaboration Mode: Default + + You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. + + Your active mode changes only when new developer instructions with a different `...<...by themselves. Known mode names are Default and Plan. [hash=e75f6b12fc52d812] + + ## request_user_input availability + + Use the `request_user_input` tool only when it is listed in the available tools for this turn. + + Use the `request_user_input` tool only for optional questions where the answer would materially improve the quality of the work. + + If `request_user_input` returns no answers, continue with best judgment instead of asking again or treating the turn as blocked. + + Never use the `request_user_input` tool for permission requests or permission-related escalations. + +03:message/developer: + You are `/root`, the primary agent in a team of agents collaborating to fulfill the user's goals. + + At the start of your turn, you are the active agent. + You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. + All agents in the team, including the agents that you can assign tasks to, are equally intelligent and capable, and have access to the same set of tools. + + You can use `spawn_agent` to create a new agent, `followup_task` to give an existing agent a new task an...message to a running agent without triggering a turn. [hash=7d40ff8b58977202] + `send_message` calls may be read by a human, so ensure they are legible. Always put proper spaces between words and/or numbers. + + - All agents use the same current working directory. + - As a result, edits made by one agent are immediately visible to all other agents. + + When calling `wait_agent`, prefer longer waits (minutes) to avoid busy polling. + + There are 4 available concurrency slots, meaning that up to 4 agents can be active at once, including you. + + Full-history forks (`fork_turns` omitted or `"all"`) inherit the parent model and reasoning effort and d... so, set `fork_turns` to `"none"`. [hash=d5928243dc643bcc] +04:message/developer: + Any earlier instruction enabling proactive multi-agent delegation no longer applies. D...elegation, or parallel agent work. [hash=2b6e037067b4390c] +05:message/user: + + + + + + <...e="unrestricted" /> [hash=d40b2f45cb028888] + +06:message/user: + Plan a team kickoff. Which planning skills are available? diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index 61cdbe4da7..44160e2b17 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -2,6 +2,7 @@ use std::future::Future; use std::pin::Pin; use std::sync::Arc; +use codex_config::types::PluginsConfigToml; use codex_context_fragments::ContextualUserFragment; use codex_protocol::items::TurnItem; use codex_protocol::protocol::TokenUsageInfo; @@ -92,10 +93,12 @@ pub trait McpServerContributor: Send + Sync { /// Declares executor plugins, including those without MCP servers or connectors. Each /// declaration identifies the plugin once; its deferred MCP data cannot change that identity. /// Return plugins in precedence order: across contributors in registration order, the first - /// plugin wins if several provide the same server. + /// plugin wins if several provide the same server. Use the supplied plugin policy for both + /// admission and deferred projection; a step's policy may be newer than its other config. fn selected_plugins<'a>( &'a self, _context: McpServerContributionContext<'a, C>, + _plugins_config: &'a PluginsConfigToml, ) -> ExtensionFuture<'a, Vec>> { Box::pin(async { Vec::new() }) } diff --git a/codex-rs/ext/mcp/src/cloud_plugin_tests.rs b/codex-rs/ext/mcp/src/cloud_plugin_tests.rs index 20c9502207..dac29686f2 100644 --- a/codex-rs/ext/mcp/src/cloud_plugin_tests.rs +++ b/codex-rs/ext/mcp/src/cloud_plugin_tests.rs @@ -237,7 +237,7 @@ async fn account_change_clears_hosted_plugins_before_waiting_for_executor() -> a &config, &init, &thread, "test", &roots, /*executor_capability_discovery*/ None, ) .with_auth_changed(/*auth_changed*/ true); - let mut loading = std::pin::pin!(contributor.selected_plugins(context)); + let mut loading = std::pin::pin!(contributor.selected_plugins(context, &config.plugins)); assert!(futures::poll!(&mut loading).is_pending()); assert_eq!(state.cloud_catalog(), None); Ok(()) diff --git a/codex-rs/ext/mcp/src/plugin.rs b/codex-rs/ext/mcp/src/plugin.rs index b5586c3373..e9ef333e92 100644 --- a/codex-rs/ext/mcp/src/plugin.rs +++ b/codex-rs/ext/mcp/src/plugin.rs @@ -1,8 +1,11 @@ -use codex_config::types::PluginMcpServerConfig; +use codex_config::types::PluginsConfigToml; use codex_connectors_extension::PluginAppProvider; use codex_core::config::Config; +use codex_core_plugins::ExecutorPluginProviderError; +use codex_core_plugins::PluginRootOwnership; use codex_core_plugins::loader::apply_configured_plugin_mcp_server_policies; -use codex_core_plugins::loader::configured_plugin_mcp_server_policies; +use codex_exec_server::CapabilityRootDiscovery; +use codex_exec_server::FileSystemSandboxContext; use codex_extension_api::ExtensionFuture; use codex_extension_api::McpServerContribution; use codex_extension_api::McpServerContributionContext; @@ -13,19 +16,75 @@ use codex_features::Feature; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use std::collections::HashMap; -use std::sync::Arc; -use std::sync::OnceLock; use self::provider::PluginMcpProvider; use crate::PluginsThreadState; use crate::cloud_plugin::hosted_plugin_connectors; use crate::plugin_contributor::PluginContributor; +use crate::plugin_contributor_state::CachedPluginMetadata; use crate::plugin_contributor_state::CachedSelectedRoot; mod discovery; mod provider; impl PluginContributor { + async fn ownership_for_root( + &self, + state: &PluginsThreadState, + selected_root: &SelectedCapabilityRoot, + discovery: Option<&CapabilityRootDiscovery>, + sandbox: Option<&FileSystemSandboxContext>, + ) -> Result { + if let Some(ownership) = state + .contributor_state() + .executor_cache + .iter() + .find(|cached| { + cached.root == *selected_root && cached.ownership_sandbox.as_deref() == sandbox + }) + .and_then(|cached| cached.ownership) + { + return Ok(ownership); + } + let ownership = self + .providers + .executor + .root_ownership(selected_root, discovery, sandbox) + .await + .inspect_err(|error| { + let CapabilityRootLocation::Environment { environment_id, .. } = + &selected_root.location; + tracing::warn!( + selected_root = selected_root.id, + environment_id, + error = %error, + "failed to resolve selected root plugin ownership" + ); + })?; + let mut state = state.contributor_state(); + if let Some(cached) = state + .executor_cache + .iter_mut() + .find(|cached| cached.root == *selected_root) + { + if cached.ownership_sandbox.as_deref() == sandbox + && let Some(ownership) = cached.ownership + { + return Ok(ownership); + } + cached.ownership = Some(ownership); + cached.ownership_sandbox = sandbox.cloned().map(Box::new); + } else { + state.executor_cache.push(CachedSelectedRoot { + root: selected_root.clone(), + ownership: Some(ownership), + ownership_sandbox: sandbox.cloned().map(Box::new), + metadata: CachedPluginMetadata::Unloaded, + }); + } + Ok(ownership) + } + /// Returns metadata for one stable selected root. /// /// Successful resolution, including a root that is not a plugin or declares no capabilities, @@ -37,13 +96,16 @@ impl PluginContributor { state: &PluginsThreadState, selected_root: &SelectedCapabilityRoot, ) -> Option { - if let Some(cached) = state + if let Some(CachedSelectedRoot { + metadata: CachedPluginMetadata::Loaded(metadata), + .. + }) = state .contributor_state() .executor_cache .iter() .find(|cached| cached.root == *selected_root) { - return cached.metadata.clone(); + return metadata.clone(); } let plugin = match self.providers.executor.resolve_bound(selected_root).await { @@ -99,13 +161,22 @@ impl PluginContributor { }; let mut state = state.contributor_state(); let cache = &mut state.executor_cache; - if let Some(cached) = cache.iter().find(|cached| cached.root == *selected_root) { - return cached.metadata.clone(); + if let Some(cached) = cache + .iter_mut() + .find(|cached| cached.root == *selected_root) + { + if let CachedPluginMetadata::Loaded(metadata) = &cached.metadata { + return metadata.clone(); + } + cached.metadata = CachedPluginMetadata::Loaded(metadata.clone()); + } else { + cache.push(CachedSelectedRoot { + root: selected_root.clone(), + ownership: None, + ownership_sandbox: None, + metadata: CachedPluginMetadata::Loaded(metadata.clone()), + }); } - cache.push(CachedSelectedRoot { - root: selected_root.clone(), - metadata: metadata.clone(), - }); metadata } } @@ -142,6 +213,7 @@ impl McpServerContributor for PluginContributor { fn selected_plugins<'a>( &'a self, context: McpServerContributionContext<'a, Config>, + plugins_config: &'a PluginsConfigToml, ) -> ExtensionFuture<'a, Vec>> { Box::pin(async move { let Some(thread_store) = context.thread_store() else { @@ -155,8 +227,6 @@ impl McpServerContributor for PluginContributor { let selected_roots = context .ready_selected_capability_roots() .unwrap_or_default(); - // All plugins in this refresh share the same policy, but step-only readers never need it. - let plugin_policies = Arc::new(OnceLock::new()); let mut plugins = Vec::new(); if let Some(snapshot) = context.executor_capability_discovery() { @@ -172,12 +242,31 @@ impl McpServerContributor for PluginContributor { continue; } }; + let CapabilityRootLocation::Environment { environment_id, .. } = + &root.selected_root.location; + // Ownership survives capability parse failures; an unresolved root cannot + // bypass a denying policy. + if !plugins_config.allows_plugin(&root.selected_root.id) + && discovery.error.is_none() + && !matches!( + self.ownership_for_root( + &state, + &root.selected_root, + Some(discovery), + snapshot.sandbox_contexts().get(environment_id), + ) + .await, + Ok(PluginRootOwnership::Standalone) + ) + { + plugins.push(denied_plugin(&root.selected_root)); + continue; + } let Some((manifest, plugin_files)) = discovery::manifest_from_discovery(&root.selected_root, discovery) else { continue; }; - let plugin_policies = Arc::clone(&plugin_policies); plugins.push(SelectedPlugin { selected_root_id: root.selected_root.id.clone(), plugin_id: root.selected_root.id.clone(), @@ -190,8 +279,8 @@ impl McpServerContributor for PluginContributor { ); project_metadata( context.config(), + plugins_config, &root.selected_root.id, - &plugin_policies, metadata, ) }), @@ -199,18 +288,32 @@ impl McpServerContributor for PluginContributor { } } else { for selected_root in selected_roots { + if !plugins_config.allows_plugin(&selected_root.id) { + if !matches!( + self.ownership_for_root( + &state, + selected_root, + /*discovery*/ None, + /*sandbox*/ None + ) + .await, + Ok(PluginRootOwnership::Standalone) + ) { + plugins.push(denied_plugin(selected_root)); + } + continue; + } let Some(metadata) = self.metadata_for_root(&state, selected_root).await else { continue; }; - let plugin_policies = Arc::clone(&plugin_policies); plugins.push(SelectedPlugin { selected_root_id: selected_root.id.clone(), plugin_id: selected_root.id.clone(), mcp: Box::pin(async move { project_metadata( context.config(), + plugins_config, &selected_root.id, - &plugin_policies, metadata, ) }), @@ -222,10 +325,25 @@ impl McpServerContributor for PluginContributor { } } +fn denied_plugin(selected_root: &SelectedCapabilityRoot) -> SelectedPlugin<'static> { + let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; + let metadata = SelectedPluginContribution { + plugin_display_name: selected_root.id.clone(), + source_environment_id: environment_id.clone(), + servers: Vec::new(), + connector_ids: Vec::new(), + }; + SelectedPlugin { + selected_root_id: selected_root.id.clone(), + plugin_id: selected_root.id.clone(), + mcp: Box::pin(async move { metadata }), + } +} + fn project_metadata( config: &Config, + plugins_config: &PluginsConfigToml, plugin_id: &str, - plugin_policies: &OnceLock>>, plugin: SelectedPluginContribution, ) -> SelectedPluginContribution { let mut servers = if config.features.enabled(Feature::Plugins) { @@ -234,11 +352,8 @@ fn project_metadata( HashMap::new() }; if !servers.is_empty() { - if let Some(plugin_policy) = plugin_policies - .get_or_init(|| configured_plugin_mcp_server_policies(&config.config_layer_stack)) - .get(plugin_id) - { - apply_configured_plugin_mcp_server_policies(plugin_policy, &mut servers); + if let Some(plugin_policy) = plugins_config.plugins.get(plugin_id) { + apply_configured_plugin_mcp_server_policies(&plugin_policy.mcp_servers, &mut servers); } config.apply_plugin_mcp_server_requirements(plugin_id, &mut servers); } diff --git a/codex-rs/ext/mcp/src/plugin_contributor_state.rs b/codex-rs/ext/mcp/src/plugin_contributor_state.rs index e86b9cb2c5..3086151b48 100644 --- a/codex-rs/ext/mcp/src/plugin_contributor_state.rs +++ b/codex-rs/ext/mcp/src/plugin_contributor_state.rs @@ -1,6 +1,8 @@ -//! Thread-owned cloud catalog and legacy selected-root cache, guarded by a short-lived mutex. +//! Thread-owned cloud catalog and stable selected-root metadata, guarded by a short-lived mutex. use codex_core_plugins::PluginCatalog; +use codex_core_plugins::PluginRootOwnership; +use codex_exec_server::FileSystemSandboxContext; use codex_extension_api::SelectedPluginContribution; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::McpResourceClient; @@ -110,5 +112,13 @@ impl PluginContributorState { pub(crate) struct CachedSelectedRoot { pub(crate) root: SelectedCapabilityRoot, - pub(crate) metadata: Option, + pub(crate) ownership: Option, + pub(crate) ownership_sandbox: Option>, + pub(crate) metadata: CachedPluginMetadata, +} + +/// Denied roots retain ownership without loading their capabilities. +pub(crate) enum CachedPluginMetadata { + Unloaded, + Loaded(Option), } diff --git a/codex-rs/ext/mcp/tests/plugin_mcp.rs b/codex-rs/ext/mcp/tests/plugin_mcp.rs index f3bf0c57f8..461397ea67 100644 --- a/codex-rs/ext/mcp/tests/plugin_mcp.rs +++ b/codex-rs/ext/mcp/tests/plugin_mcp.rs @@ -15,8 +15,11 @@ use codex_core_plugins::PluginProvider; use codex_core_plugins::PluginProviderError; use codex_core_plugins::PluginProviderFuture; use codex_core_plugins::PluginSourceLocation; +use codex_exec_server::CapabilityRootDiscovery; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecutorCapabilityDiscoveryCache; +use codex_exec_server::ExecutorCapabilityDiscoverySnapshot; +use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionDataInit; @@ -190,6 +193,226 @@ async fn selected_plugin_package_is_contributed_without_servers_or_connectors() Ok(()) } +#[tokio::test] +async fn denied_malformed_plugin_retains_identity_without_capabilities() -> TestResult { + let default_off = "[plugins._default]\nenabled = false\n"; + for (manifest, discovery_enabled, expected_denied) in [ + (Some(b"{".as_slice()), false, true), + (None, false, false), + (None, true, false), + ( + Some(br#"{"name":"denied","mcpServers":{"unexpected":{"command":"unexpected-command"}}}"#.as_slice()), + true, + true, + ), + (Some(b"{".as_slice()), true, true), + (Some(b"\xff".as_slice()), true, true), + ] { + let codex_home = tempfile::tempdir()?; + let plugin_root = tempfile::tempdir()?; + let skills_root = plugin_root.path().join("skills"); + fs::create_dir_all(&skills_root)?; + if let Some(manifest) = manifest { + fs::create_dir_all(plugin_root.path().join(".codex-plugin"))?; + fs::write(plugin_root.path().join(".codex-plugin/plugin.json"), manifest)?; + } else { + // A nested package does not own its containing skill root. + fs::create_dir_all(plugin_root.path().join("nested/.codex-plugin"))?; + fs::write( + plugin_root.path().join("nested/.codex-plugin/plugin.json"), + r#"{"name":"nested"}"#, + )?; + } + fs::write(codex_home.path().join("config.toml"), default_off)?; + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + config.features.set_enabled(Feature::ExecutorCapabilityDiscovery, discovery_enabled)?; + + let expected = if expected_denied { + vec![( + ("selected-root".to_string(), "selected-root".to_string()), + Vec::new(), + Vec::new(), + )] + } else { + Vec::new() + }; + for selected_root in [plugin_root.path(), skills_root.as_path()] { + let contributions = raw_selected_plugin_contributions(&config, selected_root).await?; + assert_eq!( + contributions + .into_iter() + .map(|(root_id, plugin_id, contribution)| ( + (root_id, plugin_id), + contribution.servers, + contribution.connector_ids, + )) + .collect::>(), + expected + ); + } + } + Ok(()) +} + +#[tokio::test] +async fn selected_root_ownership_retries_errors_and_caches_success() -> TestResult { + let codex_home = tempfile::tempdir()?; + let plugin_root = tempfile::tempdir()?; + let standalone_root = tempfile::tempdir()?; + let skills_root = plugin_root.path().join("skills"); + fs::create_dir_all(&skills_root)?; + fs::create_dir_all(plugin_root.path().join(".codex-plugin"))?; + let manifest = plugin_root.path().join(".codex-plugin/plugin.json"); + fs::write(&manifest, r#"{"name":"example"}"#)?; + fs::write(plugin_root.path().join(".mcp.json"), "{")?; + let selected_roots = [ + ("selected-root", plugin_root.path()), + ("owned-subfolder", skills_root.as_path()), + ("standalone", standalone_root.path()), + ] + .into_iter() + .map(|(id, path)| { + Ok(SelectedCapabilityRoot { + id: id.to_string(), + location: CapabilityRootLocation::Environment { + environment_id: LOCAL_ENVIRONMENT_ID.to_string(), + path: PathUri::from_host_native_path(path)?, + }, + }) + }) + .collect::>>()?; + let mut builder = ExtensionRegistryBuilder::new(); + codex_mcp_extension::install_plugins( + &mut builder, + Arc::new(EnvironmentManager::default_for_tests()), + ); + let registry = builder.build(); + let thread_init = ExtensionDataInit::new(); + let thread_store = ExtensionData::new_with_init("test-thread", thread_init.clone()); + let default_off = "[plugins._default]\nenabled = false\n"; + let standalone = &selected_roots[2]; + let CapabilityRootLocation::Environment { path, .. } = &standalone.location; + let sandbox_contexts = HashMap::from([( + LOCAL_ENVIRONMENT_ID.to_string(), + FileSystemSandboxContext::from_permission_profile(Default::default(), path.clone()), + )]); + + for (phase, enabled_root, expected) in [ + (0, None, vec![("standalone", vec![])]), + (1, None, vec![]), + (2, Some("owned-subfolder"), vec![("selected-root", vec![])]), + ( + 3, + None, + vec![("selected-root", vec![]), ("owned-subfolder", vec![])], + ), + ( + 4, + None, + vec![("selected-root", vec![]), ("owned-subfolder", vec![])], + ), + ( + 5, + Some("selected-root"), + vec![ + ("selected-root", vec!["probe"]), + ("owned-subfolder", vec![]), + ], + ), + ] { + if phase == 4 { + // Stable ownership must survive refresh without probing these files again. + fs::remove_file(&manifest)?; + fs::create_dir_all(standalone_root.path().join(".codex-plugin"))?; + fs::write( + standalone_root.path().join(".codex-plugin/plugin.json"), + "{", + )?; + } else if phase == 5 { + fs::write(&manifest, r#"{"name":"example"}"#)?; + fs::write( + plugin_root.path().join(".mcp.json"), + r#"{"mcpServers":{"probe":{"command":"probe-command"}}}"#, + )?; + fs::remove_file(standalone_root.path().join(".codex-plugin/plugin.json"))?; + } + let policy = enabled_root.map_or_else( + || default_off.to_string(), + |id| format!("{default_off}[plugins.{id}]\nenabled = true\n"), + ); + fs::write(codex_home.path().join("config.toml"), policy)?; + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + // A warning requires metadata probing, which the test filesystem cannot sandbox. + // Complete discovery retries ownership under the same sandbox, root, and policy. + let discovery = (phase < 2).then(|| { + ExecutorCapabilityDiscoverySnapshot::new( + std::slice::from_ref(standalone), + vec![Ok(Arc::new(CapabilityRootDiscovery { + id: standalone.id.clone(), + path: path.clone(), + plugin: None, + skills: Vec::new(), + namespace_manifests: Vec::new(), + warnings: if phase == 0 { + vec!["ownership metadata unavailable".to_string()] + } else { + Vec::new() + }, + error: None, + }))], + sandbox_contexts.clone(), + ) + }); + let selected = registry.mcp_server_contributors()[0] + .selected_plugins( + McpServerContributionContext::for_step( + &config, + &thread_init, + &thread_store, + "test_originator", + &selected_roots, + discovery.as_ref(), + ), + &config.plugins, + ) + .await; + let mut actual = Vec::new(); + for plugin in selected { + let contribution = plugin.mcp.await; + actual.push(( + plugin.selected_root_id, + contribution + .servers + .into_iter() + .map(|(name, _)| name) + .collect::>(), + contribution.connector_ids, + )); + } + assert_eq!( + actual, + expected + .into_iter() + .map(|(id, servers)| ( + id.to_string(), + servers.into_iter().map(str::to_string).collect::>(), + Vec::new(), + )) + .collect::>(), + "phase {phase}", + ); + } + Ok(()) +} + #[tokio::test] async fn managed_plugins_requirement_disables_selected_plugin_capabilities() -> TestResult { let codex_home = tempfile::tempdir()?; @@ -439,14 +662,17 @@ async fn raw_selected_plugin_contributions( }; let selected = registry.mcp_server_contributors()[0] - .selected_plugins(McpServerContributionContext::for_step( - config, - &thread_init, - &thread_store, - "test_originator", - &selected_capability_roots, - executor_capability_discovery.as_ref(), - )) + .selected_plugins( + McpServerContributionContext::for_step( + config, + &thread_init, + &thread_store, + "test_originator", + &selected_capability_roots, + executor_capability_discovery.as_ref(), + ), + &config.plugins, + ) .await; let mut contributions = Vec::new(); for plugin in selected { diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 0150e0302b..911952e863 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -239,6 +239,7 @@ async fn new_config( application_network_policy: Default::default(), application_auth_route_config: Some(auth_config.auth_route_config.clone()), config_layer_stack, + plugins: Default::default(), startup_warnings: Vec::new(), bypass_hook_trust: false, model,