diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 4835bc570..61be90ed7 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -93,6 +93,18 @@ fn model_tool_names(tools: &[ToolInfo]) -> HashSet { .collect::>() } +fn model_tool_name_len(name: &ToolName) -> usize { + name.namespace.as_deref().map_or(0, str::len) + name.name.len() +} + +fn is_code_mode_compatible_tool_name(name: &ToolName) -> bool { + name.namespace + .as_deref() + .into_iter() + .chain(std::iter::once(name.name.as_str())) + .flat_map(str::chars) + .all(|c| c.is_ascii_alphanumeric() || c == '_') +} #[test] fn declared_openai_file_fields_treat_names_literally() { let meta = serde_json::json!({ @@ -334,17 +346,14 @@ fn test_normalize_tools_long_names_same_server() { let names = model_tool_names(&model_tools); - assert!(names.iter().all(|name| name.display().len() == 64)); + assert!(names.iter().all(|name| model_tool_name_len(name) == 64)); assert!( names .iter() .all(|name| name.namespace.as_deref() == Some("mcp__my_server__")) ); assert!( - names.iter().all(|name| name - .display() - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_')), + names.iter().all(is_code_mode_compatible_tool_name), "model-visible names must be code-mode compatible: {names:?}" ); } @@ -363,10 +372,9 @@ fn test_normalize_tools_sanitizes_invalid_characters() { ToolName::namespaced("mcp__server_one__", "tool_two_three") ); assert_eq!( - format!("{}{}", tool.callable_namespace, tool.callable_name), - model_name.display() + ToolName::namespaced(tool.callable_namespace.clone(), tool.callable_name.clone()), + model_name ); - // The callable parts are sanitized for model-visible tool calls, but the raw // MCP name is preserved for the actual MCP call. assert_eq!(tool.server_name, "server.one"); @@ -375,10 +383,7 @@ fn test_normalize_tools_sanitizes_invalid_characters() { assert_eq!(tool.tool.name, "tool.two-three"); assert!( - model_name - .display() - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_'), + is_code_mode_compatible_tool_name(&model_name), "model-visible name must be code-mode compatible: {model_name:?}" ); } @@ -425,10 +430,7 @@ fn test_normalize_tools_disambiguates_sanitized_namespace_collisions() { assert_eq!(raw_servers, HashSet::from(["basic-server", "basic_server"])); let model_names = model_tool_names(&model_tools); assert!( - model_names.iter().all(|name| name - .display() - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_')), + model_names.iter().all(is_code_mode_compatible_tool_name), "model-visible names must be code-mode compatible: {model_names:?}" ); } diff --git a/codex-rs/core/src/memory_usage.rs b/codex-rs/core/src/memory_usage.rs index 02f74ea59..fd54044f2 100644 --- a/codex-rs/core/src/memory_usage.rs +++ b/codex-rs/core/src/memory_usage.rs @@ -1,5 +1,6 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::flat_tool_name; use crate::tools::handlers::unified_exec::ExecCommandArgs; use codex_memories_read::usage::MEMORIES_USAGE_METRIC; use codex_memories_read::usage::memories_usage_kinds_from_command; @@ -17,14 +18,14 @@ pub(crate) async fn emit_metric_for_tool_read(invocation: &ToolInvocation, succe } let success = if success { "true" } else { "false" }; - let tool_name = invocation.tool_name.display(); + let tool_name = flat_tool_name(&invocation.tool_name); for kind in kinds { invocation.turn.session_telemetry.counter( MEMORIES_USAGE_METRIC, /*inc*/ 1, &[ ("kind", kind.as_tag()), - ("tool", &tool_name), + ("tool", tool_name.as_ref()), ("success", success), ], ); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b63b16cbf..d3bbeb90c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1025,7 +1025,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an session: Arc::clone(&session), turn: Arc::clone(&turn), call_id: "probe-call".to_string(), - tool_name: "probe".to_string(), + tool_name: codex_tools::ToolName::plain("probe"), }; orchestrator diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index 29884da4a..14a4e975a 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -239,7 +239,7 @@ pub(crate) async fn handle_output_item_done( tracing::info!( thread_id = %ctx.sess.conversation_id, "ToolCall: {} {}", - call.tool_name.display(), + call.tool_name, payload_preview ); diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 2b63c1cb1..66b04f8e7 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -454,7 +454,7 @@ impl ToolHandler for ApplyPatchHandler { session: session.clone(), turn: turn.clone(), call_id: call_id.clone(), - tool_name: tool_name.display(), + tool_name: tool_name.clone(), }; let out = orchestrator .run( @@ -566,7 +566,7 @@ pub(crate) async fn intercept_apply_patch( session: session.clone(), turn: turn.clone(), call_id: call_id.to_string(), - tool_name: tool_name.to_string(), + tool_name: ToolName::plain(tool_name), }; let out = orchestrator .run( diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 4dfcb44b1..35dbc3bc0 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -8,6 +8,7 @@ use crate::tools::context::McpToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; @@ -42,8 +43,9 @@ impl ToolHandler for McpHandler { return None; }; + let tool_name = &self.tool_name; Some(PreToolUsePayload { - tool_name: HookToolName::new(self.tool_name.display()), + tool_name: HookToolName::new(flat_tool_name(tool_name).into_owned()), tool_input: mcp_hook_tool_input(raw_arguments), }) } @@ -59,8 +61,9 @@ impl ToolHandler for McpHandler { let tool_response = result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; + let tool_name = &self.tool_name; Some(PostToolUsePayload { - tool_name: HookToolName::new(self.tool_name.display()), + tool_name: HookToolName::new(flat_tool_name(tool_name).into_owned()), tool_use_id: invocation.call_id.clone(), tool_input: result.tool_input.clone(), tool_response, @@ -93,13 +96,14 @@ impl ToolHandler for McpHandler { let arguments_str = raw_arguments; let started = Instant::now(); + let hook_tool_name = flat_tool_name(&self.tool_name); let result = handle_mcp_tool_call( Arc::clone(&session), &turn, call_id.clone(), server, tool, - self.tool_name.display(), + hook_tool_name.into_owned(), arguments_str, ) .await; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index f6960bca4..81a590e07 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -29,6 +29,7 @@ use crate::tools::runtimes::shell::ShellRuntimeBackend; use crate::tools::sandboxing::ToolCtx; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::ExecCommandSource; +use codex_tools::ToolName; mod container_exec; mod local_shell; @@ -72,7 +73,7 @@ fn shell_command_payload_command(payload: &ToolPayload) -> Option { } struct RunExecLikeArgs { - tool_name: String, + tool_name: ToolName, exec_params: ExecParams, hook_command: String, additional_permissions: Option, @@ -201,7 +202,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result Ok(FunctionToolOutput::from_text( unavailable_tool_message( - self.tool_name.display(), + &self.tool_name, "Retry after the tool becomes available or ask the user to re-enable it.", ), Some(false), diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 812c36511..3073d9f9d 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -17,7 +17,10 @@ pub(crate) mod spec_plan_types; pub(crate) mod tool_dispatch_trace; pub(crate) mod tool_search_entry; +use std::borrow::Cow; + use codex_protocol::exec_output::ExecToolCallOutput; +use codex_tools::ToolName; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::formatted_truncate_text; use codex_utils_output_truncation::truncate_text; @@ -30,6 +33,21 @@ pub(crate) const TELEMETRY_PREVIEW_MAX_LINES: usize = 64; // lines pub(crate) const TELEMETRY_PREVIEW_TRUNCATION_NOTICE: &str = "[... telemetry preview truncated ...]"; +/// Legacy boundaries such as hook payloads, telemetry tags, and Responses tool +/// names still require a single flattened string. Keep comparisons and sorting +/// on `ToolName` itself; use this only when crossing those boundaries. +pub(crate) fn flat_tool_name(tool_name: &ToolName) -> Cow<'_, str> { + match tool_name.namespace.as_deref() { + Some(namespace) => { + let mut name = String::with_capacity(namespace.len() + tool_name.name.len()); + name.push_str(namespace); + name.push_str(&tool_name.name); + Cow::Owned(name) + } + None => Cow::Borrowed(tool_name.name.as_str()), + } +} + /// Format the combined exec output for sending back to the model. /// Includes exit code and duration metadata; truncates large bodies safely. pub fn format_exec_output_for_model_structured( diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index c618d778d..4fd1ea17f 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -12,6 +12,7 @@ use crate::guardian::new_guardian_review_id; use crate::guardian::routes_approval_to_guardian; use crate::hook_runtime::run_permission_request_hooks; use crate::network_policy_decision::network_approval_context_from_payload; +use crate::tools::flat_tool_name; use crate::tools::network_approval::ActiveNetworkApproval; use crate::tools::network_approval::DeferredNetworkApproval; use crate::tools::network_approval::NetworkApprovalMode; @@ -135,7 +136,7 @@ impl ToolOrchestrator { T: ToolRuntime, { let otel = turn_ctx.session_telemetry.clone(); - let otel_tn = &tool_ctx.tool_name; + let otel_tn = flat_tool_name(&tool_ctx.tool_name).into_owned(); let otel_ci = &tool_ctx.call_id; let strict_auto_review = tool_ctx.session.strict_auto_review_enabled_for_turn().await; let use_guardian = routes_approval_to_guardian(turn_ctx) || strict_auto_review; @@ -175,7 +176,7 @@ impl ToolOrchestrator { already_approved = true; } else { otel.tool_decision( - otel_tn, + &otel_tn, otel_ci, &ReviewDecision::Approved, ToolDecisionSource::Config, @@ -398,6 +399,7 @@ impl ToolOrchestrator { if evaluate_permission_request_hooks && let Some(permission_request) = tool.permission_request_payload(req) { + let tool_name = flat_tool_name(&tool_ctx.tool_name); match run_permission_request_hooks( approval_ctx.session, approval_ctx.turn, @@ -409,7 +411,7 @@ impl ToolOrchestrator { Some(PermissionRequestDecision::Allow) => { let decision = ReviewDecision::Approved; otel.tool_decision( - &tool_ctx.tool_name, + tool_name.as_ref(), &tool_ctx.call_id, &decision, ToolDecisionSource::Config, @@ -419,7 +421,7 @@ impl ToolOrchestrator { Some(PermissionRequestDecision::Deny { message }) => { let decision = ReviewDecision::Denied; otel.tool_decision( - &tool_ctx.tool_name, + tool_name.as_ref(), &tool_ctx.call_id, &decision, ToolDecisionSource::Config, @@ -436,8 +438,9 @@ impl ToolOrchestrator { ToolDecisionSource::User }; let decision = tool.start_approval_async(req, approval_ctx).await; + let tool_name = flat_tool_name(&tool_ctx.tool_name); otel.tool_decision( - &tool_ctx.tool_name, + tool_name.as_ref(), &tool_ctx.call_id, &decision, otel_source, diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index 384a800b4..d7fe22e4f 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -94,12 +94,11 @@ impl ToolCallRuntime { let lock = Arc::clone(&self.parallel_execution); let invocation_cancellation_token = cancellation_token.clone(); let started = Instant::now(); - let display_name = call.tool_name.display(); let dispatch_span = trace_span!( "dispatch_tool_call_with_code_mode_result", - otel.name = display_name.as_str(), - tool_name = display_name.as_str(), + otel.name = %call.tool_name, + tool_name = %call.tool_name, call_id = call.call_id.as_str(), aborted = false, ); diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index c1b5854b6..89c5ea0bf 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -16,6 +16,7 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use crate::util::error_or_panic; @@ -272,7 +273,7 @@ impl ToolRegistry { invocation: ToolInvocation, ) -> Result { let tool_name = invocation.tool_name.clone(); - let display_name = tool_name.display(); + let tool_name_flat = flat_tool_name(&tool_name); let call_id_owned = invocation.call_id.clone(); let otel = invocation.turn.session_telemetry.clone(); let payload_for_response = invocation.payload.clone(); @@ -325,7 +326,7 @@ impl ToolRegistry { None => { let message = unsupported_tool_call_message(&invocation.payload, &tool_name); otel.tool_result_with_tags( - &display_name, + tool_name_flat.as_ref(), &call_id_owned, log_payload.as_ref(), Duration::ZERO, @@ -342,9 +343,9 @@ impl ToolRegistry { }; if !handler.matches_kind(&invocation.payload) { - let message = format!("tool {display_name} invoked with incompatible payload"); + let message = format!("tool {tool_name} invoked with incompatible payload"); otel.tool_result_with_tags( - &display_name, + tool_name_flat.as_ref(), &call_id_owned, log_payload.as_ref(), Duration::ZERO, @@ -381,7 +382,7 @@ impl ToolRegistry { let started = Instant::now(); let result = otel .log_tool_result_with_tags( - &display_name, + tool_name_flat.as_ref(), &call_id_owned, log_payload.as_ref(), &metric_tags, @@ -573,7 +574,6 @@ impl ToolRegistryBuilder { } fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> String { - let tool_name = tool_name.display(); match payload { ToolPayload::Custom { .. } => format!("unsupported custom tool call: {tool_name}"), _ => format!("unsupported call: {tool_name}"), @@ -646,6 +646,8 @@ async fn dispatch_after_tool_use_hook( let session = invocation.session.as_ref(); let turn = invocation.turn.as_ref(); let tool_input = HookToolInput::from(&invocation.payload); + let tool_name = &invocation.tool_name; + let hook_tool_name = flat_tool_name(tool_name); let hook_outcomes = session .hooks() .dispatch(HookPayload { @@ -657,7 +659,7 @@ async fn dispatch_after_tool_use_hook( event: HookEventAfterToolUse { turn_id: turn.sub_id.clone(), call_id: invocation.call_id.clone(), - tool_name: invocation.tool_name.display(), + tool_name: hook_tool_name.into_owned(), tool_kind: hook_tool_kind(&tool_input), tool_input, executed: dispatch.executed, @@ -688,7 +690,7 @@ async fn dispatch_after_tool_use_hook( HookResult::FailedContinue(error) => { warn!( call_id = %invocation.call_id, - tool_name = %invocation.tool_name.display(), + tool_name = %invocation.tool_name, hook_name = %hook_name, error = %error, "after_tool_use hook failed; continuing" @@ -697,7 +699,7 @@ async fn dispatch_after_tool_use_hook( HookResult::FailedAbort(error) => { warn!( call_id = %invocation.call_id, - tool_name = %invocation.tool_name.display(), + tool_name = %invocation.tool_name, hook_name = %hook_name, error = %error, "after_tool_use hook failed; aborting operation" diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 7f17285db..4aa58552a 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -17,6 +17,7 @@ use crate::sandboxing::ExecOptions; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; use crate::shell::ShellType; +use crate::tools::flat_tool_name; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_sandbox_command; @@ -227,7 +228,7 @@ impl ToolRuntime for ShellRuntime { mode: NetworkApprovalMode::Immediate, trigger: GuardianNetworkAccessTrigger { call_id: ctx.call_id.clone(), - tool_name: ctx.tool_name.clone(), + tool_name: flat_tool_name(&ctx.tool_name).into_owned(), command: req.command.clone(), cwd: req.cwd.clone(), sandbox_permissions: req.sandbox_permissions, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 42f311bfc..ae080e038 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -14,6 +14,7 @@ use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecServerEnvConfig; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; +use crate::tools::flat_tool_name; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_sandbox_command; @@ -233,7 +234,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt mode: NetworkApprovalMode::Deferred, trigger: GuardianNetworkAccessTrigger { call_id: ctx.call_id.clone(), - tool_name: ctx.tool_name.clone(), + tool_name: flat_tool_name(&ctx.tool_name).into_owned(), command: req.command.clone(), cwd: req.cwd.clone(), sandbox_permissions: req.sandbox_permissions, diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index c17247beb..6bb78632f 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -27,6 +27,7 @@ use codex_sandboxing::SandboxTransformError; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; use futures::Future; use futures::future::BoxFuture; @@ -344,7 +345,7 @@ pub(crate) struct ToolCtx { pub session: Arc, pub turn: Arc, pub call_id: String, - pub tool_name: String, + pub tool_name: ToolName, } #[derive(Debug)] diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index a3e93f843..a2fe8da97 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1,5 +1,6 @@ use crate::shell::Shell; use crate::shell::ShellType; +use crate::tools::flat_tool_name; use crate::tools::handlers::multi_agents_common::DEFAULT_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_common::MAX_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; @@ -136,7 +137,7 @@ pub(crate) fn build_specs_with_discoverable_tools( .collect::>(); for unavailable_tool in unavailable_called_tools { - let tool_name = unavailable_tool.display(); + let tool_name = flat_tool_name(&unavailable_tool).into_owned(); if existing_spec_names.insert(tool_name.clone()) { let spec = codex_tools::ToolSpec::Function(ResponsesApiTool { name: tool_name.clone(), diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index ddad2cbf4..1595db7b2 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -343,7 +343,7 @@ pub fn build_tool_registry_builder( if let Some(mcp_tools) = params.mcp_tools { let mut entries = mcp_tools.to_vec(); - entries.sort_by_key(|tool| tool.name.display()); + entries.sort_by(|a, b| a.name.cmp(&b.name)); let mut namespace_entries = BTreeMap::new(); for tool in entries { diff --git a/codex-rs/core/src/tools/tool_search_entry.rs b/codex-rs/core/src/tools/tool_search_entry.rs index a0e9a726b..6a8ccd640 100644 --- a/codex-rs/core/src/tools/tool_search_entry.rs +++ b/codex-rs/core/src/tools/tool_search_entry.rs @@ -1,3 +1,4 @@ +use crate::tools::flat_tool_name; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_tools::LoadableToolSpec; @@ -22,7 +23,7 @@ pub(crate) fn build_tool_search_entries( let mut mcp_tools = mcp_tools .map(|tools| tools.iter().collect::>()) .unwrap_or_default(); - mcp_tools.sort_by_key(|info| info.canonical_tool_name().display()); + mcp_tools.sort_by_key(|info| info.canonical_tool_name()); for info in mcp_tools { match mcp_tool_search_entry(info) { Ok(entry) => entries.push(entry), @@ -94,8 +95,9 @@ fn dynamic_tool_search_entry(tool: &DynamicToolSpec) -> Result String { + let tool_name = info.canonical_tool_name(); let mut parts = vec![ - info.canonical_tool_name().display(), + flat_tool_name(&tool_name).into_owned(), info.callable_name.clone(), info.tool.name.to_string(), info.server_name.clone(), diff --git a/codex-rs/core/src/unavailable_tool.rs b/codex-rs/core/src/unavailable_tool.rs index aabf1f605..9f71facea 100644 --- a/codex-rs/core/src/unavailable_tool.rs +++ b/codex-rs/core/src/unavailable_tool.rs @@ -1,6 +1,7 @@ use std::collections::BTreeMap; use std::collections::HashSet; +use crate::tools::flat_tool_name; use codex_protocol::models::ResponseItem; use codex_tools::ToolName; @@ -9,9 +10,9 @@ pub(crate) fn collect_unavailable_called_tools( exposed_tool_names: &HashSet, ) -> Vec { let mut unavailable_tools = BTreeMap::new(); - let exposed_display_names = exposed_tool_names + let exposed_flat_names = exposed_tool_names .iter() - .map(ToolName::display) + .map(flat_tool_name) .collect::>(); for item in input { @@ -29,13 +30,13 @@ pub(crate) fn collect_unavailable_called_tools( Some(namespace) => ToolName::namespaced(namespace.clone(), name.clone()), None => ToolName::plain(name.clone()), }; - let display_name = tool_name.display(); - if exposed_display_names.contains(&display_name) { + let tool_name_flat = flat_tool_name(&tool_name).into_owned(); + if exposed_flat_names.contains(tool_name_flat.as_str()) { continue; } unavailable_tools - .entry(display_name) + .entry(tool_name_flat) .or_insert_with(|| tool_name); } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 4f85b1ddf..73afca2da 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -54,6 +54,7 @@ use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::error::CodexErr; use codex_protocol::error::SandboxErr; use codex_protocol::protocol::ExecCommandSource; +use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::approx_token_count; @@ -1040,7 +1041,7 @@ impl UnifiedExecProcessManager { session: context.session.clone(), turn: context.turn.clone(), call_id: context.call_id.clone(), - tool_name: "exec_command".to_string(), + tool_name: ToolName::plain("exec_command"), }; orchestrator .run( diff --git a/codex-rs/protocol/src/tool_name.rs b/codex-rs/protocol/src/tool_name.rs index d09bc1ce7..3d7219abe 100644 --- a/codex-rs/protocol/src/tool_name.rs +++ b/codex-rs/protocol/src/tool_name.rs @@ -1,5 +1,6 @@ use serde::Deserialize; use serde::Serialize; +use std::cmp::Ordering; use std::fmt; /// Identifies a callable tool, preserving the namespace split when the model @@ -31,13 +32,6 @@ impl ToolName { namespace: Some(namespace.into()), } } - - pub fn display(&self) -> String { - match &self.namespace { - Some(namespace) => format!("{namespace}{}", self.name), - None => self.name.clone(), - } - } } impl fmt::Display for ToolName { @@ -49,6 +43,26 @@ impl fmt::Display for ToolName { } } +impl Ord for ToolName { + fn cmp(&self, other: &Self) -> Ordering { + let lhs = match &self.namespace { + Some(namespace) => (namespace.as_str(), Some(self.name.as_str())), + None => (self.name.as_str(), None), + }; + let rhs = match &other.namespace { + Some(namespace) => (namespace.as_str(), Some(other.name.as_str())), + None => (other.name.as_str(), None), + }; + lhs.cmp(&rhs) + } +} + +impl PartialOrd for ToolName { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl From for ToolName { fn from(name: String) -> Self { Self::plain(name)