agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

[codex] [4/4] Simplify recommended plugin install schema (#28403)

Browse Source 
## 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`
This commit is contained in:
Alex Daley
2026-06-16 19:44:42 -04:00
committed by GitHub
Unverified
parent 4c79527e31
commit a397b59887
8 changed files with 174 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/tools/handlers/request_plugin_install.rs
+54 -22
View File
@@ -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<DiscoverableTool>,
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 <recommended_plugins> 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 <recommended_plugins> 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(),
codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs
+86 -50
View File
@@ -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 `<recommended_plugins>` 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 `<recommended_plugins>` 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 `<recommended_plugins>` 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 `<recommended_plugins>` 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 `<recommended_plugins>` 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,
})
);
}
}
codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs
+18
View File
@@ -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(
codex-rs/core/src/tools/spec_plan_tests.rs
+7 -1
View File
@@ -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 `<recommended_plugins>` 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"]);
}