Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 1 addition & 10 deletions codex-rs/core/src/mcp_tool_exposure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::collections::HashSet;

use codex_connectors::AppToolPolicyEvaluator;
use codex_connectors::AppToolPolicyInput;
use codex_features::Feature;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::ToolInfo as McpToolInfo;
use codex_mcp::tool_is_model_visible;
Expand All @@ -11,8 +10,6 @@ use tracing::instrument;
use crate::config::Config;
use crate::connectors;

pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100;

pub(crate) struct McpToolExposure {
pub(crate) direct_tools: Vec<McpToolInfo>,
pub(crate) deferred_tools: Option<Vec<McpToolInfo>>,
Expand All @@ -34,13 +31,7 @@ pub(crate) fn build_mcp_tool_exposure(
));
}

let should_defer = search_tool_enabled
&& (config
.features
.enabled(Feature::ToolSearchAlwaysDeferMcpTools)
|| deferred_tools.len() >= DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD);

if !should_defer {
if !search_tool_enabled {
return McpToolExposure {
direct_tools: deferred_tools,
deferred_tools: None,
Expand Down
21 changes: 8 additions & 13 deletions codex-rs/core/src/mcp_tool_exposure_test.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::collections::HashSet;
use std::sync::Arc;

use codex_features::Feature;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::ToolInfo;
use codex_tools::ToolName;
Expand Down Expand Up @@ -95,12 +94,12 @@ fn with_visibility(mut tool: ToolInfo, visibility: &[&str]) -> ToolInfo {
}

#[tokio::test]
async fn directly_exposes_small_effective_tool_sets() {
async fn directly_exposes_effective_tool_sets_when_search_is_unavailable() {
let config = test_config().await;
let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1);
let mcp_tools = numbered_mcp_tools(/*count*/ 2);

let exposure = build_mcp_tool_exposure(
&mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ true,
&mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ false,
);

assert_eq!(tool_names(&exposure.direct_tools), tool_names(&mcp_tools));
Expand Down Expand Up @@ -237,9 +236,9 @@ enabled = true
}

#[tokio::test]
async fn searches_large_effective_tool_sets() {
async fn defers_effective_tool_sets_when_search_is_available() {
let config = test_config().await;
let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD);
let mcp_tools = numbered_mcp_tools(/*count*/ 2);

let exposure = build_mcp_tool_exposure(
&mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ true,
Expand All @@ -249,17 +248,13 @@ async fn searches_large_effective_tool_sets() {
let deferred_tools = exposure
.deferred_tools
.as_ref()
.expect("large tool sets should be discoverable through tool_search");
.expect("MCP tools should be discoverable through tool_search");
assert_eq!(tool_names(deferred_tools), tool_names(&mcp_tools));
}

#[tokio::test]
async fn always_defer_feature_defers_apps_too() {
let mut config = test_config().await;
config
.features
.enable(Feature::ToolSearchAlwaysDeferMcpTools)
.expect("test config should allow feature update");
async fn defers_apps_and_non_app_mcp_tools() {
let config = test_config().await;
let mcp_tools = vec![
make_mcp_tool(
"rmcp",
Expand Down
16 changes: 7 additions & 9 deletions codex-rs/core/src/tools/spec_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ fn hosted_model_tool_specs(context: &CoreToolPlanContext<'_>) -> Vec<ToolSpec> {
}

pub(crate) fn search_tool_enabled(turn_context: &TurnContext) -> bool {
turn_context.model_info.supports_search_tool
turn_context.model_info.supports_search_tool && namespace_tools_enabled(turn_context)
}

pub(crate) fn tool_suggest_enabled(turn_context: &TurnContext) -> bool {
Expand Down Expand Up @@ -820,12 +820,11 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mu
} else {
let agent_type_description =
agent_type_description(turn_context, context.default_agent_type_description);
let exposure =
if search_tool_enabled(turn_context) && namespace_tools_enabled(turn_context) {
ToolExposure::Deferred
} else {
ToolExposure::Direct
};
let exposure = if search_tool_enabled(turn_context) {
ToolExposure::Deferred
} else {
ToolExposure::Direct
};
planned_tools.add_with_exposure(
SpawnAgentHandler::new(SpawnAgentToolOptions {
available_models: turn_context.available_models.clone(),
Expand Down Expand Up @@ -943,7 +942,7 @@ fn append_tool_search_executor(
planned_tools: &mut PlannedTools,
) {
let turn_context = context.turn_context;
if !(search_tool_enabled(turn_context) && namespace_tools_enabled(turn_context)) {
if !search_tool_enabled(turn_context) {
return;
}

Expand Down Expand Up @@ -990,7 +989,6 @@ fn append_extension_tool_executors(
reserved_tool_names.insert(ToolName::plain(codex_code_mode::WAIT_TOOL_NAME));
}
if search_tool_enabled(turn_context)
&& namespace_tools_enabled(turn_context)
&& planned_tools
.runtimes()
.iter()
Expand Down
4 changes: 0 additions & 4 deletions codex-rs/core/tests/suite/mcp_tool_exposure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ async fn code_mode_only_exposes_direct_model_only_mcp_namespaces() -> Result<()>
.features
.enable(Feature::CodeModeOnly)
.expect("test config should allow feature update");
config
.features
.enable(Feature::ToolSearchAlwaysDeferMcpTools)
.expect("test config should allow feature update");
config.code_mode.direct_only_tool_namespaces =
vec![SEARCH_CALENDAR_NAMESPACE.to_string()];
});
Expand Down
32 changes: 24 additions & 8 deletions codex-rs/core/tests/suite/openai_file_mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call_with_namespace;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::ev_tool_search_call;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::namespace_child_tool;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::test_codex::TestCodex;
Expand Down Expand Up @@ -142,18 +144,29 @@ async fn run_extract_turn(test: &TestCodex, server: &MockServer) -> Result<Respo
vec![
sse(vec![
ev_response_created("resp-1"),
ev_tool_search_call(
"extract-search-1",
&json!({
"query": "extract text from uploaded document",
"limit": 1,
}),
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_function_call_with_namespace(
"extract-call-1",
DOCUMENT_EXTRACT_NAMESPACE,
DOCUMENT_EXTRACT_TOOL,
&json!({"file": "report.txt"}).to_string(),
),
ev_completed("resp-1"),
ev_completed("resp-2"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_response_created("resp-3"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
ev_completed("resp-3"),
]),
],
)
Expand Down Expand Up @@ -190,13 +203,16 @@ async fn codex_apps_file_params_upload_environment_files_before_mcp_tool_call()
let mock = run_extract_turn(&test, &server).await?;

let requests = mock.requests();
let body = requests[0].body_json();
let search_output = requests[1].tool_search_output("extract-search-1");
let missing_tool_message = format!(
"missing tool {DOCUMENT_EXTRACT_NAMESPACE}{DOCUMENT_EXTRACT_TOOL} in /v1/responses request: {body:?}"
"missing tool {DOCUMENT_EXTRACT_NAMESPACE}{DOCUMENT_EXTRACT_TOOL} in tool_search output: {search_output:?}"
);
let extract_tool = requests[0]
.tool_by_name(DOCUMENT_EXTRACT_NAMESPACE, DOCUMENT_EXTRACT_TOOL)
.expect(&missing_tool_message);
let extract_tool = namespace_child_tool(
&search_output,
DOCUMENT_EXTRACT_NAMESPACE,
DOCUMENT_EXTRACT_TOOL,
)
.expect(&missing_tool_message);
assert_eq!(
extract_tool.pointer("/parameters/properties/file"),
Some(&json!({
Expand Down
Loading
Loading