diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 29582cb36e59..b5555ecc7cdd 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -6853,6 +6853,20 @@ "description": { "type": "string" }, + "disabledReason": { + "type": [ + "string", + "null" + ] + }, + "isEnabled": { + "default": true, + "type": "boolean" + }, + "isReadOnly": { + "default": false, + "type": "boolean" + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index e63580ba6afa..befa83c162b7 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -869,6 +869,20 @@ "description": { "type": "string" }, + "disabledReason": { + "type": [ + "string", + "null" + ] + }, + "isEnabled": { + "default": true, + "type": "boolean" + }, + "isReadOnly": { + "default": false, + "type": "boolean" + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/AppsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/AppsReadResponse.json index c4c7d48fa7b7..ef59c38899e9 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/AppsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/AppsReadResponse.json @@ -7,6 +7,20 @@ "description": { "type": "string" }, + "disabledReason": { + "type": [ + "string", + "null" + ] + }, + "isEnabled": { + "default": true, + "type": "boolean" + }, + "isReadOnly": { + "default": false, + "type": "boolean" + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolSummary.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolSummary.ts index 15677b0cce24..6ab5c16934ed 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolSummary.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolSummary.ts @@ -5,4 +5,4 @@ /** * EXPERIMENTAL - metadata returned by app/read. */ -export type AppToolSummary = { name: string, title: string | null, description: string, }; +export type AppToolSummary = { name: string, title: string | null, description: string, isEnabled: boolean, disabledReason: string | null, isReadOnly: boolean, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/apps.rs b/codex-rs/app-server-protocol/src/protocol/v2/apps.rs index 9cc69c2e899d..ea511183bdcc 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/apps.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/apps.rs @@ -190,6 +190,11 @@ pub struct AppToolSummary { pub name: String, pub title: Option, pub description: String, + #[serde(default = "default_enabled")] + pub is_enabled: bool, + pub disabled_reason: Option, + #[serde(default)] + pub is_read_only: bool, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 79ef62418686..4314f590dd81 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1990,7 +1990,10 @@ instead of failing the whole request. { "name": "search", "title": "Search", - "description": "Search the app." + "description": "Search the app.", + "isEnabled": true, + "disabledReason": null, + "isReadOnly": true } ] } @@ -2004,8 +2007,8 @@ account/workspace identity, then makes at most one `POST /ps/apps/batch` for mis expired ids. `includeTools` defaults to false and is forwarded as `include_tools`; a fresh metadata-only cache entry is refetched when tool summaries are requested. Backend or transport failures return an RPC error without replacing existing cache records. Its metadata shape can -include display-only public tool summaries and intentionally excludes runtime state, MCP tool -state, full actions, and model descriptions. +include display-only public tool summaries with enabled/read-only state and intentionally excludes +runtime state, MCP tool state, full actions, and model descriptions. Connected apps may override the thread's approval reviewer in `config.toml`. Use `apps._default.approvals_reviewer` to set the reviewer for all apps, and a diff --git a/codex-rs/app-server/src/app_info.rs b/codex-rs/app-server/src/app_info.rs index dd07182e760f..4752d5fb9324 100644 --- a/codex-rs/app-server/src/app_info.rs +++ b/codex-rs/app-server/src/app_info.rs @@ -89,11 +89,17 @@ pub(crate) fn connector_metadata_to_api(metadata: ConnectorMetadata) -> ApiConne name, title, description, + is_enabled, + disabled_reason, + is_read_only, } = tool; ApiAppToolSummary { name, title, description, + is_enabled, + disabled_reason, + is_read_only, } }) .collect() diff --git a/codex-rs/app-server/src/request_processors/apps_processor.rs b/codex-rs/app-server/src/request_processors/apps_processor.rs index 74b7db3a2f82..3dc17ae0c72f 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -477,7 +477,7 @@ fn record_legacy_apps_installed_duration(started_at: Instant, reload: bool) { ); } } -const APP_READ_MAX_IDS: usize = 100; +pub(super) const APP_READ_MAX_IDS: usize = 100; enum AppListLoadResult { Accessible(Result), diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index b0e54e39359c..c63a5531caa9 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1,3 +1,4 @@ +use super::apps_processor::APP_READ_MAX_IDS; use super::*; use crate::error_code::internal_error; use crate::error_code::invalid_request; @@ -1075,6 +1076,7 @@ impl PluginRequestProcessor { }; let app_summaries = load_plugin_app_summaries( &config, + auth.as_ref(), &outcome.plugin.apps, &outcome.plugin.app_category_by_id, ) @@ -1161,8 +1163,13 @@ impl PluginRequestProcessor { .as_ref() .map(plugin_app_category_by_id_from_value) .unwrap_or_default(); - let app_summaries = - load_plugin_app_summaries(&config, &plugin_apps, &app_category_by_id).await; + let app_summaries = load_plugin_app_summaries( + &config, + auth.as_ref(), + &plugin_apps, + &app_category_by_id, + ) + .await; remote_plugin_detail_to_info(remote_detail, app_summaries) } }; @@ -1504,14 +1511,12 @@ impl PluginRequestProcessor { } let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await; - let plugin_apps = - codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations); let apps_needing_auth = self .plugin_apps_needing_auth_for_install( &config, - auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), + auth.as_ref(), &result.plugin_id.as_key(), - &plugin_apps, + &plugin_app_declarations, ) .await; @@ -1697,35 +1702,16 @@ impl PluginRequestProcessor { .as_ref() .map(plugin_app_category_by_id_from_value) .unwrap_or_default(); - let all_connectors = connectors::list_cached_all_connectors(&config, &[]) + load_plugin_app_summaries(&config, auth.as_ref(), &plugin_apps, &app_category_by_id) .await - .unwrap_or_default(); - connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps) - .into_iter() - .map(|connector| { - let category = app_category_by_id - .get(&connector.id) - .cloned() - .or_else(|| connector.category()); - AppSummary { - category, - id: connector.id, - name: connector.name, - description: connector.description, - install_url: connector.install_url, - } - }) - .collect() } } else { let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await; - let plugin_apps = - codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations); self.plugin_apps_needing_auth_for_install( &config, - is_chatgpt_auth, + auth.as_ref(), &result.plugin_id.as_key(), - &plugin_apps, + &plugin_app_declarations, ) .await }; @@ -1775,17 +1761,31 @@ impl PluginRequestProcessor { async fn plugin_apps_needing_auth_for_install( &self, config: &Config, - is_chatgpt_auth: bool, + auth: Option<&CodexAuth>, plugin_id: &str, - plugin_apps: &[codex_plugin::AppConnectorId], + plugin_app_declarations: &[codex_plugin::AppDeclaration], ) -> Vec { - if plugin_apps.is_empty() || !config.features.apps_enabled_for_auth(is_chatgpt_auth) { + if plugin_app_declarations.is_empty() + || !config + .features + .apps_enabled_for_auth(auth.is_some_and(CodexAuth::is_chatgpt_auth)) + { return Vec::new(); } + let plugin_apps = + codex_plugin::app_connector_ids_from_declarations(plugin_app_declarations); + let app_category_by_id = plugin_app_declarations + .iter() + .filter_map(|app| { + app.category + .as_ref() + .map(|category| (app.connector_id.0.clone(), category.clone())) + }) + .collect(); let environment_manager = self.thread_manager.environment_manager(); - let (all_connectors_result, accessible_connectors_result) = tokio::join!( - connectors::list_all_connectors_with_options(config, /*force_refetch*/ false, &[]), + let (app_summaries, accessible_connectors_result) = tokio::join!( + load_plugin_app_summaries(config, auth, &plugin_apps, &app_category_by_id), connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager( config, /*force_refetch*/ true, @@ -1794,19 +1794,6 @@ impl PluginRequestProcessor { ), ); - let all_connectors = match all_connectors_result { - Ok(connectors) => connectors, - Err(err) => { - warn!( - plugin = plugin_id, - "failed to load app metadata after plugin install: {err:#}" - ); - connectors::list_cached_all_connectors(config, &[]) - .await - .unwrap_or_default() - } - }; - let all_connectors = connectors::connectors_for_plugin_apps(all_connectors, plugin_apps); let (accessible_connectors, codex_apps_ready) = match accessible_connectors_result { Ok(status) => (status.connectors, status.codex_apps_ready), Err(err) => { @@ -1827,14 +1814,17 @@ impl PluginRequestProcessor { plugin = plugin_id, "codex_apps MCP not ready after plugin install; skipping appsNeedingAuth check" ); + return Vec::new(); } - plugin_apps_needing_auth( - &all_connectors, - &accessible_connectors, - plugin_apps, - codex_apps_ready, - ) + let accessible_ids = accessible_connectors + .iter() + .map(|connector| connector.id.as_str()) + .collect::>(); + app_summaries + .into_iter() + .filter(|app| !accessible_ids.contains(app.id.as_str())) + .collect() } async fn start_plugin_mcp_oauth_logins( @@ -2078,43 +2068,66 @@ impl PluginRequestProcessor { async fn load_plugin_app_summaries( config: &Config, + auth: Option<&CodexAuth>, plugin_apps: &[codex_plugin::AppConnectorId], app_category_by_id: &HashMap, ) -> Vec { - if plugin_apps.is_empty() { - return Vec::new(); - } - - let connectors = match connectors::list_all_connectors_with_options( - config, - /*force_refetch*/ false, - &[], - ) - .await - { - Ok(connectors) => connectors, - Err(err) => { - warn!("failed to load app metadata for plugin/read: {err:#}"); - connectors::list_cached_all_connectors(config, &[]) - .await - .unwrap_or_default() + let mut seen_app_ids = HashSet::new(); + let app_ids = plugin_apps + .iter() + .map(|app| app.0.clone()) + .filter(|app_id| seen_app_ids.insert(app_id.clone())) + .collect::>(); + let mut metadata_by_id = HashMap::new(); + if let Some(auth) = auth.filter(|auth| { + config + .features + .apps_enabled_for_auth(auth.uses_codex_backend()) + }) { + metadata_by_id.extend( + codex_connectors::ConnectorMetadataStore::new( + config.chatgpt_base_url.clone(), + auth.get_account_id(), + auth.get_chatgpt_user_id(), + auth.is_workspace_account(), + ) + .fresh_records(&app_ids, /*include_tools*/ false), + ); + for app_ids in app_ids.chunks(APP_READ_MAX_IDS) { + match connectors::read_connector_metadata( + config, auth, app_ids, /*include_tools*/ false, + ) + .await + { + Ok(result) => metadata_by_id.extend( + result + .apps + .into_iter() + .map(|metadata| (metadata.id.clone(), metadata)), + ), + Err(err) => { + warn!("failed to load app metadata for plugin: {err:#}"); + break; + } + } } - }; - - let plugin_connectors = connectors::connectors_for_plugin_apps(connectors, plugin_apps); + } - plugin_connectors + app_ids .into_iter() - .map(|connector| { - let category = app_category_by_id - .get(&connector.id) - .cloned() - .or_else(|| connector.category()); + .map(|app_id| { + let (name, description) = metadata_by_id + .remove(&app_id) + .map(|metadata| (metadata.name, metadata.description)) + .unwrap_or_else(|| (app_id.clone(), None)); + let category = app_category_by_id.get(&app_id).cloned(); AppSummary { - id: connector.id, - name: connector.name, - description: connector.description, - install_url: connector.install_url, + install_url: Some(codex_connectors::metadata::connector_install_url( + &name, &app_id, + )), + id: app_id, + name, + description, category, } }) @@ -2128,45 +2141,6 @@ fn plugin_app_category_by_id_from_value(value: &serde_json::Value) -> HashMap Vec { - if !codex_apps_ready { - return Vec::new(); - } - - let accessible_ids = accessible_connectors - .iter() - .map(|connector| connector.id.as_str()) - .collect::>(); - let plugin_app_ids = plugin_apps - .iter() - .map(|connector_id| connector_id.0.as_str()) - .collect::>(); - - all_connectors - .iter() - .filter(|connector| { - plugin_app_ids.contains(connector.id.as_str()) - && !accessible_ids.contains(connector.id.as_str()) - }) - .cloned() - .map(|connector| { - let category = connector.category(); - AppSummary { - category, - id: connector.id, - name: connector.name, - description: connector.description, - install_url: connector.install_url, - } - }) - .collect() -} - fn remote_marketplace_to_info(marketplace: RemoteMarketplace) -> PluginMarketplaceEntry { PluginMarketplaceEntry { name: marketplace.name, diff --git a/codex-rs/app-server/tests/suite/v2/app_read.rs b/codex-rs/app-server/tests/suite/v2/app_read.rs index 57b9208738f5..d8564145cd65 100644 --- a/codex-rs/app-server/tests/suite/v2/app_read.rs +++ b/codex-rs/app-server/tests/suite/v2/app_read.rs @@ -34,6 +34,54 @@ use tokio::time::timeout; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); +#[test] +fn app_read_deserializes_legacy_tool_summaries() -> Result<()> { + let response: AppsReadResponse = serde_json::from_value(json!({ + "apps": [{ + "id": "alpha", + "name": "Alpha", + "description": null, + "iconUrl": null, + "iconUrlDark": null, + "distributionChannel": null, + "installUrl": null, + "pluginDisplayNames": [], + "toolSummaries": [{ + "name": "search", + "title": "Search", + "description": "Search Alpha", + }], + }], + "missingAppIds": [], + }))?; + + assert_eq!( + serde_json::to_value(response)?, + json!({ + "apps": [{ + "id": "alpha", + "name": "Alpha", + "description": null, + "iconUrl": null, + "iconUrlDark": null, + "distributionChannel": null, + "installUrl": null, + "pluginDisplayNames": [], + "toolSummaries": [{ + "name": "search", + "title": "Search", + "description": "Search Alpha", + "isEnabled": true, + "disabledReason": null, + "isReadOnly": false, + }], + }], + "missingAppIds": [], + }) + ); + Ok(()) +} + #[tokio::test] async fn app_read_deduplicates_orders_partial_misses_and_reuses_cached_metadata() -> Result<()> { let access_token = encode_id_token( @@ -450,6 +498,9 @@ fn metadata_json(id: &str, name: &str, icon_url: Option<&str>) -> Value { "name": format!("{id}_tool"), "title": format!("{name} Tool"), "description": format!("Use {name}"), + "isEnabled": false, + "disabledReason": "disabled_by_admin", + "isReadOnly": true, }], }) } @@ -473,6 +524,9 @@ fn app_response(id: &str, name: &str, icon_url: Option<&str>) -> Value { "name": format!("{id}_tool"), "title": format!("{name} Tool"), "description": format!("Use {name}"), + "is_enabled": false, + "disabled_reason": "disabled_by_admin", + "is_read_only": true, }], "branding": { "category": "PRODUCTIVITY", diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 3050ad9df766..1d7fdf25b6e8 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -20,6 +20,7 @@ use axum::http::StatusCode; use axum::http::Uri; use axum::http::header::AUTHORIZATION; use axum::routing::get; +use axum::routing::post; use codex_app_server_protocol::AppInfo; use codex_app_server_protocol::AppSummary; use codex_app_server_protocol::AppsListParams; @@ -323,6 +324,22 @@ async fn plugin_install_uses_remote_apps_needing_auth_response() -> Result<()> { .await; mount_empty_remote_installed_plugins(&server).await; mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await; + Mock::given(method("POST")) + .and(path("/backend-api/ps/apps/batch")) + .and(header("authorization", "Bearer chatgpt-token")) + .and(header("chatgpt-account-id", "account-123")) + .and(header("oai-product-sku", "codex")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "apps": [{ + "id": "alpha", + "name": "Alpha", + "description": "Alpha connector", + "icon_url": null, + "tools": null + }] + }))) + .mount(&server) + .await; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -341,13 +358,20 @@ async fn plugin_install_uses_remote_apps_needing_auth_response() -> Result<()> { auth_policy: PluginAuthPolicy::OnUse, apps_needing_auth: vec![AppSummary { id: "alpha".to_string(), - name: "alpha".to_string(), - description: None, + name: "Alpha".to_string(), + description: Some("Alpha connector".to_string()), install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), category: Some("Developer Tools".to_string()), }], } ); + wait_for_remote_plugin_request_count( + &server, + "POST", + "/backend-api/ps/apps/batch", + /*expected_count*/ 1, + ) + .await?; wait_for_remote_plugin_request_count( &server, "GET", @@ -1181,6 +1205,10 @@ async fn plugin_install_returns_apps_needing_auth() -> Result<()> { /*auth_policy*/ None, )?; write_plugin_source(repo_root.path(), "sample-plugin", &["alpha", "beta"])?; + std::fs::write( + repo_root.path().join("sample-plugin/.app.json"), + r#"{"apps":{"alpha":{"id":"alpha","category":"Communication"},"beta":{"id":"beta"}}}"#, + )?; let marketplace_path = AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; @@ -1190,6 +1218,7 @@ async fn plugin_install_returns_apps_needing_auth() -> Result<()> { .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; let directory_requests_before_install = server_control.directory_request_count(); + let batch_requests_before_install = server_control.batch_request_count(); let request_id = mcp .send_plugin_install_request(PluginInstallParams { @@ -1211,11 +1240,18 @@ async fn plugin_install_returns_apps_needing_auth() -> Result<()> { name: "Alpha".to_string(), description: Some("Alpha connector".to_string()), install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), - category: None, + category: Some("Communication".to_string()), }], } ); - assert!(server_control.directory_request_count() > directory_requests_before_install); + assert_eq!( + server_control.directory_request_count(), + directory_requests_before_install + ); + assert_eq!( + server_control.batch_request_count(), + batch_requests_before_install + 1 + ); server_handle.abort(); let _ = server_handle.await; @@ -1658,6 +1694,7 @@ async fn plugin_install_includes_formerly_disallowed_apps_needing_auth() -> Resu .await?; let directory_requests_before_install = warm_app_directory_cache(&mut mcp, &server_control, "Alpha").await?; + let batch_requests_before_install = server_control.batch_request_count(); let request_id = mcp .send_plugin_install_request(PluginInstallParams { @@ -1697,6 +1734,10 @@ async fn plugin_install_includes_formerly_disallowed_apps_needing_auth() -> Resu server_control.directory_request_count(), directory_requests_before_install ); + assert_eq!( + server_control.batch_request_count(), + batch_requests_before_install + 1 + ); server_handle.abort(); let _ = server_handle.await; @@ -1777,19 +1818,25 @@ async fn plugin_install_makes_bundled_mcp_servers_available_to_followup_requests #[derive(Clone)] struct AppsServerState { - response: Arc>, + connectors: Vec, directory_request_count: Arc, + batch_request_count: Arc, } #[derive(Clone)] struct AppsServerControl { directory_request_count: Arc, + batch_request_count: Arc, } impl AppsServerControl { fn directory_request_count(&self) -> usize { self.directory_request_count.load(Ordering::SeqCst) } + + fn batch_request_count(&self) -> usize { + self.batch_request_count.load(Ordering::SeqCst) + } } async fn warm_app_directory_cache( @@ -1852,14 +1899,15 @@ async fn start_apps_server( tools: Vec, ) -> Result<(String, JoinHandle<()>, AppsServerControl)> { let directory_request_count = Arc::new(AtomicUsize::new(0)); + let batch_request_count = Arc::new(AtomicUsize::new(0)); let state = Arc::new(AppsServerState { - response: Arc::new(StdMutex::new( - json!({ "apps": connectors, "next_token": null }), - )), + connectors, directory_request_count: directory_request_count.clone(), + batch_request_count: batch_request_count.clone(), }); let server_control = AppsServerControl { directory_request_count, + batch_request_count, }; let tools = Arc::new(StdMutex::new(tools)); @@ -1883,6 +1931,7 @@ async fn start_apps_server( "/connectors/directory/list_workspace", get(list_directory_connectors), ) + .route("/ps/apps/batch", post(batch_apps)) .with_state(state) .nest_service("/api/codex/ps/mcp", mcp_service); @@ -1917,12 +1966,58 @@ async fn list_directory_connectors( } else if !external_logos_ok { Err(StatusCode::BAD_REQUEST) } else { - let response = state - .response - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone(); - Ok(Json(response)) + Ok(Json( + json!({ "apps": &state.connectors, "next_token": null }), + )) + } +} + +async fn batch_apps( + State(state): State>, + headers: HeaderMap, + Json(body): Json, +) -> Result { + state.batch_request_count.fetch_add(1, Ordering::SeqCst); + + let bearer_ok = headers + .get(AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value == "Bearer chatgpt-token"); + let account_ok = headers + .get("chatgpt-account-id") + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value == "account-123"); + let product_sku_ok = headers + .get("oai-product-sku") + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value == "codex"); + + if !bearer_ok || !account_ok || !product_sku_ok { + Err(StatusCode::UNAUTHORIZED) + } else { + let app_ids = body + .get("app_ids") + .and_then(serde_json::Value::as_array) + .ok_or(StatusCode::BAD_REQUEST)?; + let apps = state + .connectors + .iter() + .filter(|connector| { + app_ids + .iter() + .any(|app_id| app_id.as_str() == Some(connector.id.as_str())) + }) + .map(|connector| { + json!({ + "id": connector.id, + "name": connector.name, + "description": connector.description, + "icon_url": connector.logo_url, + "tools": null + }) + }) + .collect::>(); + Ok(Json(json!({ "apps": apps }))) } } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_read.rs b/codex-rs/app-server/tests/suite/v2/plugin_read.rs index 40825d49c22c..1f2f381ae10f 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_read.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_read.rs @@ -1,5 +1,4 @@ use std::sync::Arc; -use std::sync::Mutex as StdMutex; use std::time::Duration; use anyhow::Result; @@ -12,13 +11,14 @@ use axum::Router; use axum::extract::State; use axum::http::HeaderMap; use axum::http::StatusCode; -use axum::http::Uri; use axum::http::header::AUTHORIZATION; -use axum::routing::get; +use axum::routing::post; use codex_app_server_protocol::AppInfo; use codex_app_server_protocol::AppMetadata; use codex_app_server_protocol::AppTemplateSummary; use codex_app_server_protocol::AppTemplateUnavailableReason; +use codex_app_server_protocol::AppsReadParams; +use codex_app_server_protocol::AppsReadResponse; use codex_app_server_protocol::HookEventName; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; @@ -50,6 +50,7 @@ use tokio::time::timeout; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; +use wiremock::matchers::body_json; use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; @@ -240,32 +241,19 @@ apps = true .respond_with(ResponseTemplate::new(200).set_body_string(installed_body)) .mount(&server) .await; - Mock::given(method("GET")) - .and(path("/backend-api/connectors/directory/list")) - .and(query_param("external_logos", "true")) + Mock::given(method("POST")) + .and(path("/backend-api/ps/apps/batch")) .and(header("authorization", "Bearer chatgpt-token")) .and(header("chatgpt-account-id", "account-123")) + .and(header("oai-product-sku", "codex")) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "apps": [ - AppInfo { - id: "example-app".to_string(), - name: "Example App".to_string(), - description: Some("Example app connector".to_string()), - logo_url: Some("https://example.com/example.png".to_string()), - logo_url_dark: None, - icon_assets: None, - icon_dark_assets: None, - distribution_channel: Some("featured".to_string()), - branding: None, - app_metadata: None, - labels: None, - install_url: None, - is_accessible: false, - is_enabled: true, - plugin_display_names: Vec::new(), - } - ], - "next_token": null + "apps": [{ + "id": "example-app", + "name": "Example App", + "description": "Example app connector", + "icon_url": "https://example.com/example.png", + "tools": null + }] }))) .mount(&server) .await; @@ -1656,41 +1644,16 @@ enabled = false } #[tokio::test] -async fn plugin_read_returns_app_metadata_category() -> Result<()> { - let connectors = vec![ - AppInfo { - id: "alpha".to_string(), - name: "Alpha".to_string(), - description: Some("Alpha connector".to_string()), - logo_url: Some("https://example.com/alpha.png".to_string()), - logo_url_dark: None, - icon_assets: None, - icon_dark_assets: None, - distribution_channel: Some("featured".to_string()), - branding: None, - app_metadata: Some(AppMetadata { - review: None, - categories: Some(vec!["Productivity".to_string()]), - sub_categories: None, - seo_description: None, - screenshots: None, - developer: None, - version: None, - version_id: None, - version_notes: None, - first_party_requires_install: None, - show_in_composer_when_unlinked: None, - }), - labels: None, - install_url: None, - is_accessible: false, - is_enabled: true, - plugin_display_names: Vec::new(), - }, - AppInfo { - id: "beta".to_string(), - name: "Beta".to_string(), - description: Some("Beta connector".to_string()), +async fn plugin_read_batches_large_app_metadata_requests() -> Result<()> { + let app_ids = (0..101) + .map(|index| format!("app-{index:03}")) + .collect::>(); + let connectors = app_ids + .iter() + .map(|app_id| AppInfo { + id: app_id.clone(), + name: format!("App {app_id}"), + description: Some(format!("{app_id} connector")), logo_url: None, logo_url_dark: None, icon_assets: None, @@ -1703,8 +1666,8 @@ async fn plugin_read_returns_app_metadata_category() -> Result<()> { is_accessible: false, is_enabled: true, plugin_display_names: Vec::new(), - }, - ]; + }) + .collect::>(); let (server_url, server_handle) = start_apps_server(connectors).await?; let codex_home = TempDir::new()?; @@ -1725,15 +1688,20 @@ async fn plugin_read_returns_app_metadata_category() -> Result<()> { "sample-plugin", "./sample-plugin", )?; - write_plugin_source(repo_root.path(), "sample-plugin", &["alpha", "beta"])?; + write_plugin_source( + repo_root.path(), + "sample-plugin", + &app_ids.iter().map(String::as_str).collect::>(), + )?; let marketplace_path = AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() - .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .build() .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; let request_id = mcp .send_plugin_read_request(PluginReadParams { @@ -1742,18 +1710,23 @@ async fn plugin_read_returns_app_metadata_category() -> Result<()> { plugin_name: "sample-plugin".to_string(), }) .await?; - - let response: PluginReadResponse = - timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginReadResponse = to_response(response)?; + let mut expected_app_ids = app_ids.iter().map(String::as_str).collect::>(); + expected_app_ids.sort_unstable(); assert_eq!( response .plugin .apps .iter() - .map(|app| (app.id.as_str(), app.category.as_deref())) + .map(|app| app.id.as_str()) .collect::>(), - vec![("alpha", Some("Productivity")), ("beta", None)] + expected_app_ids ); server_handle.abort(); @@ -1761,6 +1734,118 @@ async fn plugin_read_returns_app_metadata_category() -> Result<()> { Ok(()) } +#[tokio::test] +async fn plugin_read_stops_batching_after_app_metadata_failure() -> Result<()> { + let app_ids = (0..101) + .map(|index| format!("app-{index:03}")) + .collect::>(); + let server = MockServer::start().await; + // Warm one app in the failing chunk and one in the skipped chunk, then fail exactly one refresh. + Mock::given(method("POST")) + .and(path("/ps/apps/batch")) + .and(body_json(json!({ + "app_ids": ["app-000", "app-100"], + "include_tools": false, + }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "apps": [ + {"id": "app-000", "name": "Cached first", "description": "First cached app", "tools": null}, + {"id": "app-100", "name": "Cached last", "description": "Last cached app", "tools": null}, + ] + }))) + .expect(1) + .with_priority(/*p*/ 1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/ps/apps/batch")) + .respond_with(ResponseTemplate::new(503)) + .expect(1) + .with_priority(/*p*/ 2) + .mount(&server) + .await; + + let codex_home = TempDir::new()?; + write_connectors_config(codex_home.path(), &server.uri())?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + )?; + write_plugin_source( + repo_root.path(), + "sample-plugin", + &app_ids.iter().map(String::as_str).collect::>(), + )?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_apps_read_request(AppsReadParams { + app_ids: vec!["app-000".to_string(), "app-100".to_string()], + include_tools: false, + }) + .await?; + let _: AppsReadResponse = timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + + let request_id = mcp + .send_plugin_read_request(PluginReadParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginReadResponse = to_response(response)?; + let mut expected_apps = app_ids + .iter() + .map(|app_id| match app_id.as_str() { + "app-000" => ("app-000", "Cached first", Some("First cached app")), + "app-100" => ("app-100", "Cached last", Some("Last cached app")), + app_id => (app_id, app_id, None), + }) + .collect::>(); + expected_apps.sort_unstable(); + + assert_eq!( + response + .plugin + .apps + .iter() + .map(|app| ( + app.id.as_str(), + app.name.as_str(), + app.description.as_deref() + )) + .collect::>(), + expected_apps + ); + + Ok(()) +} + #[tokio::test] async fn plugin_read_hides_apps_for_api_key_auth() -> Result<()> { let connectors = vec![AppInfo { @@ -2117,24 +2202,16 @@ plugins = true #[derive(Clone)] struct AppsServerState { - response: Arc>, + connectors: Vec, } async fn start_apps_server(connectors: Vec) -> Result<(String, JoinHandle<()>)> { - let state = Arc::new(AppsServerState { - response: Arc::new(StdMutex::new( - json!({ "apps": connectors, "next_token": null }), - )), - }); + let state = Arc::new(AppsServerState { connectors }); let listener = TcpListener::bind("127.0.0.1:0").await?; let addr = listener.local_addr()?; let router = Router::new() - .route("/connectors/directory/list", get(list_directory_connectors)) - .route( - "/connectors/directory/list_workspace", - get(list_directory_connectors), - ) + .route("/ps/apps/batch", post(batch_apps)) .with_state(state); let handle = tokio::spawn(async move { @@ -2144,10 +2221,10 @@ async fn start_apps_server(connectors: Vec) -> Result<(String, JoinHand Ok((format!("http://{addr}"), handle)) } -async fn list_directory_connectors( +async fn batch_apps( State(state): State>, headers: HeaderMap, - uri: Uri, + Json(body): Json, ) -> Result { let bearer_ok = headers .get(AUTHORIZATION) @@ -2157,21 +2234,40 @@ async fn list_directory_connectors( .get("chatgpt-account-id") .and_then(|value| value.to_str().ok()) .is_some_and(|value| value == "account-123"); - let external_logos_ok = uri - .query() - .is_some_and(|query| query.split('&').any(|pair| pair == "external_logos=true")); + let product_sku_ok = headers + .get("oai-product-sku") + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value == "codex"); - if !bearer_ok || !account_ok { + if !bearer_ok || !account_ok || !product_sku_ok { Err(StatusCode::UNAUTHORIZED) - } else if !external_logos_ok { - Err(StatusCode::BAD_REQUEST) } else { - let response = state - .response - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone(); - Ok(Json(response)) + let app_ids = body + .get("app_ids") + .and_then(serde_json::Value::as_array) + .ok_or(StatusCode::BAD_REQUEST)?; + if app_ids.len() > 100 { + return Err(StatusCode::BAD_REQUEST); + } + let apps = state + .connectors + .iter() + .filter(|connector| { + app_ids + .iter() + .any(|app_id| app_id.as_str() == Some(connector.id.as_str())) + }) + .map(|connector| { + json!({ + "id": connector.id, + "name": connector.name, + "description": connector.description, + "icon_url": connector.logo_url, + "tools": null + }) + }) + .collect::>(); + Ok(Json(json!({ "apps": apps }))) } } diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 5d111b9e8045..538a99543a71 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -246,6 +246,12 @@ struct BatchAppToolSummary { name: String, title: Option, description: String, + #[serde(default)] + is_enabled: Option, + #[serde(default)] + disabled_reason: Option, + #[serde(default)] + is_read_only: bool, } fn batch_app_to_metadata(app: BatchApp) -> ConnectorMetadata { @@ -273,11 +279,17 @@ fn batch_app_to_metadata(app: BatchApp) -> ConnectorMetadata { name, title, description, + is_enabled, + disabled_reason, + is_read_only, } = tool; ConnectorToolSummary { name, title, description, + is_enabled: is_enabled.unwrap_or(true), + disabled_reason, + is_read_only, } }) .collect() @@ -368,7 +380,11 @@ mod tests { "name": "Alpha", "description": "Alpha description", "icon_url": null, - "tools": null, + "tools": [{ + "name": "search", + "title": "Search", + "description": "Search Alpha", + }], })) .expect("valid legacy batch app"); @@ -381,7 +397,14 @@ mod tests { icon_url: None, icon_url_dark: None, distribution_channel: None, - tool_summaries: None, + tool_summaries: Some(vec![ConnectorToolSummary { + name: "search".to_string(), + title: Some("Search".to_string()), + description: "Search Alpha".to_string(), + is_enabled: true, + disabled_reason: None, + is_read_only: false, + }]), } ); } diff --git a/codex-rs/connectors/src/metadata_store.rs b/codex-rs/connectors/src/metadata_store.rs index af3fc827d06b..c8a6449ac559 100644 --- a/codex-rs/connectors/src/metadata_store.rs +++ b/codex-rs/connectors/src/metadata_store.rs @@ -11,13 +11,16 @@ pub struct ConnectorToolSummary { pub name: String, pub title: Option, pub description: String, + pub is_enabled: bool, + pub disabled_reason: Option, + pub is_read_only: bool, } /// Metadata returned by the app batch-read API. /// /// This intentionally excludes connector runtime state, full actions, and model descriptions. -/// Tool summaries contain display text only, and icon URLs are already projected as public URLs by -/// the backend. +/// Tool summaries contain display text and enabled/read-only state only, and icon URLs are already +/// projected as public URLs by the backend. #[derive(Debug, Clone, PartialEq)] pub struct ConnectorMetadata { pub id: String, @@ -116,6 +119,9 @@ impl ConnectorMetadataStore { } } +// `apps_mcp_product_sku` affects which tools the batch API returns, but is intentionally omitted +// from this key because we assume an app-server does not change its product SKU after launch. +// If that assumption changes, the SKU must be included in the cache scope. #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct ConnectorMetadataStoreScope { backend_base_url: String, diff --git a/codex-rs/connectors/src/metadata_store_tests.rs b/codex-rs/connectors/src/metadata_store_tests.rs index d8c33d4574ce..77dbdd208cb5 100644 --- a/codex-rs/connectors/src/metadata_store_tests.rs +++ b/codex-rs/connectors/src/metadata_store_tests.rs @@ -90,6 +90,9 @@ fn tool_inclusive_reads_require_cached_tool_summaries() { name: "search".to_string(), title: Some("Search".to_string()), description: "Search the app".to_string(), + is_enabled: true, + disabled_reason: None, + is_read_only: true, }]); let ids = vec![ "metadata-only".to_string(), @@ -133,6 +136,9 @@ fn metadata_only_commit_does_not_replace_fresh_tool_summaries() { name: "search".to_string(), title: Some("Search".to_string()), description: "Search the app".to_string(), + is_enabled: true, + disabled_reason: None, + is_read_only: true, }]); let ids = vec!["with-tools".to_string()];