mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
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
This commit is contained in:
Ahmed Ibrahim
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -77,6 +77,12 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
|
||||
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
|
||||
|
||||
@@ -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<ToolRouter> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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::<Arc<dyn for<'call> ToolExecutor<ExtensionToolCall<'call>>>>(),
|
||||
&[
|
||||
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::<std::collections::HashSet<_>>(),
|
||||
[allowed_mcp, ToolName::plain("allowed_dynamic")]
|
||||
.into_iter()
|
||||
.collect(),
|
||||
);
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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::<serde_json::Value>(&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(())
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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::<serde_json::Value>(&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::<Vec<_>>();
|
||||
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}<INITIAL_BUILD_HASH>");
|
||||
insta::assert_snapshot!("strict_3p_mcp_catalog_cache", snapshot);
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
Ok(())
|
||||
}
|
||||
+103
@@ -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: "<PROMPT_CACHE_KEY 1>"
|
||||
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=<INITIAL_BUILD_HASH>):
|
||||
- namespace/functions; hash=<INITIAL_BUILD_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:
|
||||
<MODEL_INSTRUCTIONS>
|
||||
02:message/developer[2]:
|
||||
[01] <PERMISSIONS_INSTRUCTIONS>
|
||||
[02] <COLLABORATION_MODE>
|
||||
03:message/developer:
|
||||
<MULTI_AGENT_ROLE>
|
||||
04:message/developer:
|
||||
<MULTI_AGENT_MODE>
|
||||
05:message/user:
|
||||
<ENVIRONMENT_CONTEXT>
|
||||
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 <DURATION> 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 <DURATION> 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 <DURATION> 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 <DURATION> seconds
|
||||
Output:
|
||||
| []
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user