agent/codex
1
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] [2/4] Generalize plugin suggestion presentation (#28400)

Browse Source 
Summary
- Add list-backed and developer-context presentations for plugin
suggestion candidates.
- Let tool planning, install validation, and request-tool copy follow
the selected presentation.
- Keep every production caller on the existing list-backed presentation,
preserving the current list tool, request schema, connector behavior,
and model-visible copy.
- Leave developer-context presentation latent until the final PR in the
stack.

Stack
- 2/3, based on #28399.
- Follow-up: #27704 activates endpoint recommendations.

Validation
- `just test -p codex-core request_plugin_install`
- `just test -p codex-core spec_plan`
- `just fix -p codex-core`
- `just fmt`
- `git diff --check`
This commit is contained in:
Alex Daley
2026-06-16 18:44:10 -04:00
committed by GitHub
Unverified
parent 7e735b59ce
commit 587487df9e
9 changed files with 147 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/session/tests.rs
+2 -2
View File
@@ -571,9 +571,9 @@ fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> T
let router = Arc::new(ToolRouter::from_turn_context(
&turn_context,
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
mcp_tools: None,
deferred_mcp_tools: None,
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
@@ -9600,9 +9600,9 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
let router = ToolRouter::from_turn_context(
&turn_context,
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
deferred_mcp_tools,
mcp_tools: Some(tools),
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
codex-rs/core/src/session/turn.rs
+8 -3
View File
@@ -58,6 +58,8 @@ use crate::tools::context::SharedTurnDiffTracker;
use crate::tools::parallel::ToolCallRuntime;
use crate::tools::registry::ToolArgumentDiffConsumer;
use crate::tools::router::ToolRouterParams;
use crate::tools::router::ToolSuggestCandidates;
use crate::tools::router::ToolSuggestPresentation;
use crate::tools::router::extension_tool_executors;
use crate::tools::spec_plan::search_tool_enabled;
use crate::tools::spec_plan::tool_suggest_enabled;
@@ -1190,7 +1192,7 @@ pub(crate) async fn built_tools(
.into_iter()
.map(|connector_id| connector_id.0)
.collect::<Vec<_>>();
let discoverable_tools = async {
let tool_suggest_candidates = async {
if apps_enabled && tool_suggest_enabled(turn_context) {
if let Some(accessible_connectors) = accessible_connectors_with_enabled_state.as_ref() {
match connectors::list_tool_suggest_discoverable_tools_with_auth(
@@ -1208,7 +1210,10 @@ pub(crate) async fn built_tools(
)
}) {
Ok(discoverable_tools) if discoverable_tools.is_empty() => None,
Ok(discoverable_tools) => Some(discoverable_tools),
Ok(discoverable_tools) => Some(ToolSuggestCandidates {
tools: discoverable_tools,
presentation: ToolSuggestPresentation::ListTool,
}),
Err(err) => {
warn!("failed to load discoverable tool suggestions: {err:#}");
None
@@ -1237,7 +1242,7 @@ pub(crate) async fn built_tools(
ToolRouterParams {
mcp_tools,
deferred_mcp_tools,
discoverable_tools,
tool_suggest_candidates,
extension_tool_executors: extension_tool_executors(sess),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
codex-rs/core/src/stream_events_utils_tests.rs
+1 -1
View File
@@ -279,9 +279,9 @@ async fn handle_output_item_done_returns_contributed_last_agent_message() {
let router = Arc::new(ToolRouter::from_turn_context(
&turn_context,
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
mcp_tools: None,
deferred_mcp_tools: None,
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
codex-rs/core/src/tools/handlers/request_plugin_install.rs
+20 -4
View File
@@ -37,14 +37,22 @@ use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_install_tool;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use crate::tools::router::ToolSuggestPresentation;
pub struct RequestPluginInstallHandler {
discoverable_tools: Vec<DiscoverableTool>,
presentation: ToolSuggestPresentation,
}
impl RequestPluginInstallHandler {
pub(crate) fn new(discoverable_tools: Vec<DiscoverableTool>) -> Self {
Self { discoverable_tools }
pub(crate) fn new(
discoverable_tools: Vec<DiscoverableTool>,
presentation: ToolSuggestPresentation,
) -> Self {
Self {
discoverable_tools,
presentation,
}
}
}
@@ -54,7 +62,7 @@ impl ToolExecutor<ToolInvocation> for RequestPluginInstallHandler {
}
fn spec(&self) -> ToolSpec {
create_request_plugin_install_tool()
create_request_plugin_install_tool(self.presentation)
}
fn supports_parallel_tool_calls(&self) -> bool {
@@ -118,8 +126,16 @@ impl RequestPluginInstallHandler {
.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::DeveloperContext => {
"the developer recommendations".to_string()
}
};
FunctionCallError::RespondToModel(format!(
"tool_id must match one of the discoverable tools returned by {LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}"
"tool_id must match one of {source}"
))
})?;
codex-rs/core/src/tools/handlers/request_plugin_install_spec.rs
+28 -6
View File
@@ -5,7 +5,11 @@ use codex_tools::ResponsesApiTool;
use codex_tools::ToolSpec;
use std::collections::BTreeMap;
pub(crate) fn create_request_plugin_install_tool() -> ToolSpec {
use crate::tools::router::ToolSuggestPresentation;
pub(crate) fn create_request_plugin_install_tool(
presentation: ToolSuggestPresentation,
) -> ToolSpec {
let properties = BTreeMap::from([
(
"tool_type".to_string(),
@@ -31,9 +35,13 @@ pub(crate) fn create_request_plugin_install_tool() -> ToolSpec {
),
]);
let description = 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."
);
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::DeveloperContext =>
"# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the developer `<recommended_plugins>` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string(),
};
ToolSpec::Function(ResponsesApiTool {
name: REQUEST_PLUGIN_INSTALL_TOOL_NAME.to_string(),
@@ -62,7 +70,7 @@ mod tests {
use std::collections::BTreeMap;
#[test]
fn create_request_plugin_install_tool_uses_expected_wire_shape() {
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",
@@ -71,7 +79,7 @@ mod tests {
);
assert_eq!(
create_request_plugin_install_tool(),
create_request_plugin_install_tool(ToolSuggestPresentation::ListTool),
ToolSpec::Function(ResponsesApiTool {
name: "request_plugin_install".to_string(),
description: expected_description.to_string(),
@@ -116,4 +124,18 @@ mod tests {
})
);
}
#[test]
fn developer_recommendations_change_only_the_description() {
let mut expected = create_request_plugin_install_tool(ToolSuggestPresentation::ListTool);
let recommendations =
create_request_plugin_install_tool(ToolSuggestPresentation::DeveloperContext);
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 developer `<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);
}
}
codex-rs/core/src/tools/router.rs
+14 -1
View File
@@ -40,11 +40,24 @@ pub struct ToolRouter {
pub(crate) struct ToolRouterParams<'a> {
pub(crate) mcp_tools: Option<Vec<ToolInfo>>,
pub(crate) deferred_mcp_tools: Option<Vec<ToolInfo>>,
pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>,
pub(crate) tool_suggest_candidates: Option<ToolSuggestCandidates>,
pub(crate) extension_tool_executors: Vec<Arc<dyn ToolExecutor<ExtensionToolCall>>>,
pub(crate) dynamic_tools: &'a [DynamicToolSpec],
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum ToolSuggestPresentation {
ListTool,
#[allow(dead_code)]
DeveloperContext,
}
#[derive(Clone, Debug)]
pub(crate) struct ToolSuggestCandidates {
pub(crate) tools: Vec<DiscoverableTool>,
pub(crate) presentation: ToolSuggestPresentation,
}
impl ToolRouter {
pub(crate) fn from_turn_context(
turn_context: &TurnContext,
codex-rs/core/src/tools/router_tests.rs
+5 -5
View File
@@ -114,9 +114,9 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow
let router = ToolRouter::from_turn_context(
&turn,
ToolRouterParams {
tool_suggest_candidates: None,
deferred_mcp_tools: None,
mcp_tools: Some(mcp_tools),
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn.dynamic_tools.as_slice(),
},
@@ -182,6 +182,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> {
let router = ToolRouter::from_turn_context(
&turn,
ToolRouterParams {
tool_suggest_candidates: None,
deferred_mcp_tools: None,
mcp_tools: Some(vec![
mcp_tool_info(
@@ -197,7 +198,6 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> {
"query_with_delay",
),
]),
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn.dynamic_tools.as_slice(),
},
@@ -231,9 +231,9 @@ async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()>
let router = ToolRouter::from_turn_context(
&turn,
ToolRouterParams {
tool_suggest_candidates: None,
deferred_mcp_tools: None,
mcp_tools: None,
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn.dynamic_tools.as_slice(),
},
@@ -286,9 +286,9 @@ async fn specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> {
let router = ToolRouter::from_turn_context(
&turn,
ToolRouterParams {
tool_suggest_candidates: None,
deferred_mcp_tools: None,
mcp_tools: None,
discoverable_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: &dynamic_tools,
},
@@ -349,9 +349,9 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow
let router = ToolRouter::from_turn_context(
&turn,
ToolRouterParams {
tool_suggest_candidates: None,
deferred_mcp_tools: None,
mcp_tools: None,
discoverable_tools: None,
extension_tool_executors: extension_tool_executors(&session),
dynamic_tools: turn.dynamic_tools.as_slice(),
},
codex-rs/core/src/tools/spec_plan.rs
+13 -10
View File
@@ -68,7 +68,6 @@ use codex_protocol::openai_models::ToolMode;
use codex_protocol::protocol::MultiAgentVersion;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_tools::DiscoverableTool;
use codex_tools::ResponsesApiNamespace;
use codex_tools::ResponsesApiNamespaceTool;
use codex_tools::TOOL_SEARCH_TOOL_NAME;
@@ -145,7 +144,7 @@ struct CoreToolPlanContext<'a> {
turn_context: &'a TurnContext,
mcp_tools: Option<&'a [ToolInfo]>,
deferred_mcp_tools: Option<&'a [ToolInfo]>,
discoverable_tools: Option<&'a [DiscoverableTool]>,
tool_suggest_candidates: Option<&'a crate::tools::router::ToolSuggestCandidates>,
extension_tool_executors: &'a [Arc<dyn ToolExecutor<ExtensionToolCall>>],
dynamic_tools: &'a [DynamicToolSpec],
tool_search_handler_cache: &'a ToolSearchHandlerCache,
@@ -173,7 +172,7 @@ fn build_tool_specs_and_registry(
let ToolRouterParams {
mcp_tools,
deferred_mcp_tools,
discoverable_tools,
tool_suggest_candidates,
extension_tool_executors,
dynamic_tools,
} = params;
@@ -183,7 +182,7 @@ fn build_tool_specs_and_registry(
turn_context,
mcp_tools: mcp_tools.as_deref(),
deferred_mcp_tools: deferred_mcp_tools.as_deref(),
discoverable_tools: discoverable_tools.as_deref(),
tool_suggest_candidates: tool_suggest_candidates.as_ref(),
extension_tool_executors: &extension_tool_executors,
dynamic_tools,
tool_search_handler_cache,
@@ -672,14 +671,18 @@ fn add_core_utility_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut
}
if tool_suggest_enabled(turn_context)
&& let Some(discoverable_tools) =
context.discoverable_tools.filter(|tools| !tools.is_empty())
&& let Some(candidates) = context
.tool_suggest_candidates
.filter(|candidates| !candidates.tools.is_empty())
{
planned_tools.add(ListAvailablePluginsToInstallHandler::new(
collect_request_plugin_install_entries(discoverable_tools),
));
if candidates.presentation == crate::tools::router::ToolSuggestPresentation::ListTool {
planned_tools.add(ListAvailablePluginsToInstallHandler::new(
collect_request_plugin_install_entries(&candidates.tools),
));
}
planned_tools.add(RequestPluginInstallHandler::new(
discoverable_tools.to_vec(),
candidates.tools.clone(),
candidates.presentation,
));
}
codex-rs/core/src/tools/spec_plan_tests.rs
+56 -47
View File
@@ -36,12 +36,14 @@ use crate::tools::handlers::ToolSearchHandlerCache;
use crate::tools::handlers::multi_agents_spec::MULTI_AGENT_V1_NAMESPACE;
use crate::tools::router::ToolRouter;
use crate::tools::router::ToolRouterParams;
use crate::tools::router::ToolSuggestCandidates;
use crate::tools::router::ToolSuggestPresentation;
#[derive(Default)]
struct ToolPlanInputs {
mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<Vec<ToolInfo>>,
discoverable_tools: Option<Vec<DiscoverableTool>>,
tool_suggest_candidates: Option<ToolSuggestCandidates>,
extension_tool_executors: Vec<Arc<dyn ToolExecutor<ExtensionToolCall>>>,
dynamic_tools: Vec<DynamicToolSpec>,
}
@@ -179,9 +181,9 @@ async fn probe_with(
let router = ToolRouter::from_turn_context(
&turn,
ToolRouterParams {
tool_suggest_candidates: inputs.tool_suggest_candidates,
mcp_tools: inputs.mcp_tools,
deferred_mcp_tools: inputs.deferred_mcp_tools,
discoverable_tools: inputs.discoverable_tools,
extension_tool_executors: inputs.extension_tool_executors,
dynamic_tools: inputs.dynamic_tools.as_slice(),
},
@@ -410,17 +412,19 @@ fn dynamic_tool(namespace: Option<&str>, name: &str, defer_loading: bool) -> Dyn
}
}
fn discoverable_plugin(id: &str, name: &str) -> DiscoverableTool {
DiscoverablePluginInfo {
id: id.to_string(),
remote_plugin_id: None,
name: name.to_string(),
description: Some(format!("{name} plugin")),
has_skills: false,
mcp_server_names: Vec::new(),
app_connector_ids: Vec::new(),
fn plugin_candidates(presentation: ToolSuggestPresentation) -> ToolSuggestCandidates {
ToolSuggestCandidates {
tools: vec![DiscoverableTool::Plugin(Box::new(DiscoverablePluginInfo {
id: "github@openai-curated-remote".to_string(),
remote_plugin_id: None,
name: "GitHub".to_string(),
description: Some("Work with GitHub repositories".to_string()),
has_skills: true,
mcp_server_names: Vec::new(),
app_connector_ids: Vec::new(),
}))],
presentation,
}
.into()
}
fn has_parameter(spec: &ToolSpec, parameter_name: &str) -> bool {
@@ -793,7 +797,7 @@ async fn tool_search_cache_rebuilds_when_deferred_sources_change() {
ToolRouterParams {
mcp_tools: None,
deferred_mcp_tools: Some(vec![mcp_tool("first", "mcp__first", "lookup")]),
discoverable_tools: None,
tool_suggest_candidates: None,
extension_tool_executors: Vec::new(),
dynamic_tools: &[],
},
@@ -808,7 +812,7 @@ async fn tool_search_cache_rebuilds_when_deferred_sources_change() {
ToolRouterParams {
mcp_tools: None,
deferred_mcp_tools: Some(vec![mcp_tool("second", "mcp__second", "lookup")]),
discoverable_tools: None,
tool_suggest_candidates: None,
extension_tool_executors: Vec::new(),
dynamic_tools: &[],
},
@@ -853,8 +857,7 @@ async fn invalid_mcp_tools_are_not_registered() {
}
#[tokio::test]
async fn request_plugin_install_requires_all_discovery_features_and_discoverable_tools() {
let discoverable_tools = Some(vec![discoverable_plugin("github", "GitHub")]);
async fn request_plugin_install_requires_all_discovery_features() {
for disabled_feature in [Feature::ToolSuggest, Feature::Apps, Feature::Plugins] {
let plan = probe_with(
|turn| {
@@ -865,7 +868,7 @@ async fn request_plugin_install_requires_all_discovery_features_and_discoverable
set_feature(turn, disabled_feature, /*enabled*/ false);
},
ToolPlanInputs {
discoverable_tools: discoverable_tools.clone(),
tool_suggest_candidates: Some(plugin_candidates(ToolSuggestPresentation::ListTool)),
..ToolPlanInputs::default()
},
)
@@ -876,17 +879,31 @@ async fn request_plugin_install_requires_all_discovery_features_and_discoverable
]);
}
let no_candidates = probe(|turn| {
set_features(
turn,
&[Feature::ToolSuggest, Feature::Apps, Feature::Plugins],
);
})
.await;
no_candidates.assert_visible_lacks(&[
"list_available_plugins_to_install",
"request_plugin_install",
]);
for tool_suggest_candidates in [
None,
Some(ToolSuggestCandidates {
tools: Vec::new(),
presentation: ToolSuggestPresentation::DeveloperContext,
}),
] {
let plan = probe_with(
|turn| {
set_features(
turn,
&[Feature::ToolSuggest, Feature::Apps, Feature::Plugins],
);
},
ToolPlanInputs {
tool_suggest_candidates,
..ToolPlanInputs::default()
},
)
.await;
plan.assert_visible_lacks(&[
"list_available_plugins_to_install",
"request_plugin_install",
]);
}
let enabled = probe_with(
|turn| {
@@ -896,7 +913,7 @@ async fn request_plugin_install_requires_all_discovery_features_and_discoverable
);
},
ToolPlanInputs {
discoverable_tools,
tool_suggest_candidates: Some(plugin_candidates(ToolSuggestPresentation::ListTool)),
..ToolPlanInputs::default()
},
)
@@ -908,7 +925,7 @@ async fn request_plugin_install_requires_all_discovery_features_and_discoverable
}
#[tokio::test]
async fn install_suggestion_tools_stay_visible_without_tool_search() {
async fn request_plugin_install_stays_visible_without_tool_search() {
let plan = probe_with(
|turn| {
turn.model_info.supports_search_tool = false;
@@ -918,7 +935,7 @@ async fn install_suggestion_tools_stay_visible_without_tool_search() {
);
},
ToolPlanInputs {
discoverable_tools: Some(vec![discoverable_plugin("github", "GitHub")]),
tool_suggest_candidates: Some(plugin_candidates(ToolSuggestPresentation::ListTool)),
..ToolPlanInputs::default()
},
)
@@ -932,7 +949,7 @@ async fn install_suggestion_tools_stay_visible_without_tool_search() {
}
#[tokio::test]
async fn request_plugin_install_description_defers_inventory_to_list_tool() {
async fn request_plugin_install_description_refers_to_recommended_plugins_hint() {
let plan = probe_with(
|turn| {
set_features(
@@ -941,23 +958,14 @@ async fn request_plugin_install_description_defers_inventory_to_list_tool() {
);
},
ToolPlanInputs {
discoverable_tools: Some(vec![discoverable_plugin("github", "GitHub")]),
tool_suggest_candidates: Some(plugin_candidates(
ToolSuggestPresentation::DeveloperContext,
)),
..ToolPlanInputs::default()
},
)
.await;
let ToolSpec::Function(ResponsesApiTool {
description: list_description,
..
}) = plan.visible_spec("list_available_plugins_to_install")
else {
panic!("expected list_available_plugins_to_install function spec");
};
assert!(list_description.contains(
"Returns known plugins and connectors that can be passed to `request_plugin_install`."
));
let ToolSpec::Function(ResponsesApiTool {
description: request_description,
..
@@ -965,10 +973,11 @@ async fn request_plugin_install_description_defers_inventory_to_list_tool() {
else {
panic!("expected request_plugin_install function spec");
};
assert!(request_description.contains(
"Use this tool only after `list_available_plugins_to_install` returns a plugin or connector that exactly matches the user's explicit request."
));
assert!(request_description.contains("developer `<recommended_plugins>` list"));
assert!(!request_description.contains("list_available_plugins_to_install"));
assert!(!request_description.contains("github"));
plan.assert_visible_lacks(&["list_available_plugins_to_install"]);
plan.assert_registered_lacks(&["list_available_plugins_to_install"]);
}
#[tokio::test]