mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Fix dynamic namespace description precedence
This commit is contained in:
@@ -22,7 +22,6 @@ use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSearchSourceInfo;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::default_namespace_description;
|
||||
use codex_tools::dynamic_tool_to_responses_api_tool;
|
||||
use serde_json::Value;
|
||||
use std::time::Instant;
|
||||
@@ -48,9 +47,7 @@ impl DynamicToolHandler {
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|description| !description.is_empty());
|
||||
let description = custom_description
|
||||
.map(str::to_string)
|
||||
.unwrap_or_else(|| default_namespace_description(namespace));
|
||||
let description = custom_description.unwrap_or_default().to_string();
|
||||
let search_source_info = custom_description.map_or_else(
|
||||
|| ToolSearchSourceInfo {
|
||||
name: "Dynamic tools".to_string(),
|
||||
|
||||
@@ -149,21 +149,35 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn mixed_search_results_coalesce_mcp_namespaces() {
|
||||
let dynamic_tools = [DynamicToolSpec {
|
||||
namespace: Some("codex_app".to_string()),
|
||||
namespace_description: None,
|
||||
name: "automation_update".to_string(),
|
||||
description: "Create, update, view, or delete recurring automations.".to_string(),
|
||||
input_schema: serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"mode": { "type": "string" },
|
||||
},
|
||||
"required": ["mode"],
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: true,
|
||||
}];
|
||||
let dynamic_tools = [
|
||||
DynamicToolSpec {
|
||||
namespace: Some("codex_app".to_string()),
|
||||
namespace_description: None,
|
||||
name: "automation_list".to_string(),
|
||||
description: "List recurring automations.".to_string(),
|
||||
input_schema: serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: true,
|
||||
},
|
||||
DynamicToolSpec {
|
||||
namespace: Some("codex_app".to_string()),
|
||||
namespace_description: Some("Create and update Codex automations.".to_string()),
|
||||
name: "automation_update".to_string(),
|
||||
description: "Create, update, view, or delete recurring automations.".to_string(),
|
||||
input_schema: serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"mode": { "type": "string" },
|
||||
},
|
||||
"required": ["mode"],
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: true,
|
||||
},
|
||||
];
|
||||
let mcp_tools = [
|
||||
tool_info("calendar", "create_event", "Create events"),
|
||||
tool_info("calendar", "list_events", "List events"),
|
||||
@@ -187,6 +201,7 @@ mod tests {
|
||||
let results = [
|
||||
&handler.entries[0],
|
||||
&handler.entries[2],
|
||||
&handler.entries[3],
|
||||
&handler.entries[1],
|
||||
];
|
||||
|
||||
@@ -229,23 +244,37 @@ mod tests {
|
||||
}),
|
||||
LoadableToolSpec::Namespace(ResponsesApiNamespace {
|
||||
name: "codex_app".to_string(),
|
||||
description: "Tools in the codex_app namespace.".to_string(),
|
||||
tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "automation_update".to_string(),
|
||||
description: "Create, update, view, or delete recurring automations."
|
||||
.to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: codex_tools::JsonSchema::object(
|
||||
std::collections::BTreeMap::from([(
|
||||
"mode".to_string(),
|
||||
codex_tools::JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
Some(vec!["mode".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
})],
|
||||
description: "Create and update Codex automations.".to_string(),
|
||||
tools: vec![
|
||||
ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "automation_list".to_string(),
|
||||
description: "List recurring automations.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: codex_tools::JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
}),
|
||||
ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "automation_update".to_string(),
|
||||
description: "Create, update, view, or delete recurring automations."
|
||||
.to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: codex_tools::JsonSchema::object(
|
||||
std::collections::BTreeMap::from([(
|
||||
"mode".to_string(),
|
||||
codex_tools::JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
Some(vec!["mode".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
],
|
||||
);
|
||||
|
||||
@@ -494,6 +494,11 @@ fn code_mode_namespace_descriptions(
|
||||
entry.description = namespace.description.clone();
|
||||
}
|
||||
}
|
||||
for namespace in namespace_descriptions.values_mut() {
|
||||
if namespace.description.trim().is_empty() {
|
||||
namespace.description = default_namespace_description(&namespace.name);
|
||||
}
|
||||
}
|
||||
namespace_descriptions
|
||||
}
|
||||
|
||||
|
||||
@@ -703,6 +703,10 @@ async fn code_mode_only_exposes_code_executor_and_hides_nested_tools() {
|
||||
plain.namespace_function_names("codex_app"),
|
||||
&["lookup".to_string()]
|
||||
);
|
||||
let ToolSpec::Namespace(namespace) = plain.visible_spec("codex_app") else {
|
||||
panic!("expected namespaced dynamic tool");
|
||||
};
|
||||
assert_eq!(namespace.description, "Tools in the codex_app namespace.");
|
||||
plain.assert_visible_lacks(&[
|
||||
codex_code_mode::PUBLIC_TOOL_NAME,
|
||||
codex_code_mode::WAIT_TOOL_NAME,
|
||||
@@ -730,6 +734,40 @@ async fn code_mode_only_exposes_code_executor_and_hides_nested_tools() {
|
||||
code_mode_only.namespace_function_names("codex_app"),
|
||||
Vec::<String>::new().as_slice()
|
||||
);
|
||||
let ToolSpec::Freeform(exec) = code_mode_only.visible_spec(codex_code_mode::PUBLIC_TOOL_NAME)
|
||||
else {
|
||||
panic!("expected code mode executor");
|
||||
};
|
||||
assert!(
|
||||
exec.description
|
||||
.contains("## codex_app\nTools in the codex_app namespace.")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dynamic_namespaces_prefer_a_later_supplied_description() {
|
||||
let mut described_tool =
|
||||
dynamic_tool(Some("codex_app"), "update", /*defer_loading*/ false);
|
||||
described_tool.namespace_description = Some("Create and update Codex automations.".to_string());
|
||||
let plan = probe_with(
|
||||
|_| {},
|
||||
ToolPlanInputs {
|
||||
dynamic_tools: vec![
|
||||
dynamic_tool(Some("codex_app"), "list", /*defer_loading*/ false),
|
||||
described_tool,
|
||||
],
|
||||
..ToolPlanInputs::default()
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let ToolSpec::Namespace(namespace) = plan.visible_spec("codex_app") else {
|
||||
panic!("expected namespaced dynamic tools");
|
||||
};
|
||||
assert_eq!(
|
||||
namespace.description,
|
||||
"Create and update Codex automations."
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -2,7 +2,6 @@ use codex_tools::LoadableToolSpec;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ToolSearchSourceInfo;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::default_namespace_description;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ToolSearchEntry {
|
||||
@@ -29,9 +28,6 @@ impl ToolSearchInfo {
|
||||
LoadableToolSpec::Function(tool)
|
||||
}
|
||||
ToolSpec::Namespace(mut namespace) => {
|
||||
if namespace.description.trim().is_empty() {
|
||||
namespace.description = default_namespace_description(&namespace.name);
|
||||
}
|
||||
for tool in &mut namespace.tools {
|
||||
let ResponsesApiNamespaceTool::Function(tool) = tool;
|
||||
tool.defer_loading = Some(true);
|
||||
|
||||
@@ -94,6 +94,11 @@ pub fn coalesce_loadable_tool_specs(
|
||||
LoadableToolSpec::Function(_) | LoadableToolSpec::Namespace(_) => None,
|
||||
})
|
||||
{
|
||||
if existing_namespace.description.trim().is_empty()
|
||||
&& !namespace.description.trim().is_empty()
|
||||
{
|
||||
existing_namespace.description = namespace.description;
|
||||
}
|
||||
existing_namespace.tools.append(&mut namespace.tools);
|
||||
} else {
|
||||
coalesced_specs.push(LoadableToolSpec::Namespace(namespace));
|
||||
@@ -101,6 +106,14 @@ pub fn coalesce_loadable_tool_specs(
|
||||
}
|
||||
}
|
||||
}
|
||||
for spec in &mut coalesced_specs {
|
||||
let LoadableToolSpec::Namespace(namespace) = spec else {
|
||||
continue;
|
||||
};
|
||||
if namespace.description.trim().is_empty() {
|
||||
namespace.description = default_namespace_description(&namespace.name);
|
||||
}
|
||||
}
|
||||
coalesced_specs
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user