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

[tool_suggest] More prompt polishes. (#20566)

Browse Source 
Tool suggest still misfires when model needs tool_search, updating the
prompts to further disambiguate it:

- [x] rename it from `tool_suggest` to `request_plugin_install`
- [x] rephrase "suggestion" to "install" in the tool descriptions.
- [x] disambiguate "the tool" vs "the plugin/connector". 

Tested with the Codex App and verified it still works.
This commit is contained in:
Matthew Zeng
2026-05-01 21:22:12 -07:00
committed by GitHub
Unverified
parent 127434cd8b
commit f88701f5c8
19 changed files with 253 additions and 232 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/session/turn.rs
+2 -2
View File
@@ -97,7 +97,7 @@ use codex_protocol::protocol::TurnDiffEvent;
use codex_protocol::protocol::WarningEvent;
use codex_protocol::user_input::UserInput;
use codex_tools::ToolName;
use codex_tools::filter_tool_suggest_discoverable_tools_for_client;
use codex_tools::filter_request_plugin_install_discoverable_tools_for_client;
use codex_utils_stream_parser::AssistantTextChunk;
use codex_utils_stream_parser::AssistantTextStreamParser;
use codex_utils_stream_parser::ProposedPlanSegment;
@@ -1170,7 +1170,7 @@ pub(crate) async fn built_tools(
)
.await
.map(|discoverable_tools| {
filter_tool_suggest_discoverable_tools_for_client(
filter_request_plugin_install_discoverable_tools_for_client(
discoverable_tools,
turn_context.app_server_client_name.as_deref(),
)
codex-rs/core/src/tools/handlers/mod.rs
+2 -2
View File
@@ -10,11 +10,11 @@ pub(crate) mod multi_agents_common;
pub(crate) mod multi_agents_v2;
mod plan;
mod request_permissions;
mod request_plugin_install;
mod request_user_input;
mod shell;
mod test_sync;
mod tool_search;
mod tool_suggest;
mod unavailable_tool;
pub(crate) mod unified_exec;
mod view_image;
@@ -43,12 +43,12 @@ pub use mcp::McpHandler;
pub use mcp_resource::McpResourceHandler;
pub use plan::PlanHandler;
pub use request_permissions::RequestPermissionsHandler;
pub use request_plugin_install::RequestPluginInstallHandler;
pub use request_user_input::RequestUserInputHandler;
pub use shell::ShellCommandHandler;
pub use shell::ShellHandler;
pub use test_sync::TestSyncHandler;
pub use tool_search::ToolSearchHandler;
pub use tool_suggest::ToolSuggestHandler;
pub use unavailable_tool::UnavailableToolHandler;
pub(crate) use unavailable_tool::unavailable_tool_message;
pub use unified_exec::UnifiedExecHandler;
codex-rs/core/src/tools/handlers/tool_suggest.rs → codex-rs/core/src/tools/handlers/request_plugin_install.rs
+47 -44
View File
@@ -8,15 +8,15 @@ use codex_rmcp_client::ElicitationResponse;
use codex_tools::DiscoverableTool;
use codex_tools::DiscoverableToolAction;
use codex_tools::DiscoverableToolType;
use codex_tools::TOOL_SUGGEST_PERSIST_ALWAYS_VALUE;
use codex_tools::TOOL_SUGGEST_PERSIST_KEY;
use codex_tools::TOOL_SUGGEST_TOOL_NAME;
use codex_tools::ToolSuggestArgs;
use codex_tools::ToolSuggestResult;
use codex_tools::all_suggested_connectors_picked_up;
use codex_tools::build_tool_suggestion_elicitation_request;
use codex_tools::filter_tool_suggest_discoverable_tools_for_client;
use codex_tools::verified_connector_suggestion_completed;
use codex_tools::REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE;
use codex_tools::REQUEST_PLUGIN_INSTALL_PERSIST_KEY;
use codex_tools::REQUEST_PLUGIN_INSTALL_TOOL_NAME;
use codex_tools::RequestPluginInstallArgs;
use codex_tools::RequestPluginInstallResult;
use codex_tools::all_requested_connectors_picked_up;
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_json::Value;
use tracing::warn;
@@ -32,9 +32,9 @@ use crate::tools::handlers::parse_arguments;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
pub struct ToolSuggestHandler;
pub struct RequestPluginInstallHandler;
impl ToolHandler for ToolSuggestHandler {
impl ToolHandler for RequestPluginInstallHandler {
type Output = FunctionToolOutput;
fn kind(&self) -> ToolKind {
@@ -43,7 +43,7 @@ impl ToolHandler for ToolSuggestHandler {
#[expect(
clippy::await_holding_invalid_type,
reason = "tool suggestion discovery reads through the session-owned manager guard"
reason = "plugin install discovery reads through the session-owned manager guard"
)]
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
@@ -58,12 +58,12 @@ impl ToolHandler for ToolSuggestHandler {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::Fatal(format!(
"{TOOL_SUGGEST_TOOL_NAME} handler received unsupported payload"
"{REQUEST_PLUGIN_INSTALL_TOOL_NAME} handler received unsupported payload"
)));
}
};
let args: ToolSuggestArgs = parse_arguments(&arguments)?;
let args: RequestPluginInstallArgs = parse_arguments(&arguments)?;
let suggest_reason = args.suggest_reason.trim();
if suggest_reason.is_empty() {
return Err(FunctionCallError::RespondToModel(
@@ -72,14 +72,15 @@ impl ToolHandler for ToolSuggestHandler {
}
if args.action_type != DiscoverableToolAction::Install {
return Err(FunctionCallError::RespondToModel(
"tool suggestions currently support only action_type=\"install\"".to_string(),
"plugin install requests currently support only action_type=\"install\""
.to_string(),
));
}
if args.tool_type == DiscoverableToolType::Plugin
&& turn.app_server_client_name.as_deref() == Some("codex-tui")
{
return Err(FunctionCallError::RespondToModel(
"plugin tool suggestions are not available in codex-tui yet".to_string(),
"plugin install requests are not available in codex-tui yet".to_string(),
));
}
@@ -98,14 +99,14 @@ impl ToolHandler for ToolSuggestHandler {
)
.await
.map(|discoverable_tools| {
filter_tool_suggest_discoverable_tools_for_client(
filter_request_plugin_install_discoverable_tools_for_client(
discoverable_tools,
turn.app_server_client_name.as_deref(),
)
})
.map_err(|err| {
FunctionCallError::RespondToModel(format!(
"tool suggestions are unavailable right now: {err}"
"plugin install requests are unavailable right now: {err}"
))
})?;
@@ -114,12 +115,12 @@ impl ToolHandler for ToolSuggestHandler {
.find(|tool| tool.tool_type() == args.tool_type && tool.id() == args.tool_id)
.ok_or_else(|| {
FunctionCallError::RespondToModel(format!(
"tool_id must match one of the discoverable tools exposed by {TOOL_SUGGEST_TOOL_NAME}"
"tool_id must match one of the discoverable tools exposed by {REQUEST_PLUGIN_INSTALL_TOOL_NAME}"
))
})?;
let request_id = RequestId::String(format!("tool_suggestion_{call_id}").into());
let params = build_tool_suggestion_elicitation_request(
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.conversation_id.to_string(),
turn.sub_id.clone(),
@@ -131,14 +132,14 @@ impl ToolHandler for ToolSuggestHandler {
.request_mcp_server_elicitation(turn.as_ref(), request_id, params)
.await;
if let Some(response) = response.as_ref() {
maybe_persist_tool_suggest_disable(&session, &turn, &tool, response).await;
maybe_persist_disabled_install_request(&session, &turn, &tool, response).await;
}
let user_confirmed = response
.as_ref()
.is_some_and(|response| response.action == ElicitationAction::Accept);
let completed = if user_confirmed {
verify_tool_suggestion_completed(&session, &turn, &tool, auth.as_ref()).await
verify_request_plugin_install_completed(&session, &turn, &tool, auth.as_ref()).await
} else {
false
};
@@ -149,7 +150,7 @@ impl ToolHandler for ToolSuggestHandler {
.await;
}
let content = serde_json::to_string(&ToolSuggestResult {
let content = serde_json::to_string(&RequestPluginInstallResult {
completed,
user_confirmed,
tool_type: args.tool_type,
@@ -160,7 +161,7 @@ impl ToolHandler for ToolSuggestHandler {
})
.map_err(|err| {
FunctionCallError::Fatal(format!(
"failed to serialize {TOOL_SUGGEST_TOOL_NAME} response: {err}"
"failed to serialize {REQUEST_PLUGIN_INSTALL_TOOL_NAME} response: {err}"
))
})?;
@@ -168,17 +169,17 @@ impl ToolHandler for ToolSuggestHandler {
}
}
async fn maybe_persist_tool_suggest_disable(
async fn maybe_persist_disabled_install_request(
session: &crate::session::session::Session,
turn: &crate::session::turn_context::TurnContext,
tool: &DiscoverableTool,
response: &ElicitationResponse,
) {
if !tool_suggest_response_requests_persistent_disable(response) {
if !request_plugin_install_response_requests_persistent_disable(response) {
return;
}
if let Err(err) = persist_tool_suggest_disable(&turn.config.codex_home, tool).await {
if let Err(err) = persist_disabled_install_request(&turn.config.codex_home, tool).await {
warn!(
error = %err,
tool_id = tool.id(),
@@ -190,7 +191,9 @@ async fn maybe_persist_tool_suggest_disable(
session.reload_user_config_layer().await;
}
fn tool_suggest_response_requests_persistent_disable(response: &ElicitationResponse) -> bool {
fn request_plugin_install_response_requests_persistent_disable(
response: &ElicitationResponse,
) -> bool {
if response.action != ElicitationAction::Decline {
return false;
}
@@ -199,24 +202,24 @@ fn tool_suggest_response_requests_persistent_disable(response: &ElicitationRespo
.meta
.as_ref()
.and_then(Value::as_object)
.and_then(|meta| meta.get(TOOL_SUGGEST_PERSIST_KEY))
.and_then(|meta| meta.get(REQUEST_PLUGIN_INSTALL_PERSIST_KEY))
.and_then(Value::as_str)
== Some(TOOL_SUGGEST_PERSIST_ALWAYS_VALUE)
== Some(REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE)
}
async fn persist_tool_suggest_disable(
async fn persist_disabled_install_request(
codex_home: &codex_utils_absolute_path::AbsolutePathBuf,
tool: &DiscoverableTool,
) -> anyhow::Result<()> {
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::AddToolSuggestDisabledTool(
disabled_tool_suggestion(tool),
disabled_install_request(tool),
)])
.apply()
.await
}
fn disabled_tool_suggestion(tool: &DiscoverableTool) -> ToolSuggestDisabledTool {
fn disabled_install_request(tool: &DiscoverableTool) -> ToolSuggestDisabledTool {
match tool {
DiscoverableTool::Connector(connector) => {
ToolSuggestDisabledTool::connector(connector.id.as_str())
@@ -225,14 +228,14 @@ fn disabled_tool_suggestion(tool: &DiscoverableTool) -> ToolSuggestDisabledTool
}
}
async fn verify_tool_suggestion_completed(
async fn verify_request_plugin_install_completed(
session: &crate::session::session::Session,
turn: &crate::session::turn_context::TurnContext,
tool: &DiscoverableTool,
auth: Option<&codex_login::CodexAuth>,
) -> bool {
match tool {
DiscoverableTool::Connector(connector) => refresh_missing_suggested_connectors(
DiscoverableTool::Connector(connector) => refresh_missing_requested_connectors(
session,
turn,
auth,
@@ -241,17 +244,17 @@ async fn verify_tool_suggestion_completed(
)
.await
.is_some_and(|accessible_connectors| {
verified_connector_suggestion_completed(connector.id.as_str(), &accessible_connectors)
verified_connector_install_completed(connector.id.as_str(), &accessible_connectors)
}),
DiscoverableTool::Plugin(plugin) => {
session.reload_user_config_layer().await;
let config = session.get_config().await;
let completed = verified_plugin_suggestion_completed(
let completed = verified_plugin_install_completed(
plugin.id.as_str(),
config.as_ref(),
session.services.plugins_manager.as_ref(),
);
let _ = refresh_missing_suggested_connectors(
let _ = refresh_missing_requested_connectors(
session,
turn,
auth,
@@ -268,7 +271,7 @@ async fn verify_tool_suggestion_completed(
clippy::await_holding_invalid_type,
reason = "connector cache refresh reads through the session-owned manager guard"
)]
async fn refresh_missing_suggested_connectors(
async fn refresh_missing_requested_connectors(
session: &crate::session::session::Session,
turn: &crate::session::turn_context::TurnContext,
auth: Option<&codex_login::CodexAuth>,
@@ -285,7 +288,7 @@ async fn refresh_missing_suggested_connectors(
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
&turn.config,
);
if all_suggested_connectors_picked_up(expected_connector_ids, &accessible_connectors) {
if all_requested_connectors_picked_up(expected_connector_ids, &accessible_connectors) {
return Some(accessible_connectors);
}
@@ -304,14 +307,14 @@ async fn refresh_missing_suggested_connectors(
}
Err(err) => {
warn!(
"failed to refresh codex apps tools cache after tool suggestion for {tool_id}: {err:#}"
"failed to refresh codex apps tools cache after plugin install request for {tool_id}: {err:#}"
);
None
}
}
}
fn verified_plugin_suggestion_completed(
fn verified_plugin_install_completed(
tool_id: &str,
config: &crate::config::Config,
plugins_manager: &codex_core_plugins::PluginsManager,
@@ -327,5 +330,5 @@ fn verified_plugin_suggestion_completed(
}
#[cfg(test)]
#[path = "tool_suggest_tests.rs"]
#[path = "request_plugin_install_tests.rs"]
mod tests;
codex-rs/core/src/tools/handlers/tool_suggest_tests.rs → codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs
+30 -26
View File
@@ -22,7 +22,7 @@ use serde_json::json;
use tempfile::tempdir;
#[tokio::test]
async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
async fn verified_plugin_install_completed_requires_installed_plugin() {
let codex_home = tempdir().expect("tempdir should succeed");
let curated_root = curated_plugins_repo_path(codex_home.path());
write_openai_curated_marketplace(&curated_root, &["sample"]);
@@ -32,7 +32,7 @@ async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
let config = load_plugins_config(codex_home.path()).await;
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
assert!(!verified_plugin_suggestion_completed(
assert!(!verified_plugin_install_completed(
"sample@openai-curated",
&config,
&plugins_manager,
@@ -50,7 +50,7 @@ async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
.expect("plugin should install");
let refreshed_config = load_plugins_config(codex_home.path()).await;
assert!(verified_plugin_suggestion_completed(
assert!(verified_plugin_install_completed(
"sample@openai-curated",
&refreshed_config,
&plugins_manager,
@@ -58,43 +58,47 @@ async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
}
#[test]
fn tool_suggest_response_persists_only_decline_always_mode() {
assert!(tool_suggest_response_requests_persistent_disable(
fn request_plugin_install_response_persists_only_decline_always_mode() {
assert!(request_plugin_install_response_requests_persistent_disable(
&ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
meta: Some(json!({ TOOL_SUGGEST_PERSIST_KEY: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE })),
meta: Some(json!({
REQUEST_PLUGIN_INSTALL_PERSIST_KEY: REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE
})),
}
));
assert!(!tool_suggest_response_requests_persistent_disable(
&ElicitationResponse {
assert!(
!request_plugin_install_response_requests_persistent_disable(&ElicitationResponse {
action: ElicitationAction::Accept,
content: None,
meta: Some(json!({ TOOL_SUGGEST_PERSIST_KEY: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE })),
}
));
assert!(!tool_suggest_response_requests_persistent_disable(
&ElicitationResponse {
meta: Some(json!({
REQUEST_PLUGIN_INSTALL_PERSIST_KEY: REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE
})),
})
);
assert!(
!request_plugin_install_response_requests_persistent_disable(&ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
meta: Some(json!({ TOOL_SUGGEST_PERSIST_KEY: "session" })),
}
));
assert!(!tool_suggest_response_requests_persistent_disable(
&ElicitationResponse {
meta: Some(json!({ REQUEST_PLUGIN_INSTALL_PERSIST_KEY: "session" })),
})
);
assert!(
!request_plugin_install_response_requests_persistent_disable(&ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
meta: None,
}
));
})
);
}
#[tokio::test]
async fn persist_tool_suggest_disable_writes_connector_config() {
async fn persist_disabled_install_request_writes_connector_config() {
let codex_home = tempdir().expect("tempdir should succeed");
let tool = connector_tool("connector_calendar", "Google Calendar");
persist_tool_suggest_disable(&codex_home.path().abs(), &tool)
persist_disabled_install_request(&codex_home.path().abs(), &tool)
.await
.expect("persist connector disable");
@@ -111,7 +115,7 @@ async fn persist_tool_suggest_disable_writes_connector_config() {
}
#[tokio::test]
async fn persist_tool_suggest_disable_writes_plugin_config() {
async fn persist_disabled_install_request_writes_plugin_config() {
let codex_home = tempdir().expect("tempdir should succeed");
let tool = DiscoverableTool::Plugin(Box::new(DiscoverablePluginInfo {
id: "slack@openai-curated".to_string(),
@@ -122,7 +126,7 @@ async fn persist_tool_suggest_disable_writes_plugin_config() {
app_connector_ids: Vec::new(),
}));
persist_tool_suggest_disable(&codex_home.path().abs(), &tool)
persist_disabled_install_request(&codex_home.path().abs(), &tool)
.await
.expect("persist plugin disable");
@@ -139,7 +143,7 @@ async fn persist_tool_suggest_disable_writes_plugin_config() {
}
#[tokio::test]
async fn persist_tool_suggest_disable_dedupes_existing_disabled_tools() {
async fn persist_disabled_install_request_dedupes_existing_disabled_tools() {
let codex_home = tempdir().expect("tempdir should succeed");
let tool = connector_tool("connector_calendar", "Google Calendar");
std::fs::write(
@@ -169,7 +173,7 @@ id = "slack@openai-curated"
)
.expect("write config");
persist_tool_suggest_disable(&codex_home.path().abs(), &tool)
persist_disabled_install_request(&codex_home.path().abs(), &tool)
.await
.expect("persist connector disable");
codex-rs/core/src/tools/spec.rs
+4 -4
View File
@@ -86,12 +86,12 @@ pub(crate) fn build_specs_with_discoverable_tools(
use crate::tools::handlers::McpResourceHandler;
use crate::tools::handlers::PlanHandler;
use crate::tools::handlers::RequestPermissionsHandler;
use crate::tools::handlers::RequestPluginInstallHandler;
use crate::tools::handlers::RequestUserInputHandler;
use crate::tools::handlers::ShellCommandHandler;
use crate::tools::handlers::ShellHandler;
use crate::tools::handlers::TestSyncHandler;
use crate::tools::handlers::ToolSearchHandler;
use crate::tools::handlers::ToolSuggestHandler;
use crate::tools::handlers::UnavailableToolHandler;
use crate::tools::handlers::UnifiedExecHandler;
use crate::tools::handlers::ViewImageHandler;
@@ -174,7 +174,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
.cloned()
.collect::<Vec<_>>();
let mut tool_search_handler = None;
let tool_suggest_handler = Arc::new(ToolSuggestHandler);
let request_plugin_install_handler = Arc::new(RequestPluginInstallHandler);
let code_mode_handler = Arc::new(CodeModeExecuteHandler);
let code_mode_wait_handler = Arc::new(CodeModeWaitHandler);
let unavailable_tool_handler = Arc::new(UnavailableToolHandler);
@@ -281,8 +281,8 @@ pub(crate) fn build_specs_with_discoverable_tools(
builder.register_handler(handler.name, tool_search_handler.clone());
}
}
ToolHandlerKind::ToolSuggest => {
builder.register_handler(handler.name, tool_suggest_handler.clone());
ToolHandlerKind::RequestPluginInstall => {
builder.register_handler(handler.name, request_plugin_install_handler.clone());
}
ToolHandlerKind::UnifiedExec => {
builder.register_handler(handler.name, unified_exec_handler.clone());
codex-rs/core/src/tools/spec_tests.rs
+3 -3
View File
@@ -21,11 +21,11 @@ use codex_tools::ConfiguredToolSpec;
use codex_tools::DiscoverableTool;
use codex_tools::JsonSchema;
use codex_tools::LoadableToolSpec;
use codex_tools::REQUEST_PLUGIN_INSTALL_TOOL_NAME;
use codex_tools::ResponsesApiNamespaceTool;
use codex_tools::ResponsesApiTool;
use codex_tools::ShellCommandBackendConfig;
use codex_tools::TOOL_SEARCH_TOOL_NAME;
use codex_tools::TOOL_SUGGEST_TOOL_NAME;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use codex_tools::ToolsConfig;
@@ -791,7 +791,7 @@ async fn multi_agent_v2_wait_agent_schema_uses_configured_min_timeout() {
}
#[tokio::test]
async fn tool_suggest_requires_apps_and_plugins_features() {
async fn request_plugin_install_requires_apps_and_plugins_features() {
let model_info = search_capable_model_info().await;
let discoverable_tools = Some(vec![discoverable_connector(
"connector_2128aebfecb84f64a069897515042a44",
@@ -831,7 +831,7 @@ async fn tool_suggest_requires_apps_and_plugins_features() {
assert!(
!tools
.iter()
.any(|tool| tool.name() == TOOL_SUGGEST_TOOL_NAME),
.any(|tool| tool.name() == REQUEST_PLUGIN_INSTALL_TOOL_NAME),
"tool_suggest should be absent when {disabled_feature:?} is disabled"
);
}