openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-09 20:50:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix flattened namespace tool collisions

Browse Source
This commit is contained in:
Mitsuhiro Kotake
2026-07-01 18:12:47 +09:00
parent acef5a0965
commit 05d99c83f0
12 changed files with 489 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/session/tests.rs
+16 -12
View File
@@ -623,17 +623,20 @@ async fn preview_session_start_hooks(
fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> ToolCallRuntime {
let step_context = StepContext::for_test(Arc::clone(&turn_context));
let router = Arc::new(ToolRouter::from_context(
step_context.as_ref(),
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
mcp_tools: None,
deferred_mcp_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
&Default::default(),
));
let router = Arc::new(
ToolRouter::from_context(
step_context.as_ref(),
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
mcp_tools: None,
deferred_mcp_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
&Default::default(),
)
.expect("test tool router should build"),
);
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
ToolCallRuntime::new(router, session, step_context, tracker)
}
@@ -10476,7 +10479,8 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
&Default::default(),
);
)
.expect("test tool router should build");
let item = ResponseItem::CustomToolCall {
id: None,
status: None,
codex-rs/core/src/session/turn.rs
+4 -2
View File
@@ -1296,7 +1296,7 @@ pub(crate) async fn built_tools(
);
let mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.direct_tools);
let deferred_mcp_tools = mcp_tool_exposure.deferred_tools;
Ok(Arc::new(ToolRouter::from_context(
let router = ToolRouter::from_context(
step_context,
ToolRouterParams {
mcp_tools,
@@ -1306,7 +1306,9 @@ pub(crate) async fn built_tools(
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
&sess.services.tool_search_handler_cache,
)))
)
.map_err(CodexErr::InvalidRequest)?;
Ok(Arc::new(router))
}
#[derive(Debug)]
codex-rs/core/src/stream_events_utils_tests.rs
+14 -11
View File
@@ -282,17 +282,20 @@ async fn handle_output_item_done_returns_contributed_last_agent_message() {
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let step_context = StepContext::for_test(Arc::clone(&turn_context));
let router = Arc::new(ToolRouter::from_context(
step_context.as_ref(),
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
mcp_tools: None,
deferred_mcp_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
&Default::default(),
));
let router = Arc::new(
ToolRouter::from_context(
step_context.as_ref(),
crate::tools::router::ToolRouterParams {
tool_suggest_candidates: None,
mcp_tools: None,
deferred_mcp_tools: None,
extension_tool_executors: Vec::new(),
dynamic_tools: turn_context.dynamic_tools.as_slice(),
},
&Default::default(),
)
.expect("test tool router should build"),
);
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let tool_runtime = ToolCallRuntime::new(router, Arc::clone(&session), step_context, tracker);
let item = assistant_output_text("original assistant text");
codex-rs/core/src/tools/hook_names.rs
+9
View File
@@ -55,6 +55,15 @@ impl HookToolName {
Self::new("Bash")
}
/// Adds a matcher-only compatibility name without changing the canonical
/// name serialized into hook stdin.
pub(crate) fn add_matcher_alias(&mut self, alias: impl Into<String>) {
let alias = alias.into();
if alias != self.name && !self.matcher_aliases.contains(&alias) {
self.matcher_aliases.push(alias);
}
}
/// Returns the canonical hook name serialized into hook stdin.
pub(crate) fn name(&self) -> &str {
&self.name
codex-rs/core/src/tools/registry.rs
+42 -10
View File
@@ -1,3 +1,4 @@
use std::collections::BTreeSet;
use std::collections::HashMap;
use std::sync::Arc;
use std::sync::atomic::AtomicBool;
@@ -336,13 +337,18 @@ impl ToolRegistry {
#[cfg(test)]
#[instrument(level = "trace", skip_all)]
pub(crate) fn from_tools(tools: impl IntoIterator<Item = Arc<dyn CoreToolRuntime>>) -> Self {
Self::from_tools_with_namespace_tool_spec_mode(tools, NamespaceToolSpecMode::Preserve)
Self::from_tools_with_namespace_tool_spec_mode(
tools,
NamespaceToolSpecMode::Preserve,
&BTreeSet::new(),
)
}
#[instrument(level = "trace", skip_all)]
pub(crate) fn from_tools_with_namespace_tool_spec_mode(
tools: impl IntoIterator<Item = Arc<dyn CoreToolRuntime>>,
namespace_tool_spec_mode: NamespaceToolSpecMode,
suppressed_flat_aliases: &BTreeSet<String>,
) -> Self {
let mut tools_by_name = HashMap::new();
for tool in tools {
@@ -356,7 +362,7 @@ impl ToolRegistry {
let mut registry = Self::new(tools_by_name);
if namespace_tool_spec_mode == NamespaceToolSpecMode::Flatten {
registry.add_flat_tool_aliases();
registry.add_flat_tool_aliases(suppressed_flat_aliases);
}
registry
}
@@ -390,12 +396,15 @@ impl ToolRegistry {
})
}
fn add_flat_tool_aliases(&mut self) {
fn add_flat_tool_aliases(&mut self, suppressed_flat_aliases: &BTreeSet<String>) {
for (name, tool) in &self.tools {
if name.namespace.is_none() {
continue;
}
let flat_alias = ToolName::plain(name.canonical_flat_name().into_owned());
if suppressed_flat_aliases.contains(flat_alias.name.as_str()) {
continue;
}
if self.tools.contains_key(&flat_alias)
|| self.flat_tool_aliases.contains_key(&flat_alias)
{
@@ -485,11 +494,8 @@ impl ToolRegistry {
}
let dispatch_trace = ToolDispatchTrace::start(&invocation);
let tool = match self.resolved_tool(&tool_name) {
Some((resolved_tool_name, tool)) => {
invocation.tool_name = resolved_tool_name;
tool
}
let (resolved_tool_name, tool) = match self.resolved_tool(&tool_name) {
Some(resolved_tool) => resolved_tool,
None => {
let message = unsupported_tool_call_message(&invocation.payload, &tool_name);
let log_payload = invocation.payload.log_payload();
@@ -508,6 +514,8 @@ impl ToolRegistry {
return Err(err);
}
};
let resolved_tool_name_for_hooks = resolved_tool_name.clone();
invocation.tool_name = resolved_tool_name;
let telemetry_tags = tool.telemetry_tags(&invocation).await;
let mut tool_result_tags =
@@ -541,7 +549,12 @@ impl ToolRegistry {
notify_tool_start(&invocation).await;
if let Some(pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) {
if let Some(mut pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) {
add_model_visible_flat_alias(
&mut pre_tool_use_payload.tool_name,
&tool_name,
&resolved_tool_name_for_hooks,
);
match run_pre_tool_use_hooks(
&invocation.session,
&invocation.turn,
@@ -621,7 +634,7 @@ impl ToolRegistry {
Err(_) => false,
};
emit_metric_for_tool_read(&invocation, success);
let post_tool_use_payload = if success {
let mut post_tool_use_payload = if success {
let guard = response_cell.lock().await;
guard
.as_ref()
@@ -629,6 +642,13 @@ impl ToolRegistry {
} else {
None
};
if let Some(post_tool_use_payload) = post_tool_use_payload.as_mut() {
add_model_visible_flat_alias(
&mut post_tool_use_payload.tool_name,
&tool_name,
&resolved_tool_name_for_hooks,
);
}
let post_tool_use_outcome = if let Some(post_tool_use_payload) = post_tool_use_payload {
Some(
run_post_tool_use_hooks(
@@ -772,6 +792,18 @@ fn function_hook_tool_name(invocation: &ToolInvocation) -> HookToolName {
HookToolName::new(flat_tool_name(&invocation.tool_name).into_owned())
}
fn add_model_visible_flat_alias(
hook_tool_name: &mut HookToolName,
requested_tool_name: &ToolName,
resolved_tool_name: &ToolName,
) {
// A differing plain request can only come from `flat_tool_aliases`, whose
// keys are the function names exposed when namespace wrappers are flattened.
if requested_tool_name.namespace.is_none() && requested_tool_name != resolved_tool_name {
hook_tool_name.add_matcher_alias(requested_tool_name.name.clone());
}
}
fn function_hook_tool_input(arguments: &str) -> Value {
if arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
codex-rs/core/src/tools/registry_tests.rs
+54
View File
@@ -188,6 +188,7 @@ fn handler_resolves_flat_aliases_when_namespace_specs_are_flattened() {
let registry = ToolRegistry::from_tools_with_namespace_tool_spec_mode(
[handler.clone()],
codex_tools::NamespaceToolSpecMode::Flatten,
&std::collections::BTreeSet::new(),
);
let flat_name = codex_tools::ToolName::plain("mcp__calendar__create_event");
@@ -204,6 +205,59 @@ fn handler_resolves_flat_aliases_when_namespace_specs_are_flattened() {
);
}
#[test]
fn suppressed_flat_aliases_keep_logical_runtimes_registered() {
let first_name = codex_tools::ToolName::namespaced("agents", "spawn_agent");
let second_name = codex_tools::ToolName::namespaced("agents_", "spawn_agent");
let first_handler = Arc::new(TestHandler {
tool_name: first_name.clone(),
}) as Arc<dyn CoreToolRuntime>;
let second_handler = Arc::new(TestHandler {
tool_name: second_name.clone(),
}) as Arc<dyn CoreToolRuntime>;
let registry = ToolRegistry::from_tools_with_namespace_tool_spec_mode(
[first_handler, second_handler],
codex_tools::NamespaceToolSpecMode::Flatten,
&std::collections::BTreeSet::from(["agents__spawn_agent".to_string()]),
);
assert_eq!(
registry.tool_names_for_test(),
vec![first_name.clone(), second_name.clone()]
);
assert!(registry.tool(&first_name).is_some());
assert!(registry.tool(&second_name).is_some());
assert!(
registry
.tool(&codex_tools::ToolName::plain("agents__spawn_agent"))
.is_none()
);
}
#[test]
fn flattened_model_visible_name_is_only_a_hook_matcher_alias() {
let requested_tool_name = codex_tools::ToolName::plain("web__run");
let resolved_tool_name = codex_tools::ToolName::namespaced("web", "run");
let mut hook_tool_name = HookToolName::new("webrun");
add_model_visible_flat_alias(
&mut hook_tool_name,
&requested_tool_name,
&resolved_tool_name,
);
assert_eq!(hook_tool_name.name(), "webrun");
assert_eq!(hook_tool_name.matcher_aliases(), ["web__run".to_string()]);
let mut exact_hook_tool_name = HookToolName::new("webrun");
add_model_visible_flat_alias(
&mut exact_hook_tool_name,
&resolved_tool_name,
&resolved_tool_name,
);
assert!(exact_hook_tool_name.matcher_aliases().is_empty());
}
#[tokio::test]
async fn function_tools_expose_default_hook_payloads_and_rewrites() -> anyhow::Result<()> {
let (session, turn) = crate::session::tests::make_session_and_context().await;
codex-rs/core/src/tools/router.rs
+1 -1
View File
@@ -62,7 +62,7 @@ impl ToolRouter {
step_context: &StepContext,
params: ToolRouterParams<'_>,
tool_search_handler_cache: &ToolSearchHandlerCache,
) -> Self {
) -> Result<Self, String> {
build_tool_router(step_context, params, tool_search_handler_cache)
}
codex-rs/core/src/tools/router_tests.rs
+10 -5
View File
@@ -124,7 +124,8 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow
dynamic_tools: turn.dynamic_tools.as_slice(),
},
&Default::default(),
);
)
.expect("test tool router should build");
let parallel_tool_name = ["exec_command", "shell_command"]
.into_iter()
@@ -236,7 +237,8 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> {
dynamic_tools: turn.dynamic_tools.as_slice(),
},
&Default::default(),
);
)
.expect("test tool router should build");
let call = ToolCall {
tool_name: ToolName::namespaced("mcp__echo__", "query_with_delay"),
@@ -274,7 +276,8 @@ async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()>
dynamic_tools: turn.dynamic_tools.as_slice(),
},
&Default::default(),
);
)
.expect("test tool router should build");
assert!(!router.tool_supports_parallel(&ToolCall {
tool_name: ToolName::plain("web_search"),
@@ -331,7 +334,8 @@ async fn specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> {
dynamic_tools: &dynamic_tools,
},
&Default::default(),
);
)
.expect("test tool router should build");
assert_eq!(
namespace_function_names(&router.model_visible_specs(), "codex_app"),
@@ -398,7 +402,8 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow
dynamic_tools: turn.dynamic_tools.as_slice(),
},
&Default::default(),
);
)
.expect("test tool router should build");
assert!(
router.model_visible_specs().iter().any(
codex-rs/core/src/tools/spec_plan.rs
+112 -12
View File
@@ -93,6 +93,7 @@ use codex_tools::serialize_tool_specs;
use codex_tools::shell_command_backend_for_features;
use codex_tools::shell_type_for_model_and_features;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::collections::HashSet;
use std::sync::Arc;
use tracing::instrument;
@@ -164,10 +165,10 @@ pub(crate) fn build_tool_router(
step_context: &StepContext,
params: ToolRouterParams<'_>,
tool_search_handler_cache: &ToolSearchHandlerCache,
) -> ToolRouter {
) -> Result<ToolRouter, String> {
let (model_visible_specs, registry) =
build_tool_specs_and_registry(step_context, params, tool_search_handler_cache);
ToolRouter::from_parts(registry, model_visible_specs)
build_tool_specs_and_registry(step_context, params, tool_search_handler_cache)?;
Ok(ToolRouter::from_parts(registry, model_visible_specs))
}
#[instrument(level = "trace", skip_all)]
@@ -175,7 +176,7 @@ fn build_tool_specs_and_registry(
step_context: &StepContext,
params: ToolRouterParams<'_>,
tool_search_handler_cache: &ToolSearchHandlerCache,
) -> (Vec<ToolSpec>, ToolRegistry) {
) -> Result<(Vec<ToolSpec>, ToolRegistry), String> {
let turn_context = step_context.turn.as_ref();
let ToolRouterParams {
mcp_tools,
@@ -200,9 +201,83 @@ fn build_tool_specs_and_registry(
let mut planned_tools = PlannedTools::default();
add_tool_sources(&context, &mut planned_tools);
apply_direct_model_only_namespace_overrides(turn_context, &mut planned_tools);
// Reject aliases that could be exposed before deriving tool-search or
// code-mode tools. Keep unreachable logical runtimes registered, but
// suppress their ambiguous flat aliases before building the registry.
let suppressed_flat_aliases =
if namespace_tool_spec_mode(turn_context) == NamespaceToolSpecMode::Flatten {
validate_flattened_tool_name_collisions(turn_context, planned_tools.runtimes())?
} else {
BTreeSet::new()
};
append_tool_search_executor(&context, &mut planned_tools);
prepend_code_mode_executors(&context, &mut planned_tools);
build_model_visible_specs_and_registry(turn_context, planned_tools)
build_model_visible_specs_and_registry(turn_context, planned_tools, &suppressed_flat_aliases)
}
fn validate_flattened_tool_name_collisions(
turn_context: &TurnContext,
runtimes: &[PlannedRuntime],
) -> Result<BTreeSet<String>, String> {
let mut logical_names_by_flat_name = BTreeMap::<String, BTreeMap<ToolName, bool>>::new();
for runtime in runtimes {
let tool_name = runtime.tool_name();
let exposure = runtime.exposure();
let reachable = (exposure.is_direct()
&& !is_hidden_by_code_mode_only(turn_context, &tool_name, exposure))
|| (exposure == ToolExposure::Deferred
&& search_tool_enabled(turn_context)
&& runtime.search_info().is_some());
logical_names_by_flat_name
.entry(tool_name.canonical_flat_name().into_owned())
.or_default()
.entry(tool_name)
.and_modify(|already_reachable| *already_reachable |= reachable)
.or_insert(reachable);
}
let mut collisions = Vec::new();
let mut suppressed_flat_aliases = BTreeSet::new();
for (flat_name, logical_names) in logical_names_by_flat_name {
if logical_names.len() <= 1 {
continue;
}
let has_reachable_namespaced_tool = logical_names
.iter()
.any(|(tool_name, reachable)| *reachable && tool_name.namespace.is_some());
let reachable_tool_count = logical_names
.values()
.filter(|reachable| **reachable)
.count();
let logical_names = logical_names
.keys()
.map(|tool_name| match tool_name.namespace.as_deref() {
Some(namespace) => {
format!("namespace `{namespace}` tool `{}`", tool_name.name)
}
None => format!("plain tool `{}`", tool_name.name),
})
.collect::<Vec<_>>()
.join(", ");
let collision = format!("`{flat_name}`: {logical_names}");
if has_reachable_namespaced_tool || reachable_tool_count > 1 {
collisions.push(collision);
} else {
warn!(
"ignoring unreachable flattened tool name collision and suppressing its flat alias: {collision}"
);
suppressed_flat_aliases.insert(flat_name);
}
}
if collisions.is_empty() {
Ok(suppressed_flat_aliases)
} else {
Err(format!(
"cannot flatten namespace tools because canonical flat tool names collide: {}",
collisions.join("; ")
))
}
}
fn apply_direct_model_only_namespace_overrides(
@@ -238,7 +313,8 @@ fn apply_direct_model_only_namespace_overrides(
fn build_model_visible_specs_and_registry(
turn_context: &TurnContext,
planned_tools: PlannedTools,
) -> (Vec<ToolSpec>, ToolRegistry) {
suppressed_flat_aliases: &BTreeSet<String>,
) -> Result<(Vec<ToolSpec>, ToolRegistry), String> {
let PlannedTools {
runtimes,
hosted_specs,
@@ -265,14 +341,38 @@ fn build_model_visible_specs_and_registry(
specs.extend(hosted_specs);
let namespace_tool_spec_mode = namespace_tool_spec_mode(turn_context);
let registry =
ToolRegistry::from_tools_with_namespace_tool_spec_mode(runtimes, namespace_tool_spec_mode);
let model_visible_specs = dedupe_model_visible_function_names(serialize_tool_specs(
merge_into_namespaces(specs),
let registry = ToolRegistry::from_tools_with_namespace_tool_spec_mode(
runtimes,
namespace_tool_spec_mode,
));
suppressed_flat_aliases,
);
let model_visible_specs =
serialize_tool_specs(merge_into_namespaces(specs), namespace_tool_spec_mode);
let model_visible_specs = match namespace_tool_spec_mode {
NamespaceToolSpecMode::Preserve => dedupe_model_visible_function_names(model_visible_specs),
NamespaceToolSpecMode::Flatten => {
let mut function_name_counts = BTreeMap::<&str, usize>::new();
for spec in &model_visible_specs {
if let ToolSpec::Function(tool) = spec {
*function_name_counts.entry(tool.name.as_str()).or_default() += 1;
}
}
let duplicate_function_names = function_name_counts
.into_iter()
.filter(|(_, count)| *count > 1)
.map(|(name, _)| format!("`{name}`"))
.collect::<Vec<_>>();
if !duplicate_function_names.is_empty() {
return Err(format!(
"cannot expose duplicate model-visible function names: {}",
duplicate_function_names.join(", ")
));
}
model_visible_specs
}
};
(model_visible_specs, registry)
Ok((model_visible_specs, registry))
}
fn spec_for_model_request(
codex-rs/core/src/tools/spec_plan_tests.rs
+148 -17
View File
@@ -177,10 +177,10 @@ impl ToolPlanProbe {
}
}
async fn probe_with(
async fn probe_result_with(
configure_turn: impl FnOnce(&mut TurnContext),
inputs: ToolPlanInputs,
) -> ToolPlanProbe {
) -> Result<ToolPlanProbe, String> {
let (_session, mut turn) = make_session_and_context().await;
configure_turn(&mut turn);
let turn = Arc::new(turn);
@@ -195,8 +195,17 @@ async fn probe_with(
dynamic_tools: inputs.dynamic_tools.as_slice(),
},
&Default::default(),
);
ToolPlanProbe::from_router(router)
)?;
Ok(ToolPlanProbe::from_router(router))
}
async fn probe_with(
configure_turn: impl FnOnce(&mut TurnContext),
inputs: ToolPlanInputs,
) -> ToolPlanProbe {
probe_result_with(configure_turn, inputs)
.await
.expect("tool plan should build")
}
async fn probe(configure_turn: impl FnOnce(&mut TurnContext)) -> ToolPlanProbe {
@@ -701,17 +710,20 @@ async fn environment_tools_follow_the_step_context() {
/*loaded_agents_md*/ None,
));
let plan = ToolPlanProbe::from_router(ToolRouter::from_context(
step_context.as_ref(),
ToolRouterParams {
mcp_tools: None,
deferred_mcp_tools: None,
tool_suggest_candidates: None,
extension_tool_executors: Vec::new(),
dynamic_tools: &[],
},
&Default::default(),
));
let plan = ToolPlanProbe::from_router(
ToolRouter::from_context(
step_context.as_ref(),
ToolRouterParams {
mcp_tools: None,
deferred_mcp_tools: None,
tool_suggest_candidates: None,
extension_tool_executors: Vec::new(),
dynamic_tools: &[],
},
&Default::default(),
)
.expect("tool plan should build"),
);
plan.assert_visible_contains(&["exec_command", "apply_patch", "view_image"]);
}
@@ -858,6 +870,123 @@ async fn mcp_and_tool_search_follow_direct_and_deferred_tool_exposure() {
flattened_searchable_mcp.assert_visible_contains(&["tool_search"]);
}
#[tokio::test]
async fn namespace_tools_disabled_rejects_flat_alias_collision_with_plain_tool() {
let err = match probe_result_with(
|turn| {
turn.model_info.supports_search_tool = false;
disable_namespace_tools(turn);
set_feature(turn, Feature::MultiAgentV2, /*enabled*/ true);
update_config(turn, |config| {
config.multi_agent_v2.tool_namespace = Some("agents".to_string());
});
},
ToolPlanInputs {
dynamic_tools: vec![dynamic_tool(
None,
"agents__spawn_agent",
// This plain runtime is not discoverable with search disabled,
// but its exact registry name would still steal the alias.
/*defer_loading*/
true,
)],
..ToolPlanInputs::default()
},
)
.await
{
Ok(_) => panic!("flat alias collision should reject the tool plan"),
Err(err) => err,
};
assert_eq!(
err,
"cannot flatten namespace tools because canonical flat tool names collide: `agents__spawn_agent`: namespace `agents` tool `spawn_agent`, plain tool `agents__spawn_agent`"
);
}
#[tokio::test]
async fn namespace_tools_disabled_rejects_cross_source_namespaced_flat_collisions() {
let err = match probe_result_with(
|turn| {
disable_namespace_tools(turn);
set_feature(turn, Feature::MultiAgentV2, /*enabled*/ true);
update_config(turn, |config| {
config.multi_agent_v2.tool_namespace = Some("agents".to_string());
});
},
ToolPlanInputs {
dynamic_tools: vec![dynamic_tool(
Some("agents_"),
"spawn_agent",
/*defer_loading*/ false,
)],
..ToolPlanInputs::default()
},
)
.await
{
Ok(_) => panic!("cross-source flat collision should reject the tool plan"),
Err(err) => err,
};
assert_eq!(
err,
"cannot flatten namespace tools because canonical flat tool names collide: `agents__spawn_agent`: namespace `agents` tool `spawn_agent`, namespace `agents_` tool `spawn_agent`"
);
}
#[tokio::test]
async fn namespace_tools_disabled_rejects_discoverable_flat_collisions() {
let err = match probe_result_with(
|turn| {
turn.model_info.supports_search_tool = true;
disable_namespace_tools(turn);
},
ToolPlanInputs {
dynamic_tools: vec![
dynamic_tool(Some("agents"), "spawn_agent", /*defer_loading*/ true),
dynamic_tool(Some("agents_"), "spawn_agent", /*defer_loading*/ true),
],
..ToolPlanInputs::default()
},
)
.await
{
Ok(_) => panic!("discoverable flat collision should reject the tool plan"),
Err(err) => err,
};
assert_eq!(
err,
"cannot flatten namespace tools because canonical flat tool names collide: `agents__spawn_agent`: namespace `agents` tool `spawn_agent`, namespace `agents_` tool `spawn_agent`"
);
}
#[tokio::test]
async fn namespace_tools_disabled_ignores_unreachable_flat_collisions() {
let plan = probe_with(
|turn| {
turn.model_info.supports_search_tool = false;
disable_namespace_tools(turn);
},
ToolPlanInputs {
dynamic_tools: vec![
dynamic_tool(Some("agents"), "spawn_agent", /*defer_loading*/ true),
dynamic_tool(Some("agents_"), "spawn_agent", /*defer_loading*/ true),
],
..ToolPlanInputs::default()
},
)
.await;
plan.assert_visible_lacks(&["agents__spawn_agent", "tool_search"]);
plan.assert_registered_contains(&[
&ToolName::namespaced("agents", "spawn_agent").to_string(),
&ToolName::namespaced("agents_", "spawn_agent").to_string(),
]);
}
#[tokio::test]
async fn deferred_extension_tools_are_discoverable_with_tool_search() {
let plan = probe_with(
@@ -895,7 +1024,8 @@ async fn tool_search_cache_rebuilds_when_deferred_sources_change() {
dynamic_tools: &[],
},
&cache,
);
)
.expect("first tool plan should build");
let first_plan = ToolPlanProbe::from_router(first_router);
let (_session, mut second_turn) = make_session_and_context().await;
@@ -912,7 +1042,8 @@ async fn tool_search_cache_rebuilds_when_deferred_sources_change() {
dynamic_tools: &[],
},
&cache,
);
)
.expect("second tool plan should build");
let second_plan = ToolPlanProbe::from_router(second_router);
let ToolSpec::ToolSearch {
codex-rs/core/tests/suite/hooks.rs
+78 -4
View File
@@ -668,7 +668,11 @@ elif mode == "exit_2":
Ok(())
}
fn write_logging_pre_and_blocking_post_tool_use_hooks(home: &Path, feedback: &str) -> Result<()> {
fn write_logging_pre_and_blocking_post_tool_use_hooks(
home: &Path,
matcher: &str,
feedback: &str,
) -> Result<()> {
let pre_script_path = home.join("pre_tool_use_hook.py");
let pre_log_path = home.join("pre_tool_use_hook_log.jsonl");
let post_script_path = home.join("post_tool_use_hook.py");
@@ -702,7 +706,7 @@ raise SystemExit(2)
let hooks = serde_json::json!({
"hooks": {
"PreToolUse": [{
"matcher": "Bash",
"matcher": matcher,
"hooks": [{
"type": "command",
"command": format!("python3 {}", pre_script_path.display()),
@@ -710,7 +714,7 @@ raise SystemExit(2)
}]
}],
"PostToolUse": [{
"matcher": "Bash",
"matcher": matcher,
"hooks": [{
"type": "command",
"command": format!("python3 {}", post_script_path.display()),
@@ -4009,6 +4013,76 @@ async fn post_tool_use_spills_large_feedback_message() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn flattened_alias_matches_pre_and_post_tool_use_hooks() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "flattened-hook-alias";
let feedback = "flattened alias post hook ran";
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "agents__list_agents", "{}"),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "hook alias observed"),
ev_completed("resp-2"),
]),
],
)
.await;
let mut builder = test_codex()
.with_pre_build_hook(move |home| {
write_logging_pre_and_blocking_post_tool_use_hooks(
home,
"agents__list_agents",
feedback,
)
.expect("failed to write flattened alias hook fixture");
})
.with_config(|config| {
config
.features
.enable(Feature::MultiAgentV2)
.expect("test config should allow feature update");
config.multi_agent_v2.tool_namespace = Some("agents".to_string());
config.model_provider.namespace_tools = Some(false);
trust_discovered_hooks(config);
});
let test = builder.build(&server).await?;
test.submit_turn("list agents").await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("list_agents output string");
assert_eq!(output, feedback);
let pre_hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?;
assert_eq!(pre_hook_inputs.len(), 1);
assert_eq!(pre_hook_inputs[0]["hook_event_name"], "PreToolUse");
assert_eq!(pre_hook_inputs[0]["tool_name"], "agentslist_agents");
assert_eq!(pre_hook_inputs[0]["tool_use_id"], call_id);
let post_hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?;
assert_eq!(post_hook_inputs.len(), 1);
assert_eq!(post_hook_inputs[0]["hook_event_name"], "PostToolUse");
assert_eq!(post_hook_inputs[0]["tool_name"], "agentslist_agents");
assert_eq!(post_hook_inputs[0]["tool_use_id"], call_id);
Ok(())
}
#[tokio::test]
async fn post_tool_use_blocks_when_exec_session_completes_via_write_stdin() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -4063,7 +4137,7 @@ async fn post_tool_use_blocks_when_exec_session_completes_via_write_stdin() -> R
let mut builder = test_codex()
.with_pre_build_hook(|home| {
write_logging_pre_and_blocking_post_tool_use_hooks(home, feedback)
write_logging_pre_and_blocking_post_tool_use_hooks(home, "Bash", feedback)
.expect("failed to write tool use hook test fixture");
})
.with_config(|config| {
codex-rs/core/tests/suite/responses_lite.rs
+1
View File
@@ -282,6 +282,7 @@ async fn responses_lite_exposes_standalone_tools_for_actor_authorized_provider()
configure_responses_tools(config);
config.model_provider.name = "local".to_string();
config.model_provider.requires_openai_auth = false;
config.model_provider.namespace_tools = Some(true);
config.model_provider.http_headers = Some(HashMap::from([(
"x-openai-actor-authorization".to_string(),
"test-actor-authorization".to_string(),