mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Add configurable default plugin activation policy (#52220)
## What changed Support `plugins._default.enabled` with explicit per-plugin `enabled` overrides. Omission preserves existing source and account enablement; configuration cannot activate a plugin disabled at its source. Plugin settings such as remembered tool approvals inherit the default unless `enabled` is explicitly set. Apply the policy to plugin skills, MCP servers, Apps, and hooks, including selected executor roots and runtime configuration refreshes. Preserve the latest plugin policy when adding MCP dependencies and rebuild hooks when concurrent configuration updates replace their inputs. Report effective enablement in `plugin/list`, `plugin/installed`, `plugin/read`, and `plugin/share/list`. Remote installation still installs disabled packages, but skips MCP and Apps authentication setup without writing an enablement override. ## Testing Add regression coverage for default inheritance and explicit overrides, source-disabled plugins, policy refreshes during active turns, disabled or malformed selected plugin roots, and remote installation authentication behavior. GitOrigin-RevId: 83b70c5b47fc95474fec292f248683f1f887221e
This commit is contained in:
@@ -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."<plugin-name>@<marketplace-name>".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
|
||||
`<plugin-name>@<marketplace-name>` format. A supplied list replaces the selection;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<PluginMarketplaceEntry> {
|
||||
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<PluginShareListResponse, JSONRPCErrorError> {
|
||||
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<St
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn remote_marketplace_to_info(marketplace: RemoteMarketplace) -> 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<AppSummary>,
|
||||
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::<Vec<_>>();
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
));
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
|
||||
@@ -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<_>>(),
|
||||
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<_>>(),
|
||||
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)
|
||||
|
||||
@@ -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<bool>,
|
||||
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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<HooksToml>,
|
||||
|
||||
/// User-level plugin config entries keyed by plugin name.
|
||||
/// Default and per-plugin settings keyed by exact `<plugin>@<marketplace>` IDs.
|
||||
#[serde(default)]
|
||||
pub plugins: HashMap<String, PluginConfig>,
|
||||
pub plugins: PluginsConfigToml,
|
||||
|
||||
/// User-level marketplace entries keyed by marketplace name.
|
||||
#[serde(default)]
|
||||
|
||||
@@ -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<PluginsDefaultConfig>,
|
||||
|
||||
#[serde(default, flatten)]
|
||||
pub plugins: HashMap<String, PluginConfig>,
|
||||
}
|
||||
|
||||
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<bool>,
|
||||
|
||||
/// Per-MCP-server policy overlays for MCP servers contributed by this plugin.
|
||||
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
|
||||
|
||||
@@ -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)] {
|
||||
|
||||
@@ -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<ExecutorPluginHookSource> {
|
||||
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;
|
||||
};
|
||||
|
||||
@@ -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::<ExecutorPluginHookSource>::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::<ExecutorPluginHookSource>::new()
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<McpServerConfig>])
|
||||
|
||||
/// 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<PluginSkillRoot>>,
|
||||
@@ -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<String, PluginConfig>,
|
||||
store: &PluginStore,
|
||||
remote_global_catalog_active: bool,
|
||||
scope: PluginLoadScope<'_>,
|
||||
) -> Vec<LoadedPlugin<McpServerConfig>> {
|
||||
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<String, PluginConfig>,
|
||||
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<String, HashMap<String, PluginMcpServerConfig>> {
|
||||
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<String, PluginMcpServerConfig>,
|
||||
|
||||
@@ -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::<codex_config::config_toml::ConfigToml>()
|
||||
.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::<codex_config::config_toml::ConfigToml>()
|
||||
.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::<codex_config::config_toml::ConfigToml>()
|
||||
.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::<ConfigToml>()
|
||||
.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]
|
||||
|
||||
@@ -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<dyn Fn(EffectivePluginsChange) + Send
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PluginsConfigInput {
|
||||
pub config_layer_stack: ConfigLayerStack,
|
||||
pub plugins: PluginsConfigToml,
|
||||
pub model_provider_id: String,
|
||||
pub plugins_enabled: bool,
|
||||
pub remote_plugin_enabled: bool,
|
||||
@@ -167,8 +169,11 @@ pub struct PluginsConfigInput {
|
||||
}
|
||||
|
||||
impl PluginsConfigInput {
|
||||
// Require validated policy at construction; a default followed by a setter could fail open.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn new(
|
||||
config_layer_stack: ConfigLayerStack,
|
||||
plugins: PluginsConfigToml,
|
||||
model_provider_id: String,
|
||||
plugins_enabled: bool,
|
||||
remote_plugin_enabled: bool,
|
||||
@@ -178,6 +183,7 @@ impl PluginsConfigInput {
|
||||
) -> 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<String, PluginConfig>,
|
||||
plugin_policy: PluginsConfigToml,
|
||||
skill_config_rules: SkillConfigRules,
|
||||
remote_global_catalog_active: bool,
|
||||
auth_identity: Option<RemoteInstalledPluginsAuthIdentity>,
|
||||
@@ -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::<HashSet<_>>();
|
||||
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::<HashSet<_>>();
|
||||
ConfiguredPluginStates { installed, enabled }
|
||||
}
|
||||
|
||||
@@ -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::<codex_config::config_toml::ConfigToml>(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::<codex_config::config_toml::ConfigToml>()
|
||||
.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::<Vec<_>>(),
|
||||
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::<codex_config::config_toml::ConfigToml>()
|
||||
.unwrap()
|
||||
.plugins,
|
||||
crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot::default(),
|
||||
&PluginStore::new(codex_home.path().to_path_buf()),
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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::<codex_config::config_toml::ConfigToml>()
|
||||
.unwrap()
|
||||
.plugins,
|
||||
"openai".to_string(),
|
||||
/*plugins_enabled*/ true,
|
||||
/*remote_plugin_enabled*/ false,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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::<codex_config::config_toml::ConfigToml>()
|
||||
.unwrap()
|
||||
.plugins,
|
||||
model_provider_id,
|
||||
feature_enabled(&effective_config, "plugins", /*default_enabled*/ true),
|
||||
feature_enabled(
|
||||
|
||||
@@ -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 `<plugin>@<marketplace>` IDs."
|
||||
},
|
||||
"profile": {
|
||||
"description": "Profile to use from the `profiles` map.",
|
||||
|
||||
@@ -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<String>,
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -31,6 +31,7 @@ pub(crate) enum RuntimeConfigRefresh {
|
||||
#[serde(default)]
|
||||
struct McpRefreshToml {
|
||||
features: Option<FeaturesToml>,
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -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::<codex_config::config_toml::ConfigToml>()
|
||||
.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;
|
||||
|
||||
@@ -332,9 +332,9 @@ fn executor_hook_sources_for_step(step_context: &StepContext) -> Vec<ExecutorPlu
|
||||
.executor_capability_discovery
|
||||
.as_deref()
|
||||
.map(|snapshot| {
|
||||
let app_tool_policy =
|
||||
AppToolPolicyEvaluator::new(&step_context.mcp.config().config_layer_stack);
|
||||
executor_plugin_hook_sources(snapshot, |server, tool| {
|
||||
let config = step_context.mcp.config();
|
||||
let app_tool_policy = AppToolPolicyEvaluator::new(&config.config_layer_stack);
|
||||
executor_plugin_hook_sources(snapshot, &config.plugins, |server, tool| {
|
||||
step_context
|
||||
.mcp
|
||||
.tool_info(server, tool)
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::sync::Arc;
|
||||
use crate::config::Config;
|
||||
use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::types::PluginsConfigToml;
|
||||
use codex_connectors::ConnectorRuntimeManager;
|
||||
use codex_connectors::PluginConnectorSource;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
@@ -191,6 +192,7 @@ impl McpManager {
|
||||
pub(crate) async fn selected_plugins_for_step(
|
||||
&self,
|
||||
context: McpServerContributionContext<'_, Config>,
|
||||
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);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
@@ -690,8 +690,7 @@ impl Session {
|
||||
|
||||
pub(crate) async fn refresh_mcp_servers_now(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
refresh_config: &Config,
|
||||
added_servers: HashMap<String, McpServerConfig>,
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) {
|
||||
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,
|
||||
|
||||
@@ -2167,27 +2167,31 @@ impl Session {
|
||||
state.session_configuration.provider.info().clone()
|
||||
}
|
||||
|
||||
pub(crate) async fn refresh_hooks(&self, config: Arc<Config>) {
|
||||
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<Config>) {
|
||||
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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -1577,6 +1577,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
struct InitialDataRecorder {
|
||||
lifecycle_observed: Arc<std::sync::Mutex<Vec<(String, String)>>>,
|
||||
mcp_observed: Arc<std::sync::Mutex<Vec<(String, SessionSource)>>>,
|
||||
mcp_loaded: Arc<std::sync::atomic::AtomicUsize>,
|
||||
}
|
||||
|
||||
impl codex_extension_api::ThreadLifecycleContributor<Config> 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<codex_extension_api::SelectedPlugin<'a>>>
|
||||
{
|
||||
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::<codex_config::config_toml::ConfigToml>(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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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::<Value>::new());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn executor_stop_hook_runs_after_attachment() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -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::<Vec<_>>();
|
||||
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(())
|
||||
}
|
||||
@@ -38,6 +38,7 @@ impl McpServerContributor<Config> for ExecutorSelectionRecorder {
|
||||
fn selected_plugins<'a>(
|
||||
&'a self,
|
||||
context: McpServerContributionContext<'a, Config>,
|
||||
_plugins_config: &'a codex_config::types::PluginsConfigToml,
|
||||
) -> ExtensionFuture<'a, Vec<SelectedPlugin<'a>>> {
|
||||
Box::pin(async move {
|
||||
self.plugin_contexts
|
||||
|
||||
@@ -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"))]
|
||||
|
||||
@@ -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<WorldStateSectionContribution>> {
|
||||
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, "<SKILLS_ROOT>/"))?;
|
||||
// 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::<Vec<_>>();
|
||||
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(),
|
||||
)
|
||||
|
||||
+139
@@ -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: "<PROMPT_CACHE_KEY 1>"
|
||||
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]
|
||||
|
||||
<OMITTED 155 LINES; ~4935 TOKENS; hash=5573869716192298>
|
||||
|
||||
- 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] <permissions instructions>
|
||||
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: &&, ||
|
||||
<OMITTED 44 LINES; ~790 TOKENS; hash=3a8900343e968d61>
|
||||
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"]
|
||||
</permissions instructions>
|
||||
[02] <collaboration_mode># 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 `<collaboration_mode>...<...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.
|
||||
</collaboration_mode>
|
||||
[03] <skills_instructions>
|
||||
## 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/<SKILLS_ROOT>/plugin/skills`
|
||||
- `e1` = `skill://workspace-skills/<SKILLS_ROOT>/skills`
|
||||
Read a skill package directly with `skills.read({"package":"<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)
|
||||
</skills_instructions>
|
||||
03:message/developer:
|
||||
<multi_agent_role>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.
|
||||
<OMITTED 17 LINES; ~246 TOKENS; hash=780031e2df907944>
|
||||
- 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"`.</multi_agent_role> [hash=d5928243dc643bcc]
|
||||
04:message/developer:
|
||||
<multi_agent_mode>Any earlier instruction enabling proactive multi-agent delegation no longer applies. D...elegation, or parallel agent work.</multi_agent_mode> [hash=2b6e037067b4390c]
|
||||
05:message/user:
|
||||
<environment_context>
|
||||
<cwd><CWD></cwd>
|
||||
<shell><HOST_SHELL></shell>
|
||||
<current_date><CURRENT_DATE></current_date>
|
||||
<timezone><HOST_TIMEZONE></timezone>
|
||||
<filesystem><workspace_roots><root><CWD></root></workspace_roots><permission_profile type="managed"><f...ntry></file_system></permission_profile></filesystem> [hash=e8c1de60307d0a3d]
|
||||
</environment_context>
|
||||
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_ROOT>/skills/active-helper"}],"warnings":[]} [hash=a640b563d0b76db9]
|
||||
09:message/developer:
|
||||
<skills_instructions>
|
||||
## 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/<SKILLS_ROOT>/other-skills`
|
||||
Read a skill package directly with `skills.read({"package":"<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)
|
||||
</skills_instructions>
|
||||
+113
@@ -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: "<PROMPT_CACHE_KEY 1>"
|
||||
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]
|
||||
|
||||
<OMITTED 155 LINES; ~4935 TOKENS; hash=5573869716192298>
|
||||
|
||||
- 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_instructions>
|
||||
## 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` = `<SKILLS_ROOT>`
|
||||
- `r1` = `<PLUGINS_CACHE>/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)
|
||||
</skills_instructions>
|
||||
[02] <permissions instructions>
|
||||
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.
|
||||
</permissions instructions>
|
||||
[03] <collaboration_mode># 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 `<collaboration_mode>...<...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.
|
||||
</collaboration_mode>
|
||||
03:message/developer:
|
||||
<multi_agent_role>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.
|
||||
<OMITTED 17 LINES; ~246 TOKENS; hash=780031e2df907944>
|
||||
- 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"`.</multi_agent_role> [hash=d5928243dc643bcc]
|
||||
04:message/developer:
|
||||
<multi_agent_mode>Any earlier instruction enabling proactive multi-agent delegation no longer applies. D...elegation, or parallel agent work.</multi_agent_mode> [hash=2b6e037067b4390c]
|
||||
05:message/user:
|
||||
<environment_context>
|
||||
<cwd><CWD></cwd>
|
||||
<shell><HOST_SHELL></shell>
|
||||
<current_date><CURRENT_DATE></current_date>
|
||||
<timezone><HOST_TIMEZONE></timezone>
|
||||
<filesystem><workspace_roots><root><CWD></root></workspace_roots><permission_profile type="disabled"><...e="unrestricted" /></permission_profile></filesystem> [hash=d40b2f45cb028888]
|
||||
</environment_context>
|
||||
06:message/user:
|
||||
Plan a team kickoff. Which planning skills are available?
|
||||
@@ -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<C: Sync>: 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<SelectedPlugin<'a>>> {
|
||||
Box::pin(async { Vec::new() })
|
||||
}
|
||||
|
||||
@@ -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(())
|
||||
|
||||
+139
-24
@@ -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<PluginRootOwnership, ExecutorPluginProviderError> {
|
||||
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<SelectedPluginContribution> {
|
||||
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<Config> for PluginContributor {
|
||||
fn selected_plugins<'a>(
|
||||
&'a self,
|
||||
context: McpServerContributionContext<'a, Config>,
|
||||
plugins_config: &'a PluginsConfigToml,
|
||||
) -> ExtensionFuture<'a, Vec<SelectedPlugin<'a>>> {
|
||||
Box::pin(async move {
|
||||
let Some(thread_store) = context.thread_store() else {
|
||||
@@ -155,8 +227,6 @@ impl McpServerContributor<Config> 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<Config> 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<Config> for PluginContributor {
|
||||
);
|
||||
project_metadata(
|
||||
context.config(),
|
||||
plugins_config,
|
||||
&root.selected_root.id,
|
||||
&plugin_policies,
|
||||
metadata,
|
||||
)
|
||||
}),
|
||||
@@ -199,18 +288,32 @@ impl McpServerContributor<Config> 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<Config> 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<HashMap<String, HashMap<String, PluginMcpServerConfig>>>,
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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<SelectedPluginContribution>,
|
||||
pub(crate) ownership: Option<PluginRootOwnership>,
|
||||
pub(crate) ownership_sandbox: Option<Box<FileSystemSandboxContext>>,
|
||||
pub(crate) metadata: CachedPluginMetadata,
|
||||
}
|
||||
|
||||
/// Denied roots retain ownership without loading their capabilities.
|
||||
pub(crate) enum CachedPluginMetadata {
|
||||
Unloaded,
|
||||
Loaded(Option<SelectedPluginContribution>),
|
||||
}
|
||||
|
||||
@@ -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::<Vec<_>>(),
|
||||
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::<anyhow::Result<Vec<_>>>()?;
|
||||
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::<Vec<_>>(),
|
||||
contribution.connector_ids,
|
||||
));
|
||||
}
|
||||
assert_eq!(
|
||||
actual,
|
||||
expected
|
||||
.into_iter()
|
||||
.map(|(id, servers)| (
|
||||
id.to_string(),
|
||||
servers.into_iter().map(str::to_string).collect::<Vec<_>>(),
|
||||
Vec::new(),
|
||||
))
|
||||
.collect::<Vec<_>>(),
|
||||
"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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user