From 4c7a1d800fe83cb2c96bdaed3162c614feddb663 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Tue, 10 Mar 2026 00:13:15 -0700 Subject: [PATCH] update --- codex-rs/core/src/apps/render.rs | 2 +- codex-rs/core/src/codex.rs | 26 +- codex-rs/core/src/connectors.rs | 77 +++++- codex-rs/core/src/mcp_tool_call.rs | 4 +- .../src/tools/handlers/search_tool_bm25.rs | 244 ++++++++++++++---- .../core/src/tools/handlers/tool_suggest.rs | 162 ++++++++++-- codex-rs/core/src/tools/spec.rs | 66 +++-- codex-rs/core/src/unified_exec/mod.rs | 2 +- .../templates/search_tool/tool_description.md | 23 +- codex-rs/core/tests/suite/search_tool.rs | 44 +++- codex-rs/tui/src/app.rs | 1 + codex-rs/tui/src/bottom_pane/app_link_view.rs | 205 ++++++++++++++- codex-rs/tui/src/bottom_pane/mod.rs | 2 +- ...ests__app_link_view_enable_suggestion.snap | 30 +++ codex-rs/tui/src/chatwidget.rs | 3 +- 15 files changed, 752 insertions(+), 139 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__app_link_view__tests__app_link_view_enable_suggestion.snap diff --git a/codex-rs/core/src/apps/render.rs b/codex-rs/core/src/apps/render.rs index 41aed96de3..7f706f737e 100644 --- a/codex-rs/core/src/apps/render.rs +++ b/codex-rs/core/src/apps/render.rs @@ -2,6 +2,6 @@ use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; pub(crate) fn render_apps_section() -> String { format!( - "## Apps\nApps are mentioned in the prompt in the format `[$app-name](app://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, its tools may already be available, or the app may need to be installed first.\nUse `search_tool_bm25` with `mode: \"available\"` to search for installed tools, before trying to call hidden app tools. If the user clearly wants a specific app and it is not available, use `search_tool_bm25` with `mode: \"installable\"`, then call `tool_suggest` to prompt the user to install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned." + "## Apps\nApps are mentioned in the prompt in the format `[$app-name](app://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either already provided in `{CODEX_APPS_MCP_SERVER_NAME}`, or do not exist because the user did not install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned." ) } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 842b256fdf..5e4774653a 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -5984,7 +5984,9 @@ fn filter_connectors_for_input( ) -> Vec { let connectors: Vec = connectors .iter() - .filter(|connector| connector.is_enabled) + .filter(|connector| { + connector.is_enabled || explicitly_enabled_connectors.contains(&connector.id) + }) .cloned() .collect::>(); if connectors.is_empty() { @@ -6056,6 +6058,7 @@ fn filter_codex_apps_mcp_tools( mcp_tools: &HashMap, connectors: &[connectors::AppInfo], config: &Config, + enabled_connector_overrides: &HashSet, ) -> HashMap { let allowed: HashSet<&str> = connectors .iter() @@ -6071,7 +6074,12 @@ fn filter_codex_apps_mcp_tools( let Some(connector_id) = codex_apps_connector_id(tool) else { return false; }; - allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool) + allowed.contains(connector_id) + && connectors::codex_app_tool_is_enabled_with_enabled_connector_overrides( + config, + tool, + enabled_connector_overrides, + ) }) .map(|(name, tool)| (name.clone(), tool.clone())) .collect() @@ -6266,7 +6274,12 @@ async fn built_tools( // Keep the connector-grouped app view around for the router even though // app tools only become prompt-visible after explicit selection/discovery. let app_tools = connectors.as_ref().map(|connectors| { - filter_codex_apps_mcp_tools(&mcp_tools, connectors, &turn_context.config) + filter_codex_apps_mcp_tools( + &mcp_tools, + connectors, + &turn_context.config, + &effective_explicitly_enabled_connectors, + ) }); if let Some(connectors) = connectors.as_ref() { @@ -6292,8 +6305,11 @@ async fn built_tools( explicitly_enabled.as_ref(), )); - mcp_tools = - connectors::filter_codex_apps_tools_by_policy(selected_mcp_tools, &turn_context.config); + mcp_tools = connectors::filter_codex_apps_tools_by_policy_with_enabled_connector_overrides( + selected_mcp_tools, + &turn_context.config, + &effective_explicitly_enabled_connectors, + ); } Ok(Arc::new(ToolRouter::from_config( diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index dee7419fb9..8c032314ee 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -652,46 +652,57 @@ pub fn with_app_plugin_sources( connectors } -pub(crate) fn app_tool_policy( +pub(crate) fn app_tool_policy_with_enabled_connector_overrides( config: &Config, connector_id: Option<&str>, tool_name: &str, tool_title: Option<&str>, annotations: Option<&ToolAnnotations>, + enabled_connector_overrides: &HashSet, ) -> AppToolPolicy { let apps_config = read_apps_config(config); - app_tool_policy_from_apps_config( + app_tool_policy_from_apps_config_with_enabled_connector_overrides( apps_config.as_ref(), connector_id, tool_name, tool_title, annotations, + enabled_connector_overrides, ) } -pub(crate) fn codex_app_tool_is_enabled( +pub(crate) fn codex_app_tool_is_enabled_with_enabled_connector_overrides( config: &Config, tool_info: &crate::mcp_connection_manager::ToolInfo, + enabled_connector_overrides: &HashSet, ) -> bool { if tool_info.server_name != CODEX_APPS_MCP_SERVER_NAME { return true; } - app_tool_policy( + app_tool_policy_with_enabled_connector_overrides( config, tool_info.connector_id.as_deref(), &tool_info.tool_name, tool_info.tool.title.as_deref(), tool_info.tool.annotations.as_ref(), + enabled_connector_overrides, ) .enabled } -pub(crate) fn filter_codex_apps_tools_by_policy( +pub(crate) fn filter_codex_apps_tools_by_policy_with_enabled_connector_overrides( mut mcp_tools: HashMap, config: &Config, + enabled_connector_overrides: &HashSet, ) -> HashMap { - mcp_tools.retain(|_, tool_info| codex_app_tool_is_enabled(config, tool_info)); + mcp_tools.retain(|_, tool_info| { + codex_app_tool_is_enabled_with_enabled_connector_overrides( + config, + tool_info, + enabled_connector_overrides, + ) + }); mcp_tools } @@ -756,12 +767,31 @@ fn app_is_enabled(apps_config: &AppsConfigToml, connector_id: Option<&str>) -> b .unwrap_or(default_enabled) } +#[cfg(test)] fn app_tool_policy_from_apps_config( apps_config: Option<&AppsConfigToml>, connector_id: Option<&str>, tool_name: &str, tool_title: Option<&str>, annotations: Option<&ToolAnnotations>, +) -> AppToolPolicy { + app_tool_policy_from_apps_config_with_enabled_connector_overrides( + apps_config, + connector_id, + tool_name, + tool_title, + annotations, + &HashSet::new(), + ) +} + +fn app_tool_policy_from_apps_config_with_enabled_connector_overrides( + apps_config: Option<&AppsConfigToml>, + connector_id: Option<&str>, + tool_name: &str, + tool_title: Option<&str>, + annotations: Option<&ToolAnnotations>, + enabled_connector_overrides: &HashSet, ) -> AppToolPolicy { let Some(apps_config) = apps_config else { return AppToolPolicy::default(); @@ -780,7 +810,10 @@ fn app_tool_policy_from_apps_config( .or_else(|| app.and_then(|app| app.default_tools_approval_mode)) .unwrap_or(AppToolApproval::Auto); - if !app_is_enabled(apps_config, connector_id) { + let connector_is_temporarily_enabled = + connector_id.is_some_and(|connector_id| enabled_connector_overrides.contains(connector_id)); + + if !connector_is_temporarily_enabled && !app_is_enabled(apps_config, connector_id) { return AppToolPolicy { enabled: false, approval, @@ -1523,6 +1556,36 @@ mod tests { ); } + #[test] + fn app_tool_policy_allows_temporarily_enabled_connector_when_default_is_disabled() { + let apps_config = AppsConfigToml { + default: Some(AppsDefaultConfig { + enabled: false, + destructive_enabled: true, + open_world_enabled: true, + }), + apps: HashMap::new(), + }; + let enabled_connector_overrides = HashSet::from(["calendar".to_string()]); + + let policy = app_tool_policy_from_apps_config_with_enabled_connector_overrides( + Some(&apps_config), + Some("calendar"), + "events/list", + None, + Some(&annotations(None, None)), + &enabled_connector_overrides, + ); + + assert_eq!( + policy, + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Auto, + } + ); + } + #[test] fn app_tool_policy_allows_per_app_enable_when_default_is_disabled() { let apps_config = AppsConfigToml { diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 29d1de5b6d..7631531e67 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -87,8 +87,9 @@ pub(crate) async fn handle_mcp_tool_call( let metadata = lookup_mcp_tool_metadata(sess.as_ref(), turn_context.as_ref(), &server, &tool_name).await; + let enabled_connector_overrides = sess.get_connector_selection().await; let app_tool_policy = if server == CODEX_APPS_MCP_SERVER_NAME { - connectors::app_tool_policy( + connectors::app_tool_policy_with_enabled_connector_overrides( &turn_context.config, metadata .as_ref() @@ -100,6 +101,7 @@ pub(crate) async fn handle_mcp_tool_call( metadata .as_ref() .and_then(|metadata| metadata.annotations.as_ref()), + &enabled_connector_overrides, ) } else { connectors::AppToolPolicy::default() diff --git a/codex-rs/core/src/tools/handlers/search_tool_bm25.rs b/codex-rs/core/src/tools/handlers/search_tool_bm25.rs index e8eec19851..884aa4be60 100644 --- a/codex-rs/core/src/tools/handlers/search_tool_bm25.rs +++ b/codex-rs/core/src/tools/handlers/search_tool_bm25.rs @@ -17,6 +17,8 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::handlers::parse_arguments; +use crate::tools::handlers::tool_suggest::ToolSuggestionToolType; +use crate::tools::handlers::tool_suggest::ToolSuggestionType; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -33,8 +35,8 @@ fn default_limit() -> usize { #[serde(rename_all = "lowercase")] pub(crate) enum SearchToolMode { #[default] - Available, - Installable, + Enabled, + Discoverable, } #[derive(Deserialize)] @@ -87,16 +89,18 @@ struct ConnectorEntry { connector_id: String, connector_name: String, connector_description: Option, + suggestion_type: ToolSuggestionType, search_text: String, } impl ConnectorEntry { - fn new(connector: AppInfo) -> Self { + fn new(connector: AppInfo, suggestion_type: ToolSuggestionType) -> Self { let search_text = build_connector_search_text(&connector); Self { connector_id: connector.id, connector_name: connector.name, connector_description: connector.description, + suggestion_type, search_text, } } @@ -142,7 +146,7 @@ impl ToolHandler for SearchToolBm25Handler { } let content = match args.mode { - SearchToolMode::Available => { + SearchToolMode::Enabled => { let mcp_tools = session .services .mcp_connection_manager @@ -150,14 +154,23 @@ impl ToolHandler for SearchToolBm25Handler { .await .list_all_tools() .await; + let enabled_connector_overrides = session.get_connector_selection().await; let connectors = connectors::with_app_enabled_state( connectors::accessible_connectors_from_mcp_tools(&mcp_tools), &turn.config, ); - let mcp_tools = filter_codex_apps_mcp_tools(mcp_tools, &connectors); + let mcp_tools = filter_codex_apps_mcp_tools( + mcp_tools, + &connectors, + &enabled_connector_overrides, + ); let mcp_tools = - connectors::filter_codex_apps_tools_by_policy(mcp_tools, &turn.config); + connectors::filter_codex_apps_tools_by_policy_with_enabled_connector_overrides( + mcp_tools, + &turn.config, + &enabled_connector_overrides, + ); let mut entries: Vec = mcp_tools .into_iter() @@ -170,7 +183,7 @@ impl ToolHandler for SearchToolBm25Handler { session.get_mcp_tool_selection().await.unwrap_or_default(); json!({ "query": query, - "mode": SearchToolMode::Available, + "mode": SearchToolMode::Enabled, "total_tools": 0, "active_selected_tools": active_selected_tools, "tools": [], @@ -210,7 +223,7 @@ impl ToolHandler for SearchToolBm25Handler { json!({ "query": query, - "mode": SearchToolMode::Available, + "mode": SearchToolMode::Enabled, "total_tools": entries.len(), "active_selected_tools": active_selected_tools, "tools": result_payloads, @@ -218,67 +231,67 @@ impl ToolHandler for SearchToolBm25Handler { .to_string() } } - SearchToolMode::Installable => { + SearchToolMode::Discoverable => { let all_connectors = match connectors::list_cached_connectors(&turn.config).await { Some(connectors) => connectors, None => connectors::list_connectors(&turn.config) .await .map_err(|err| { FunctionCallError::RespondToModel(format!( - "failed to load installable apps: {err}" + "failed to load discoverable apps: {err}" )) })?, }; let active_selected_tools = session.get_mcp_tool_selection().await.unwrap_or_default(); - let mut entries: Vec = all_connectors - .into_iter() - .filter(|connector| !connector.is_accessible && connector.is_enabled) - .map(ConnectorEntry::new) - .collect(); - entries.sort_by(|a, b| { - a.connector_name - .cmp(&b.connector_name) - .then_with(|| a.connector_id.cmp(&b.connector_id)) - }); + let (installable_entries, disabled_entries) = + discoverable_connector_entries(all_connectors); + let total_connectors = installable_entries.len() + disabled_entries.len(); - if entries.is_empty() { + if total_connectors == 0 { json!({ "query": query, - "mode": SearchToolMode::Installable, + "mode": SearchToolMode::Discoverable, "total_connectors": 0, "active_selected_tools": active_selected_tools, "connectors": [], }) .to_string() } else { - let documents: Vec> = entries - .iter() - .enumerate() - .map(|(idx, entry)| Document::new(idx, entry.search_text.clone())) - .collect(); - let search_engine = - SearchEngineBuilder::::with_documents(Language::English, documents) - .build(); - let results = search_engine.search(query, args.limit); - let mut result_payloads = Vec::new(); - for result in results { - let Some(entry) = entries.get(result.document.id) else { - continue; - }; + let installable_results = + search_connector_entries(&installable_entries, query, args.limit); + for (entry, score) in installable_results { result_payloads.push(json!({ "connector_id": entry.connector_id.clone(), "connector_name": entry.connector_name.clone(), "connector_description": entry.connector_description.clone(), - "score": result.score, + "tool_type": ToolSuggestionToolType::Connector, + "suggestion_type": entry.suggestion_type, + "score": score, })); } + let remaining = args.limit.saturating_sub(result_payloads.len()); + if remaining > 0 { + for (entry, score) in + search_connector_entries(&disabled_entries, query, remaining) + { + result_payloads.push(json!({ + "connector_id": entry.connector_id.clone(), + "connector_name": entry.connector_name.clone(), + "connector_description": entry.connector_description.clone(), + "tool_type": ToolSuggestionToolType::Connector, + "suggestion_type": entry.suggestion_type, + "score": score, + })); + } + } + json!({ "query": query, - "mode": SearchToolMode::Installable, - "total_connectors": entries.len(), + "mode": SearchToolMode::Discoverable, + "total_connectors": total_connectors, "active_selected_tools": active_selected_tools, "connectors": result_payloads, }) @@ -294,10 +307,13 @@ impl ToolHandler for SearchToolBm25Handler { fn filter_codex_apps_mcp_tools( mut mcp_tools: HashMap, connectors: &[AppInfo], + enabled_connector_overrides: &HashSet, ) -> HashMap { let enabled_connectors: HashSet<&str> = connectors .iter() - .filter(|connector| connector.is_enabled) + .filter(|connector| { + connector.is_enabled || enabled_connector_overrides.contains(connector.id.as_str()) + }) .map(|connector| connector.id.as_str()) .collect(); @@ -313,6 +329,62 @@ fn filter_codex_apps_mcp_tools( mcp_tools } +fn discoverable_connector_entries( + connectors: Vec, +) -> (Vec, Vec) { + let mut installable_entries = Vec::new(); + let mut disabled_entries = Vec::new(); + + for connector in connectors { + if !connector.is_accessible { + installable_entries.push(ConnectorEntry::new(connector, ToolSuggestionType::Install)); + } else if !connector.is_enabled { + disabled_entries.push(ConnectorEntry::new(connector, ToolSuggestionType::Enable)); + } + } + + installable_entries.sort_by(|a, b| { + a.connector_name + .cmp(&b.connector_name) + .then_with(|| a.connector_id.cmp(&b.connector_id)) + }); + disabled_entries.sort_by(|a, b| { + a.connector_name + .cmp(&b.connector_name) + .then_with(|| a.connector_id.cmp(&b.connector_id)) + }); + + (installable_entries, disabled_entries) +} + +fn search_connector_entries<'a>( + entries: &'a [ConnectorEntry], + query: &str, + limit: usize, +) -> Vec<(&'a ConnectorEntry, f32)> { + if entries.is_empty() || limit == 0 { + return Vec::new(); + } + + let documents: Vec> = entries + .iter() + .enumerate() + .map(|(idx, entry)| Document::new(idx, entry.search_text.clone())) + .collect(); + let search_engine = + SearchEngineBuilder::::with_documents(Language::English, documents).build(); + let results = search_engine.search(query, limit); + + results + .into_iter() + .filter_map(|result| { + entries + .get(result.document.id) + .map(|entry| (entry, result.score)) + }) + .collect() +} + fn build_search_text(name: &str, info: &ToolInfo, input_keys: &[String]) -> String { let mut parts = vec![ name.to_string(), @@ -479,14 +551,37 @@ mod tests { make_connector("drive", true), ]; - let mut filtered: Vec = filter_codex_apps_mcp_tools(mcp_tools, &connectors) - .into_keys() - .collect(); + let mut filtered: Vec = + filter_codex_apps_mcp_tools(mcp_tools, &connectors, &HashSet::new()) + .into_keys() + .collect(); filtered.sort(); assert_eq!(filtered, vec!["mcp__codex_apps__drive_search".to_string()]); } + #[test] + fn filter_codex_apps_mcp_tools_keeps_temporarily_enabled_apps() { + let mcp_tools = HashMap::from([make_tool( + "mcp__codex_apps__calendar_create_event", + CODEX_APPS_MCP_SERVER_NAME, + "calendar_create_event", + Some("calendar"), + )]); + let connectors = vec![make_connector("calendar", false)]; + let enabled_connector_overrides = HashSet::from(["calendar".to_string()]); + + let filtered: Vec = + filter_codex_apps_mcp_tools(mcp_tools, &connectors, &enabled_connector_overrides) + .into_keys() + .collect(); + + assert_eq!( + filtered, + vec!["mcp__codex_apps__calendar_create_event".to_string()] + ); + } + #[test] fn filter_codex_apps_mcp_tools_drops_apps_without_connector_id() { let mcp_tools = HashMap::from([ @@ -499,12 +594,67 @@ mod tests { make_tool("mcp__rmcp__echo", "rmcp", "echo", None), ]); - let mut filtered: Vec = - filter_codex_apps_mcp_tools(mcp_tools, &[make_connector("calendar", true)]) - .into_keys() - .collect(); + let mut filtered: Vec = filter_codex_apps_mcp_tools( + mcp_tools, + &[make_connector("calendar", true)], + &HashSet::new(), + ) + .into_keys() + .collect(); filtered.sort(); assert_eq!(filtered, Vec::::new()); } + + #[test] + fn discoverable_connector_entries_prioritize_install_before_enable() { + let installable = AppInfo { + id: "calendar".to_string(), + name: "Calendar".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: false, + is_enabled: false, + plugin_display_names: Vec::new(), + }; + let disabled = AppInfo { + id: "drive".to_string(), + name: "Drive".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: true, + is_enabled: false, + plugin_display_names: Vec::new(), + }; + + let (installable_entries, disabled_entries) = + discoverable_connector_entries(vec![disabled, installable]); + + assert_eq!( + installable_entries + .iter() + .map(|entry| (entry.connector_id.clone(), entry.suggestion_type)) + .collect::>(), + vec![("calendar".to_string(), ToolSuggestionType::Install)] + ); + assert_eq!( + disabled_entries + .iter() + .map(|entry| (entry.connector_id.clone(), entry.suggestion_type)) + .collect::>(), + vec![("drive".to_string(), ToolSuggestionType::Enable)] + ); + } } diff --git a/codex-rs/core/src/tools/handlers/tool_suggest.rs b/codex-rs/core/src/tools/handlers/tool_suggest.rs index 8ab9ab2acd..eef6e1ca37 100644 --- a/codex-rs/core/src/tools/handlers/tool_suggest.rs +++ b/codex-rs/core/src/tools/handlers/tool_suggest.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::collections::HashSet; use async_trait::async_trait; use codex_app_server_protocol::McpElicitationObjectType; @@ -9,6 +10,7 @@ use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; use rmcp::model::RequestId; use serde::Deserialize; +use serde::Serialize; use serde_json::Value; use serde_json::json; @@ -26,13 +28,46 @@ pub struct ToolSuggestHandler; pub(crate) const TOOL_SUGGEST_TOOL_NAME: &str = "tool_suggest"; const TOOL_SUGGEST_DECISION_INSTALL: &str = "install"; +const TOOL_SUGGEST_DECISION_ENABLE: &str = "enable"; const TOOL_SUGGEST_DECISION_NOT_NOW: &str = "not_now"; const TOOL_SUGGEST_META_KIND_KEY: &str = "codex_approval_kind"; -const TOOL_SUGGEST_META_KIND_VALUE: &str = "app_install_suggestion"; +const TOOL_SUGGEST_META_KIND_VALUE: &str = "tool_suggestion"; + +#[derive(Clone, Copy, Debug, Deserialize, Serialize, Eq, PartialEq)] +#[serde(rename_all = "lowercase")] +pub(crate) enum ToolSuggestionToolType { + Connector, + Plugin, +} + +#[derive(Clone, Copy, Debug, Deserialize, Serialize, Eq, PartialEq)] +#[serde(rename_all = "lowercase")] +pub(crate) enum ToolSuggestionType { + Install, + Enable, +} + +impl ToolSuggestionType { + fn decision(self) -> &'static str { + match self { + Self::Install => TOOL_SUGGEST_DECISION_INSTALL, + Self::Enable => TOOL_SUGGEST_DECISION_ENABLE, + } + } + + fn verb(self) -> &'static str { + match self { + Self::Install => "Install", + Self::Enable => "Enable", + } + } +} #[derive(Deserialize)] struct ToolSuggestArgs { connector_id: String, + tool_type: ToolSuggestionToolType, + suggestion_type: ToolSuggestionType, } #[derive(Deserialize)] @@ -41,17 +76,21 @@ struct ToolSuggestElicitationContent { } fn tool_suggest_message( + suggestion_type: ToolSuggestionType, connector_name: &str, connector_description: Option<&str>, - install_url: &str, + url: &str, ) -> String { - let mut parts = vec![format!("Install {connector_name} to continue?")]; + let mut parts = vec![format!( + "{} {connector_name} to continue?", + suggestion_type.verb() + )]; if let Some(description) = connector_description.map(str::trim) && !description.is_empty() { parts.push(description.to_string()); } - parts.push(format!("Install URL: {install_url}")); + parts.push(format!("Open URL: {url}")); parts.join(" | ") } @@ -65,6 +104,8 @@ fn tool_suggest_requested_schema() -> McpElicitationSchema { } fn tool_suggest_elicitation_meta( + tool_type: ToolSuggestionToolType, + suggestion_type: ToolSuggestionType, connector_id: &str, connector_name: &str, connector_description: Option<&str>, @@ -72,6 +113,8 @@ fn tool_suggest_elicitation_meta( ) -> Value { json!({ TOOL_SUGGEST_META_KIND_KEY: TOOL_SUGGEST_META_KIND_VALUE, + "tool_type": tool_type, + "suggestion_type": suggestion_type, "connector_id": connector_id, "connector_name": connector_name, "connector_description": connector_description, @@ -81,6 +124,7 @@ fn tool_suggest_elicitation_meta( fn parse_tool_suggest_elicitation_response( response: ElicitationResponse, + suggestion_type: ToolSuggestionType, ) -> (&'static str, String) { let elicitation_action = match response.action { ElicitationAction::Accept => "accept", @@ -98,10 +142,12 @@ fn parse_tool_suggest_elicitation_response( .filter(|decision| { matches!( decision.as_str(), - TOOL_SUGGEST_DECISION_INSTALL | TOOL_SUGGEST_DECISION_NOT_NOW + TOOL_SUGGEST_DECISION_INSTALL + | TOOL_SUGGEST_DECISION_ENABLE + | TOOL_SUGGEST_DECISION_NOT_NOW ) }) - .unwrap_or_else(|| TOOL_SUGGEST_DECISION_INSTALL.to_string()), + .unwrap_or_else(|| suggestion_type.decision().to_string()), ElicitationAction::Decline | ElicitationAction::Cancel => { TOOL_SUGGEST_DECISION_NOT_NOW.to_string() } @@ -143,6 +189,15 @@ impl ToolHandler for ToolSuggestHandler { "connector_id must not be empty".to_string(), )); } + if args.tool_type != ToolSuggestionToolType::Connector { + return Err(FunctionCallError::RespondToModel(format!( + "tool_type `{}` is not supported by {TOOL_SUGGEST_TOOL_NAME} yet", + match args.tool_type { + ToolSuggestionToolType::Connector => "connector", + ToolSuggestionToolType::Plugin => "plugin", + } + ))); + } let connectors = match connectors::list_cached_connectors(&turn.config).await { Some(connectors) => connectors, @@ -150,7 +205,7 @@ impl ToolHandler for ToolSuggestHandler { .await .map_err(|err| { FunctionCallError::RespondToModel(format!( - "failed to load installable apps: {err}" + "failed to load discoverable apps: {err}" )) })?, }; @@ -162,10 +217,29 @@ impl ToolHandler for ToolSuggestHandler { FunctionCallError::RespondToModel(format!("unknown connector_id `{connector_id}`")) })?; - if connector.is_accessible { - return Err(FunctionCallError::RespondToModel(format!( - "connector_id `{connector_id}` is already available; use available tools or search_tool_bm25 with mode `available` instead" - ))); + let enabled_connector_overrides = session.get_connector_selection().await; + let connector_is_enabled = + connector.is_enabled || enabled_connector_overrides.contains(connector_id); + match args.suggestion_type { + ToolSuggestionType::Install => { + if connector.is_accessible { + return Err(FunctionCallError::RespondToModel(format!( + "connector_id `{connector_id}` is already installed; use search_tool_bm25 with mode `enabled`, or mode `discoverable` with suggestion_type `enable` if it is disabled" + ))); + } + } + ToolSuggestionType::Enable => { + if !connector.is_accessible { + return Err(FunctionCallError::RespondToModel(format!( + "connector_id `{connector_id}` is not installed; use search_tool_bm25 with mode `discoverable` and suggestion_type `install` instead" + ))); + } + if connector_is_enabled { + return Err(FunctionCallError::RespondToModel(format!( + "connector_id `{connector_id}` is already enabled; use its tools or search_tool_bm25 with mode `enabled` instead" + ))); + } + } } let install_url = connector @@ -183,12 +257,15 @@ impl ToolHandler for ToolSuggestHandler { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), request: McpServerElicitationRequest::Form { meta: Some(tool_suggest_elicitation_meta( + args.tool_type, + args.suggestion_type, &connector.id, &connector.name, connector.description.as_deref(), &install_url, )), message: tool_suggest_message( + args.suggestion_type, &connector.name, connector.description.as_deref(), &install_url, @@ -204,17 +281,30 @@ impl ToolHandler for ToolSuggestHandler { ) })?; let (elicitation_action, user_decision) = - parse_tool_suggest_elicitation_response(elicitation_response); + parse_tool_suggest_elicitation_response(elicitation_response, args.suggestion_type); + + if user_decision == TOOL_SUGGEST_DECISION_INSTALL + || user_decision == TOOL_SUGGEST_DECISION_ENABLE + { + session + .merge_connector_selection(HashSet::from([connector.id.clone()])) + .await; + } + let assistant_instruction = if user_decision == TOOL_SUGGEST_DECISION_INSTALL { - "The user confirmed they installed this app. Treat it as installed for this request, but verify its tools appear in an available-tool search before trying to use them." + "The user confirmed they completed the install flow. Treat this connector as selectable for this turn, but verify its tools appear in a `search_tool_bm25` search with `mode: \"enabled\"` before trying to use them. If they still do not appear, it may also need enabling." + } else if user_decision == TOOL_SUGGEST_DECISION_ENABLE { + "The user confirmed they enabled this connector. Treat it as selectable for this turn, but verify its tools appear in a `search_tool_bm25` search with `mode: \"enabled\"` before trying to use them." } else { - "The user did not install this app in this flow, either because they chose not to or did not finish setup. Do not try to use its tools in this turn." + "The user did not complete this suggestion flow. Do not try to use this tool in this turn." }; let content = json!({ "connector_id": connector.id, "connector_name": connector.name, "connector_description": connector.description, "install_url": install_url, + "tool_type": args.tool_type, + "suggestion_type": args.suggestion_type, "elicitation_action": elicitation_action, "user_decision": user_decision, "assistant_instruction": assistant_instruction, @@ -231,14 +321,17 @@ mod tests { use codex_rmcp_client::ElicitationResponse; use pretty_assertions::assert_eq; + use super::TOOL_SUGGEST_DECISION_ENABLE; use super::TOOL_SUGGEST_DECISION_INSTALL; use super::TOOL_SUGGEST_DECISION_NOT_NOW; + use super::ToolSuggestionType; use super::parse_tool_suggest_elicitation_response; use super::tool_suggest_message; #[test] fn tool_suggest_message_uses_single_line_text() { let message = tool_suggest_message( + ToolSuggestionType::Install, "Docs & Notes", Some("Install & sync"), "https://example.com/apps?name=Docs", @@ -246,31 +339,50 @@ mod tests { assert_eq!( message, - "Install Docs & Notes to continue? | Install & sync | Install URL: https://example.com/apps?name=Docs" + "Install Docs & Notes to continue? | Install & sync | Open URL: https://example.com/apps?name=Docs" ); } #[test] fn accepted_tool_suggest_defaults_to_install_without_form_content() { - let (elicitation_action, user_decision) = - parse_tool_suggest_elicitation_response(ElicitationResponse { + let (elicitation_action, user_decision) = parse_tool_suggest_elicitation_response( + ElicitationResponse { action: ElicitationAction::Accept, content: None, meta: None, - }); + }, + ToolSuggestionType::Install, + ); assert_eq!(elicitation_action, "accept"); assert_eq!(user_decision, TOOL_SUGGEST_DECISION_INSTALL); } + #[test] + fn accepted_tool_suggest_defaults_to_enable_without_form_content() { + let (elicitation_action, user_decision) = parse_tool_suggest_elicitation_response( + ElicitationResponse { + action: ElicitationAction::Accept, + content: None, + meta: None, + }, + ToolSuggestionType::Enable, + ); + + assert_eq!(elicitation_action, "accept"); + assert_eq!(user_decision, TOOL_SUGGEST_DECISION_ENABLE); + } + #[test] fn declined_tool_suggest_maps_to_not_now() { - let (elicitation_action, user_decision) = - parse_tool_suggest_elicitation_response(ElicitationResponse { + let (elicitation_action, user_decision) = parse_tool_suggest_elicitation_response( + ElicitationResponse { action: ElicitationAction::Decline, content: None, meta: None, - }); + }, + ToolSuggestionType::Install, + ); assert_eq!(elicitation_action, "decline"); assert_eq!(user_decision, TOOL_SUGGEST_DECISION_NOT_NOW); @@ -278,12 +390,14 @@ mod tests { #[test] fn cancelled_tool_suggest_maps_to_not_now() { - let (elicitation_action, user_decision) = - parse_tool_suggest_elicitation_response(ElicitationResponse { + let (elicitation_action, user_decision) = parse_tool_suggest_elicitation_response( + ElicitationResponse { action: ElicitationAction::Cancel, content: None, meta: None, - }); + }, + ToolSuggestionType::Install, + ); assert_eq!(elicitation_action, "cancel"); assert_eq!(user_decision, TOOL_SUGGEST_DECISION_NOT_NOW); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index c1e883f740..4ebdae10ee 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1290,7 +1290,7 @@ fn create_search_tool_bm25_tool(app_tools: &HashMap) -> ToolSp "mode".to_string(), JsonSchema::String { description: Some( - "Search mode: `available` searches currently available app tools; `installable` searches apps that are not installed yet. Defaults to `available`." + "Search mode: `enabled` searches currently enabled app tools; `discoverable` searches connectors that still need install or enable steps. Defaults to `enabled`." .to_string(), ), }, @@ -1307,7 +1307,7 @@ fn create_search_tool_bm25_tool(app_tools: &HashMap) -> ToolSp let description = if app_names.is_empty() { SEARCH_TOOL_BM25_DESCRIPTION_TEMPLATE .replace("({{app_names}})", "(None currently enabled)") - .replace("{{app_names}}", "available apps") + .replace("{{app_names}}", "enabled apps") } else { SEARCH_TOOL_BM25_DESCRIPTION_TEMPLATE.replace("{{app_names}}", app_names.as_str()) }; @@ -1325,22 +1325,47 @@ fn create_search_tool_bm25_tool(app_tools: &HashMap) -> ToolSp } fn create_tool_suggest_tool() -> ToolSpec { - let properties = BTreeMap::from([( - "connector_id".to_string(), - JsonSchema::String { - description: Some( - "Connector ID returned by `search_tool_bm25` in `installable` mode.".to_string(), - ), - }, - )]); + let properties = BTreeMap::from([ + ( + "connector_id".to_string(), + JsonSchema::String { + description: Some( + "Connector ID returned by `search_tool_bm25` in `discoverable` mode." + .to_string(), + ), + }, + ), + ( + "tool_type".to_string(), + JsonSchema::String { + description: Some( + "Tool type returned by `search_tool_bm25` in `discoverable` mode, for example `connector`." + .to_string(), + ), + }, + ), + ( + "suggestion_type".to_string(), + JsonSchema::String { + description: Some( + "Suggestion type returned by `search_tool_bm25` in `discoverable` mode: `install` or `enable`." + .to_string(), + ), + }, + ), + ]); ToolSpec::Function(ResponsesApiTool { name: TOOL_SUGGEST_TOOL_NAME.to_string(), - description: "Prompt the user to install an app that is not installed yet.".to_string(), + description: "Prompt the user to install or enable a discoverable connector.".to_string(), strict: false, parameters: JsonSchema::Object { properties, - required: Some(vec!["connector_id".to_string()]), + required: Some(vec![ + "connector_id".to_string(), + "tool_type".to_string(), + "suggestion_type".to_string(), + ]), additional_properties: Some(false.into()), }, }) @@ -3431,7 +3456,7 @@ mod tests { }; assert!(description.contains("Calendar")); assert!(!description.contains("mcp__rmcp__echo")); - assert!(description.contains("Always search `mode: \"available\"` first.")); + assert!(description.contains("Always search `mode: \"enabled\"` first.")); assert!(description.contains("call `tool_suggest`")); } @@ -3500,7 +3525,18 @@ mod tests { panic!("expected object schema"); }; assert!(properties.contains_key("connector_id")); - assert_eq!(required.as_deref(), Some(&["connector_id".to_string()][..])); + assert!(properties.contains_key("tool_type")); + assert!(properties.contains_key("suggestion_type")); + assert_eq!( + required.as_deref(), + Some( + &[ + "connector_id".to_string(), + "tool_type".to_string(), + "suggestion_type".to_string(), + ][..] + ) + ); } #[test] @@ -3567,7 +3603,7 @@ mod tests { }; assert!(description.contains("(None currently enabled)")); - assert!(description.contains("available apps.")); + assert!(description.contains("enabled apps.")); assert!(!description.contains("{{app_names}}")); } diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 0d11c107c4..c090346a28 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -444,7 +444,7 @@ mod tests { "pause should block the unified exec yield timeout" ); assert!( - response.output.contains("unified-exec-done"), + response.truncated_output().contains("unified-exec-done"), "exec_command should wait for output after the pause lifts" ); assert!( diff --git a/codex-rs/core/templates/search_tool/tool_description.md b/codex-rs/core/templates/search_tool/tool_description.md index d473b5cb99..daf6ed451c 100644 --- a/codex-rs/core/templates/search_tool/tool_description.md +++ b/codex-rs/core/templates/search_tool/tool_description.md @@ -1,6 +1,6 @@ # Apps tool discovery -Searches over apps metadata with BM25 and exposes matching installed tools for the next model call, or installable apps when requested. +Searches over apps metadata with BM25 and exposes matching enabled tools for the next model call, or discoverable apps when requested. MCP tools of the apps ({{app_names}}) are hidden until you search for them with this tool (`search_tool_bm25`). @@ -9,17 +9,20 @@ Follow this workflow: 1. Call `search_tool_bm25` with: - `query` (required): focused terms that describe the capability you need. - `limit` (optional): maximum number of tools to return (default `8`). - - `mode` (optional): `available` or `installable`. Default is `available`. -2. Always search `mode: "available"` first. -3. If `available` finds the right tool, use the returned `tools` list to decide which Apps tools are relevant. -4. Matching available tools are added to available `tools` for the remainder of the current session/thread. -5. If `available` does not find the right tool and the user strongly wants a specific app, search again with `mode: "installable"`. -6. If `installable` finds the right app, call `tool_suggest` with the returned `connector_id` to prompt the user to install it. + - `mode` (optional): `enabled` or `discoverable`. Default is `enabled`. +2. Always search `mode: "enabled"` first. +3. If `enabled` finds the right tool, use the returned `tools` list to decide which Apps tools are relevant. +4. Matching enabled tools are added to available `tools` for the remainder of the current session/thread. +5. If `enabled` does not find the right tool and the user strongly wants a specific app, search again with `mode: "discoverable"`. +6. `discoverable` results prefer: + - connectors that are not installed + - connectors that are installed but disabled +7. If `discoverable` finds the right app, call `tool_suggest` with the returned `connector_id`, `tool_type`, and `suggestion_type`. Notes: - Core tools remain available without searching. - If you are unsure, start with `limit` between 5 and 10 to see a broader set of tools. -- In `available` mode, `query` is matched against Apps tool metadata fields: +- In `enabled` mode, `query` is matched against Apps tool metadata fields: - `name` - `tool_name` - `server_name` @@ -27,7 +30,7 @@ Notes: - `description` - `connector_name` - input schema property keys (`input_keys`) -- In `installable` mode, `query` is matched against app metadata such as connector id, name, description, labels, categories, and plugin display names. +- In `discoverable` mode, `query` is matched against app metadata such as connector id, name, description, labels, categories, and plugin display names. - If the needed app is already explicit in the prompt (for example `[$app-name](app://{connector_id})`) or already present in the current `tools` list, you can call that tool directly. -- Do not call app MCP tools for apps returned only by `installable` mode until the user installs them. +- Do not call app MCP tools for apps returned only by `discoverable` mode until the user installs or enables them. - Do not use `search_tool_bm25` for non-apps/local tasks (filesystem, repo search, or shell-only workflows) or anything not related to {{app_names}}. diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 910158d9a7..1a378c84b3 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -46,8 +46,8 @@ use wiremock::matchers::method; use wiremock::matchers::path; const SEARCH_TOOL_INSTRUCTION_SNIPPETS: [&str; 2] = [ - "Always search `mode: \"available\"` first.", - "If `installable` finds the right app, call `tool_suggest` with the returned `connector_id` to prompt the user to install it.", + "Always search `mode: \"enabled\"` first.", + "If `discoverable` finds the right app, call `tool_suggest` with the returned `connector_id`, `tool_type`, and `suggestion_type`.", ]; const SEARCH_TOOL_BM25_TOOL_NAME: &str = "search_tool_bm25"; const TOOL_SUGGEST_TOOL_NAME: &str = "tool_suggest"; @@ -1362,7 +1362,7 @@ async fn installable_search_returns_inaccessible_connectors_without_selecting_to let args = json!({ "query": "notion docs", "limit": 5, - "mode": "installable", + "mode": "discoverable", }); let mock = mount_sse_sequence( &server, @@ -1405,7 +1405,7 @@ async fn installable_search_returns_inaccessible_connectors_without_selecting_to let payload = search_tool_output_payload(&requests[1], call_id); assert_eq!( payload.get("mode").and_then(Value::as_str), - Some("installable") + Some("discoverable") ); assert_eq!( payload.get("total_connectors").and_then(Value::as_u64), @@ -1427,6 +1427,14 @@ async fn installable_search_returns_inaccessible_connectors_without_selecting_to connectors[0].get("connector_id").and_then(Value::as_str), Some(NOTION_CONNECTOR_ID), ); + assert_eq!( + connectors[0].get("tool_type").and_then(Value::as_str), + Some("connector"), + ); + assert_eq!( + connectors[0].get("suggestion_type").and_then(Value::as_str), + Some("install"), + ); assert_eq!( connectors[0].get("connector_name").and_then(Value::as_str), Some(NOTION_CONNECTOR_NAME), @@ -1475,7 +1483,7 @@ async fn installable_search_preserves_available_selection() -> Result<()> { &serde_json::to_string(&json!({ "query": "notion docs", "limit": 5, - "mode": "installable", + "mode": "discoverable", }))?, ), ev_completed("resp-3"), @@ -1545,7 +1553,7 @@ async fn installable_search_reports_catalog_load_errors() -> Result<()> { let call_id = "installable-search"; let args = json!({ "query": "notion docs", - "mode": "installable", + "mode": "discoverable", }); let mock = mount_sse_sequence( &server, @@ -1597,7 +1605,7 @@ async fn installable_search_reports_catalog_load_errors() -> Result<()> { .and_then(|(content, _)| content) .expect("installable search output should contain an error"); assert!( - error.contains("failed to load installable apps"), + error.contains("failed to load discoverable apps"), "unexpected installable search error: {error:?}" ); @@ -1614,6 +1622,8 @@ async fn tool_suggest_prompts_install_via_elicitation() -> Result<()> { let call_id = "tool-suggest"; let args = json!({ "connector_id": NOTION_CONNECTOR_ID, + "tool_type": "connector", + "suggestion_type": "install", }); let mock = mount_sse_sequence( &server, @@ -1654,13 +1664,15 @@ async fn tool_suggest_prompts_install_via_elicitation() -> Result<()> { assert_eq!( message, format!( - "Install {NOTION_CONNECTOR_NAME} to continue? | {NOTION_CONNECTOR_DESCRIPTION} | Install URL: https://chatgpt.com/apps/notion/notion" + "Install {NOTION_CONNECTOR_NAME} to continue? | {NOTION_CONNECTOR_DESCRIPTION} | Open URL: https://chatgpt.com/apps/notion/notion" ) ); assert_eq!( meta, Some(json!({ - "codex_approval_kind": "app_install_suggestion", + "codex_approval_kind": "tool_suggestion", + "tool_type": "connector", + "suggestion_type": "install", "connector_id": NOTION_CONNECTOR_ID, "connector_name": NOTION_CONNECTOR_NAME, "connector_description": NOTION_CONNECTOR_DESCRIPTION, @@ -1727,7 +1739,7 @@ async fn tool_suggest_prompts_install_via_elicitation() -> Result<()> { payload .get("assistant_instruction") .and_then(Value::as_str) - .is_some_and(|instruction| instruction.contains("confirmed they installed")), + .is_some_and(|instruction| instruction.contains("completed the install flow")), "unexpected tool_suggest payload: {payload:?}" ); @@ -1744,6 +1756,8 @@ async fn tool_suggest_rejects_accessible_connector() -> Result<()> { let call_id = "tool-suggest"; let args = json!({ "connector_id": "calendar", + "tool_type": "connector", + "suggestion_type": "install", }); let mock = mount_sse_sequence( &server, @@ -1788,7 +1802,7 @@ async fn tool_suggest_rejects_accessible_connector() -> Result<()> { .and_then(|(content, _)| content) .expect("tool_suggest should return an error"); assert!( - error.contains("already available"), + error.contains("already installed"), "unexpected accessible connector rejection: {error:?}" ); @@ -1805,6 +1819,8 @@ async fn tool_suggest_reports_not_now_when_user_declines_install() -> Result<()> let call_id = "tool-suggest"; let args = json!({ "connector_id": NOTION_CONNECTOR_ID, + "tool_type": "connector", + "suggestion_type": "install", }); let mock = mount_sse_sequence( &server, @@ -1868,7 +1884,9 @@ async fn tool_suggest_reports_not_now_when_user_declines_install() -> Result<()> payload .get("assistant_instruction") .and_then(Value::as_str) - .is_some_and(|instruction| instruction.contains("did not install this app")), + .is_some_and( + |instruction| instruction.contains("did not complete this suggestion flow") + ), "unexpected tool_suggest payload: {payload:?}" ); @@ -1885,6 +1903,8 @@ async fn tool_suggest_rejects_unknown_connector() -> Result<()> { let call_id = "tool-suggest"; let args = json!({ "connector_id": "unknown-app", + "tool_type": "connector", + "suggestion_type": "install", }); let mock = mount_sse_sequence( &server, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index ad1631599b..2d3603a545 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -2382,6 +2382,7 @@ impl App { url, is_installed, is_enabled, + suggestion_type: None, elicitation_resolution: None, }); } diff --git a/codex-rs/tui/src/bottom_pane/app_link_view.rs b/codex-rs/tui/src/bottom_pane/app_link_view.rs index a8698d7f60..7173406ace 100644 --- a/codex-rs/tui/src/bottom_pane/app_link_view.rs +++ b/codex-rs/tui/src/bottom_pane/app_link_view.rs @@ -43,10 +43,34 @@ enum AppLinkScreen { InstallConfirmation, } -const APP_INSTALL_SUGGESTION_META_KIND_VALUE: &str = "app_install_suggestion"; -const APP_INSTALL_SUGGESTION_DECISION_INSTALL: &str = "install"; +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +enum ToolSuggestionToolType { + Connector, + Plugin, +} + +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub(crate) enum ToolSuggestionType { + Install, + Enable, +} + +impl ToolSuggestionType { + fn decision(self) -> &'static str { + match self { + Self::Install => "install", + Self::Enable => "enable", + } + } +} + +const TOOL_SUGGESTION_META_KIND_VALUE: &str = "tool_suggestion"; pub(crate) const APP_LINK_INSTALL_INSTRUCTIONS: &str = "Install this app in your browser, then reload Codex."; +const APP_LINK_ENABLE_INSTRUCTIONS: &str = + "Enable this app in Codex to use its tools in this session."; #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct AppLinkElicitationResolution { @@ -64,20 +88,23 @@ pub(crate) struct AppLinkViewParams { pub(crate) url: String, pub(crate) is_installed: bool, pub(crate) is_enabled: bool, + pub(crate) suggestion_type: Option, pub(crate) elicitation_resolution: Option, } #[derive(Deserialize)] -struct AppInstallSuggestionMeta { +struct ToolSuggestionMeta { #[serde(rename = "codex_approval_kind")] approval_kind: String, + tool_type: ToolSuggestionToolType, + suggestion_type: ToolSuggestionType, connector_id: String, connector_name: String, connector_description: Option, install_url: String, } -pub(crate) fn app_install_suggestion_params_from_event( +pub(crate) fn tool_suggestion_params_from_event( thread_id: ThreadId, request: &ElicitationRequestEvent, ) -> Option { @@ -88,19 +115,27 @@ pub(crate) fn app_install_suggestion_params_from_event( return None; }; - let meta = serde_json::from_value::(meta.clone()).ok()?; - if meta.approval_kind != APP_INSTALL_SUGGESTION_META_KIND_VALUE { + let meta = serde_json::from_value::(meta.clone()).ok()?; + if meta.approval_kind != TOOL_SUGGESTION_META_KIND_VALUE + || meta.tool_type != ToolSuggestionToolType::Connector + { return None; } + let (instructions, is_installed, is_enabled) = match meta.suggestion_type { + ToolSuggestionType::Install => (APP_LINK_INSTALL_INSTRUCTIONS.to_string(), false, false), + ToolSuggestionType::Enable => (APP_LINK_ENABLE_INSTRUCTIONS.to_string(), true, false), + }; + Some(AppLinkViewParams { app_id: meta.connector_id, title: meta.connector_name, description: meta.connector_description, - instructions: APP_LINK_INSTALL_INSTRUCTIONS.to_string(), + instructions, url: meta.install_url, - is_installed: false, - is_enabled: false, + is_installed, + is_enabled, + suggestion_type: Some(meta.suggestion_type), elicitation_resolution: Some(AppLinkElicitationResolution { thread_id, server_name: request.server_name.clone(), @@ -117,6 +152,7 @@ pub(crate) struct AppLinkView { url: String, is_installed: bool, is_enabled: bool, + suggestion_type: Option, elicitation_resolution: Option, app_event_tx: AppEventSender, screen: AppLinkScreen, @@ -134,6 +170,7 @@ impl AppLinkView { url, is_installed, is_enabled, + suggestion_type, elicitation_resolution, } = params; Self { @@ -144,6 +181,7 @@ impl AppLinkView { url, is_installed, is_enabled, + suggestion_type, elicitation_resolution, app_event_tx, screen: AppLinkScreen::Link, @@ -198,7 +236,10 @@ impl AppLinkView { self.resolve_elicitation( ElicitationAction::Accept, Some(json!({ - "decision": APP_INSTALL_SUGGESTION_DECISION_INSTALL, + "decision": self + .suggestion_type + .unwrap_or(ToolSuggestionType::Install) + .decision(), })), ); self.complete = true; @@ -220,6 +261,18 @@ impl AppLinkView { id: self.app_id.clone(), enabled: self.is_enabled, }); + if self.is_enabled + && self.elicitation_resolution.is_some() + && self.suggestion_type == Some(ToolSuggestionType::Enable) + { + self.resolve_elicitation( + ElicitationAction::Accept, + Some(json!({ + "decision": ToolSuggestionType::Enable.decision(), + })), + ); + self.complete = true; + } } fn resolve_elicitation(&self, decision: ElicitationAction, content: Option) { @@ -555,10 +608,17 @@ mod tests { url: "https://example.test/notion".to_string(), is_installed: true, is_enabled: true, + suggestion_type: None, elicitation_resolution: None, } } + fn render_snapshot(view: &AppLinkView, area: Rect) -> String { + let mut buf = Buffer::empty(area); + view.render(area, &mut buf); + format!("{buf:?}") + } + fn elicitation_resolution(thread_id: ThreadId) -> AppLinkElicitationResolution { AppLinkElicitationResolution { thread_id, @@ -673,7 +733,7 @@ mod tests { } #[test] - fn app_install_suggestion_event_builds_app_link_params() { + fn tool_install_suggestion_event_builds_app_link_params() { let thread_id = ThreadId::default(); let request = ElicitationRequestEvent { turn_id: Some("turn-1".to_string()), @@ -681,7 +741,9 @@ mod tests { id: McpRequestId::String("request-1".to_string()), request: ElicitationRequest::Form { meta: Some(json!({ - "codex_approval_kind": "app_install_suggestion", + "codex_approval_kind": "tool_suggestion", + "tool_type": "connector", + "suggestion_type": "install", "connector_id": "connector_1", "connector_name": "Notion", "connector_description": "Docs and notes", @@ -695,7 +757,7 @@ mod tests { }, }; - let params = app_install_suggestion_params_from_event(thread_id, &request); + let params = tool_suggestion_params_from_event(thread_id, &request); assert_eq!( params, @@ -707,11 +769,72 @@ mod tests { url: "https://example.test/notion".to_string(), is_installed: false, is_enabled: false, + suggestion_type: Some(ToolSuggestionType::Install), elicitation_resolution: Some(elicitation_resolution(thread_id)), }) ); } + #[test] + fn tool_enable_suggestion_event_builds_app_link_params() { + let thread_id = ThreadId::default(); + let request = ElicitationRequestEvent { + turn_id: Some("turn-1".to_string()), + server_name: "codex_apps".to_string(), + id: McpRequestId::String("request-1".to_string()), + request: ElicitationRequest::Form { + meta: Some(json!({ + "codex_approval_kind": "tool_suggestion", + "tool_type": "connector", + "suggestion_type": "enable", + "connector_id": "connector_1", + "connector_name": "Notion", + "connector_description": "Docs and notes", + "install_url": "https://example.test/notion", + })), + message: "Enable Notion to continue?".to_string(), + requested_schema: json!({ + "type": "object", + "properties": {}, + }), + }, + }; + + let params = tool_suggestion_params_from_event(thread_id, &request); + + assert_eq!( + params, + Some(AppLinkViewParams { + app_id: "connector_1".to_string(), + title: "Notion".to_string(), + description: Some("Docs and notes".to_string()), + instructions: APP_LINK_ENABLE_INSTRUCTIONS.to_string(), + url: "https://example.test/notion".to_string(), + is_installed: true, + is_enabled: false, + suggestion_type: Some(ToolSuggestionType::Enable), + elicitation_resolution: Some(elicitation_resolution(thread_id)), + }) + ); + } + + #[test] + fn enable_suggestion_snapshot() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut params = base_params(); + params.is_enabled = false; + params.instructions = APP_LINK_ENABLE_INSTRUCTIONS.to_string(); + params.suggestion_type = Some(ToolSuggestionType::Enable); + let view = AppLinkView::new(params, tx); + let area = Rect::new(0, 0, 80, view.desired_height(80)); + + insta::assert_snapshot!( + "app_link_view_enable_suggestion", + render_snapshot(&view, area) + ); + } + #[test] fn install_confirmation_resolves_elicitation_after_refresh() { let expected_thread_id = ThreadId::default(); @@ -721,6 +844,7 @@ mod tests { params.is_installed = false; params.is_enabled = false; params.instructions = APP_LINK_INSTALL_INSTRUCTIONS.to_string(); + params.suggestion_type = Some(ToolSuggestionType::Install); params.elicitation_resolution = Some(elicitation_resolution(expected_thread_id)); let mut view = AppLinkView::new(params, tx); view.screen = AppLinkScreen::InstallConfirmation; @@ -775,6 +899,7 @@ mod tests { params.is_installed = false; params.is_enabled = false; params.instructions = APP_LINK_INSTALL_INSTRUCTIONS.to_string(); + params.suggestion_type = Some(ToolSuggestionType::Install); params.elicitation_resolution = Some(elicitation_resolution(expected_thread_id)); let mut view = AppLinkView::new(params, tx); view.selected_action = 1; @@ -806,4 +931,58 @@ mod tests { assert!(view.is_complete()); } + + #[test] + fn enabling_suggested_app_resolves_elicitation() { + let expected_thread_id = ThreadId::default(); + let (tx_raw, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut params = base_params(); + params.is_enabled = false; + params.instructions = APP_LINK_ENABLE_INSTRUCTIONS.to_string(); + params.suggestion_type = Some(ToolSuggestionType::Enable); + params.elicitation_resolution = Some(elicitation_resolution(expected_thread_id)); + let mut view = AppLinkView::new(params, tx); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('2'), KeyModifiers::NONE)); + + match rx.try_recv() { + Ok(AppEvent::SetAppEnabled { id, enabled }) => { + assert_eq!(id, "connector_1"); + assert!(enabled); + } + Ok(other) => panic!("unexpected app event: {other:?}"), + Err(err) => panic!("missing app event: {err}"), + } + + match rx.try_recv() { + Ok(AppEvent::SubmitThreadOp { + thread_id, + op: + Op::ResolveElicitation { + server_name, + request_id, + decision, + content, + meta, + }, + }) => { + assert_eq!(thread_id, expected_thread_id); + assert_eq!(server_name, "codex_apps"); + assert_eq!(request_id, McpRequestId::String("request-1".to_string())); + assert_eq!(decision, ElicitationAction::Accept); + assert_eq!( + content, + Some(json!({ + "decision": "enable", + })) + ); + assert_eq!(meta, None); + } + Ok(other) => panic!("unexpected app event: {other:?}"), + Err(err) => panic!("missing app event: {err}"), + } + + assert!(view.is_complete()); + } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5fb987c1bf..da1bdfae1d 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -50,7 +50,7 @@ mod status_line_setup; pub(crate) use app_link_view::APP_LINK_INSTALL_INSTRUCTIONS; pub(crate) use app_link_view::AppLinkView; pub(crate) use app_link_view::AppLinkViewParams; -pub(crate) use app_link_view::app_install_suggestion_params_from_event; +pub(crate) use app_link_view::tool_suggestion_params_from_event; pub(crate) use approval_overlay::ApprovalOverlay; pub(crate) use approval_overlay::ApprovalRequest; pub(crate) use approval_overlay::format_additional_permissions_rule; diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__app_link_view__tests__app_link_view_enable_suggestion.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__app_link_view__tests__app_link_view_enable_suggestion.snap new file mode 100644 index 0000000000..446d03943f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__app_link_view__tests__app_link_view_enable_suggestion.snap @@ -0,0 +1,30 @@ +--- +source: tui/src/bottom_pane/app_link_view.rs +expression: "render_snapshot(&view, area)" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 80, height: 13 }, + content: [ + " ", + " Notion ", + " ", + " Use $ to insert this app into the prompt. ", + " ", + " Enable this app in Codex to use its tools in this session. ", + " Newly installed apps can take a few minutes to appear in /apps. ", + " ", + " ", + " › 1. Manage on ChatGPT ", + " 2. Enable app ", + " 3. Back ", + " Use tab / ↑ ↓ to move, enter to select, esc to close ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 8, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 9, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 24, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 12, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + ] +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e60e8b753c..18a0f1f2c9 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2973,8 +2973,7 @@ impl ChatWidget { }); let thread_id = self.thread_id.unwrap_or_default(); - if let Some(params) = - crate::bottom_pane::app_install_suggestion_params_from_event(thread_id, &ev) + if let Some(params) = crate::bottom_pane::tool_suggestion_params_from_event(thread_id, &ev) { self.bottom_pane.push_app_link_view(params); } else if let Some(request) =