mirror of
https://github.com/openai/codex.git
synced 2026-05-03 21:01:55 +03:00
register all mcp tools with namespace (#17404)
stacked on #17402. MCP tools returned by `tool_search` (deferred tools) get registered in our `ToolRegistry` with a different format than directly available tools. this leads to two different ways of accessing MCP tools from our tool catalog, only one of which works for each. fix this by registering all MCP tools with the namespace format, since this info is already available. also, direct MCP tools are registered to responsesapi without a namespace, while deferred MCP tools have a namespace. this means we can receive MCP `FunctionCall`s in both formats from namespaces. fix this by always registering MCP tools with namespace, regardless of deferral status. make code mode track `ToolName` provenance of tools so it can map the literal JS function name string to the correct `ToolName` for invocation, rather than supporting both in core. this lets us unify to a single canonical `ToolName` representation for each MCP tool and force everywhere to use that one, without supporting fallbacks.
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
use codex_protocol::ToolName;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
@@ -129,6 +130,7 @@ pub enum CodeModeToolKind {
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub struct ToolDefinition {
|
||||
pub name: String,
|
||||
pub tool_name: ToolName,
|
||||
pub description: String,
|
||||
pub kind: CodeModeToolKind,
|
||||
pub input_schema: Option<JsonValue>,
|
||||
@@ -269,11 +271,15 @@ pub fn build_exec_tool_description(
|
||||
for tool in enabled_tools {
|
||||
let name = tool.name.as_str();
|
||||
let nested_description = render_code_mode_sample_for_definition(tool);
|
||||
let next_namespace = namespace_descriptions
|
||||
.get(name)
|
||||
let namespace_description = tool
|
||||
.tool_name
|
||||
.namespace
|
||||
.as_ref()
|
||||
.and_then(|namespace| namespace_descriptions.get(namespace));
|
||||
let next_namespace = namespace_description
|
||||
.map(|namespace_description| namespace_description.name.as_str());
|
||||
if next_namespace != current_namespace {
|
||||
if let Some(namespace_description) = namespace_descriptions.get(name) {
|
||||
if let Some(namespace_description) = namespace_description {
|
||||
let namespace_description_text = namespace_description.description.trim();
|
||||
if !namespace_description_text.is_empty() {
|
||||
nested_tool_sections.push(format!(
|
||||
@@ -346,7 +352,7 @@ pub fn augment_tool_definition(mut definition: ToolDefinition) -> ToolDefinition
|
||||
|
||||
pub fn enabled_tool_metadata(definition: &ToolDefinition) -> EnabledToolMetadata {
|
||||
EnabledToolMetadata {
|
||||
tool_name: definition.name.clone(),
|
||||
tool_name: definition.tool_name.clone(),
|
||||
global_name: normalize_code_mode_identifier(&definition.name),
|
||||
description: definition.description.clone(),
|
||||
kind: definition.kind,
|
||||
@@ -355,7 +361,7 @@ pub fn enabled_tool_metadata(definition: &ToolDefinition) -> EnabledToolMetadata
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
|
||||
pub struct EnabledToolMetadata {
|
||||
pub tool_name: String,
|
||||
pub tool_name: ToolName,
|
||||
pub global_name: String,
|
||||
pub description: String,
|
||||
pub kind: CodeModeToolKind,
|
||||
@@ -706,6 +712,7 @@ mod tests {
|
||||
use super::build_exec_tool_description;
|
||||
use super::normalize_code_mode_identifier;
|
||||
use super::parse_exec_source;
|
||||
use codex_protocol::ToolName;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value as JsonValue;
|
||||
use serde_json::json;
|
||||
@@ -770,6 +777,7 @@ mod tests {
|
||||
fn augment_tool_definition_appends_typed_declaration() {
|
||||
let definition = ToolDefinition {
|
||||
name: "hidden_dynamic_tool".to_string(),
|
||||
tool_name: ToolName::plain("hidden_dynamic_tool"),
|
||||
description: "Test tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -798,6 +806,7 @@ mod tests {
|
||||
fn augment_tool_definition_includes_property_descriptions_as_comments() {
|
||||
let definition = ToolDefinition {
|
||||
name: "weather_tool".to_string(),
|
||||
tool_name: ToolName::plain("weather_tool"),
|
||||
description: "Weather tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -846,6 +855,7 @@ mod tests {
|
||||
let description = build_exec_tool_description(
|
||||
&[ToolDefinition {
|
||||
name: "foo".to_string(),
|
||||
tool_name: ToolName::plain("foo"),
|
||||
description: "bar".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: None,
|
||||
@@ -870,26 +880,18 @@ bar"
|
||||
|
||||
#[test]
|
||||
fn code_mode_only_description_groups_namespace_instructions_once() {
|
||||
let namespace_descriptions = BTreeMap::from([
|
||||
(
|
||||
"mcp__sample__alpha".to_string(),
|
||||
ToolNamespaceDescription {
|
||||
name: "mcp__sample".to_string(),
|
||||
description: "Shared namespace guidance.".to_string(),
|
||||
},
|
||||
),
|
||||
(
|
||||
"mcp__sample__beta".to_string(),
|
||||
ToolNamespaceDescription {
|
||||
name: "mcp__sample".to_string(),
|
||||
description: "Shared namespace guidance.".to_string(),
|
||||
},
|
||||
),
|
||||
]);
|
||||
let namespace_descriptions = BTreeMap::from([(
|
||||
"mcp__sample__".to_string(),
|
||||
ToolNamespaceDescription {
|
||||
name: "mcp__sample".to_string(),
|
||||
description: "Shared namespace guidance.".to_string(),
|
||||
},
|
||||
)]);
|
||||
let description = build_exec_tool_description(
|
||||
&[
|
||||
ToolDefinition {
|
||||
name: "mcp__sample__alpha".to_string(),
|
||||
tool_name: ToolName::namespaced("mcp__sample__", "alpha"),
|
||||
description: "First tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -905,6 +907,7 @@ bar"
|
||||
},
|
||||
ToolDefinition {
|
||||
name: "mcp__sample__beta".to_string(),
|
||||
tool_name: ToolName::namespaced("mcp__sample__", "beta"),
|
||||
description: "Second tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -935,7 +938,7 @@ bar"
|
||||
#[test]
|
||||
fn code_mode_only_description_omits_empty_namespace_sections() {
|
||||
let namespace_descriptions = BTreeMap::from([(
|
||||
"mcp__sample__alpha".to_string(),
|
||||
"mcp__sample__".to_string(),
|
||||
ToolNamespaceDescription {
|
||||
name: "mcp__sample".to_string(),
|
||||
description: String::new(),
|
||||
@@ -944,6 +947,7 @@ bar"
|
||||
let description = build_exec_tool_description(
|
||||
&[ToolDefinition {
|
||||
name: "mcp__sample__alpha".to_string(),
|
||||
tool_name: ToolName::namespaced("mcp__sample__", "alpha"),
|
||||
description: "First tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -969,6 +973,7 @@ bar"
|
||||
fn code_mode_only_description_renders_shared_mcp_types_once() {
|
||||
let first_tool = augment_tool_definition(ToolDefinition {
|
||||
name: "mcp__sample__alpha".to_string(),
|
||||
tool_name: ToolName::namespaced("mcp__sample__", "alpha"),
|
||||
description: "First tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -1002,6 +1007,7 @@ bar"
|
||||
});
|
||||
let second_tool = augment_tool_definition(ToolDefinition {
|
||||
name: "mcp__sample__beta".to_string(),
|
||||
tool_name: ToolName::namespaced("mcp__sample__", "beta"),
|
||||
description: "Second tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
@@ -1038,6 +1044,7 @@ bar"
|
||||
&[
|
||||
ToolDefinition {
|
||||
name: first_tool.name,
|
||||
tool_name: first_tool.tool_name,
|
||||
description: "First tool".to_string(),
|
||||
kind: first_tool.kind,
|
||||
input_schema: first_tool.input_schema,
|
||||
@@ -1045,6 +1052,7 @@ bar"
|
||||
},
|
||||
ToolDefinition {
|
||||
name: second_tool.name,
|
||||
tool_name: second_tool.tool_name,
|
||||
description: "Second tool".to_string(),
|
||||
kind: second_tool.kind,
|
||||
input_schema: second_tool.input_schema,
|
||||
|
||||
@@ -15,7 +15,13 @@ pub(super) fn tool_callback(
|
||||
args: v8::FunctionCallbackArguments,
|
||||
mut retval: v8::ReturnValue<v8::Value>,
|
||||
) {
|
||||
let tool_name = args.data().to_rust_string_lossy(scope);
|
||||
let tool_index = match args.data().to_rust_string_lossy(scope).parse::<usize>() {
|
||||
Ok(tool_index) => tool_index,
|
||||
Err(_) => {
|
||||
throw_type_error(scope, "invalid tool callback data");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let input = if args.length() == 0 {
|
||||
Ok(None)
|
||||
} else {
|
||||
@@ -36,6 +42,22 @@ pub(super) fn tool_callback(
|
||||
let promise = resolver.get_promise(scope);
|
||||
|
||||
let resolver = v8::Global::new(scope, resolver);
|
||||
let tool_name = {
|
||||
let Some(state) = scope.get_slot::<RuntimeState>() else {
|
||||
throw_type_error(scope, "runtime state unavailable");
|
||||
return;
|
||||
};
|
||||
let Some(tool_name) = state
|
||||
.enabled_tools
|
||||
.get(tool_index)
|
||||
.map(|tool| tool.tool_name.clone())
|
||||
else {
|
||||
throw_type_error(scope, "tool callback data is out of range");
|
||||
return;
|
||||
};
|
||||
tool_name
|
||||
};
|
||||
|
||||
let Some(state) = scope.get_slot_mut::<RuntimeState>() else {
|
||||
throw_type_error(scope, "runtime state unavailable");
|
||||
return;
|
||||
|
||||
@@ -53,10 +53,10 @@ fn build_tools_object<'s>(
|
||||
.map(|state| state.enabled_tools.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
for tool in enabled_tools {
|
||||
for (tool_index, tool) in enabled_tools.iter().enumerate() {
|
||||
let name = v8::String::new(scope, &tool.global_name)
|
||||
.ok_or_else(|| "failed to allocate tool name".to_string())?;
|
||||
let function = tool_function(scope, &tool.tool_name)?;
|
||||
let function = tool_function(scope, tool_index)?;
|
||||
tools.set(scope, name.into(), function.into());
|
||||
}
|
||||
Ok(tools)
|
||||
@@ -116,9 +116,9 @@ where
|
||||
|
||||
fn tool_function<'s>(
|
||||
scope: &mut v8::PinScope<'s, '_>,
|
||||
tool_name: &str,
|
||||
tool_index: usize,
|
||||
) -> Result<v8::Local<'s, v8::Function>, String> {
|
||||
let data = v8::String::new(scope, tool_name)
|
||||
let data = v8::String::new(scope, &tool_index.to_string())
|
||||
.ok_or_else(|| "failed to allocate tool callback data".to_string())?;
|
||||
let template = v8::FunctionTemplate::builder(tool_callback)
|
||||
.data(data.into())
|
||||
|
||||
@@ -9,6 +9,7 @@ use std::sync::OnceLock;
|
||||
use std::sync::mpsc as std_mpsc;
|
||||
use std::thread;
|
||||
|
||||
use codex_protocol::ToolName;
|
||||
use serde_json::Value as JsonValue;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
@@ -62,7 +63,7 @@ pub(crate) enum TurnMessage {
|
||||
ToolCall {
|
||||
cell_id: String,
|
||||
id: String,
|
||||
name: String,
|
||||
name: ToolName,
|
||||
input: Option<JsonValue>,
|
||||
},
|
||||
Notify {
|
||||
@@ -87,7 +88,7 @@ pub(crate) enum RuntimeEvent {
|
||||
YieldRequested,
|
||||
ToolCall {
|
||||
id: String,
|
||||
name: String,
|
||||
name: ToolName,
|
||||
input: Option<JsonValue>,
|
||||
},
|
||||
Notify {
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use codex_protocol::ToolName;
|
||||
use serde_json::Value as JsonValue;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::mpsc;
|
||||
@@ -26,7 +27,7 @@ use crate::runtime::spawn_runtime;
|
||||
pub trait CodeModeTurnHost: Send + Sync {
|
||||
async fn invoke_tool(
|
||||
&self,
|
||||
tool_name: String,
|
||||
tool_name: ToolName,
|
||||
input: Option<JsonValue>,
|
||||
cancellation_token: CancellationToken,
|
||||
) -> Result<JsonValue, String>;
|
||||
|
||||
Reference in New Issue
Block a user