mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
[apps][tool_suggest] Remove tool_suggest's dependency on tool search. (#14856)
- [x] Remove tool_suggest's dependency on tool search.
This commit is contained in:
@@ -12,7 +12,7 @@ pub(crate) fn render_apps_section(connectors: &[AppInfo]) -> Option<String> {
|
||||
}
|
||||
|
||||
let body = format!(
|
||||
"## Apps (Connectors)\nApps (Connectors) can be explicitly triggered in user messages in the format `[$app-name](app://{{connector_id}})`. Apps can also be implicitly triggered as long as the context suggests usage of available apps, the available apps will be listed by the `tool_search` tool.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nAn installed app's MCP tools are either provided to you already, or can be lazy-loaded through the `tool_search` tool.\nDo not additionally call list_mcp_resources or list_mcp_resource_templates for apps."
|
||||
"## Apps (Connectors)\nApps (Connectors) can be explicitly triggered in user messages in the format `[$app-name](app://{{connector_id}})`. Apps can also be implicitly triggered as long as the context suggests usage of available apps.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nAn installed app's MCP tools are either provided to you already, or can be lazy-loaded through the `tool_search` tool. If `tool_search` is available, the apps that are searchable by `tools_search` will be listed by it.\nDo not additionally call list_mcp_resources or list_mcp_resource_templates for apps."
|
||||
);
|
||||
Some(format!(
|
||||
"{APPS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{APPS_INSTRUCTIONS_CLOSE_TAG}"
|
||||
|
||||
@@ -6615,10 +6615,7 @@ pub(crate) async fn built_tools(
|
||||
None
|
||||
};
|
||||
let auth = sess.services.auth_manager.auth().await;
|
||||
let discoverable_tools = if apps_enabled
|
||||
&& turn_context.tools_config.search_tool
|
||||
&& turn_context.tools_config.tool_suggest
|
||||
{
|
||||
let discoverable_tools = if apps_enabled && turn_context.tools_config.tool_suggest {
|
||||
if let Some(accessible_connectors) = accessible_connectors_with_enabled_state.as_ref() {
|
||||
match connectors::list_tool_suggest_discoverable_tools_with_auth(
|
||||
&turn_context.config,
|
||||
|
||||
@@ -252,45 +252,90 @@ async fn verify_tool_suggestion_completed(
|
||||
auth: Option<&crate::CodexAuth>,
|
||||
) -> bool {
|
||||
match tool {
|
||||
DiscoverableTool::Connector(connector) => {
|
||||
let manager = session.services.mcp_connection_manager.read().await;
|
||||
match manager.hard_refresh_codex_apps_tools_cache().await {
|
||||
Ok(mcp_tools) => {
|
||||
let accessible_connectors = connectors::with_app_enabled_state(
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn.config,
|
||||
);
|
||||
connectors::refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
&turn.config,
|
||||
auth,
|
||||
&mcp_tools,
|
||||
);
|
||||
verified_connector_suggestion_completed(
|
||||
connector.id.as_str(),
|
||||
&accessible_connectors,
|
||||
)
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(
|
||||
"failed to refresh codex apps tools cache after tool suggestion for {}: {err:#}",
|
||||
connector.id
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
DiscoverableTool::Connector(connector) => refresh_missing_suggested_connectors(
|
||||
session,
|
||||
turn,
|
||||
auth,
|
||||
std::slice::from_ref(&connector.id),
|
||||
connector.id.as_str(),
|
||||
)
|
||||
.await
|
||||
.is_some_and(|accessible_connectors| {
|
||||
verified_connector_suggestion_completed(connector.id.as_str(), &accessible_connectors)
|
||||
}),
|
||||
DiscoverableTool::Plugin(plugin) => {
|
||||
session.reload_user_config_layer().await;
|
||||
let config = session.get_config().await;
|
||||
verified_plugin_suggestion_completed(
|
||||
let completed = verified_plugin_suggestion_completed(
|
||||
plugin.id.as_str(),
|
||||
config.as_ref(),
|
||||
session.services.plugins_manager.as_ref(),
|
||||
);
|
||||
let _ = refresh_missing_suggested_connectors(
|
||||
session,
|
||||
turn,
|
||||
auth,
|
||||
&plugin.app_connector_ids,
|
||||
plugin.id.as_str(),
|
||||
)
|
||||
.await;
|
||||
completed
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn refresh_missing_suggested_connectors(
|
||||
session: &crate::codex::Session,
|
||||
turn: &crate::codex::TurnContext,
|
||||
auth: Option<&crate::CodexAuth>,
|
||||
expected_connector_ids: &[String],
|
||||
tool_id: &str,
|
||||
) -> Option<Vec<AppInfo>> {
|
||||
if expected_connector_ids.is_empty() {
|
||||
return Some(Vec::new());
|
||||
}
|
||||
|
||||
let manager = session.services.mcp_connection_manager.read().await;
|
||||
let mcp_tools = manager.list_all_tools().await;
|
||||
let accessible_connectors = connectors::with_app_enabled_state(
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn.config,
|
||||
);
|
||||
if all_suggested_connectors_picked_up(expected_connector_ids, &accessible_connectors) {
|
||||
return Some(accessible_connectors);
|
||||
}
|
||||
|
||||
match manager.hard_refresh_codex_apps_tools_cache().await {
|
||||
Ok(mcp_tools) => {
|
||||
let accessible_connectors = connectors::with_app_enabled_state(
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn.config,
|
||||
);
|
||||
connectors::refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
&turn.config,
|
||||
auth,
|
||||
&mcp_tools,
|
||||
);
|
||||
Some(accessible_connectors)
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(
|
||||
"failed to refresh codex apps tools cache after tool suggestion for {tool_id}: {err:#}"
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn all_suggested_connectors_picked_up(
|
||||
expected_connector_ids: &[String],
|
||||
accessible_connectors: &[AppInfo],
|
||||
) -> bool {
|
||||
expected_connector_ids.iter().all(|connector_id| {
|
||||
verified_connector_suggestion_completed(connector_id, accessible_connectors)
|
||||
})
|
||||
}
|
||||
|
||||
fn verified_connector_suggestion_completed(
|
||||
tool_id: &str,
|
||||
accessible_connectors: &[AppInfo],
|
||||
|
||||
@@ -235,6 +235,34 @@ fn verified_connector_suggestion_completed_requires_accessible_connector() {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn all_suggested_connectors_picked_up_requires_every_expected_connector() {
|
||||
let accessible_connectors = vec![AppInfo {
|
||||
id: "calendar".to_string(),
|
||||
name: "Google Calendar".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: None,
|
||||
is_accessible: true,
|
||||
is_enabled: false,
|
||||
plugin_display_names: Vec::new(),
|
||||
}];
|
||||
|
||||
assert!(all_suggested_connectors_picked_up(
|
||||
&["calendar".to_string()],
|
||||
&accessible_connectors,
|
||||
));
|
||||
assert!(!all_suggested_connectors_picked_up(
|
||||
&["calendar".to_string(), "gmail".to_string()],
|
||||
&accessible_connectors,
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
|
||||
@@ -390,8 +390,7 @@ impl ToolsConfig {
|
||||
include_request_user_input && features.enabled(Feature::DefaultModeRequestUserInput);
|
||||
let include_search_tool =
|
||||
model_info.supports_search_tool && features.enabled(Feature::ToolSearch);
|
||||
let include_tool_suggest = include_search_tool
|
||||
&& features.enabled(Feature::ToolSuggest)
|
||||
let include_tool_suggest = features.enabled(Feature::ToolSuggest)
|
||||
&& features.enabled(Feature::Apps)
|
||||
&& features.enabled(Feature::Plugins);
|
||||
let include_original_image_detail = can_request_original_image_detail(features, model_info);
|
||||
|
||||
@@ -2029,6 +2029,8 @@ fn tool_suggest_is_not_registered_without_feature_flag() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
features.disable(Feature::ToolSuggest);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
@@ -2060,6 +2062,54 @@ fn tool_suggest_is_not_registered_without_feature_flag() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_suggest_can_be_registered_without_search_tool() {
|
||||
let model_info = ModelInfo {
|
||||
supports_search_tool: false,
|
||||
..search_capable_model_info()
|
||||
};
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
features.enable(Feature::ToolSuggest);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
let (tools, _) = build_specs_with_discoverable_tools(
|
||||
&tools_config,
|
||||
None,
|
||||
None,
|
||||
Some(vec![discoverable_connector(
|
||||
"connector_2128aebfecb84f64a069897515042a44",
|
||||
"Google Calendar",
|
||||
"Plan events and schedules.",
|
||||
)]),
|
||||
&[],
|
||||
)
|
||||
.build();
|
||||
|
||||
assert_contains_tool_names(&tools, &[TOOL_SUGGEST_TOOL_NAME]);
|
||||
assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
|
||||
let tool_suggest = find_tool(&tools, TOOL_SUGGEST_TOOL_NAME);
|
||||
let ToolSpec::Function(ResponsesApiTool { description, .. }) = &tool_suggest.spec else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request"
|
||||
)
|
||||
);
|
||||
assert!(description.contains("This includes `tool_search` (if available) and other means."));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_suggest_requires_apps_and_plugins_features() {
|
||||
let model_info = search_capable_model_info();
|
||||
@@ -2315,7 +2365,14 @@ fn tool_suggest_description_lists_discoverable_tools() {
|
||||
assert!(
|
||||
description.contains("skills; MCP servers: sample-docs; app connectors: connector_sample")
|
||||
);
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request"
|
||||
)
|
||||
);
|
||||
assert!(description.contains("This includes `tool_search` (if available) and other means."));
|
||||
assert!(description.contains("DO NOT explore or recommend tools that are not on this list."));
|
||||
assert!(!description.contains("tool_search fails to find a good match"));
|
||||
let JsonSchema::Object { required, .. } = parameters else {
|
||||
panic!("expected object parameters");
|
||||
};
|
||||
|
||||
@@ -3,8 +3,7 @@
|
||||
Suggests a discoverable connector or plugin when the user clearly wants a capability that is not currently available in the active `tools` list.
|
||||
|
||||
Use this ONLY when:
|
||||
- There's no available tool to handle the user's request
|
||||
- And tool_search fails to find a good match
|
||||
- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.
|
||||
- AND the user's request strongly matches one of the discoverable tools listed below.
|
||||
|
||||
Tool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.
|
||||
@@ -14,12 +13,13 @@ Discoverable tools:
|
||||
|
||||
Workflow:
|
||||
|
||||
1. Match the user's request against the discoverable tools list above.
|
||||
2. If one tool clearly fits, call `tool_suggest` with:
|
||||
1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.
|
||||
2. Match the user's request against the discoverable tools list above.
|
||||
3. If one tool clearly fits, call `tool_suggest` with:
|
||||
- `tool_type`: `connector` or `plugin`
|
||||
- `action_type`: `install` or `enable`
|
||||
- `tool_id`: exact id from the discoverable tools list above
|
||||
- `suggest_reason`: concise one-line user-facing reason this tool can help with the current request
|
||||
3. After the suggestion flow completes:
|
||||
4. After the suggestion flow completes:
|
||||
- if the user finished the install or enable flow, continue by searching again or using the newly available tool
|
||||
- if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks you to.
|
||||
|
||||
@@ -130,6 +130,7 @@ mod text_encoding_fix;
|
||||
mod thread_metadata;
|
||||
mod tool_harness;
|
||||
mod tool_parallelism;
|
||||
mod tool_suggest;
|
||||
mod tools;
|
||||
mod truncation;
|
||||
mod turn_state;
|
||||
|
||||
144
codex-rs/core/tests/suite/tool_suggest.rs
Normal file
144
codex-rs/core/tests/suite/tool_suggest.rs
Normal file
@@ -0,0 +1,144 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::types::ToolSuggestDiscoverable;
|
||||
use codex_core::config::types::ToolSuggestDiscoverableType;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use core_test_support::apps_test_server::AppsTestServer;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use serde_json::Value;
|
||||
|
||||
const TOOL_SEARCH_TOOL_NAME: &str = "tool_search";
|
||||
const TOOL_SUGGEST_TOOL_NAME: &str = "tool_suggest";
|
||||
const DISCOVERABLE_GMAIL_ID: &str = "connector_68df038e0ba48191908c8434991bbac2";
|
||||
|
||||
fn tool_names(body: &Value) -> Vec<String> {
|
||||
body.get("tools")
|
||||
.and_then(Value::as_array)
|
||||
.map(|tools| {
|
||||
tools
|
||||
.iter()
|
||||
.filter_map(|tool| {
|
||||
tool.get("name")
|
||||
.or_else(|| tool.get("type"))
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_string)
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn function_tool_description(body: &Value, name: &str) -> Option<String> {
|
||||
body.get("tools")
|
||||
.and_then(Value::as_array)
|
||||
.and_then(|tools| {
|
||||
tools.iter().find_map(|tool| {
|
||||
if tool.get("name").and_then(Value::as_str) == Some(name) {
|
||||
tool.get("description")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_string)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn configure_apps_without_search_tool(config: &mut Config, apps_base_url: &str) {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::Apps)
|
||||
.expect("test config should allow feature update");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::Plugins)
|
||||
.expect("test config should allow feature update");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ToolSuggest)
|
||||
.expect("test config should allow feature update");
|
||||
config.chatgpt_base_url = apps_base_url.to_string();
|
||||
config.model = Some("gpt-5-codex".to_string());
|
||||
config.tool_suggest.discoverables = vec![ToolSuggestDiscoverable {
|
||||
kind: ToolSuggestDiscoverableType::Connector,
|
||||
id: DISCOVERABLE_GMAIL_ID.to_string(),
|
||||
}];
|
||||
|
||||
let mut model_catalog: ModelsResponse =
|
||||
serde_json::from_str(include_str!("../../models.json")).expect("valid models.json");
|
||||
let model = model_catalog
|
||||
.models
|
||||
.iter_mut()
|
||||
.find(|model| model.slug == "gpt-5-codex")
|
||||
.expect("gpt-5-codex exists in bundled models.json");
|
||||
model.supports_search_tool = false;
|
||||
config.model_catalog = Some(model_catalog);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn tool_suggest_is_available_without_search_tool_after_discovery_attempts() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let apps_server = AppsTestServer::mount(&server).await?;
|
||||
let mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
configure_apps_without_search_tool(config, apps_server.chatgpt_base_url.as_str())
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.submit_turn_with_policies(
|
||||
"list tools",
|
||||
AskForApproval::Never,
|
||||
SandboxPolicy::DangerFullAccess,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let body = mock.single_request().body_json();
|
||||
let tools = tool_names(&body);
|
||||
assert!(
|
||||
!tools.iter().any(|name| name == TOOL_SEARCH_TOOL_NAME),
|
||||
"tools list should not include {TOOL_SEARCH_TOOL_NAME}: {tools:?}"
|
||||
);
|
||||
assert!(
|
||||
tools.iter().any(|name| name == TOOL_SUGGEST_TOOL_NAME),
|
||||
"tools list should include {TOOL_SUGGEST_TOOL_NAME}: {tools:?}"
|
||||
);
|
||||
|
||||
let description =
|
||||
function_tool_description(&body, TOOL_SUGGEST_TOOL_NAME).expect("description");
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request"
|
||||
)
|
||||
);
|
||||
assert!(description.contains("This includes `tool_search` (if available) and other means."));
|
||||
assert!(!description.contains("tool_search fails to find a good match"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -717,7 +717,7 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
id: Feature::Apps,
|
||||
key: "apps",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ToolSearch,
|
||||
@@ -729,13 +729,13 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
id: Feature::ToolSuggest,
|
||||
key: "tool_suggest",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Plugins,
|
||||
key: "plugins",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ImageGeneration,
|
||||
@@ -787,7 +787,7 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
id: Feature::ToolCallMcpElicitation,
|
||||
key: "tool_call_mcp_elicitation",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Personality,
|
||||
|
||||
@@ -115,9 +115,9 @@ fn request_permissions_tool_is_under_development() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_suggest_is_stable_and_enabled_by_default() {
|
||||
fn tool_suggest_is_stable_and_disabled_by_default() {
|
||||
assert_eq!(Feature::ToolSuggest.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::ToolSuggest.default_enabled(), true);
|
||||
assert_eq!(Feature::ToolSuggest.default_enabled(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -145,9 +145,9 @@ fn image_generation_is_under_development() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_call_mcp_elicitation_is_stable_and_enabled_by_default() {
|
||||
fn tool_call_mcp_elicitation_is_stable_and_disabled_by_default() {
|
||||
assert_eq!(Feature::ToolCallMcpElicitation.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::ToolCallMcpElicitation.default_enabled(), true);
|
||||
assert_eq!(Feature::ToolCallMcpElicitation.default_enabled(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user