mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Render namespace description for tools (#16879)
This commit is contained in:
committed by
GitHub
Unverified
parent
9091999c83
commit
d47b755aa2
@@ -6888,16 +6888,18 @@ pub(crate) async fn built_tools(
|
||||
} else {
|
||||
app_tools
|
||||
};
|
||||
let mcp_tool_router_inputs =
|
||||
has_mcp_servers.then(|| crate::tools::router::map_mcp_tool_infos(&mcp_tools));
|
||||
|
||||
Ok(Arc::new(ToolRouter::from_config(
|
||||
&turn_context.tools_config,
|
||||
ToolRouterParams {
|
||||
mcp_tools: has_mcp_servers.then(|| {
|
||||
mcp_tools
|
||||
.into_iter()
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect()
|
||||
}),
|
||||
mcp_tools: mcp_tool_router_inputs
|
||||
.as_ref()
|
||||
.map(|inputs| inputs.mcp_tools.clone()),
|
||||
tool_namespaces: mcp_tool_router_inputs
|
||||
.as_ref()
|
||||
.map(|inputs| inputs.tool_namespaces.clone()),
|
||||
app_tools,
|
||||
discoverable_tools,
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -121,7 +121,7 @@ mod guardian_tests;
|
||||
|
||||
struct InstructionsTestCase {
|
||||
slug: &'static str,
|
||||
expects_apply_patch_instructions: bool,
|
||||
expects_apply_patch_description: bool,
|
||||
}
|
||||
|
||||
fn user_message(text: &str) -> ResponseItem {
|
||||
@@ -305,6 +305,7 @@ fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> T
|
||||
&turn_context.tools_config,
|
||||
crate::tools::router::ToolRouterParams {
|
||||
mcp_tools: None,
|
||||
tool_namespaces: None,
|
||||
app_tools: None,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
@@ -413,6 +414,7 @@ fn make_mcp_tool(
|
||||
server_name: server_name.to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
tool_namespace,
|
||||
server_instructions: None,
|
||||
tool: Tool {
|
||||
name: tool_name.to_string().into(),
|
||||
title: None,
|
||||
@@ -695,19 +697,19 @@ async fn get_base_instructions_no_user_content() {
|
||||
let test_cases = vec![
|
||||
InstructionsTestCase {
|
||||
slug: "gpt-5",
|
||||
expects_apply_patch_instructions: false,
|
||||
expects_apply_patch_description: false,
|
||||
},
|
||||
InstructionsTestCase {
|
||||
slug: "gpt-5.1",
|
||||
expects_apply_patch_instructions: false,
|
||||
expects_apply_patch_description: false,
|
||||
},
|
||||
InstructionsTestCase {
|
||||
slug: "gpt-5.1-codex",
|
||||
expects_apply_patch_instructions: false,
|
||||
expects_apply_patch_description: false,
|
||||
},
|
||||
InstructionsTestCase {
|
||||
slug: "gpt-5.1-codex-max",
|
||||
expects_apply_patch_instructions: false,
|
||||
expects_apply_patch_description: false,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -716,7 +718,7 @@ async fn get_base_instructions_no_user_content() {
|
||||
|
||||
for test_case in test_cases {
|
||||
let model_info = model_info_for_slug(test_case.slug, &config);
|
||||
if test_case.expects_apply_patch_instructions {
|
||||
if test_case.expects_apply_patch_description {
|
||||
assert_eq!(
|
||||
model_info.base_instructions.as_str(),
|
||||
prompt_with_apply_patch_instructions
|
||||
@@ -5292,15 +5294,12 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
|
||||
.await
|
||||
};
|
||||
let app_tools = Some(tools.clone());
|
||||
let mcp_tool_router_inputs = crate::tools::router::map_mcp_tool_infos(&tools);
|
||||
let router = ToolRouter::from_config(
|
||||
&turn_context.tools_config,
|
||||
crate::tools::router::ToolRouterParams {
|
||||
mcp_tools: Some(
|
||||
tools
|
||||
.into_iter()
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect(),
|
||||
),
|
||||
mcp_tools: Some(mcp_tool_router_inputs.mcp_tools),
|
||||
tool_namespaces: Some(mcp_tool_router_inputs.tool_namespaces),
|
||||
app_tools,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -112,6 +112,7 @@ fn codex_app_tool(
|
||||
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
tool_namespace,
|
||||
server_instructions: None,
|
||||
tool: test_tool_definition(tool_name),
|
||||
connector_id: Some(connector_id.to_string()),
|
||||
connector_name: connector_name.map(ToOwned::to_owned),
|
||||
@@ -190,6 +191,7 @@ fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
|
||||
server_name: "sample".to_string(),
|
||||
tool_name: "echo".to_string(),
|
||||
tool_namespace: "sample".to_string(),
|
||||
server_instructions: None,
|
||||
tool: test_tool_definition("echo"),
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
@@ -314,6 +316,7 @@ fn accessible_connectors_from_mcp_tools_preserves_description() {
|
||||
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
tool_name: "calendar_create_event".to_string(),
|
||||
tool_namespace: "mcp__codex_apps__calendar".to_string(),
|
||||
server_instructions: None,
|
||||
tool: Tool {
|
||||
name: "calendar_create_event".to_string().into(),
|
||||
title: None,
|
||||
|
||||
@@ -257,15 +257,14 @@ async fn build_nested_router(exec: &ExecContext) -> ToolRouter {
|
||||
.read()
|
||||
.await
|
||||
.list_all_tools()
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|(name, tool_info)| (name, tool_info.tool))
|
||||
.collect();
|
||||
.await;
|
||||
let mcp_tool_router_inputs = crate::tools::router::map_mcp_tool_infos(&mcp_tools);
|
||||
|
||||
ToolRouter::from_config(
|
||||
&nested_tools_config,
|
||||
ToolRouterParams {
|
||||
mcp_tools: Some(mcp_tools),
|
||||
mcp_tools: Some(mcp_tool_router_inputs.mcp_tools),
|
||||
tool_namespaces: Some(mcp_tool_router_inputs.tool_namespaces),
|
||||
app_tools: None,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: exec.turn.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -1561,16 +1561,13 @@ impl JsReplManager {
|
||||
.await
|
||||
.list_all_tools()
|
||||
.await;
|
||||
let mcp_tool_router_inputs = crate::tools::router::map_mcp_tool_infos(&mcp_tools);
|
||||
|
||||
let router = ToolRouter::from_config(
|
||||
&exec.turn.tools_config,
|
||||
crate::tools::router::ToolRouterParams {
|
||||
mcp_tools: Some(
|
||||
mcp_tools
|
||||
.into_iter()
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect(),
|
||||
),
|
||||
mcp_tools: Some(mcp_tool_router_inputs.mcp_tools),
|
||||
tool_namespaces: Some(mcp_tool_router_inputs.tool_namespaces),
|
||||
app_tools: None,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: exec.turn.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -16,6 +16,7 @@ use codex_protocol::models::SearchToolCallParams;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use codex_tools::ConfiguredToolSpec;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ToolNamespace;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::ToolsConfig;
|
||||
use rmcp::model::Tool;
|
||||
@@ -41,15 +42,43 @@ pub struct ToolRouter {
|
||||
|
||||
pub(crate) struct ToolRouterParams<'a> {
|
||||
pub(crate) mcp_tools: Option<HashMap<String, Tool>>,
|
||||
pub(crate) tool_namespaces: Option<HashMap<String, ToolNamespace>>,
|
||||
pub(crate) app_tools: Option<HashMap<String, ToolInfo>>,
|
||||
pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
pub(crate) dynamic_tools: &'a [DynamicToolSpec],
|
||||
}
|
||||
|
||||
pub(crate) struct McpToolRouterInputs {
|
||||
pub(crate) mcp_tools: HashMap<String, Tool>,
|
||||
pub(crate) tool_namespaces: HashMap<String, ToolNamespace>,
|
||||
}
|
||||
|
||||
pub(crate) fn map_mcp_tool_infos(mcp_tools: &HashMap<String, ToolInfo>) -> McpToolRouterInputs {
|
||||
McpToolRouterInputs {
|
||||
mcp_tools: mcp_tools
|
||||
.iter()
|
||||
.map(|(name, tool)| (name.clone(), tool.tool.clone()))
|
||||
.collect(),
|
||||
tool_namespaces: mcp_tools
|
||||
.iter()
|
||||
.map(|(name, tool)| {
|
||||
(
|
||||
name.clone(),
|
||||
ToolNamespace {
|
||||
name: tool.tool_namespace.clone(),
|
||||
description: tool.server_instructions.clone(),
|
||||
},
|
||||
)
|
||||
})
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
impl ToolRouter {
|
||||
pub fn from_config(config: &ToolsConfig, params: ToolRouterParams<'_>) -> Self {
|
||||
let ToolRouterParams {
|
||||
mcp_tools,
|
||||
tool_namespaces,
|
||||
app_tools,
|
||||
discoverable_tools,
|
||||
dynamic_tools,
|
||||
@@ -58,6 +87,7 @@ impl ToolRouter {
|
||||
config,
|
||||
mcp_tools,
|
||||
app_tools,
|
||||
tool_namespaces,
|
||||
discoverable_tools,
|
||||
dynamic_tools,
|
||||
);
|
||||
|
||||
@@ -35,6 +35,7 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> {
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect(),
|
||||
),
|
||||
tool_namespaces: None,
|
||||
app_tools,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
@@ -93,6 +94,7 @@ async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()>
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect(),
|
||||
),
|
||||
tool_namespaces: None,
|
||||
app_tools,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ToolHandlerKind;
|
||||
use codex_tools::ToolNamespace;
|
||||
use codex_tools::ToolRegistryPlanAppTool;
|
||||
use codex_tools::ToolRegistryPlanParams;
|
||||
use codex_tools::ToolUserShellType;
|
||||
@@ -33,6 +34,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
config: &ToolsConfig,
|
||||
mcp_tools: Option<HashMap<String, rmcp::model::Tool>>,
|
||||
app_tools: Option<HashMap<String, ToolInfo>>,
|
||||
tool_namespaces: Option<HashMap<String, ToolNamespace>>,
|
||||
discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> ToolRegistryBuilder {
|
||||
@@ -86,6 +88,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
config,
|
||||
ToolRegistryPlanParams {
|
||||
mcp_tools: mcp_tools.as_ref(),
|
||||
tool_namespaces: tool_namespaces.as_ref(),
|
||||
app_tools: app_tool_sources.as_deref(),
|
||||
discoverable_tools: discoverable_tools.as_deref(),
|
||||
dynamic_tools,
|
||||
|
||||
@@ -181,6 +181,7 @@ fn build_specs(
|
||||
config,
|
||||
mcp_tools,
|
||||
app_tools,
|
||||
/*tool_namespaces*/ None,
|
||||
/*discoverable_tools*/ None,
|
||||
dynamic_tools,
|
||||
)
|
||||
@@ -261,6 +262,7 @@ fn assert_model_tools(
|
||||
&tools_config,
|
||||
ToolRouterParams {
|
||||
mcp_tools: None,
|
||||
tool_namespaces: None,
|
||||
app_tools: None,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: &[],
|
||||
@@ -628,6 +630,7 @@ fn tool_suggest_requires_apps_and_plugins_features() {
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*app_tools*/ None,
|
||||
/*tool_namespaces*/ None,
|
||||
discoverable_tools.clone(),
|
||||
&[],
|
||||
)
|
||||
@@ -701,6 +704,7 @@ fn search_tool_description_falls_back_to_connector_name_without_description() {
|
||||
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
tool_name: "_create_event".to_string(),
|
||||
tool_namespace: "mcp__codex_apps__calendar".to_string(),
|
||||
server_instructions: None,
|
||||
tool: mcp_tool(
|
||||
"calendar_create_event",
|
||||
"Create calendar event",
|
||||
@@ -751,6 +755,7 @@ fn search_tool_registers_namespaced_app_tool_aliases() {
|
||||
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
tool_name: "_create_event".to_string(),
|
||||
tool_namespace: "mcp__codex_apps__calendar".to_string(),
|
||||
server_instructions: None,
|
||||
tool: mcp_tool(
|
||||
"calendar-create-event",
|
||||
"Create calendar event",
|
||||
@@ -768,6 +773,7 @@ fn search_tool_registers_namespaced_app_tool_aliases() {
|
||||
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
tool_name: "_list_events".to_string(),
|
||||
tool_namespace: "mcp__codex_apps__calendar".to_string(),
|
||||
server_instructions: None,
|
||||
tool: mcp_tool(
|
||||
"calendar-list-events",
|
||||
"List calendar events",
|
||||
|
||||
Reference in New Issue
Block a user