mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Make multi-agent v2 tool namespace configurable (#23147)
## Summary - Add `features.multi_agent_v2.tool_namespace` with config/schema validation for Responses-compatible namespace values. - Thread the resolved namespace into `ToolsConfig` for normal turns and review turns. - Wrap MultiAgentV2 tool specs and registry names in the configured namespace when namespace tools are supported, while falling back to the plain tool names when they are not. ## Validation - `just fmt` - `just write-config-schema` - `cargo test -p codex-features multi_agent_v2_feature_config -- --nocapture` - `cargo test -p codex-core test_build_specs_multi_agent_v2 -- --nocapture` - `cargo test -p codex-core multi_agent_v2_config -- --nocapture` - `cargo test -p codex-core multi_agent_v2_rejects_invalid_tool_namespace -- --nocapture` - `cargo test -p codex-tools` - `git diff --check`
This commit is contained in:
jif-oai
committed by
GitHub
Unverified
GitHub
parent
f0166cadbb
commit
545ede569c
@@ -281,10 +281,6 @@ impl ToolRegistry {
|
||||
self.tools.get(name).map(Arc::clone)
|
||||
}
|
||||
|
||||
pub(crate) fn tool_exposure(&self, name: &ToolName) -> Option<ToolExposure> {
|
||||
self.tools.get(name).map(|tool| tool.exposure())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn has_tool(&self, name: &ToolName) -> bool {
|
||||
self.tool(name).is_some()
|
||||
|
||||
@@ -2,6 +2,7 @@ use crate::config::DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::tools::code_mode::execute_spec::create_code_mode_tool;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::handlers::ApplyPatchHandler;
|
||||
use crate::tools::handlers::CodeModeExecuteHandler;
|
||||
use crate::tools::handlers::CodeModeWaitHandler;
|
||||
@@ -58,12 +59,14 @@ use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ResponsesApiNamespace;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::TOOL_SEARCH_TOOL_NAME;
|
||||
use codex_tools::ToolCall as ExtensionToolCall;
|
||||
use codex_tools::ToolEnvironmentMode;
|
||||
use codex_tools::ToolExecutor;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolOutput;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::ToolsConfig;
|
||||
use codex_tools::collect_code_mode_exec_prompt_tool_definitions;
|
||||
@@ -73,6 +76,8 @@ use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use tracing::warn;
|
||||
|
||||
const MULTI_AGENT_V2_NAMESPACE_DESCRIPTION: &str = "Tools for spawning and managing sub-agents.";
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct ToolRegistryBuildParams<'a> {
|
||||
mcp_tools: Option<&'a [ToolInfo]>,
|
||||
@@ -127,22 +132,29 @@ fn build_model_visible_specs_and_registry(
|
||||
let mut specs = Vec::new();
|
||||
let mut seen_tool_names = HashSet::new();
|
||||
for executor in &executors {
|
||||
if !seen_tool_names.insert(executor.tool_name()) {
|
||||
let tool_name = executor.tool_name();
|
||||
if !seen_tool_names.insert(tool_name.clone()) {
|
||||
continue;
|
||||
}
|
||||
if executor.exposure().is_direct()
|
||||
let exposure = executor.exposure();
|
||||
if exposure.is_direct()
|
||||
&& !is_hidden_by_code_mode_only(config, &tool_name, exposure)
|
||||
&& let Some(spec) = executor.spec()
|
||||
{
|
||||
specs.push(spec_for_model_request(config, executor.exposure(), spec));
|
||||
specs.push(spec_for_model_request(config, exposure, spec));
|
||||
}
|
||||
}
|
||||
for spec in hosted_specs {
|
||||
if !is_hidden_by_code_mode_only(config, &ToolName::plain(spec.name()), ToolExposure::Direct)
|
||||
{
|
||||
specs.push(spec);
|
||||
}
|
||||
}
|
||||
specs.extend(hosted_specs);
|
||||
|
||||
let registry = ToolRegistry::from_tools(executors);
|
||||
let model_visible_specs = merge_into_namespaces(specs)
|
||||
.into_iter()
|
||||
.filter(|spec| config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)))
|
||||
.filter(|spec| !is_hidden_by_code_mode_only(config, ®istry, spec))
|
||||
.collect();
|
||||
|
||||
(model_visible_specs, registry)
|
||||
@@ -210,17 +222,14 @@ fn agent_type_description(config: &ToolsConfig, default_agent_type_description:
|
||||
|
||||
fn is_hidden_by_code_mode_only(
|
||||
config: &ToolsConfig,
|
||||
registry: &ToolRegistry,
|
||||
spec: &ToolSpec,
|
||||
tool_name: &ToolName,
|
||||
exposure: ToolExposure,
|
||||
) -> bool {
|
||||
if !config.code_mode_only_enabled || !codex_code_mode::is_code_mode_nested_tool(spec.name()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let exposure = registry
|
||||
.tool_exposure(&ToolName::plain(spec.name()))
|
||||
.unwrap_or(ToolExposure::Direct);
|
||||
exposure != ToolExposure::DirectModelOnly
|
||||
config.code_mode_only_enabled
|
||||
&& exposure != ToolExposure::DirectModelOnly
|
||||
&& codex_code_mode::is_code_mode_nested_tool(&codex_tools::code_mode_name_for_tool_name(
|
||||
tool_name,
|
||||
))
|
||||
}
|
||||
|
||||
fn build_code_mode_executors(
|
||||
@@ -442,6 +451,10 @@ fn collect_tool_executors(
|
||||
} else {
|
||||
ToolExposure::Direct
|
||||
};
|
||||
let tool_namespace = config
|
||||
.namespace_tools
|
||||
.then_some(config.multi_agent_v2_tool_namespace.as_deref())
|
||||
.flatten();
|
||||
let agent_type_description =
|
||||
agent_type_description(config, params.default_agent_type_description);
|
||||
executors.push(multi_agent_v2_handler(
|
||||
@@ -454,15 +467,33 @@ fn collect_tool_executors(
|
||||
max_concurrent_threads_per_session: config.max_concurrent_threads_per_session,
|
||||
}),
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
SendMessageHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
FollowupTaskHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(SendMessageHandlerV2, exposure));
|
||||
executors.push(multi_agent_v2_handler(FollowupTaskHandlerV2, exposure));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
WaitAgentHandlerV2::new(params.wait_agent_timeouts),
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
CloseAgentHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
ListAgentsHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(CloseAgentHandlerV2, exposure));
|
||||
executors.push(multi_agent_v2_handler(ListAgentsHandlerV2, exposure));
|
||||
} else {
|
||||
let agent_type_description =
|
||||
agent_type_description(config, params.default_agent_type_description);
|
||||
@@ -592,8 +623,70 @@ fn append_extension_tool_executors(
|
||||
fn multi_agent_v2_handler(
|
||||
handler: impl CoreToolRuntime + 'static,
|
||||
exposure: ToolExposure,
|
||||
namespace: Option<&str>,
|
||||
) -> Arc<dyn CoreToolRuntime> {
|
||||
override_tool_exposure(Arc::new(handler), exposure)
|
||||
let handler: Arc<dyn CoreToolRuntime> = match namespace {
|
||||
Some(namespace) => Arc::new(MultiAgentV2NamespaceOverride {
|
||||
handler: Arc::new(handler),
|
||||
namespace: namespace.to_string(),
|
||||
}),
|
||||
None => Arc::new(handler),
|
||||
};
|
||||
override_tool_exposure(handler, exposure)
|
||||
}
|
||||
|
||||
struct MultiAgentV2NamespaceOverride {
|
||||
handler: Arc<dyn CoreToolRuntime>,
|
||||
namespace: String,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ToolExecutor<ToolInvocation> for MultiAgentV2NamespaceOverride {
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::namespaced(self.namespace.clone(), self.handler.tool_name().name)
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<ToolSpec> {
|
||||
match self.handler.spec()? {
|
||||
ToolSpec::Function(tool) => Some(ToolSpec::Namespace(ResponsesApiNamespace {
|
||||
name: self.namespace.clone(),
|
||||
description: MULTI_AGENT_V2_NAMESPACE_DESCRIPTION.to_string(),
|
||||
tools: vec![ResponsesApiNamespaceTool::Function(tool)],
|
||||
})),
|
||||
spec => Some(spec),
|
||||
}
|
||||
}
|
||||
|
||||
fn exposure(&self) -> ToolExposure {
|
||||
self.handler.exposure()
|
||||
}
|
||||
|
||||
fn supports_parallel_tool_calls(&self) -> bool {
|
||||
self.handler.supports_parallel_tool_calls()
|
||||
}
|
||||
|
||||
async fn handle(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
) -> Result<Box<dyn ToolOutput>, codex_tools::FunctionCallError> {
|
||||
self.handler.handle(invocation).await
|
||||
}
|
||||
}
|
||||
|
||||
impl CoreToolRuntime for MultiAgentV2NamespaceOverride {
|
||||
fn matches_kind(&self, payload: &crate::tools::context::ToolPayload) -> bool {
|
||||
self.handler.matches_kind(payload)
|
||||
}
|
||||
|
||||
fn search_info(&self) -> Option<crate::tools::tool_search_entry::ToolSearchInfo> {
|
||||
self.handler.search_info()
|
||||
}
|
||||
|
||||
fn create_diff_consumer(
|
||||
&self,
|
||||
) -> Option<Box<dyn crate::tools::registry::ToolArgumentDiffConsumer>> {
|
||||
self.handler.create_diff_consumer()
|
||||
}
|
||||
}
|
||||
|
||||
fn compare_code_mode_tools(
|
||||
|
||||
@@ -1341,3 +1341,67 @@ async fn code_mode_only_can_expose_multi_agent_v2_as_normal_tools() {
|
||||
};
|
||||
assert!(!spawn_agent.description.contains("exec tool declaration"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn code_mode_only_can_expose_namespaced_multi_agent_v2_as_normal_tools() {
|
||||
let config = test_config().await;
|
||||
let model_info = construct_model_info_offline("gpt-5.4", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::CodeModeOnly);
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Live),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
})
|
||||
.with_multi_agent_v2_tool_namespace(Some("agents".to_string()))
|
||||
.with_multi_agent_v2_non_code_mode_only(/*multi_agent_v2_non_code_mode_only*/ true);
|
||||
let router = ToolRouter::from_config(
|
||||
&tools_config,
|
||||
ToolRouterParams {
|
||||
mcp_tools: None,
|
||||
deferred_mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: &[],
|
||||
},
|
||||
);
|
||||
let model_visible_specs = router.model_visible_specs();
|
||||
let tool_names = model_visible_specs
|
||||
.iter()
|
||||
.map(ToolSpec::name)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(tool_names, vec!["exec", "wait", "agents"]);
|
||||
|
||||
let exec = find_tool(&model_visible_specs, "exec");
|
||||
let ToolSpec::Freeform(exec) = exec else {
|
||||
panic!("exec should be a freeform tool");
|
||||
};
|
||||
assert!(!exec.description.contains("spawn_agent"));
|
||||
assert!(!exec.description.contains("wait_agent"));
|
||||
assert!(
|
||||
!exec
|
||||
.description
|
||||
.contains("do not attempt to use any other tools directly")
|
||||
);
|
||||
|
||||
for tool_name in [
|
||||
"spawn_agent",
|
||||
"send_message",
|
||||
"followup_task",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"list_agents",
|
||||
] {
|
||||
let tool = find_namespace_function_tool(&model_visible_specs, "agents", tool_name);
|
||||
assert!(!tool.description.contains("exec tool declaration"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -577,6 +577,77 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
|
||||
assert_lacks_tool_name(&tools, "resume_agent");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_multi_agent_v2_uses_configured_tool_namespace() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
})
|
||||
.with_multi_agent_v2_tool_namespace(Some("agents".to_string()));
|
||||
let (tools, registry) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
|
||||
assert_contains_tool_names(&tools, &["agents"]);
|
||||
for tool_name in [
|
||||
"spawn_agent",
|
||||
"send_message",
|
||||
"followup_task",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"list_agents",
|
||||
] {
|
||||
assert_lacks_tool_name(&tools, tool_name);
|
||||
assert!(registry.has_tool(&ToolName::namespaced("agents", tool_name)));
|
||||
assert!(!registry.has_tool(&ToolName::plain(tool_name)));
|
||||
assert_namespace_contains_function(&tools, "agents", tool_name);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_multi_agent_v2_ignores_tool_namespace_without_namespace_support() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
let available_models = Vec::new();
|
||||
let mut tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
})
|
||||
.with_multi_agent_v2_tool_namespace(Some("agents".to_string()));
|
||||
tools_config.namespace_tools = false;
|
||||
let (tools, registry) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
|
||||
assert_contains_tool_names(&tools, &["spawn_agent", "send_message", "list_agents"]);
|
||||
assert_lacks_tool_name(&tools, "agents");
|
||||
assert!(registry.has_tool(&ToolName::plain("spawn_agent")));
|
||||
assert!(!registry.has_tool(&ToolName::namespaced("agents", "spawn_agent")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_multi_agent_v2_does_not_require_collab_feature() {
|
||||
let model_info = model_info();
|
||||
@@ -2657,6 +2728,23 @@ fn find_tool<'a>(tools: &'a [ToolSpec], expected_name: &str) -> &'a ToolSpec {
|
||||
.unwrap_or_else(|| panic!("expected tool {expected_name}"))
|
||||
}
|
||||
|
||||
fn assert_namespace_contains_function(
|
||||
tools: &[ToolSpec],
|
||||
expected_namespace: &str,
|
||||
expected_name: &str,
|
||||
) {
|
||||
let namespace_tool = find_tool(tools, expected_namespace);
|
||||
let ToolSpec::Namespace(namespace) = namespace_tool else {
|
||||
panic!("expected namespace tool {expected_namespace}");
|
||||
};
|
||||
assert!(
|
||||
namespace.tools.iter().any(|tool| {
|
||||
matches!(tool, ResponsesApiNamespaceTool::Function(tool) if tool.name == expected_name)
|
||||
}),
|
||||
"expected tool {expected_name} in namespace {expected_namespace}"
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_process_tool_environment_id(
|
||||
tools: &[ToolSpec],
|
||||
expected_name: &str,
|
||||
|
||||
Reference in New Issue
Block a user