Skip to content

[plugins] Pass remote IDs through request_plugin_install #31713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adaley-openai wants to merge 1 commit into from
Closed

[plugins] Pass remote IDs through request_plugin_install #31713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions codex-rs/core/src/tools/handlers/request_plugin_install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use codex_core_plugins::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_rmcp_client::ElicitationAction;
use codex_rmcp_client::ElicitationResponse;
use codex_tools::DiscoverablePluginInfo;
use codex_tools::DiscoverableTool;
use codex_tools::DiscoverableToolAction;
use codex_tools::DiscoverableToolType;
Expand Down Expand Up @@ -45,6 +46,8 @@ use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use crate::tools::router::ToolSuggestPresentation;

const MAX_REMOTE_PLUGIN_ID_BYTES: usize = 256;

#[derive(Debug, Deserialize, PartialEq, Eq)]
struct RecommendedPluginInstallArgs {
#[serde(alias = "tool_id")]
Expand Down Expand Up @@ -146,19 +149,38 @@ impl RequestPluginInstallHandler {
self.discoverable_tools.clone(),
turn.app_server_client_name.as_deref(),
);

let tool = discoverable_tools
.into_iter()
.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(_))
}
let requested_plugin = requested_tool_type == Some(DiscoverableToolType::Plugin)
|| self.presentation == ToolSuggestPresentation::RecommendationContext;

let matching_tool = discoverable_tools.into_iter().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(_))
}
}
});
let direct_remote = matching_tool.is_none();
let tool = matching_tool
.or_else(|| {
(requested_plugin
&& turn.config.plugins_config_input().remote_plugin_enabled
&& !requested_tool_id.trim().is_empty()
&& requested_tool_id.len() <= MAX_REMOTE_PLUGIN_ID_BYTES)
.then(|| {
DiscoverableTool::Plugin(Box::new(DiscoverablePluginInfo {
id: requested_tool_id.clone(),
remote_plugin_id: Some(requested_tool_id.clone()),
name: requested_tool_id.clone(),
description: None,
has_skills: false,
mcp_server_names: Vec::new(),
app_connector_ids: Vec::new(),
}))
})
})
.ok_or_else(|| {
let (argument_name, source) = match self.presentation {
Expand All @@ -174,13 +196,17 @@ impl RequestPluginInstallHandler {
),
};
FunctionCallError::RespondToModel(format!(
"{argument_name} must match one of {source}"
"{argument_name} must match one of {source} or be a non-empty remote plugin id"
))
})?;
let tool_type = tool.tool_type();

