mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Filter plugin install suggestions by installed apps (#24996)
## Summary - Keep the original `TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST` as a fallback seed list, so users with no installed plugins still get initial install suggestions. - Allow additional install suggestions from trusted marketplaces: `openai-curated` and `openai-bundled`. - Require non-fallback, non-configured marketplace candidates to share `.app.json` connector IDs with already installed plugins. - Preserve explicit configured plugin discoverables as an override, while still omitting installed, disabled, and `NOT_AVAILABLE` plugins. ## Context `list_available_plugins_to_install` controls which plugins the model can trigger via `request_plugin_install`. We want a small starter set for empty/new users, but we also want installed workflow plugins to unlock relevant source plugins without maintaining every source plugin ID by hand. This keeps the legacy plugin ID allowlist only as the starter fallback. For everything else, the trusted marketplace is the candidate boundary, and installed app connector overlap is the relevance filter. For example, an installed Sales plugin can make HubSpot and Granola suggestible when those source plugins are in `openai-curated` and share Sales app connector IDs, while an unrelated test-source plugin with an app connector not declared by Sales stays hidden. ## Test Coverage - Empty/no-installed-plugin case: returns the fallback seed plugins from the original allowlist. - Installed-app expansion: returns non-fallback marketplace plugins only when their app connector IDs overlap with an installed plugin. - Sales workflow case: installed Sales declares HubSpot and Granola apps, so `hubspot@openai-curated` and `granola@openai-curated` are returned. - Sales negative case: `test-source@openai-curated` has an app connector not declared by Sales, so it is not returned. - Existing guardrails: installed plugins, disabled suggestions, and `NOT_AVAILABLE` plugins remain omitted; explicit configured discoverables still work as an override. ## Validation - `just fmt` - `just test -p codex-core plugins::discoverable::tests` - `just test -p codex-core` was attempted earlier, but current `main` / local env failed with unrelated existing failures around missing `test_stdio_server`, CLI/code-mode MCP tool setup, and unified_exec/shell snapshot flakes/timeouts. The touched discoverable tests pass.
This commit is contained in:
committed by
GitHub
Unverified
parent
a076b21730
commit
8e5f561697
@@ -37,7 +37,15 @@ use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_i
|
||||
use crate::tools::registry::CoreToolRuntime;
|
||||
use crate::tools::registry::ToolExecutor;
|
||||
|
||||
pub struct RequestPluginInstallHandler;
|
||||
pub struct RequestPluginInstallHandler {
|
||||
discoverable_tools: Vec<DiscoverableTool>,
|
||||
}
|
||||
|
||||
impl RequestPluginInstallHandler {
|
||||
pub(crate) fn new(discoverable_tools: Vec<DiscoverableTool>) -> Self {
|
||||
Self { discoverable_tools }
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ToolExecutor<ToolInvocation> for RequestPluginInstallHandler {
|
||||
@@ -53,10 +61,6 @@ impl ToolExecutor<ToolInvocation> for RequestPluginInstallHandler {
|
||||
true
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "plugin install discovery reads through the session-owned manager guard"
|
||||
)]
|
||||
async fn handle(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
@@ -99,31 +103,10 @@ impl ToolExecutor<ToolInvocation> for RequestPluginInstallHandler {
|
||||
));
|
||||
}
|
||||
|
||||
let auth = session.services.auth_manager.auth().await;
|
||||
let manager = session.services.mcp_connection_manager.read().await;
|
||||
let mcp_tools = manager.list_all_tools().await;
|
||||
drop(manager);
|
||||
let accessible_connectors = connectors::with_app_enabled_state(
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn.config,
|
||||
let discoverable_tools = filter_request_plugin_install_discoverable_tools_for_client(
|
||||
self.discoverable_tools.clone(),
|
||||
turn.app_server_client_name.as_deref(),
|
||||
);
|
||||
let discoverable_tools = connectors::list_tool_suggest_discoverable_tools_with_auth(
|
||||
&turn.config,
|
||||
auth.as_ref(),
|
||||
&accessible_connectors,
|
||||
)
|
||||
.await
|
||||
.map(|discoverable_tools| {
|
||||
filter_request_plugin_install_discoverable_tools_for_client(
|
||||
discoverable_tools,
|
||||
turn.app_server_client_name.as_deref(),
|
||||
)
|
||||
})
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
"plugin install requests are unavailable right now: {err}"
|
||||
))
|
||||
})?;
|
||||
|
||||
let tool = discoverable_tools
|
||||
.into_iter()
|
||||
@@ -154,6 +137,7 @@ impl ToolExecutor<ToolInvocation> for RequestPluginInstallHandler {
|
||||
.as_ref()
|
||||
.is_some_and(|response| response.action == ElicitationAction::Accept);
|
||||
|
||||
let auth = session.services.auth_manager.auth().await;
|
||||
let completed = if user_confirmed {
|
||||
verify_request_plugin_install_completed(&session, &turn, &tool, auth.as_ref()).await
|
||||
} else {
|
||||
|
||||
@@ -615,7 +615,9 @@ fn add_core_utility_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut
|
||||
planned_tools.add(ListAvailablePluginsToInstallHandler::new(
|
||||
collect_request_plugin_install_entries(discoverable_tools),
|
||||
));
|
||||
planned_tools.add(RequestPluginInstallHandler);
|
||||
planned_tools.add(RequestPluginInstallHandler::new(
|
||||
discoverable_tools.to_vec(),
|
||||
));
|
||||
}
|
||||
|
||||
if environment_mode.has_environment() && turn_context.model_info.apply_patch_tool_type.is_some()
|
||||
|
||||
Reference in New Issue
Block a user