From 7e37354a587920eaf65fbb3c3005f7b8e6c7c208 Mon Sep 17 00:00:00 2001 From: Alex Daley Date: Thu, 18 Jun 2026 20:08:04 -0400 Subject: [PATCH] [plugins] Refresh plugin and tool caches after remote install (#28951) Summary - Refresh the installed remote-plugin snapshot and Codex Apps tools after completing a remote JIT install. - Gate `completed: true` on every expected `app_connector_id` appearing after the uncached `tools/list` refresh, while continuing to skip local bundle verification for server-side installs. - Keep the cached recommendations response and filter refreshed installed remote IDs locally, so this does not add another recommendations fetch. - Add regression coverage for tools appearing after the hard refresh and remaining absent after the refresh. The resumed model request sees the refreshed tool router when installation completes. Root Cause - Remote suggestions from `openai-curated-remote` returned `true` before taking the existing connector refresh path, leaving the resumed turn with the pre-install Apps tool catalog. Validation - `just test -p codex-core request_plugin_install` - `just test -p codex-core-plugins recommended_plugin_candidates_filter_installed_and_disabled_plugins` - `just test -p codex-core-plugins` - `just fix -p codex-core-plugins` - `just fix -p codex-core` - `just fmt` - `just test -p codex-core` was not fully clean locally: 2,729 passed, 26 failed, and 16 skipped. The failures were dominated by local Seatbelt/network/timing issues, including plugin-install timeouts under full-suite contention; the focused plugin-install runs pass. --- codex-rs/core-plugins/src/manager.rs | 15 + codex-rs/core-plugins/src/manager_tests.rs | 6 +- .../tools/handlers/request_plugin_install.rs | 45 ++- .../core/tests/common/apps_test_server.rs | 71 ++++- .../tests/suite/request_plugin_install.rs | 286 +++++++++++++++--- 5 files changed, 370 insertions(+), 53 deletions(-) diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 9d6c09633..5e2224bb7 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -39,6 +39,7 @@ use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError; use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; use crate::marketplace_upgrade::configured_git_marketplace_names; use crate::marketplace_upgrade::upgrade_configured_git_marketplaces; +use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RecommendedPluginsMode; use crate::remote::RemoteInstalledPlugin; use crate::remote::RemotePluginCatalogError; @@ -1098,6 +1099,19 @@ impl PluginsManager { .iter() .map(|plugin| plugin.config_name.as_str()) .collect::>(); + let installed_remote_plugin_ids = { + let cache = match self.remote_installed_plugins_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache + .as_deref() + .unwrap_or_default() + .iter() + .filter(|plugin| plugin.marketplace_name == REMOTE_GLOBAL_MARKETPLACE_NAME) + .map(|plugin| plugin.id.clone()) + .collect::>() + }; let disabled_plugin_ids = input .disabled_tools .iter() @@ -1109,6 +1123,7 @@ impl PluginsManager { .into_iter() .filter(|plugin| { !installed_plugin_ids.contains(plugin.config_id.as_str()) + && !installed_remote_plugin_ids.contains(plugin.remote_plugin_id.as_str()) && !disabled_plugin_ids.contains(plugin.config_id.as_str()) }) .map(|plugin| { diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index cc8ee1913..4f448ff16 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -4464,8 +4464,6 @@ plugins = true remote_plugin = true "#, ); - write_cached_plugin(tmp.path(), REMOTE_GLOBAL_MARKETPLACE_NAME, "linear"); - let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/ps/plugins/suggested")) @@ -4496,7 +4494,9 @@ remote_plugin = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = server.uri(); let manager = PluginsManager::new(tmp.path().to_path_buf()); - manager.write_remote_installed_plugins_cache(vec![remote_installed_plugin("linear")]); + let mut installed_linear = remote_installed_plugin("linear"); + installed_linear.id = "plugin_linear".to_string(); + manager.write_remote_installed_plugins_cache(vec![installed_linear]); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let disabled_tools = [ToolSuggestDisabledTool::plugin( "github@openai-curated-remote", diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 804721314..ff7b421fd 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -328,7 +328,27 @@ async fn verify_request_plugin_install_completed( }), DiscoverableTool::Plugin(plugin) => { if is_remote_plugin_install_suggestion(&plugin.id) { - return true; + let (_, accessible_connectors) = tokio::join!( + refresh_remote_installed_plugins_cache_after_install( + session, + turn, + auth, + plugin.id.as_str(), + ), + refresh_missing_requested_connectors( + session, + turn, + auth, + &plugin.app_connector_ids, + plugin.id.as_str(), + ) + ); + return accessible_connectors.is_some_and(|accessible_connectors| { + all_requested_connectors_picked_up( + &plugin.app_connector_ids, + &accessible_connectors, + ) + }); } session.reload_user_config_layer().await; @@ -351,6 +371,29 @@ async fn verify_request_plugin_install_completed( } } +async fn refresh_remote_installed_plugins_cache_after_install( + session: &crate::session::session::Session, + turn: &crate::session::turn_context::TurnContext, + auth: Option<&codex_login::CodexAuth>, + tool_id: &str, +) { + let plugins_manager = &session.services.plugins_manager; + let plugins_config = turn.config.plugins_config_input(); + if let Err(err) = plugins_manager + .build_and_cache_remote_installed_plugin_marketplaces( + &plugins_config, + auth, + &[REMOTE_GLOBAL_MARKETPLACE_NAME], + /*on_effective_plugins_changed*/ None, + ) + .await + { + warn!( + "failed to refresh remote installed plugins cache after plugin install request for {tool_id}: {err:#}" + ); + } +} + fn is_remote_plugin_install_suggestion(plugin_id: &str) -> bool { plugin_id .rsplit_once('@') diff --git a/codex-rs/core/tests/common/apps_test_server.rs b/codex-rs/core/tests/common/apps_test_server.rs index 8a671ea5b..0849087d0 100644 --- a/codex-rs/core/tests/common/apps_test_server.rs +++ b/codex-rs/core/tests/common/apps_test_server.rs @@ -7,6 +7,8 @@ use codex_login::CodexAuth; use codex_models_manager::bundled_models_response; use serde_json::Value; use serde_json::json; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; use wiremock::Mock; use wiremock::MockServer; use wiremock::Request; @@ -59,6 +61,13 @@ pub enum AppsTestToolLoading { Searchable, } +#[derive(Clone, Copy)] +enum AppsTestToolsListBehavior { + AlwaysAvailable, + AvailableAfterInitialList, + AlwaysUnavailable, +} + impl AppsTestServer { pub async fn mount(server: &MockServer) -> Result { Self::mount_with_connector_name(server, CONNECTOR_NAME).await @@ -73,6 +82,7 @@ impl AppsTestServer { CONNECTOR_DESCRIPTION.to_string(), /*searchable*/ true, /*include_app_only_tool*/ false, + AppsTestToolsListBehavior::AlwaysAvailable, ) .await; Ok(Self { @@ -92,6 +102,7 @@ impl AppsTestServer { CONNECTOR_DESCRIPTION.to_string(), /*searchable*/ false, /*include_app_only_tool*/ false, + AppsTestToolsListBehavior::AlwaysAvailable, ) .await; Ok(Self { @@ -111,6 +122,42 @@ impl AppsTestServer { CONNECTOR_DESCRIPTION.to_string(), matches!(tool_loading, AppsTestToolLoading::Searchable), /*include_app_only_tool*/ true, + AppsTestToolsListBehavior::AlwaysAvailable, + ) + .await; + Ok(Self { + chatgpt_base_url: server.uri(), + }) + } + + pub async fn mount_with_tools_available_after_initial_list( + server: &MockServer, + ) -> Result { + Self::mount_with_tools_list_behavior( + server, + AppsTestToolsListBehavior::AvailableAfterInitialList, + ) + .await + } + + pub async fn mount_without_tools(server: &MockServer) -> Result { + Self::mount_with_tools_list_behavior(server, AppsTestToolsListBehavior::AlwaysUnavailable) + .await + } + + async fn mount_with_tools_list_behavior( + server: &MockServer, + tools_list_behavior: AppsTestToolsListBehavior, + ) -> Result { + mount_oauth_metadata(server).await; + mount_connectors_directory(server).await; + mount_streamable_http_json_rpc( + server, + CONNECTOR_NAME.to_string(), + CONNECTOR_DESCRIPTION.to_string(), + /*searchable*/ false, + /*include_app_only_tool*/ false, + tools_list_behavior, ) .await; Ok(Self { @@ -264,6 +311,7 @@ async fn mount_streamable_http_json_rpc( connector_description: String, searchable: bool, include_app_only_tool: bool, + tools_list_behavior: AppsTestToolsListBehavior, ) { Mock::given(method("POST")) .and(path_regex("^/api/codex/apps/?$")) @@ -272,6 +320,8 @@ async fn mount_streamable_http_json_rpc( connector_description, searchable, include_app_only_tool, + tools_list_behavior, + tools_list_calls: AtomicUsize::new(0), }) .mount(server) .await; @@ -282,6 +332,8 @@ struct CodexAppsJsonRpcResponder { connector_description: String, searchable: bool, include_app_only_tool: bool, + tools_list_behavior: AppsTestToolsListBehavior, + tools_list_calls: AtomicUsize, } impl Respond for CodexAppsJsonRpcResponder { @@ -327,6 +379,12 @@ impl Respond for CodexAppsJsonRpcResponder { } "notifications/initialized" => ResponseTemplate::new(202), "tools/list" => { + let list_index = self.tools_list_calls.fetch_add(1, Ordering::SeqCst); + let tools_available = match self.tools_list_behavior { + AppsTestToolsListBehavior::AlwaysAvailable => true, + AppsTestToolsListBehavior::AvailableAfterInitialList => list_index > 0, + AppsTestToolsListBehavior::AlwaysUnavailable => false, + }; let id = body.get("id").cloned().unwrap_or(Value::Null); let mut response = json!({ "jsonrpc": "2.0", @@ -428,7 +486,15 @@ impl Respond for CodexAppsJsonRpcResponder { "nextCursor": null } }); - if self.searchable + if !tools_available + && let Some(tools) = response + .pointer_mut("/result/tools") + .and_then(Value::as_array_mut) + { + tools.clear(); + } + if tools_available + && self.searchable && let Some(tools) = response .pointer_mut("/result/tools") .and_then(Value::as_array_mut) @@ -455,7 +521,8 @@ impl Respond for CodexAppsJsonRpcResponder { })); } } - if self.include_app_only_tool + if tools_available + && self.include_app_only_tool && let Some(tools) = response .pointer_mut("/result/tools") .and_then(Value::as_array_mut) diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index f3da1bea4..f307fce05 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -11,6 +11,7 @@ use codex_login::CodexAuth; use codex_models_manager::bundled_models_response; use codex_protocol::approvals::ElicitationAction; use codex_protocol::approvals::ElicitationRequest; +use codex_protocol::approvals::ElicitationRequestEvent; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -38,6 +39,7 @@ use core_test_support::wait_for_event_match; use serde_json::Value; use serde_json::json; use wiremock::Mock; +use wiremock::MockGuard; use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; @@ -47,6 +49,11 @@ const TOOL_SEARCH_TOOL_NAME: &str = "tool_search"; const LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME: &str = "list_available_plugins_to_install"; const REQUEST_PLUGIN_INSTALL_TOOL_NAME: &str = "request_plugin_install"; const DISCOVERABLE_GMAIL_ID: &str = "connector_68df038e0ba48191908c8434991bbac2"; +const REMOTE_CALENDAR_PLUGIN_CONFIG_ID: &str = "calendar@openai-curated-remote"; +const REMOTE_CALENDAR_PLUGIN_ID: &str = "plugin_calendar"; +const CALENDAR_CONNECTOR_ID: &str = "calendar"; +const CALENDAR_NAMESPACE: &str = "mcp__codex_apps__calendar"; +const CALENDAR_CREATE_EVENT_TOOL: &str = "_create_event"; fn tool_names(body: &Value) -> Vec { body.get("tools") @@ -133,6 +140,134 @@ async fn build_test( builder.build(server).await } +async fn start_install_turn(test: &TestCodex, prompt: &str) -> Result { + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: prompt.to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: ThreadSettingsOverrides { + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(CollaborationMode { + mode: ModeKind::Default, + settings: Settings { + model: test.session_configured.model.clone(), + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + Ok(wait_for_event_match(&test.codex, |event| match event { + EventMsg::ElicitationRequest(request) => Some(request.clone()), + _ => None, + }) + .await) +} + +async fn resolve_install_elicitation( + test: &TestCodex, + elicitation: ElicitationRequestEvent, + decision: ElicitationAction, +) -> Result<()> { + test.codex + .submit(Op::ResolveElicitation { + server_name: elicitation.server_name, + request_id: elicitation.id, + decision, + content: None, + meta: None, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + Ok(()) +} + +async fn mount_remote_calendar_recommendation(server: &wiremock::MockServer) { + Mock::given(method("GET")) + .and(path("/ps/plugins/suggested")) + .and(query_param("scope", "GLOBAL")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "enabled": true, + "plugins": [{ + "id": REMOTE_CALENDAR_PLUGIN_ID, + "name": "calendar", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "release": { + "display_name": "Calendar", + "app_ids": [CALENDAR_CONNECTOR_ID] + } + }] + }))) + .expect(1) + .mount(server) + .await; +} + +fn remote_installed_plugins_response(plugins: Vec) -> ResponseTemplate { + ResponseTemplate::new(200).set_body_json(json!({ + "plugins": plugins, + "pagination": { + "next_page_token": null + } + })) +} + +async fn mount_empty_remote_installed_plugins(server: &wiremock::MockServer) -> MockGuard { + Mock::given(method("GET")) + .and(path("/ps/plugins/installed")) + .respond_with(remote_installed_plugins_response(Vec::new())) + .mount_as_scoped(server) + .await +} + +async fn mount_remote_calendar_installed_plugins(server: &wiremock::MockServer) { + Mock::given(method("GET")) + .and(path("/ps/plugins/installed")) + .and(query_param("scope", "GLOBAL")) + .respond_with(remote_installed_plugins_response(vec![json!({ + "id": REMOTE_CALENDAR_PLUGIN_ID, + "name": "calendar", + "scope": "GLOBAL", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { + "display_name": "Calendar", + "description": "Manage calendar events.", + "interface": {} + }, + "enabled": true + })])) + .with_priority(1) + .mount(server) + .await; + for scope in ["WORKSPACE", "USER"] { + Mock::given(method("GET")) + .and(path("/ps/plugins/installed")) + .and(query_param("scope", scope)) + .respond_with(remote_installed_plugins_response(Vec::new())) + .with_priority(1) + .mount(server) + .await; + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn explicit_false_preserves_legacy_workflow() -> Result<()> { skip_if_no_network!(Ok(())); @@ -278,6 +413,10 @@ async fn endpoint_recommendation_adds_install_identity_only_to_elicitation_metad { skip_if_no_network!(Ok(())); + run_remote_plugin_install_metadata_case().await +} + +async fn run_remote_plugin_install_metadata_case() -> Result<()> { const REMOTE_PLUGIN_ID: &str = "plugin_connector_github"; const APP_CONNECTOR_ID: &str = "connector_github"; @@ -325,40 +464,7 @@ async fn endpoint_recommendation_adds_install_identity_only_to_elicitation_metad ) .await; let test = build_test(&server, &apps_server).await?; - let (sandbox_policy, permission_profile) = - turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); - - test.codex - .submit(Op::UserInput { - items: vec![UserInput::Text { - text: "use GitHub".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - responsesapi_client_metadata: None, - additional_context: Default::default(), - thread_settings: ThreadSettingsOverrides { - approval_policy: Some(AskForApproval::Never), - sandbox_policy: Some(sandbox_policy), - permission_profile, - collaboration_mode: Some(CollaborationMode { - mode: ModeKind::Default, - settings: Settings { - model: test.session_configured.model.clone(), - reasoning_effort: None, - developer_instructions: None, - }, - }), - ..Default::default() - }, - }) - .await?; - - let elicitation = wait_for_event_match(&test.codex, |event| match event { - EventMsg::ElicitationRequest(request) => Some(request.clone()), - _ => None, - }) - .await; + let elicitation = start_install_turn(&test, "use GitHub").await?; let ElicitationRequest::Form { meta: Some(meta), .. } = &elicitation.request @@ -368,19 +474,7 @@ async fn endpoint_recommendation_adds_install_identity_only_to_elicitation_metad assert_eq!(meta["remote_plugin_id"], REMOTE_PLUGIN_ID); assert_eq!(meta["app_connector_ids"], json!([APP_CONNECTOR_ID])); - test.codex - .submit(Op::ResolveElicitation { - server_name: elicitation.server_name, - request_id: elicitation.id, - decision: ElicitationAction::Decline, - content: None, - meta: None, - }) - .await?; - wait_for_event(&test.codex, |event| { - matches!(event, EventMsg::TurnComplete(_)) - }) - .await; + resolve_install_elicitation(&test, elicitation, ElicitationAction::Decline).await?; let requests = mock.requests(); assert_eq!(requests.len(), 2); @@ -392,6 +486,104 @@ async fn endpoint_recommendation_adds_install_identity_only_to_elicitation_metad Ok(()) } +#[derive(Clone, Copy)] +enum RefreshedAppsTools { + Available, + Missing, +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_plugin_install_refreshes_plugin_and_apps_tool_caches() -> Result<()> { + skip_if_no_network!(Ok(())); + + run_remote_plugin_install_refresh_case(RefreshedAppsTools::Available).await?; + run_remote_plugin_install_refresh_case(RefreshedAppsTools::Missing).await +} + +async fn run_remote_plugin_install_refresh_case(refreshed_tools: RefreshedAppsTools) -> Result<()> { + let server = start_mock_server().await; + let apps_server = match refreshed_tools { + RefreshedAppsTools::Available => { + AppsTestServer::mount_with_tools_available_after_initial_list(&server).await? + } + RefreshedAppsTools::Missing => AppsTestServer::mount_without_tools(&server).await?, + }; + mount_remote_calendar_recommendation(&server).await; + let initial_remote_installed_plugins = mount_empty_remote_installed_plugins(&server).await; + + let install_call_id = "install-calendar"; + let suggest_reason = "Use Calendar for this request"; + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + install_call_id, + REQUEST_PLUGIN_INSTALL_TOOL_NAME, + &serde_json::to_string(&json!({ + "plugin_id": REMOTE_CALENDAR_PLUGIN_CONFIG_ID, + "suggest_reason": suggest_reason + }))?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + let test = build_test(&server, &apps_server).await?; + + let elicitation = start_install_turn(&test, "use Calendar").await?; + mount_remote_calendar_installed_plugins(&server).await; + drop(initial_remote_installed_plugins); + resolve_install_elicitation(&test, elicitation, ElicitationAction::Accept).await?; + + let requests = mock.requests(); + assert_eq!(requests.len(), 2); + assert!( + requests[0] + .tool_by_name(CALENDAR_NAMESPACE, CALENDAR_CREATE_EVENT_TOOL) + .is_none(), + "calendar tool should be absent before the remote install" + ); + let completed = matches!(refreshed_tools, RefreshedAppsTools::Available); + assert_eq!( + serde_json::from_str::( + &requests[1] + .function_call_output_text(install_call_id) + .expect("install tool output") + )?, + json!({ + "completed": completed, + "user_confirmed": true, + "tool_type": "plugin", + "action_type": "install", + "tool_id": REMOTE_CALENDAR_PLUGIN_CONFIG_ID, + "tool_name": "Calendar", + "suggest_reason": suggest_reason + }) + ); + assert_eq!( + requests[1] + .tool_by_name(CALENDAR_NAMESPACE, CALENDAR_CREATE_EVENT_TOOL) + .is_some(), + completed, + "the resumed router should reflect the refreshed Apps tools" + ); + assert!( + !tool_names(&requests[1].body_json()) + .iter() + .any(|name| name == REQUEST_PLUGIN_INSTALL_TOOL_NAME), + "the refreshed installed-plugin cache should filter the cached recommendation" + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn endpoint_mode_with_no_eligible_candidates_exposes_no_suggestion_tools() -> Result<()> { skip_if_no_network!(Ok(()));