[codex] Simplify default namespace migration

This commit is contained in:
Rohit Arunachalam
2026-06-16 20:13:53 -07:00
parent d06ffdfbcb
commit 98ecf3f7e7
6 changed files with 32 additions and 54 deletions

View File

@@ -325,10 +325,6 @@ pub struct ToolRegistry {
impl ToolRegistry {
fn new(tools: HashMap<ToolName, Arc<dyn CoreToolRuntime>>) -> Self {
let tools = tools
.into_iter()
.map(|(name, tool)| (name.with_default_namespace(), tool))
.collect();
Self { tools }
}
@@ -356,8 +352,7 @@ impl ToolRegistry {
where
T: CoreToolRuntime + 'static,
{
let name = handler.tool_name();
Self::new(HashMap::from([(name, handler as Arc<dyn CoreToolRuntime>)]))
Self::from_tools([handler as Arc<dyn CoreToolRuntime>])
}
fn tool(&self, name: &ToolName) -> Option<Arc<dyn CoreToolRuntime>> {

View File

@@ -151,10 +151,8 @@ fn handler_normalizes_only_the_default_namespace() {
let namespaced_handler = Arc::new(TestHandler {
tool_name: namespaced_name.clone(),
}) as Arc<dyn CoreToolRuntime>;
let registry = ToolRegistry::new(HashMap::from([
(plain_name.clone(), Arc::clone(&plain_handler)),
(namespaced_name.clone(), Arc::clone(&namespaced_handler)),
]));
let registry =
ToolRegistry::from_tools([Arc::clone(&plain_handler), Arc::clone(&namespaced_handler)]);
let plain = registry.tool(&plain_name);
let default_namespaced = registry.tool(&codex_tools::ToolName::function(tool_name));
@@ -164,10 +162,7 @@ fn handler_normalizes_only_the_default_namespace() {
tool_name,
));
assert_eq!(plain.is_some(), true);
assert_eq!(default_namespaced.is_some(), true);
assert_eq!(namespaced.is_some(), true);
assert_eq!(missing_namespaced.is_none(), true);
assert!(missing_namespaced.is_none());
assert!(
plain
.as_ref()
@@ -397,10 +392,7 @@ async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> {
tool_name: failing_tool.clone(),
result: LifecycleTestResult::Err,
}) as Arc<dyn CoreToolRuntime>;
let registry = ToolRegistry::new(HashMap::from([
(ok_tool.clone(), ok_handler),
(failing_tool.clone(), failing_handler),
]));
let registry = ToolRegistry::from_tools([ok_handler, failing_handler]);
let session = Arc::new(session);
let turn = Arc::new(turn);

View File

@@ -120,13 +120,6 @@ async fn responses_lite_uses_input_items_for_instructions_and_tools() -> Result<
Ok(())
}
fn functions_namespace(tools: &[Value]) -> Option<&Value> {
tools.iter().find(|tool| {
tool.get("type").and_then(Value::as_str) == Some("namespace")
&& tool.get("name").and_then(Value::as_str) == Some("functions")
})
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_lite_rejects_oversized_additional_tools_item() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -279,7 +272,12 @@ async fn responses_lite_uses_standalone_web_search_and_image_generation() -> Res
assert!(body.get("tools").is_none());
let tools = additional_tools(&body)?;
assert!(!tools.is_empty());
let function_tools = functions_namespace(tools)
let function_tools = tools
.iter()
.find(|tool| {
tool.get("type").and_then(Value::as_str) == Some("namespace")
&& tool.get("name").and_then(Value::as_str) == Some("functions")
})
.and_then(|namespace| namespace.get("tools"))
.and_then(Value::as_array)
.context("Responses Lite should group default tools under functions")?;

View File

@@ -30,7 +30,6 @@ use core_test_support::test_codex::turn_permission_fields;
use core_test_support::wait_for_event;
use serde_json::Value;
use serde_json::json;
use test_case::test_case;
fn call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option<bool>) {
let raw = req.function_call_output(call_id);
assert_eq!(
@@ -136,11 +135,8 @@ async fn shell_command_tool_executes_command_and_streams_output() -> anyhow::Res
Ok(())
}
#[test_case(None; "omitted_namespace")]
#[test_case(Some(""); "empty_namespace")]
#[test_case(Some("functions"); "explicit_functions_namespace")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn update_plan_tool_emits_plan_update_event(namespace: Option<&str>) -> anyhow::Result<()> {
async fn update_plan_tool_emits_plan_update_event() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -163,15 +159,9 @@ async fn update_plan_tool_emits_plan_update_event(namespace: Option<&str>) -> an
})
.to_string();
let function_call = match namespace {
Some(namespace) => {
ev_function_call_with_namespace(call_id, namespace, "update_plan", &plan_args)
}
None => ev_function_call(call_id, "update_plan", &plan_args),
};
let first_response = sse(vec![
ev_response_created("resp-1"),
function_call,
ev_function_call_with_namespace(call_id, "functions", "update_plan", &plan_args),
ev_completed("resp-1"),
]);
responses::mount_sse_once(&server, first_response).await;

View File

@@ -48,9 +48,10 @@ impl ToolName {
}
pub fn is_default_namespace(&self) -> bool {
self.namespace
.as_deref()
.is_none_or(|namespace| namespace.is_empty() || namespace == DEFAULT_FUNCTION_NAMESPACE)
matches!(
self.namespace.as_deref(),
None | Some("") | Some(DEFAULT_FUNCTION_NAMESPACE)
)
}
}

View File

@@ -96,24 +96,31 @@ pub fn create_tools_json_for_responses_api(
pub fn create_tools_json_for_responses_lite(
tools: &[ToolSpec],
) -> Result<Vec<Value>, serde_json::Error> {
let mut functions = Vec::new();
let mut functions_description = None;
let mut functions = ResponsesApiNamespace {
name: DEFAULT_FUNCTION_NAMESPACE.to_string(),
description: default_namespace_description(DEFAULT_FUNCTION_NAMESPACE),
tools: Vec::new(),
};
let mut functions_index = None;
let mut tools_json = Vec::new();
for tool in tools {
match tool {
ToolSpec::Function(tool) => {
functions.push(ResponsesApiNamespaceTool::Function(tool.clone()));
functions
.tools
.push(ResponsesApiNamespaceTool::Function(tool.clone()));
}
ToolSpec::Freeform(tool) => {
functions.push(ResponsesApiNamespaceTool::Freeform(tool.clone()));
functions
.tools
.push(ResponsesApiNamespaceTool::Freeform(tool.clone()));
}
ToolSpec::Namespace(namespace) if namespace.name == DEFAULT_FUNCTION_NAMESPACE => {
if !namespace.description.trim().is_empty() {
functions_description = Some(namespace.description.clone());
functions.description = namespace.description.clone();
}
functions.extend(namespace.tools.clone());
functions.tools.extend(namespace.tools.clone());
}
tool => {
tools_json.push(serde_json::to_value(tool)?);
@@ -124,16 +131,11 @@ pub fn create_tools_json_for_responses_lite(
}
if let Some(functions_index) = functions_index
&& !functions.is_empty()
&& !functions.tools.is_empty()
{
tools_json.insert(
functions_index,
serde_json::to_value(ToolSpec::Namespace(ResponsesApiNamespace {
name: DEFAULT_FUNCTION_NAMESPACE.to_string(),
description: functions_description
.unwrap_or_else(|| default_namespace_description(DEFAULT_FUNCTION_NAMESPACE)),
tools: functions,
}))?,
serde_json::to_value(ToolSpec::Namespace(functions))?,
);
}