Gate plugin recommendation cache warming on feature flags (#60721)

## Why

Background plugin cache refreshes fetched recommendations even when plugin recommendations were disabled.

## What changed

Pass the effective recommendation feature state into `PluginsConfigInput` and skip recommendation cache warming unless apps, plugins, and either `tool_suggest` or `recommended_plugins` are enabled. Installed-plugin cache refreshes continue independently.

Disable `tool_suggest` and `recommended_plugins` by default in core and app-server test helpers, with explicit overrides for recommendation tests.

## Testing

Update cache-warming and external-login recommendation tests to opt in explicitly. Update config RPC assertions to verify the test helpers' session flag overrides.

GitOrigin-RevId: dc04e2847c94e2b15842fcd95cc0cf1f104e1fac
This commit is contained in:
Alex Daley
2026-11-11 05:38:52 +00:00
committed by copyberry
parent 5228d94a27
commit fe2f185208
13 changed files with 104 additions and 89 deletions
+1 -1
View File
@@ -53,8 +53,8 @@ pub use rollout::create_fake_rollout_with_text_elements;
pub use rollout::create_fake_rollout_with_token_usage;
pub use rollout::rollout_path;
use serde::de::DeserializeOwned;
pub use test_app_server::DEFAULT_APP_SERVER_TEST_ARGS;
pub use test_app_server::DEFAULT_CLIENT_NAME;
pub use test_app_server::DISABLE_PLUGIN_STARTUP_TASKS_ARG;
pub use test_app_server::TestAppServer;
pub use test_app_server::TestAppServerBuilder;
@@ -167,7 +167,14 @@ pub struct TestAppServer {
}
pub const DEFAULT_CLIENT_NAME: &str = "codex-app-server-tests";
pub const DISABLE_PLUGIN_STARTUP_TASKS_ARG: &str = "--disable-plugin-startup-tasks-for-tests";
const DISABLE_PLUGIN_STARTUP_TASKS_ARG: &str = "--disable-plugin-startup-tasks-for-tests";
pub const DEFAULT_APP_SERVER_TEST_ARGS: [&str; 5] = [
DISABLE_PLUGIN_STARTUP_TASKS_ARG,
"-c",
"features.tool_suggest=false",
"-c",
"features.recommended_plugins=false",
];
const DISABLE_MANAGED_CONFIG_ENV_VAR: &str = "CODEX_APP_SERVER_DISABLE_MANAGED_CONFIG";
#[cfg(windows)]
const DEFAULT_REQUEST_TIMEOUT: Duration = Duration::from_secs(25);
@@ -183,7 +190,7 @@ impl TestAppServer {
environment: TestAppServerEnvironment::Auto,
program: None,
env_overrides: Vec::new(),
args: vec![DISABLE_PLUGIN_STARTUP_TASKS_ARG.to_string()],
args: DEFAULT_APP_SERVER_TEST_ARGS.map(str::to_owned).into(),
exec_server_delay: None,
mock_chatgpt_backend: false,
}
@@ -899,7 +899,7 @@ sandbox_mode = "workspace-write"
.all(|origin| !matches!(&origin.name, ConfigLayerSource::PackagedDefaults { .. }))
);
let layers = layers.expect("layers present");
assert_layers_user_then_optional_system(&layers, user_file)?;
assert_config_layers(&layers, user_file, /*managed_file*/ None);
Ok(())
}
@@ -970,7 +970,7 @@ allowed_domains = ["example.com"]
}
);
let layers = layers.expect("layers present");
assert_layers_user_then_optional_system(&layers, user_file)?;
assert_config_layers(&layers, user_file, /*managed_file*/ None);
Ok(())
}
@@ -1570,7 +1570,7 @@ enabled = false
origins["apps.app1.tools.list.enabled"],
origins["plugins.sample@test.apps.app1.tools.list.enabled"],
);
assert_layers_user_then_optional_system(&layers, user_file)?;
assert_config_layers(&layers, user_file, /*managed_file*/ None);
Ok(())
}
@@ -1854,7 +1854,7 @@ writable_roots = [{}]
);
let layers = layers.expect("layers present");
assert_layers_managed_user_then_optional_system(&layers, managed_file, user_file)?;
assert_config_layers(&layers, user_file, Some(managed_file));
Ok(())
}
@@ -2875,59 +2875,48 @@ async fn config_batch_write_updates_multiple_desktop_settings() -> Result<()> {
Ok(())
}
fn assert_layers_user_then_optional_system(
fn assert_config_layers(
layers: &[codex_app_server_protocol::ConfigLayer],
user_file: AbsolutePathBuf,
) -> Result<()> {
let mut first_index = 0;
if matches!(
managed_file: Option<AbsolutePathBuf>,
) {
let layers = if matches!(
layers.first().map(|layer| &layer.name),
Some(ConfigLayerSource::LegacyManagedConfigTomlFromMdm)
) {
first_index = 1;
}
assert_eq!(layers.len(), first_index + 2);
&layers[1..]
} else {
layers
};
let layers = if let Some(file) = managed_file {
let (managed, remaining) = layers.split_first().expect("managed config layer");
assert_eq!(
managed.name,
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file }
);
remaining
} else {
layers
};
let [flags, user, system] = layers else {
panic!("expected session flags, user, and system layers, got {layers:?}");
};
assert_eq!(flags.name, ConfigLayerSource::SessionFlags);
assert_eq!(
layers[first_index].name,
flags.config,
json!({
"features": {
"tool_suggest": false,
"recommended_plugins": false,
},
})
);
assert_eq!(
user.name,
ConfigLayerSource::User {
file: user_file,
profile: None
}
);
assert!(matches!(
layers[first_index + 1].name,
ConfigLayerSource::System { .. }
));
Ok(())
}
fn assert_layers_managed_user_then_optional_system(
layers: &[codex_app_server_protocol::ConfigLayer],
managed_file: AbsolutePathBuf,
user_file: AbsolutePathBuf,
) -> Result<()> {
let mut first_index = 0;
if matches!(
layers.first().map(|layer| &layer.name),
Some(ConfigLayerSource::LegacyManagedConfigTomlFromMdm)
) {
first_index = 1;
}
assert_eq!(layers.len(), first_index + 3);
assert_eq!(
layers[first_index].name,
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
);
assert_eq!(
layers[first_index + 1].name,
ConfigLayerSource::User {
file: user_file,
profile: None
}
);
assert!(matches!(
layers[first_index + 2].name,
ConfigLayerSource::System { .. }
));
Ok(())
assert!(matches!(system.name, ConfigLayerSource::System { .. }));
}
@@ -2,7 +2,7 @@
use anyhow::Context;
use anyhow::Result;
use app_test_support::DISABLE_PLUGIN_STARTUP_TASKS_ARG;
use app_test_support::DEFAULT_APP_SERVER_TEST_ARGS;
use app_test_support::TestAppServer;
use base64::Engine;
use base64::engine::general_purpose::STANDARD;
@@ -53,7 +53,7 @@ async fn stdio_sigterm_times_out_with_blocked_stderr() -> Result<()> {
// TestAppServer drains stderr, so keep this subprocess's stderr pipe unread.
let codex_home = TempDir::new()?;
let mut process = tokio::process::Command::new(cargo_bin("codex-app-server")?)
.arg(DISABLE_PLUGIN_STARTUP_TASKS_ARG)
.args(DEFAULT_APP_SERVER_TEST_ARGS)
.env("CODEX_HOME", codex_home.path())
.env(
"CODEX_APP_SERVER_MANAGED_CONFIG_PATH",
@@ -1,7 +1,7 @@
use anyhow::Context;
use anyhow::Result;
use anyhow::bail;
use app_test_support::DISABLE_PLUGIN_STARTUP_TASKS_ARG;
use app_test_support::DEFAULT_APP_SERVER_TEST_ARGS;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::to_response;
use base64::Engine;
@@ -573,7 +573,7 @@ pub(super) async fn spawn_websocket_server_with_args(
let mut cmd = Command::new(program);
cmd.arg("--listen")
.arg(listen_url)
.arg(DISABLE_PLUGIN_STARTUP_TASKS_ARG)
.args(DEFAULT_APP_SERVER_TEST_ARGS)
.args(extra_args)
.stdin(Stdio::null())
.stdout(Stdio::null())
@@ -711,7 +711,7 @@ async fn run_websocket_server_to_completion_with_args(
let mut cmd = Command::new(program);
cmd.arg("--listen")
.arg(listen_url)
.arg(DISABLE_PLUGIN_STARTUP_TASKS_ARG)
.args(DEFAULT_APP_SERVER_TEST_ARGS)
.args(extra_args)
.stdin(Stdio::null())
.stdout(Stdio::null())
@@ -4,7 +4,7 @@ use super::connection_handling_websocket::DEFAULT_READ_TIMEOUT;
use super::connection_handling_websocket::create_config_toml;
use anyhow::Context;
use anyhow::Result;
use app_test_support::DISABLE_PLUGIN_STARTUP_TASKS_ARG;
use app_test_support::DEFAULT_APP_SERVER_TEST_ARGS;
use app_test_support::create_final_assistant_message_sse_response;
use codex_app_server_protocol::InitializeCapabilities;
use codex_app_server_protocol::JSONRPCMessage;
@@ -814,7 +814,7 @@ pub(super) fn spawn_server(home: &Path, socket_path: &Path) -> Result<Child> {
let binary = codex_utils_cargo_bin::cargo_bin("codex-app-server")?;
Ok(Command::new(binary)
.args(["--listen", &format!("unix://{}", socket_path.display())])
.arg(DISABLE_PLUGIN_STARTUP_TASKS_ARG)
.args(DEFAULT_APP_SERVER_TEST_ARGS)
.env("CODEX_HOME", home)
// Match TestAppServer's isolation from the host's managed configuration.
.env(
@@ -36,6 +36,10 @@ async fn experimental_feature_list_returns_feature_metadata_with_stage() -> Resu
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cli_overrides(vec![
("features.tool_suggest".to_string(), false.into()),
("features.recommended_plugins".to_string(), false.into()),
])
.loader_overrides(LoaderOverrides::with_managed_config_path_for_tests(
codex_home.path().join("managed_config.toml"),
))
@@ -31,30 +31,26 @@ use wiremock::matchers::query_param;
const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(20);
const WORKSPACE_ID: &str = "123e4567-e89b-42d3-a456-426614174010";
enum ToolSuggestFeature {
Enabled,
Disabled,
enum RecommendationFeature {
ToolSuggest,
RecommendedPlugins,
}
#[tokio::test]
async fn first_turn_after_external_login_waits_for_recommended_plugins() -> Result<()> {
recommended_plugins_after_external_login(ToolSuggestFeature::Enabled).await
recommended_plugins_after_external_login(RecommendationFeature::ToolSuggest).await
}
#[tokio::test]
async fn first_turn_after_external_login_waits_for_recommended_plugins_without_tool_suggest()
-> Result<()> {
recommended_plugins_after_external_login(ToolSuggestFeature::Disabled).await
recommended_plugins_after_external_login(RecommendationFeature::RecommendedPlugins).await
}
async fn recommended_plugins_after_external_login(
tool_suggest_feature: ToolSuggestFeature,
) -> Result<()> {
let tool_suggest_enabled = matches!(tool_suggest_feature, ToolSuggestFeature::Enabled);
let recommended_plugins_config = if tool_suggest_enabled {
""
} else {
"recommended_plugins = true\n"
async fn recommended_plugins_after_external_login(feature: RecommendationFeature) -> Result<()> {
let feature_args: &[&str] = match feature {
RecommendationFeature::ToolSuggest => &["-c", "features.tool_suggest=true"],
RecommendationFeature::RecommendedPlugins => &["-c", "features.recommended_plugins=true"],
};
let server = responses::start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
@@ -89,19 +85,12 @@ async fn recommended_plugins_after_external_login(
&server.uri(),
&apps_server.chatgpt_base_url,
)?;
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[features]\napps = true\nplugins = true\ntool_suggest = {tool_suggest_enabled}\n{recommended_plugins_config}"
),
)?;
let sqlite_home = codex_home.path().to_string_lossy();
let mut app_server = TestAppServer::builder()
.with_codex_home(codex_home.path())
.without_managed_config()
.with_args(&["-c", "features.apps=true", "-c", "features.plugins=true"])
.with_args(feature_args)
.with_env_overrides(&[("CODEX_SQLITE_HOME", Some(sqlite_home.as_ref()))])
.build()
.await?;
@@ -218,7 +207,7 @@ async fn recommended_plugins_after_external_login(
.collect::<Vec<_>>();
assert_eq!(
tool_names.contains(&"request_plugin_install"),
tool_suggest_enabled
matches!(feature, RecommendationFeature::ToolSuggest)
);
assert!(!tool_names.contains(&"list_available_plugins_to_install"));
Ok(())
+11
View File
@@ -175,6 +175,7 @@ pub struct PluginsConfigInput {
pub remote_plugin_enabled: bool,
pub chatgpt_base_url: String,
pub product_sku: Option<String>,
plugin_recommendations_enabled: bool,
http_client_factory: HttpClientFactory,
remote_http_clients: Arc<OnceLock<Arc<dyn HttpClientSelector>>>,
runtime_http_clients: Arc<OnceLock<Arc<dyn HttpClientSelector>>>,
@@ -203,12 +204,18 @@ impl PluginsConfigInput {
chatgpt_base_url,
http_client_factory,
product_sku,
plugin_recommendations_enabled: false,
remote_http_clients: Arc::new(OnceLock::new()),
runtime_http_clients: Arc::new(OnceLock::new()),
runtime_auth_mode: None,
}
}
pub fn with_plugin_recommendations_enabled(mut self, enabled: bool) -> Self {
self.plugin_recommendations_enabled = enabled;
self
}
pub fn with_runtime_auth_mode(mut self, mode: Option<RuntimeAuthMode>) -> Self {
self.runtime_auth_mode = mode;
self
@@ -1699,6 +1706,10 @@ impl PluginsManager {
EffectivePluginsChange::default(),
);
if !config.plugin_recommendations_enabled {
return;
}
let manager = Arc::clone(self);
let config = config.clone();
tokio::spawn(async move {
+3 -1
View File
@@ -6151,7 +6151,9 @@ plugins = true
.mount(&server)
.await;
let mut config = load_config(tmp.path(), tmp.path()).await;
let mut config = load_config(tmp.path(), tmp.path())
.await
.with_plugin_recommendations_enabled(/*enabled*/ true);
config.chatgpt_base_url = server.uri();
let manager = std::sync::Arc::new(test_plugins_manager(tmp.path().to_path_buf()));
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
+8
View File
@@ -331,6 +331,13 @@ pub(crate) async fn test_config() -> Config {
Config::load_from_base_config_with_overrides(
ConfigToml {
model: Some("gpt-5.5".to_string()),
features: Some(
BTreeMap::from([
(Feature::ToolSuggest.key().to_string(), false),
(Feature::RecommendedPlugins.key().to_string(), false),
])
.into(),
),
..Default::default()
},
ConfigOverrides::default(),
@@ -1890,6 +1897,7 @@ impl Config {
self.http_client_factory(),
self.apps_mcp_product_sku.clone(),
)
.with_plugin_recommendations_enabled(self.features.plugin_recommendations_enabled())
.with_runtime_auth_mode(
(self.features.enabled(Feature::RuntimeAgentInstances)
&& codex_runtime_agent_instance::is_openai_api_url(&self.chatgpt_base_url))
+10 -2
View File
@@ -17,6 +17,7 @@ use codex_core::config::ConfigBuilder;
use codex_core::config::ConfigOverrides;
pub use codex_core::test_support::TestCodexResponsesRequestKind;
pub use codex_core::test_support::responses_metadata;
use codex_features::Feature;
use codex_utils_absolute_path::AbsolutePathBuf;
pub use codex_utils_absolute_path::test_support::PathBufExt;
pub use codex_utils_absolute_path::test_support::PathExt;
@@ -213,14 +214,21 @@ pub async fn load_default_config_for_test_with_cloud_config_bundle(
codex_home: &TempDir,
cloud_config_bundle: CloudConfigBundleLoader,
) -> Config {
ConfigBuilder::default()
let mut config = ConfigBuilder::default()
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(default_test_overrides())
.cloud_config_bundle(cloud_config_bundle)
.build()
.await
.expect("defaults for test should always succeed")
.expect("defaults for test should always succeed");
for feature in [Feature::ToolSuggest, Feature::RecommendedPlugins] {
config
.features
.disable(feature)
.expect("test config should allow disabling plugin recommendations");
}
config
}
pub fn managed_network_requirements_loader() -> CloudConfigBundleLoader {
@@ -2,9 +2,9 @@
#![cfg(test)]
use anyhow::Result;
use codex_core::config::ConfigBuilder;
use codex_login::CodexAuth;
use codex_models_manager::bundled_models_response;
use core_test_support::load_default_config_for_test;
use core_test_support::responses;
use core_test_support::test_codex::test_codex;
use pretty_assertions::assert_eq;
@@ -49,10 +49,7 @@ model_catalog_url = "https://chatgpt.com/backend-api/codex/models"
inference.uri(),
),
)?;
let mut loaded = ConfigBuilder::default()
.codex_home(home.path().to_path_buf())
.build()
.await?;
let mut loaded = load_default_config_for_test(&home).await;
assert_eq!(
loaded
.model_provider