From 58ae3ba61186c39b849a6ebe60e60f4b11690373 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Sat, 3 Oct 2026 18:07:22 +0000 Subject: [PATCH] Keep third-party tools deferred in strict Code Mode Only (#50687) ## Why Third-party exposure settings can eagerly expose tool schemas or exclude tools from `exec`. Keeping eligible tools deferred lets MCP catalogs change between turns without changing the model's eager tool prefix. ## What changed Add the disabled-by-default `code_mode_only_strict_3p_tools` feature, applied only when the effective model tool mode is Code Mode Only. - Defer eligible MCP, app, and client-supplied dynamic tools while keeping them discoverable and callable through `exec`. - Override `deferLoading`, MCP/app `omit_tools_from`, and direct-only or excluded namespace settings. Log conflicts without adding prompt warnings. - Preserve disabled, app-only, policy-denied, and over-budget tool restrictions. Keep built-in tool exposure unchanged, including when a dynamic tool has the same name. ## Testing Add unit and integration coverage for exposure overrides, effective model modes, tool eligibility, and app discovery and dispatch. Add an MCP refresh scenario that verifies the eager tool prefix stays unchanged as tools are added, replaced, and removed while `exec` tracks the current catalog. GitOrigin-RevId: a6a8e401214119bb02ea4711473c8fa3ec85c81a --- codex-rs/core/config.schema.json | 6 + codex-rs/core/src/tools/handlers/dynamic.rs | 6 +- codex-rs/core/src/tools/handlers/mcp.rs | 4 + codex-rs/core/src/tools/registry.rs | 6 + codex-rs/core/src/tools/spec_plan.rs | 53 +++- .../spec_plan_strict_third_party_tests.rs | 233 ++++++++++++++++++ codex-rs/core/src/tools/spec_plan_tests.rs | 3 + .../core/tests/suite/app_tool_exposure.rs | 3 + .../suite/app_tool_exposure_strict_tests.rs | 105 ++++++++ codex-rs/core/tests/suite/scenarios.rs | 3 + .../tests/suite/scenarios_strict_3p_cache.rs | 197 +++++++++++++++ ...3p_cache__strict_3p_mcp_catalog_cache.snap | 103 ++++++++ codex-rs/features/src/lib.rs | 9 + 13 files changed, 729 insertions(+), 2 deletions(-) create mode 100644 codex-rs/core/src/tools/spec_plan_strict_third_party_tests.rs create mode 100644 codex-rs/core/tests/suite/app_tool_exposure_strict_tests.rs create mode 100644 codex-rs/core/tests/suite/scenarios_strict_3p_cache.rs create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 62edaa6824..a98e06fe49 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -797,6 +797,9 @@ "code_mode_only": { "type": "boolean" }, + "code_mode_only_strict_3p_tools": { + "type": "boolean" + }, "code_mode_prewarm": { "type": "boolean" }, @@ -6952,6 +6955,9 @@ "code_mode_only": { "type": "boolean" }, + "code_mode_only_strict_3p_tools": { + "type": "boolean" + }, "code_mode_prewarm": { "type": "boolean" }, diff --git a/codex-rs/core/src/tools/handlers/dynamic.rs b/codex-rs/core/src/tools/handlers/dynamic.rs index f2b7860d06..f658d6742b 100644 --- a/codex-rs/core/src/tools/handlers/dynamic.rs +++ b/codex-rs/core/src/tools/handlers/dynamic.rs @@ -165,7 +165,11 @@ impl DynamicToolHandler { } } -impl CoreToolRuntime for DynamicToolHandler {} +impl CoreToolRuntime for DynamicToolHandler { + fn is_third_party_tool(&self) -> bool { + true + } +} #[expect( clippy::await_holding_invalid_type, diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 2287f7db58..4cce55018f 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -308,6 +308,10 @@ impl CoreToolRuntime for McpHandler { })) } + fn is_third_party_tool(&self) -> bool { + true + } + fn mcp_server_name(&self) -> Option<&str> { Some(&self.tool_info.server_name) } diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 9ac7b76ebd..fb2fad72bd 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -77,6 +77,12 @@ pub(crate) trait CoreToolRuntime: ToolExecutor { None } + /// True for MCP/app and client-supplied dynamic tools. + /// A built-in stays false even if a client's tool has the same name. + fn is_third_party_tool(&self) -> bool { + false + } + /// Returns the owning server only for MCP-backed tool runtimes. fn mcp_server_name(&self) -> Option<&str> { None diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 7ed57821ba..a3ec808099 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -187,6 +187,15 @@ pub(crate) fn build_tool_router( ) } +/// Use the effective mode because the model can override the thread's configured mode. +fn code_mode_only_strict_3p_tools(turn_context: &TurnContext, model_info: &ModelInfo) -> bool { + effective_tool_mode(turn_context, model_info) == ToolMode::CodeModeOnly + && turn_context + .config + .features + .enabled(Feature::CodeModeOnlyStrictThirdPartyTools) +} + fn apply_mcp_tool_exposure_policy( turn_context: &TurnContext, model_info: &ModelInfo, @@ -198,6 +207,7 @@ fn apply_mcp_tool_exposure_policy( let apps_config = apps_config_from_layer_stack(&turn_context.config.config_layer_stack); for tool in mcp.tools() { let tool_name = tool.canonical_tool_name(); + // Disabled, denied, app-only and over-budget MCP tools are absent from this set. if !registered_mcp_tools.contains(&tool_name) { continue; } @@ -230,6 +240,17 @@ fn apply_mcp_tool_exposure_policy( let Some(omitted_exposures) = omitted_exposures_by_tool.get(&tool_name) else { continue; }; + if code_mode_only_strict_3p_tools(turn_context, model_info) { + // Ignore MCP/app omit_tools_from; eligible tools must stay deferred in exec. + // Log conflicts without adding a warning to the model's prompt. + if omitted_exposures.intersects(ToolExposures::DEFERRED | ToolExposures::CODE_MODE) { + tracing::warn!( + tool_name = %tool_name, + "Ignoring MCP/app omit_tools_from because code_mode_only_strict_3p_tools is enabled" + ); + } + continue; + } let tool_name = tool_name.with_default_namespace(); let mut exposures = ToolExposures::ALL.difference(*omitted_exposures); @@ -355,6 +376,7 @@ pub(crate) fn finalize_tool_router( ) -> CodexResult { hosted_specs.retain(|spec| registry.tool_policy.allows(&ToolName::plain(spec.name()))); apply_direct_model_only_namespace_overrides(turn_context, &mut registry); + enforce_strict_3p_tools(turn_context, model_info, &mut registry); let tool_mode = effective_tool_mode(turn_context, model_info); let code_mode_enabled = matches!(tool_mode, ToolMode::CodeMode | ToolMode::CodeModeOnly); if code_mode_enabled { @@ -550,6 +572,31 @@ fn apply_direct_model_only_namespace_overrides( } } +/// Run after namespace overrides so eligible third-party tools always remain deferred. +fn enforce_strict_3p_tools( + turn_context: &TurnContext, + model_info: &ModelInfo, + registry: &mut ToolRegistry, +) { + if !code_mode_only_strict_3p_tools(turn_context, model_info) { + return; + } + + for tool in registry.entries_mut() { + // Remaining Hidden entries failed eligibility or budget checks; keep them hidden. + if !tool.runtime.is_third_party_tool() || tool.exposure == ToolExposure::Hidden { + continue; + } + if tool.exposure != ToolExposure::Deferred { + tracing::warn!( + tool_name = %tool.runtime.tool_name().with_default_namespace(), + "Deferring third-party tool because code_mode_only_strict_3p_tools is enabled" + ); + tool.exposure = ToolExposure::Deferred; + } + } +} + #[instrument(level = "trace", skip_all)] fn build_model_visible_specs( turn_context: &TurnContext, @@ -824,7 +871,11 @@ fn register_code_mode_executors( } let tool_name = tool.runtime.tool_name(); - if is_excluded_from_code_mode(turn_context, &tool_name) { + // Deferred third-party tools still need an exec route to be discoverable. + if is_excluded_from_code_mode(turn_context, &tool_name) + && !(code_mode_only_strict_3p_tools(turn_context, model_info) + && tool.runtime.is_third_party_tool()) + { continue; } diff --git a/codex-rs/core/src/tools/spec_plan_strict_third_party_tests.rs b/codex-rs/core/src/tools/spec_plan_strict_third_party_tests.rs new file mode 100644 index 0000000000..197a82b7e7 --- /dev/null +++ b/codex-rs/core/src/tools/spec_plan_strict_third_party_tests.rs @@ -0,0 +1,233 @@ +//! Strict Code Mode Only: deferral, mode overrides, and hidden/denied tools. +use super::*; +use crate::tools::registry::ToolRegistry; +use codex_extension_api::ToolPolicy; +use pretty_assertions::assert_eq; + +#[tokio::test] +#[tracing_test::traced_test] +async fn strict_defers_third_party_tools_despite_caller_settings_and_logs_overrides() { + for supports_search_tool in [false, true] { + let plan = probe_with( + |turn| { + set_features( + turn, + &[ + Feature::CodeMode, + Feature::CodeModeOnly, + Feature::CodeModeOnlyStrictThirdPartyTools, + ], + ); + update_turn_settings_for_test(turn, |settings| { + Arc::make_mut(&mut settings.model_info).supports_search_tool = + supports_search_tool; + }); + update_config(turn, |config| { + config.update_plan_enabled = true; + config.code_mode.direct_only_tool_namespaces = + vec!["direct_client".to_string()]; + config.code_mode.excluded_tool_namespaces = vec!["excluded_client".to_string()]; + }); + }, + ToolPlanInputs { + dynamic_tools: vec![ + dynamic_tool(Some("client"), "eager", /*defer_loading*/ false), + dynamic_tool(Some("client"), "deferred", /*defer_loading*/ true), + dynamic_tool(Some("direct_client"), "eager", /*defer_loading*/ false), + dynamic_tool( + Some("excluded_client"), + "eager", + /*defer_loading*/ false, + ), + // Legal legacy input: app-server forbids deferLoading=true on a plain tool. + dynamic_tool( + /*namespace*/ None, + "client_echo", + /*defer_loading*/ false, + ), + // A collision must not make the built-in tool third-party. + dynamic_tool( + /*namespace*/ None, + "update_plan", + /*defer_loading*/ false, + ), + ], + ..ToolPlanInputs::default() + }, + ) + .await; + plan.assert_visible_contains(&["exec", "wait"]); + plan.assert_visible_lacks(&[ + "tool_search", + "client", + "direct_client", + "excluded_client", + "client_echo", + ]); + assert_eq!(plan.exposure("update_plan"), ToolExposure::Direct); + let ToolSpec::Freeform(exec) = plan.visible_spec("exec") else { + panic!("expected exec"); + }; + for name in [ + ToolName::namespaced("client", "eager"), + ToolName::namespaced("client", "deferred"), + ToolName::namespaced("direct_client", "eager"), + ToolName::namespaced("excluded_client", "eager"), + ToolName::plain("client_echo"), + ] { + assert_eq!(plan.exposure(&name.to_string()), ToolExposure::Deferred); + assert!( + plan.code_mode_tool_names + .values() + .any(|nested| nested == &name), + "{name} must be callable via exec" + ); + assert!( + !exec.description.contains(&format!("{}(args:", name.name)), + "{name} schema must not be eager" + ); + } + assert!( + !exec.description.contains("Deferring third-party tool"), + "warnings must not enter the model prompt" + ); + } + assert!(logs_contain( + "Deferring third-party tool because code_mode_only_strict_3p_tools is enabled" + )); + assert!(logs_contain("client_echo")); +} + +#[tokio::test] +async fn strict_flag_does_nothing_outside_effective_code_mode_only() { + let (_, mut turn) = make_session_and_context().await; + set_features(&mut turn, &[Feature::CodeMode, Feature::CodeModeOnly]); + turn.code_mode_available = true; + update_config(&mut turn, |config| { + config.code_mode.direct_only_tool_namespaces = vec!["client".to_string()]; + config.code_mode.excluded_tool_namespaces = vec!["client".to_string()]; + }); + for mode in [ToolMode::Direct, ToolMode::CodeMode] { + update_turn_settings_for_test(&mut turn, |settings| { + let model = Arc::make_mut(&mut settings.model_info); + model.tool_mode = Some(mode); + model.supports_search_tool = true; + }); + let inputs = || ToolPlanInputs { + dynamic_tools: vec![ + dynamic_tool(Some("client"), "eager", /*defer_loading*/ false), + dynamic_tool(Some("client"), "deferred", /*defer_loading*/ true), + dynamic_tool( + /*namespace*/ None, + "client_echo", + /*defer_loading*/ false, + ), + ], + tool_runtimes: vec![mcp_runtime( + "sample", + "mcp__sample", + "lookup", + ToolExposure::Direct, + )], + ..ToolPlanInputs::default() + }; + set_feature( + &mut turn, + Feature::CodeModeOnlyStrictThirdPartyTools, + /*enabled*/ false, + ); + let off = ToolPlanProbe::from_router(plan_with_model(&turn, turn.model_info(), inputs())); + assert_eq!(off.tool_mode, mode); + set_feature( + &mut turn, + Feature::CodeModeOnlyStrictThirdPartyTools, + /*enabled*/ true, + ); + let on = ToolPlanProbe::from_router(plan_with_model(&turn, turn.model_info(), inputs())); + assert_eq!(on, off, "strict flag changed effective {mode:?}"); + } +} + +#[tokio::test] +async fn strict_finalization_cannot_resurrect_hidden_or_policy_denied_tools() { + let (_, mut turn) = make_session_and_context().await; + set_features( + &mut turn, + &[ + Feature::CodeMode, + Feature::CodeModeOnly, + Feature::CodeModeOnlyStrictThirdPartyTools, + ], + ); + update_config(&mut turn, |config| { + config.code_mode.direct_only_tool_namespaces = vec!["mcp__allowed".to_string()]; + }); + let allowed_mcp = ToolName::namespaced("mcp__allowed", "lookup"); + let hidden_mcp = ToolName::namespaced("mcp__allowed", "over_budget"); + let mut registry = ToolRegistry::with_tool_policy(Arc::new(ToolPolicy { + allowed_tools: Some(vec![ + ToolName::plain("exec"), + ToolName::plain("wait"), + allowed_mcp.clone(), + hidden_mcp.clone(), + ToolName::plain("allowed_dynamic"), + ]), + ..Default::default() + })); + let hosted = append_source_tools( + &turn, + turn.model_info(), + &mut registry, + vec![ + mcp_runtime("allowed", "mcp__allowed", "lookup", ToolExposure::Direct), + // MCP registration keeps budget-overflow tools in the registry as Hidden. + // This is different from a caller setting omit_tools_from. + mcp_runtime( + "allowed", + "mcp__allowed", + "over_budget", + ToolExposure::Hidden, + ), + mcp_runtime("denied", "mcp__denied", "lookup", ToolExposure::Direct), + ], + std::iter::empty:: ToolExecutor>>>(), + &[ + dynamic_tool( + /*namespace*/ None, + "allowed_dynamic", + /*defer_loading*/ false, + ), + dynamic_tool( + /*namespace*/ None, + "denied_dynamic", + /*defer_loading*/ false, + ), + ], + ); + let plan = ToolPlanProbe::from_router(ToolRouter::from_registry( + &turn, + turn.model_info(), + registry, + hosted, + &Default::default(), + )); + plan.assert_registered_lacks(&[ + &ToolName::namespaced("mcp__denied", "lookup").to_string(), + "denied_dynamic", + ]); + assert_eq!( + plan.exposure(&allowed_mcp.to_string()), + ToolExposure::Deferred + ); + assert_eq!(plan.exposure(&hidden_mcp.to_string()), ToolExposure::Hidden); + assert_eq!(plan.exposure("allowed_dynamic"), ToolExposure::Deferred); + assert_eq!( + plan.code_mode_tool_names + .values() + .cloned() + .collect::>(), + [allowed_mcp, ToolName::plain("allowed_dynamic")] + .into_iter() + .collect(), + ); +} diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 07a0bea574..ce5a708749 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -3409,3 +3409,6 @@ async fn hosted_web_search_and_standalone_image_generation_follow_runtime_gates( bedrock_with_standalone_web_search.assert_visible_contains(&["web_search"]); bedrock_with_standalone_web_search.assert_visible_lacks(&["web"]); } + +#[path = "spec_plan_strict_third_party_tests.rs"] +mod strict_third_party; diff --git a/codex-rs/core/tests/suite/app_tool_exposure.rs b/codex-rs/core/tests/suite/app_tool_exposure.rs index 977d7b49ff..95bdcb3d2a 100644 --- a/codex-rs/core/tests/suite/app_tool_exposure.rs +++ b/codex-rs/core/tests/suite/app_tool_exposure.rs @@ -230,3 +230,6 @@ pub(super) async fn connector_exposure_requests( test.codex.shutdown_and_wait().await?; Ok(requests) } + +#[path = "app_tool_exposure_strict_tests.rs"] +mod strict_third_party; diff --git a/codex-rs/core/tests/suite/app_tool_exposure_strict_tests.rs b/codex-rs/core/tests/suite/app_tool_exposure_strict_tests.rs new file mode 100644 index 0000000000..894695ff91 --- /dev/null +++ b/codex-rs/core/tests/suite/app_tool_exposure_strict_tests.rs @@ -0,0 +1,105 @@ +//! App omissions cannot bypass strict deferral; app access restrictions still apply. +use super::*; +use pretty_assertions::assert_eq; +use std::sync::Mutex; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tracing_test::traced_test] +async fn strict_ignores_app_omissions_but_preserves_app_access() -> anyhow::Result<()> { + let server = responses::start_mock_server().await; + let mut tools = vec![]; + for name in ["lookup", "disabled", "app_only"] { + let mut tool = json!({ + "name": name, + "description": format!("Calendar {name}"), + "inputSchema": {"type": "object", "properties": {}, "additionalProperties": false}, + "annotations": {"readOnlyHint": true}, + "_meta": {"connector_id": "calendar", "connector_name": "Calendar"} + }); + if name == "app_only" { + tool["_meta"]["ui"] = json!({"visibility": ["app"]}); + } + tools.push(tool); + } + AppsTestServer::mount_with_tools(&server, Arc::new(Mutex::new(tools))).await?; + let mut extensions = ExtensionRegistryBuilder::new(); + extensions.mcp_server_contributor(Arc::new(AppsServer(vec![]))); + let script = r#"const names = ALL_TOOLS.filter(t => t.name.startsWith("mcp__codex_apps__calendar__")).map(t => t.name).sort(); +const t = ALL_TOOLS.find(t => t.name === "mcp__codex_apps__calendar__lookup"); +text(JSON.stringify({names, result: t ? (await tools[t.name]({})).content[0].text : null}));"#; + let mock = responses::mount_sse_sequence( + &server, + vec![ + responses::sse(vec![ + responses::ev_custom_tool_call("lookup", "exec", script), + responses::ev_completed("first"), + ]), + responses::sse(vec![responses::ev_completed("second")]), + ], + ) + .await; + let mut builder = search_capable_apps_builder(server.uri()) + .with_code_mode_host_program(codex_utils_cargo_bin::cargo_bin("codex-code-mode-host")?) + .with_auth(codex_login::CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_extensions(Arc::new(extensions.build())) + .with_pre_build_hook(|home| { + std::fs::write( + home.join("config.toml"), + r#" +[apps.calendar] +omit_tools_from = ["deferred"] +[apps.calendar.tools.disabled] +enabled = false +"#, + ) + .expect("write app policy"); + }) + .with_model_info_override("gpt-5.5", |model| { + model.tool_mode = Some(ToolMode::CodeModeOnly); + model.supports_search_tool = false; + }) + .with_config(|config| { + config + .features + .enable(Feature::CodeModeHost) + .expect("code mode host"); + config + .features + .enable(Feature::CodeModeOnlyStrictThirdPartyTools) + .expect("strict third-party tools"); + config.analytics_enabled = Some(false); + }); + let test = builder.build_with_auto_env(&server).await?; + test.submit_turn("Look up my calendar.").await?; + let requests = mock.requests(); + assert_eq!(requests.len(), 2); + assert!( + requests[0] + .tool_by_name("mcp__codex_apps__calendar", "lookup") + .is_none() + ); + let output = + super::super::code_mode::custom_tool_output_last_non_empty_text(&requests[1], "lookup") + .expect("exec reports actual discovery and dispatch"); + assert_eq!( + serde_json::from_str::(&output)?, + json!({ + "names": ["mcp__codex_apps__calendar__lookup"], + "result": "called lookup for at with " + }), + "app omissions are ignored; disabled and app-only tools never enter ALL_TOOLS" + ); + // The spawned Core session does not inherit the test span. + let logs = String::from_utf8( + tracing_test::internal::global_buf() + .lock() + .expect("captured logs") + .clone(), + )?; + let thread_id = test.session_configured.thread_id.to_string(); + assert!(logs.lines().any(|line| { + line.contains(&thread_id) && line.contains("Ignoring MCP/app omit_tools_from") + })); + test.codex.shutdown_and_wait().await?; + Ok(()) +} diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 07401f7b92..722362710d 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -92,6 +92,9 @@ const ONE_PIXEL_PNG_BASE64: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJ #[path = "scenarios_incremental_tools.rs"] mod incremental_tools; +#[path = "scenarios_strict_3p_cache.rs"] +mod strict_3p_cache; + #[path = "scenarios_agent_message_board.rs"] mod agent_message_board; diff --git a/codex-rs/core/tests/suite/scenarios_strict_3p_cache.rs b/codex-rs/core/tests/suite/scenarios_strict_3p_cache.rs new file mode 100644 index 0000000000..ecc78d2c10 --- /dev/null +++ b/codex-rs/core/tests/suite/scenarios_strict_3p_cache.rs @@ -0,0 +1,197 @@ +//! A server named "environment" adds, replaces, then removes MCP tools between turns. +//! The execution environment runs throughout; this tests MCP refresh, not executor shutdown. + +use anyhow::Result; +use codex_core::ConfigRefreshOutcome; +use codex_features::Feature; +use codex_protocol::openai_models::ToolMode; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::McpStartupStatus; +use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::context_snapshot; +use core_test_support::context_snapshot::ContextSnapshotOptions; +use core_test_support::responses; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event_match; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::collections::HashMap; +use std::sync::Arc; +use std::sync::Mutex; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn strict_3p_mcp_refresh_preserves_cache_while_exec_tracks_available_tools() -> Result<()> { + let server = responses::start_mock_server().await; + let first_mcp = responses::start_mock_server().await; + let second_mcp = responses::start_mock_server().await; + let initial = AppsTestServer::mount_with_tools( + &first_mcp, + Arc::new(Mutex::new(vec![json!({ + "name": "lookup", + "description": "First environment tool.", + "inputSchema": {"type": "object", "properties": {}, "additionalProperties": false}, + })])), + ) + .await?; + let changed = AppsTestServer::mount_with_tools( + &second_mcp, + Arc::new(Mutex::new(vec![ + json!({ + "name": "search", + "description": "Replacement environment tool.", + "inputSchema": {"type": "object", "properties": {}, "additionalProperties": false}, + }), + json!({ + "name": "resolve", + "description": "Additional tool from the refreshed environment.", + "inputSchema": {"type": "object", "properties": {}, "additionalProperties": false}, + }), + ])), + ) + .await?; + let test = test_codex() + .with_config(|config| { + super::configure_scenario_catalog(config); + config.code_mode.disable_in_process_fallback = true; + config.features.enable(Feature::CodeModeHost).unwrap(); + config + .features + .enable(Feature::CodeModeOnlyStrictThirdPartyTools) + .unwrap(); + config.code_mode.direct_only_tool_namespaces = vec!["mcp__environment".to_string()]; + config.code_mode.excluded_tool_namespaces = vec!["mcp__environment".to_string()]; + }) + .with_model_info_override("gpt-6-astra", |model| { + model.tool_mode = Some(ToolMode::CodeModeOnly); + model.use_responses_lite = true; + model.supports_search_tool = false; + }) + .build_with_auto_env(&server) + .await?; + + let script = r#"const available = ALL_TOOLS.filter(t => t.name.startsWith("mcp__environment__")).sort((a, b) => a.name.localeCompare(b.name)); +const results = []; +for (const t of available) { + const result = await tools[t.name]({}); + results.push({name: t.name, result: result.content[0].text}); +} +text(JSON.stringify(results));"#; + let mut events = Vec::new(); + for phase in ["before", "ready", "changed", "gone"] { + events.push(responses::sse(vec![ + responses::ev_custom_tool_call(phase, "exec", script), + responses::ev_completed(phase), + ])); + events.push(responses::sse_completed(&format!("{phase}-done"))); + } + let mock = responses::mount_sse_sequence(&server, events).await; + + test.submit_turn("List and call the tools provided by the environment MCP server.") + .await?; + for endpoint in [ + Some(initial.chatgpt_base_url), + Some(changed.chatgpt_base_url), + None, + ] { + let current = test.codex.config().await; + let mut next = current.as_ref().clone(); + let mut servers = HashMap::new(); + if let Some(url) = &endpoint { + servers.insert( + "environment".to_string(), + serde_json::from_value(json!({ + "url": format!("{url}/api/codex/ps/mcp"), + "omit_tools_from": ["direct", "deferred", "code_mode"], + }))?, + ); + } + next.mcp_servers.set(servers)?; + assert_eq!( + test.codex.refresh_mcp_config(current, next).await, + ConfigRefreshOutcome::Published, + ); + if endpoint.is_some() { + let status = wait_for_event_match(&test.codex, |event| match event { + EventMsg::McpStartupUpdate(update) + if update.server == "environment" + && !matches!(update.status, McpStartupStatus::Starting) => + { + Some(update.status.clone()) + } + _ => None, + }) + .await; + assert!(matches!(status, McpStartupStatus::Ready), "{status:?}"); + } + test.submit_turn("List and call the tools provided by the environment MCP server.") + .await?; + } + + let requests = mock.requests(); + // Each of four turns needs two requests: one calls exec, the next includes its output. + assert_eq!(requests.len(), 8); + let prefix = requests[0].inputs_of_type("additional_tools"); + assert!( + !prefix.is_empty(), + "Responses Lite must supply a real tool prefix" + ); + for (index, request) in requests.iter().enumerate() { + assert_eq!( + request.inputs_of_type("additional_tools"), + prefix, + "request {}: MCP registration or refresh changed the eager tool prefix", + index + 1, + ); + } + for (request, (phase, expected)) in requests.iter().skip(1).step_by(2).zip([ + ("before", json!([])), + ( + "ready", + json!([{"name": "mcp__environment__lookup", "result": "called lookup for at with "}]), + ), + ( + "changed", + json!([ + {"name": "mcp__environment__resolve", "result": "called resolve for at with "}, + {"name": "mcp__environment__search", "result": "called search for at with "}, + ]), + ), + ("gone", json!([])), + ]) { + let output = + super::super::code_mode::custom_tool_output_last_non_empty_text(request, phase) + .expect("exec reports the current MCP catalog and dispatched tool results"); + assert_eq!( + serde_json::from_str::(&output)?, + expected, + "phase {phase}: exec must discover and call exactly the currently registered MCP tools", + ); + } + let labels = [ + "turn 1: no MCP server registered", + "turn 1: exec discovery and call results", + "turn 2: register MCP with lookup; omit-all, direct-only and excluded", + "turn 2: exec discovery and call results", + "turn 3: same MCP name, new endpoint; replace lookup with resolve and search", + "turn 3: exec discovery and call results", + "turn 4: remove MCP server from config", + "turn 4: exec discovery and call results", + ]; + let sections = labels.into_iter().zip(&requests).collect::>(); + let snapshot = context_snapshot::format_labeled_requests_snapshot( + "Strict Code Mode Only: MCP config changes between turns, with omit-all and namespace skips. Exec calls the live catalog. One window means the captured prefix stayed stable. The execution environment remains running.", + §ions, + &ContextSnapshotOptions::default() + .rewrite_known_segments() + .include_request_settings(), + ); + // The starting exec schema varies across Cargo and Bazel builds. The assertions above + // still compare every real prefix byte; the formatter has already grouped the windows. + let snapshot = regex_lite::Regex::new( + r"(?m)(^00:additional_tools/developer \(\d+; hash=|^ +- namespace/functions; hash=)[0-9a-f]{16}", + )? + .replace_all(&snapshot, "${1}"); + insta::assert_snapshot!("strict_3p_mcp_catalog_cache", snapshot); + test.codex.shutdown_and_wait().await?; + Ok(()) +} diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap new file mode 100644 index 0000000000..b6de242b41 --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap @@ -0,0 +1,103 @@ +--- +source: core/tests/suite/scenarios_strict_3p_cache.rs +expression: snapshot +--- +Scenario: Strict Code Mode Only: MCP config changes between turns, with omit-all and namespace skips. Exec calls the live catalog. One window means the captured prefix stayed stable. The execution environment remains running. + +## Window 1 +Settings: + include: ["reasoning.encrypted_content"] + model: "gpt-6-astra" + parallel_tool_calls: false + prompt_cache_key: "" + reasoning: {"context":"all_turns","effort":"low"} + store: false + stream: true + text: {"verbosity":"low"} + tool_choice: "auto" +-- request 1 (turn; turn 1: no MCP server registered) -- +00:additional_tools/developer (3; hash=): + - namespace/functions; hash= + - custom/exec + - function/wait + - function/request_user_input + - function/request_user_input_async + - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 + - function/sleep + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=b263e3c84ad07c37 + - function/followup_task + - function/interrupt_agent + - function/list_agents + - function/send_message + - function/spawn_agent + - function/wait_agent +01:message/developer: + +02:message/developer[2]: + [01] + [02] +03:message/developer: + +04:message/developer: + +05:message/user: + +06:message/user: + List and call the tools provided by the environment MCP server. +-- request 2 (turn; turn 1: exec discovery and call results) -- +07:custom_tool_call/exec:const available = ALL_TOOLS.filter(t => t.name.startsWith("mcp__environment__")).sort((a, b) => a.name.localeCompare(b.name)); + const results = []; + for (const t of available) { + const result = await tools[t.name]({}); + results.push({name: t.name, result: result.content[0].text}); + } + text(JSON.stringify(results)); +08:custom_tool_call_output:Script completed + Wall time seconds + Output: + | [] +-- request 3 (turn; turn 2: register MCP with lookup; omit-all, direct-only and excluded) -- +09:message/user: + List and call the tools provided by the environment MCP server. +-- request 4 (turn; turn 2: exec discovery and call results) -- +10:custom_tool_call/exec:const available = ALL_TOOLS.filter(t => t.name.startsWith("mcp__environment__")).sort((a, b) => a.name.localeCompare(b.name)); + const results = []; + for (const t of available) { + const result = await tools[t.name]({}); + results.push({name: t.name, result: result.content[0].text}); + } + text(JSON.stringify(results)); +11:custom_tool_call_output:Script completed + Wall time seconds + Output: + | [{"name":"mcp__environment__lookup","result":"called lookup for at with "}] +-- request 5 (turn; turn 3: same MCP name, new endpoint; replace lookup with resolve and search) -- +12:message/user: + List and call the tools provided by the environment MCP server. +-- request 6 (turn; turn 3: exec discovery and call results) -- +13:custom_tool_call/exec:const available = ALL_TOOLS.filter(t => t.name.startsWith("mcp__environment__")).sort((a, b) => a.name.localeCompare(b.name)); + const results = []; + for (const t of available) { + const result = await tools[t.name]({}); + results.push({name: t.name, result: result.content[0].text}); + } + text(JSON.stringify(results)); +14:custom_tool_call_output:Script completed + Wall time seconds + Output: + | [{"name":"mcp__environment__resolve","result":"called resolve for at with "},{"name":"mcp__environment__search","result":"called search for at with "}] +-- request 7 (turn; turn 4: remove MCP server from config) -- +15:message/user: + List and call the tools provided by the environment MCP server. +-- request 8 (turn; turn 4: exec discovery and call results) -- +16:custom_tool_call/exec:const available = ALL_TOOLS.filter(t => t.name.startsWith("mcp__environment__")).sort((a, b) => a.name.localeCompare(b.name)); + const results = []; + for (const t of available) { + const result = await tools[t.name]({}); + results.push({name: t.name, result: result.content[0].text}); + } + text(JSON.stringify(results)); +17:custom_tool_call_output:Script completed + Wall time seconds + Output: + | [] diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 56bae8c8c1..8d7fa099ad 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -132,6 +132,9 @@ pub enum Feature { CodeModeInterrupt, /// Restrict model-visible tools to code mode entrypoints (`exec`, `wait`). CodeModeOnly, + /// Keep eligible MCP/app and dynamic tools deferred in exec, only in Code Mode Only. + /// Ignores deferLoading, omit_tools_from, and namespace skip settings. + CodeModeOnlyStrictThirdPartyTools, /// Use the single unified PTY-backed exec tool. UnifiedExec, /// Allow unified exec commands to allocate an interactive terminal. @@ -1134,6 +1137,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::CodeModeOnlyStrictThirdPartyTools, + key: "code_mode_only_strict_3p_tools", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::JsReplToolsOnly, key: "js_repl_tools_only",