From a397b59887d6b5df7498dd7c52d1000db78b6b4e Mon Sep 17 00:00:00 2001 From: Alex Daley Date: Tue, 16 Jun 2026 19:44:42 -0400 Subject: [PATCH] [codex] [4/4] Simplify recommended plugin install schema (#28403) ## Summary - Simplify recommendation-context `request_plugin_install` arguments to `plugin_id` and `suggest_reason`. - Derive plugin type and install action from the matched candidate while preserving Codex-owned elicitation metadata. - Keep the legacy list-backed schema unchanged and accept resumed calls that still use `tool_id`. ## Stack - #28399 - #28400 - #27704 - This PR ## Validation - `just test -p codex-tools -p codex-core request_plugin_install` (25 passed) - `just fix -p codex-tools -p codex-core` - `just fmt` - `git diff --check` --- .../recommended_plugins_instructions.rs | 2 +- .../tools/handlers/request_plugin_install.rs | 76 +++++++--- .../handlers/request_plugin_install_spec.rs | 136 +++++++++++------- .../handlers/request_plugin_install_tests.rs | 18 +++ codex-rs/core/src/tools/spec_plan_tests.rs | 8 +- .../tests/suite/request_plugin_install.rs | 8 +- codex-rs/tools/src/request_plugin_install.rs | 12 +- .../tools/src/request_plugin_install_tests.rs | 22 +-- 8 files changed, 174 insertions(+), 108 deletions(-) diff --git a/codex-rs/core/src/context/recommended_plugins_instructions.rs b/codex-rs/core/src/context/recommended_plugins_instructions.rs index a7b3ea78f..9ad3c9a0d 100644 --- a/codex-rs/core/src/context/recommended_plugins_instructions.rs +++ b/codex-rs/core/src/context/recommended_plugins_instructions.rs @@ -1,7 +1,7 @@ use super::ContextualUserFragment; use codex_tools::DiscoverableTool; -const RECOMMENDED_PLUGINS_INTRO: &str = "Here is a list of plugins that are available but not installed. If the user's query would benefit from one of these plugins, use the `request_plugin_install` tool to suggest that they install it. All entries have `tool_type: plugin`; pass `plugin` as `tool_type` and the parenthesized ID as `tool_id`. For example, suggest the Google Drive plugin if the query could possibly be better answered with access to Google Drive."; +const RECOMMENDED_PLUGINS_INTRO: &str = "Here is a list of plugins that are available but not installed. If the user's query would benefit from one of these plugins, use the `request_plugin_install` tool to suggest that they install it. Pass the parenthesized ID as `plugin_id`. For example, suggest the Google Drive plugin if the query could possibly be better answered with access to Google Drive."; const MAX_RECOMMENDED_PLUGINS: usize = 50; #[derive(Debug, Clone, PartialEq)] diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 0a7f19546..804721314 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -22,6 +22,7 @@ use codex_tools::build_request_plugin_install_elicitation_request; use codex_tools::filter_request_plugin_install_discoverable_tools_for_client; use codex_tools::verified_connector_install_completed; use rmcp::model::RequestId; +use serde::Deserialize; use serde_json::Value; use tracing::warn; @@ -39,6 +40,13 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; use crate::tools::router::ToolSuggestPresentation; +#[derive(Debug, Deserialize, PartialEq, Eq)] +struct RecommendedPluginInstallArgs { + #[serde(alias = "tool_id")] + plugin_id: String, + suggest_reason: String, +} + pub struct RequestPluginInstallHandler { discoverable_tools: Vec, presentation: ToolSuggestPresentation, @@ -96,20 +104,30 @@ impl RequestPluginInstallHandler { } }; - let args: RequestPluginInstallArgs = parse_arguments(&arguments)?; - let suggest_reason = args.suggest_reason.trim(); + let (requested_tool_id, requested_tool_type, suggest_reason) = match self.presentation { + ToolSuggestPresentation::ListTool => { + let args: RequestPluginInstallArgs = parse_arguments(&arguments)?; + if args.action_type != DiscoverableToolAction::Install { + return Err(FunctionCallError::RespondToModel( + "plugin install requests currently support only action_type=\"install\"" + .to_string(), + )); + } + (args.tool_id, Some(args.tool_type), args.suggest_reason) + } + ToolSuggestPresentation::RecommendationContext => { + let args: RecommendedPluginInstallArgs = parse_arguments(&arguments)?; + (args.plugin_id, None, args.suggest_reason) + } + }; + let suggest_reason = suggest_reason.trim(); if suggest_reason.is_empty() { return Err(FunctionCallError::RespondToModel( "suggest_reason must not be empty".to_string(), )); } - if args.action_type != DiscoverableToolAction::Install { - return Err(FunctionCallError::RespondToModel( - "plugin install requests currently support only action_type=\"install\"" - .to_string(), - )); - } - if args.tool_type == DiscoverableToolType::Plugin + if (requested_tool_type == Some(DiscoverableToolType::Plugin) + || self.presentation == ToolSuggestPresentation::RecommendationContext) && turn.app_server_client_name.as_deref() == Some("codex-tui") { return Err(FunctionCallError::RespondToModel( @@ -124,27 +142,41 @@ impl RequestPluginInstallHandler { let tool = discoverable_tools .into_iter() - .find(|tool| tool.tool_type() == args.tool_type && tool.id() == args.tool_id) - .ok_or_else(|| { - let source = match self.presentation { - ToolSuggestPresentation::ListTool => format!( - "the discoverable tools returned by {LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}" - ), - ToolSuggestPresentation::RecommendationContext => { - "the list".to_string() + .find(|tool| { + tool.id() == requested_tool_id + && match self.presentation { + ToolSuggestPresentation::ListTool => { + Some(tool.tool_type()) == requested_tool_type + } + ToolSuggestPresentation::RecommendationContext => { + matches!(tool, DiscoverableTool::Plugin(_)) + } } + }) + .ok_or_else(|| { + let (argument_name, source) = match self.presentation { + ToolSuggestPresentation::ListTool => ( + "tool_id", + format!( + "the discoverable tools returned by {LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}" + ), + ), + ToolSuggestPresentation::RecommendationContext => ( + "plugin_id", + "the entries in the list".to_string(), + ), }; FunctionCallError::RespondToModel(format!( - "tool_id must match one of {source}" + "{argument_name} must match one of {source}" )) })?; + let tool_type = tool.tool_type(); let request_id = RequestId::String(format!("request_plugin_install_{call_id}").into()); let params = build_request_plugin_install_elicitation_request( CODEX_APPS_MCP_SERVER_NAME, session.thread_id.to_string(), turn.sub_id.clone(), - &args, suggest_reason, &tool, ); @@ -173,7 +205,7 @@ impl RequestPluginInstallHandler { } if elicitation.sent { - let tool_type = match args.tool_type { + let tool_type = match tool_type { DiscoverableToolType::Connector => "connector", DiscoverableToolType::Plugin => "plugin", }; @@ -196,8 +228,8 @@ impl RequestPluginInstallHandler { let content = serde_json::to_string(&RequestPluginInstallResult { completed, user_confirmed, - tool_type: args.tool_type, - action_type: args.action_type, + tool_type, + action_type: DiscoverableToolAction::Install, tool_id: tool.id().to_string(), tool_name: tool.name().to_string(), suggest_reason: suggest_reason.to_string(), diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs b/codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs index fa3bbdab0..e37e22383 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs @@ -10,37 +10,63 @@ use crate::tools::router::ToolSuggestPresentation; pub(crate) fn create_request_plugin_install_tool( presentation: ToolSuggestPresentation, ) -> ToolSpec { - let properties = BTreeMap::from([ - ( - "tool_type".to_string(), - JsonSchema::string(Some( - "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"." - .to_string(), - )), + let (properties, required, description) = match presentation { + ToolSuggestPresentation::ListTool => ( + BTreeMap::from([ + ( + "tool_type".to_string(), + JsonSchema::string(Some( + "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"." + .to_string(), + )), + ), + ( + "action_type".to_string(), + JsonSchema::string(Some( + "Suggested action for the tool. Use \"install\".".to_string(), + )), + ), + ( + "tool_id".to_string(), + JsonSchema::string(Some("Connector or plugin id to suggest.".to_string())), + ), + ( + "suggest_reason".to_string(), + JsonSchema::string(Some( + "Concise one-line user-facing reason why this plugin or connector can help with the current request." + .to_string(), + )), + ), + ]), + vec![ + "tool_type".to_string(), + "action_type".to_string(), + "tool_id".to_string(), + "suggest_reason".to_string(), + ], + format!( + "# Request plugin/connector install\n\nUse this tool only after `{LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}` returns a plugin or connector that exactly matches the user's explicit request.\n\nDo not use it for adjacent capabilities, broad recommendations, or tools that merely seem useful. Pass the returned `tool_type` through directly, and pass the returned `id` as `tool_id`.\n\nIMPORTANT: DO NOT call this tool in parallel with other tools." + ), ), - ( - "action_type".to_string(), - JsonSchema::string(Some("Suggested action for the tool. Use \"install\".".to_string())), - ), - ( - "tool_id".to_string(), - JsonSchema::string(Some("Connector or plugin id to suggest.".to_string())), - ), - ( - "suggest_reason".to_string(), - JsonSchema::string(Some( - "Concise one-line user-facing reason why this plugin or connector can help with the current request." - .to_string(), - )), - ), - ]); - - let description = match presentation { - ToolSuggestPresentation::ListTool => format!( - "# Request plugin/connector install\n\nUse this tool only after `{LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}` returns a plugin or connector that exactly matches the user's explicit request.\n\nDo not use it for adjacent capabilities, broad recommendations, or tools that merely seem useful. Pass the returned `tool_type` through directly, and pass the returned `id` as `tool_id`.\n\nIMPORTANT: DO NOT call this tool in parallel with other tools." - ), - ToolSuggestPresentation::RecommendationContext => + ToolSuggestPresentation::RecommendationContext => ( + BTreeMap::from([ + ( + "plugin_id".to_string(), + JsonSchema::string(Some( + "Plugin id from the `` list.".to_string(), + )), + ), + ( + "suggest_reason".to_string(), + JsonSchema::string(Some( + "Concise one-line user-facing reason why this plugin can help with the current request." + .to_string(), + )), + ), + ]), + vec!["plugin_id".to_string(), "suggest_reason".to_string()], "# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the `` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string(), + ), }; ToolSpec::Function(ResponsesApiTool { @@ -48,16 +74,7 @@ pub(crate) fn create_request_plugin_install_tool( description, strict: false, defer_loading: None, - parameters: JsonSchema::object( - properties, - Some(vec![ - "tool_type".to_string(), - "action_type".to_string(), - "tool_id".to_string(), - "suggest_reason".to_string(), - ]), - Some(false.into()), - ), + parameters: JsonSchema::object(properties, Some(required), Some(false.into())), output_schema: None, }) } @@ -126,16 +143,35 @@ mod tests { } #[test] - fn recommendation_context_changes_only_the_description() { - let mut expected = create_request_plugin_install_tool(ToolSuggestPresentation::ListTool); - let recommendations = - create_request_plugin_install_tool(ToolSuggestPresentation::RecommendationContext); - - let ToolSpec::Function(expected_function) = &mut expected else { - panic!("expected function tool specs"); - }; - expected_function.description = "# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the `` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string(); - - assert_eq!(recommendations, expected); + fn recommendation_context_uses_simplified_plugin_wire_shape() { + assert_eq!( + create_request_plugin_install_tool(ToolSuggestPresentation::RecommendationContext), + ToolSpec::Function(ResponsesApiTool { + name: "request_plugin_install".to_string(), + description: "# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the `` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string(), + strict: false, + defer_loading: None, + parameters: JsonSchema::object( + BTreeMap::from([ + ( + "plugin_id".to_string(), + JsonSchema::string(Some( + "Plugin id from the `` list.".to_string(), + )), + ), + ( + "suggest_reason".to_string(), + JsonSchema::string(Some( + "Concise one-line user-facing reason why this plugin can help with the current request." + .to_string(), + )), + ), + ]), + Some(vec!["plugin_id".to_string(), "suggest_reason".to_string()]), + Some(false.into()), + ), + output_schema: None, + }) + ); } } diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs index cacc25ff9..62c8afaa6 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs @@ -68,6 +68,24 @@ fn remote_plugin_install_suggestions_skip_core_installed_verification() { assert!(!is_remote_plugin_install_suggestion("Plugin_123")); } +#[test] +fn recommended_plugin_install_args_accept_legacy_tool_id() { + let current: RecommendedPluginInstallArgs = serde_json::from_value(json!({ + "plugin_id": "google-drive@openai-curated-remote", + "suggest_reason": "Use Google Drive for this request" + })) + .expect("current arguments should deserialize"); + let legacy: RecommendedPluginInstallArgs = serde_json::from_value(json!({ + "tool_type": "plugin", + "action_type": "install", + "tool_id": "google-drive@openai-curated-remote", + "suggest_reason": "Use Google Drive for this request" + })) + .expect("legacy arguments should deserialize"); + + assert_eq!(current, legacy); +} + #[test] fn request_plugin_install_response_persists_only_decline_always_mode() { assert!(request_plugin_install_response_requests_persistent_disable( diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index fc17ff5d3..8a324dcc8 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -966,16 +966,22 @@ async fn request_plugin_install_description_refers_to_recommended_plugins_hint() ) .await; + let request_spec = plan.visible_spec("request_plugin_install"); let ToolSpec::Function(ResponsesApiTool { description: request_description, .. - }) = plan.visible_spec("request_plugin_install") + }) = request_spec else { panic!("expected request_plugin_install function spec"); }; assert!(request_description.contains("the `` list")); assert!(!request_description.contains("list_available_plugins_to_install")); assert!(!request_description.contains("github")); + assert!(has_parameter(request_spec, "plugin_id")); + assert!(has_parameter(request_spec, "suggest_reason")); + assert!(!has_parameter(request_spec, "tool_id")); + assert!(!has_parameter(request_spec, "tool_type")); + assert!(!has_parameter(request_spec, "action_type")); plan.assert_visible_lacks(&["list_available_plugins_to_install"]); plan.assert_registered_lacks(&["list_available_plugins_to_install"]); } diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index 9812d3941..f3da1bea4 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -230,9 +230,7 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids() call_id, REQUEST_PLUGIN_INSTALL_TOOL_NAME, &serde_json::to_string(&json!({ - "tool_type": "plugin", - "action_type": "install", - "tool_id": "invented@openai-curated-remote", + "plugin_id": "invented@openai-curated-remote", "suggest_reason": "Try this" }))?, ), @@ -312,9 +310,7 @@ async fn endpoint_recommendation_adds_install_identity_only_to_elicitation_metad call_id, REQUEST_PLUGIN_INSTALL_TOOL_NAME, &serde_json::to_string(&json!({ - "tool_type": "plugin", - "action_type": "install", - "tool_id": "github@openai-curated-remote", + "plugin_id": "github@openai-curated-remote", "suggest_reason": "Use GitHub for this request" }))?, ), diff --git a/codex-rs/tools/src/request_plugin_install.rs b/codex-rs/tools/src/request_plugin_install.rs index 5d6352b7f..7bc0f6d4c 100644 --- a/codex-rs/tools/src/request_plugin_install.rs +++ b/codex-rs/tools/src/request_plugin_install.rs @@ -57,7 +57,6 @@ pub fn build_request_plugin_install_elicitation_request( server_name: &str, thread_id: String, turn_id: String, - args: &RequestPluginInstallArgs, suggest_reason: &str, tool: &DiscoverableTool, ) -> McpServerElicitationRequestParams { @@ -69,8 +68,6 @@ pub fn build_request_plugin_install_elicitation_request( server_name: server_name.to_string(), request: McpServerElicitationRequest::Form { meta: Some(json!(build_request_plugin_install_meta( - args.tool_type, - args.action_type, suggest_reason, tool, ))), @@ -105,14 +102,13 @@ pub fn verified_connector_install_completed( } fn build_request_plugin_install_meta<'a>( - tool_type: DiscoverableToolType, - action_type: DiscoverableToolAction, suggest_reason: &'a str, tool: &'a DiscoverableTool, ) -> RequestPluginInstallMeta<'a> { - let (remote_plugin_id, app_connector_ids) = match tool { - DiscoverableTool::Connector(_) => (None, None), + let (tool_type, remote_plugin_id, app_connector_ids) = match tool { + DiscoverableTool::Connector(_) => (DiscoverableToolType::Connector, None, None), DiscoverableTool::Plugin(plugin) => ( + DiscoverableToolType::Plugin, plugin.remote_plugin_id.as_deref(), Some(plugin.app_connector_ids.as_slice()), ), @@ -121,7 +117,7 @@ fn build_request_plugin_install_meta<'a>( codex_approval_kind: REQUEST_PLUGIN_INSTALL_APPROVAL_KIND_VALUE, persist: REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE, tool_type, - suggest_type: action_type, + suggest_type: DiscoverableToolAction::Install, suggest_reason, tool_id: tool.id(), tool_name: tool.name(), diff --git a/codex-rs/tools/src/request_plugin_install_tests.rs b/codex-rs/tools/src/request_plugin_install_tests.rs index ab8af9be6..400126dc6 100644 --- a/codex-rs/tools/src/request_plugin_install_tests.rs +++ b/codex-rs/tools/src/request_plugin_install_tests.rs @@ -5,12 +5,6 @@ use serde_json::json; #[test] fn build_request_plugin_install_elicitation_request_uses_expected_shape() { - let args = RequestPluginInstallArgs { - tool_type: DiscoverableToolType::Connector, - action_type: DiscoverableToolAction::Install, - tool_id: "connector_2128aebfecb84f64a069897515042a44".to_string(), - suggest_reason: "Plan and reference events from your calendar".to_string(), - }; let connector = DiscoverableTool::Connector(Box::new(AppInfo { id: "connector_2128aebfecb84f64a069897515042a44".to_string(), name: "Google Calendar".to_string(), @@ -34,7 +28,6 @@ fn build_request_plugin_install_elicitation_request_uses_expected_shape() { "codex-apps", "thread-1".to_string(), "turn-1".to_string(), - &args, "Plan and reference events from your calendar", &connector, ); @@ -74,12 +67,6 @@ fn build_request_plugin_install_elicitation_request_uses_expected_shape() { #[test] fn build_request_plugin_install_elicitation_request_injects_plugin_metadata() { - let args = RequestPluginInstallArgs { - tool_type: DiscoverableToolType::Plugin, - action_type: DiscoverableToolAction::Install, - tool_id: "sample@openai-curated-remote".to_string(), - suggest_reason: "Use the sample plugin's skills and MCP server".to_string(), - }; let plugin = DiscoverableTool::Plugin(Box::new(DiscoverablePluginInfo { id: "sample@openai-curated-remote".to_string(), remote_plugin_id: Some("plugins~Plugin_sample".to_string()), @@ -94,7 +81,6 @@ fn build_request_plugin_install_elicitation_request_injects_plugin_metadata() { "codex-apps", "thread-1".to_string(), "turn-1".to_string(), - &args, "Use the sample plugin's skills and MCP server", &plugin, ); @@ -149,12 +135,8 @@ fn build_request_plugin_install_meta_uses_expected_shape() { is_enabled: true, plugin_display_names: Vec::new(), })); - let meta = build_request_plugin_install_meta( - DiscoverableToolType::Connector, - DiscoverableToolAction::Install, - "Find and reference emails from your inbox", - &connector, - ); + let meta = + build_request_plugin_install_meta("Find and reference emails from your inbox", &connector); assert_eq!( meta,