let suggestion_id = format!("request_plugin_install_{call_id}");
if let DiscoverableTool::Plugin(plugin) = &tool {
// The analytics contract has no source value for direct remote lookups, so do not
// misattribute them to endpoint recommendations or legacy discovery.
if let DiscoverableTool::Plugin(plugin) = &tool
&& !direct_remote
{
let source = match self.presentation {
ToolSuggestPresentation::ListTool => PluginInstallRequestSource::LegacyDiscovery,
ToolSuggestPresentation::RecommendationContext => {
Expand Down Expand Up @@ -228,8 +254,10 @@ impl RequestPluginInstallHandler {
.as_ref()
.is_some_and(|response| response.action == ElicitationAction::Accept);

let auth = session.services.auth_manager.auth().await;
let completed = if user_confirmed {
let completed = if user_confirmed && direct_remote {
true
} else if user_confirmed {
let auth = session.services.auth_manager.auth().await;
verify_request_plugin_install_completed(&session, &turn, manager, &tool, auth.as_ref())
.await
} else {
Expand Down
23 changes: 14 additions & 9 deletions codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ pub(crate) fn create_request_plugin_install_tool(
),
(
"tool_id".to_string(),
JsonSchema::string(Some("Connector or plugin id to suggest.".to_string())),
JsonSchema::string(Some(
"Connector or plugin id from discovery, or an exact remote plugin id returned by another tool."
.to_string(),
)),
),
(
"suggest_reason".to_string(),
Expand All @@ -45,15 +48,16 @@ pub(crate) fn create_request_plugin_install_tool(
"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."
"# 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, or another tool returns an exact remote plugin id relevant to that request.\n\nDo not use it for adjacent capabilities, broad recommendations, or tools that merely seem useful. Do not guess remote plugin ids. For discoverable tools, pass the returned `tool_type` and `id` through directly. For a remote plugin, pass its exact id as `tool_id` and use `tool_type=\"plugin\"`.\n\nIMPORTANT: DO NOT call this tool in parallel with other tools."

@cravuri-oai cravuri-oai Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing, I'm seeing this:
image

I think this behavior is a bit too restrictive. Maybe changing the wording here so it removes some of the details around another tool needing to return the plugin id.

),
),
ToolSuggestPresentation::RecommendationContext => (
BTreeMap::from([
(
"plugin_id".to_string(),
JsonSchema::string(Some(
"Plugin id from the `<recommended_plugins>` list.".to_string(),
"Plugin id from the `<recommended_plugins>` list, or an exact remote plugin id returned by another tool."
.to_string(),
)),
),
(
Expand All @@ -65,7 +69,7 @@ pub(crate) fn create_request_plugin_install_tool(
),
]),
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(),
"# Suggest a plugin installation\n\nSuggest installing a plugin from the `<recommended_plugins>` list, or by exact remote plugin id returned by another tool, when it would help with the user's current request. Do not guess remote plugin ids. Briefly explain why in `suggest_reason`.".to_string(),
),
};

Expand All @@ -90,8 +94,8 @@ mod tests {
fn create_request_plugin_install_tool_uses_expected_legacy_wire_shape() {
let expected_description = concat!(
"# Request plugin/connector install\n\n",
"Use this tool only after `list_available_plugins_to_install` returns a plugin or connector that exactly matches the user's explicit request.\n\n",
"Do 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\n",
"Use this tool only after `list_available_plugins_to_install` returns a plugin or connector that exactly matches the user's explicit request, or another tool returns an exact remote plugin id relevant to that request.\n\n",
"Do not use it for adjacent capabilities, broad recommendations, or tools that merely seem useful. Do not guess remote plugin ids. For discoverable tools, pass the returned `tool_type` and `id` through directly. For a remote plugin, pass its exact id as `tool_id` and use `tool_type=\"plugin\"`.\n\n",
"IMPORTANT: DO NOT call this tool in parallel with other tools.",
);

Expand Down Expand Up @@ -120,7 +124,7 @@ mod tests {
(
"tool_id".to_string(),
JsonSchema::string(Some(
"Connector or plugin id to suggest."
"Connector or plugin id from discovery, or an exact remote plugin id returned by another tool."
.to_string(),
),),
),
Expand Down Expand Up @@ -148,15 +152,16 @@ mod tests {
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(),
description: "# Suggest a plugin installation\n\nSuggest installing a plugin from the `<recommended_plugins>` list, or by exact remote plugin id returned by another tool, when it would help with the user's current request. Do not guess remote plugin ids. 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(),
"Plugin id from the `<recommended_plugins>` list, or an exact remote plugin id returned by another tool."
.to_string(),
)),
),
(
Expand Down
30 changes: 17 additions & 13 deletions codex-rs/core/src/tools/spec_plan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,20 +771,24 @@ fn add_core_utility_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut
}
}

if tool_suggest_enabled(turn_context)
&& let Some(candidates) = context
.tool_suggest_candidates
.filter(|candidates| !candidates.tools.is_empty())
{
if candidates.presentation == crate::tools::router::ToolSuggestPresentation::ListTool {
planned_tools.add(ListAvailablePluginsToInstallHandler::new(
collect_request_plugin_install_entries(&candidates.tools),
));
if tool_suggest_enabled(turn_context) {
let accepts_remote_plugin_ids = features.enabled(Feature::RemotePlugin)
&& turn_context.app_server_client_name.as_deref() != Some("codex-tui");
if let Some(candidates) = context.tool_suggest_candidates {
if candidates.presentation == crate::tools::router::ToolSuggestPresentation::ListTool
&& !candidates.tools.is_empty()
{
planned_tools.add(ListAvailablePluginsToInstallHandler::new(
collect_request_plugin_install_entries(&candidates.tools),
));
}
if !candidates.tools.is_empty() || accepts_remote_plugin_ids {
planned_tools.add(RequestPluginInstallHandler::new(
candidates.tools.clone(),
candidates.presentation,
));
}
}
planned_tools.add(RequestPluginInstallHandler::new(
candidates.tools.clone(),
candidates.presentation,
));
}

if environment_mode.has_environment() && turn_context.model_info.apply_patch_tool_type.is_some()
Expand Down
4 changes: 3 additions & 1 deletion codex-rs/core/src/tools/spec_plan_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,7 @@ async fn request_plugin_install_requires_all_discovery_features() {
turn,
&[Feature::ToolSuggest, Feature::Apps, Feature::Plugins],
);
set_feature(turn, Feature::RemotePlugin, /*enabled*/ false);
},
ToolPlanInputs {
tool_suggest_candidates,
Expand Down Expand Up @@ -1011,7 +1012,7 @@ async fn request_plugin_install_stays_visible_without_tool_search() {
}

#[tokio::test]
async fn request_plugin_install_description_refers_to_recommended_plugins_hint() {
async fn request_plugin_install_description_accepts_recommendations_and_remote_ids() {
let plan = probe_with(
|turn| {
set_features(
Expand All @@ -1037,6 +1038,7 @@ async fn request_plugin_install_description_refers_to_recommended_plugins_hint()
panic!("expected request_plugin_install function spec");
};
assert!(request_description.contains("the `<recommended_plugins>` list"));
assert!(request_description.contains("exact remote plugin id"));
assert!(!request_description.contains("list_available_plugins_to_install"));
assert!(!request_description.contains("github"));
assert!(has_parameter(request_spec, "plugin_id"));
Expand Down
56 changes: 42 additions & 14 deletions codex-rs/core/tests/suite/request_plugin_install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,11 @@ async fn explicit_false_preserves_legacy_workflow() -> Result<()> {
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids() -> Result<()> {
async fn endpoint_mode_accepts_remote_plugin_ids_outside_recommendations() -> Result<()> {
skip_if_no_network!(Ok(()));

const REMOTE_PLUGIN_ID: &str = "Plugin_slack";

let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
mount_recommendations(
Expand All @@ -357,7 +359,8 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids()
})),
)
.await;
let call_id = "invented-plugin";
let call_id = "remote-plugin";
let suggest_reason = "Use Slack for this request";
let mock = mount_sse_sequence(
&server,
vec![
Expand All @@ -367,8 +370,8 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids()
call_id,
REQUEST_PLUGIN_INSTALL_TOOL_NAME,
&serde_json::to_string(&json!({
"plugin_id": "invented@openai-curated-remote",
"suggest_reason": "Try this"
"plugin_id": REMOTE_PLUGIN_ID,
"suggest_reason": suggest_reason
}))?,
),
ev_completed("resp-1"),
Expand All @@ -383,14 +386,26 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids()
.await;
let test = build_test(&server, &apps_server).await?;

test.submit_turn("suggest a plugin").await?;
let elicitation = start_install_turn(&test, "suggest a plugin").await?;
let ElicitationRequest::Form {
meta: Some(meta), ..
} = &elicitation.request
else {
panic!("expected form elicitation metadata");
};
assert_eq!(meta["tool_id"], REMOTE_PLUGIN_ID);
assert_eq!(meta["tool_name"], REMOTE_PLUGIN_ID);
assert_eq!(meta["remote_plugin_id"], REMOTE_PLUGIN_ID);
assert_eq!(meta["app_connector_ids"], json!([]));
resolve_install_elicitation(&test, elicitation, ElicitationAction::Accept).await?;

let requests = mock.requests();
assert_eq!(requests.len(), 2);
let contextual_user_message = requests[0].message_input_texts("user").join("\n");
assert!(contextual_user_message.contains("<recommended_plugins>"));
assert!(contextual_user_message.contains("github@openai-curated-remote"));
assert!(contextual_user_message.contains("google-calendar@openai-curated-remote"));
assert!(!contextual_user_message.contains(REMOTE_PLUGIN_ID));
let body = requests[0].body_json();
let tools = tool_names(&body);
assert!(
Expand All @@ -403,10 +418,22 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids()
.iter()
.any(|name| name == REQUEST_PLUGIN_INSTALL_TOOL_NAME)
);
let output = requests[1]
.function_call_output_text(call_id)
.expect("request tool output");
assert!(output.contains("<recommended_plugins> list"));
assert_eq!(
serde_json::from_str::<Value>(
&requests[1]
.function_call_output_text(call_id)
.expect("request tool output")
)?,
json!({
"completed": true,
"user_confirmed": true,
"tool_type": "plugin",
"action_type": "install",
"tool_id": REMOTE_PLUGIN_ID,
"tool_name": REMOTE_PLUGIN_ID,
"suggest_reason": suggest_reason
})
);
Ok(())
}

Expand Down Expand Up @@ -624,16 +651,16 @@ async fn run_remote_plugin_install_refresh_case(refreshed_tools: RefreshedAppsTo
"the resumed router should reflect the refreshed Apps tools"
);
assert!(
!tool_names(&requests[1].body_json())
tool_names(&requests[1].body_json())
.iter()
.any(|name| name == REQUEST_PLUGIN_INSTALL_TOOL_NAME),
"the refreshed installed-plugin cache should filter the cached recommendation"
"direct remote plugin ids should remain installable after recommendations are filtered"
);
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn endpoint_mode_with_no_eligible_candidates_exposes_no_suggestion_tools() -> Result<()> {
async fn endpoint_mode_with_no_eligible_candidates_keeps_remote_install_tool() -> Result<()> {
skip_if_no_network!(Ok(()));

let server = start_mock_server().await;
Expand Down Expand Up @@ -688,9 +715,10 @@ async fn endpoint_mode_with_no_eligible_candidates_exposes_no_suggestion_tools()
.any(|name| name == LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME)
);
assert!(
!tools
tools
.iter()
.any(|name| name == REQUEST_PLUGIN_INSTALL_TOOL_NAME)
.any(|name| name == REQUEST_PLUGIN_INSTALL_TOOL_NAME),
"remote plugin ids should remain installable without recommendation candidates"
);
Ok(())
}
Loading