mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Split multi-agent handlers per tool (#14535)
Summary - move the existing multi-agent handler logic into each tool-specific handler and inline helper implementations - remove the old central dispatcher now that each handler encapsulates its own behavior - adjust handler specs and tests to match the new structure without macros Testing - Not run (not requested)
This commit is contained in:
committed by
GitHub
Unverified
parent
76d8d174b1
commit
793bf32585
@@ -44,7 +44,6 @@ pub use js_repl::JsReplResetHandler;
|
||||
pub use list_dir::ListDirHandler;
|
||||
pub use mcp::McpHandler;
|
||||
pub use mcp_resource::McpResourceHandler;
|
||||
pub use multi_agents::MultiAgentHandler;
|
||||
pub use plan::PlanHandler;
|
||||
pub use read_file::ReadFileHandler;
|
||||
pub use request_permissions::RequestPermissionsHandler;
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -78,7 +78,7 @@ async fn handler_rejects_non_function_payloads() {
|
||||
input: "hello".to_string(),
|
||||
},
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SpawnAgentHandler.handle(invocation).await else {
|
||||
panic!("payload should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -89,24 +89,6 @@ async fn handler_rejects_non_function_payloads() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn handler_rejects_unknown_tool() {
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
let invocation = invocation(
|
||||
Arc::new(session),
|
||||
Arc::new(turn),
|
||||
"unknown_tool",
|
||||
function_payload(json!({})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
panic!("tool should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
err,
|
||||
FunctionCallError::RespondToModel("unsupported collab tool unknown_tool".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_rejects_empty_message() {
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
@@ -116,7 +98,7 @@ async fn spawn_agent_rejects_empty_message() {
|
||||
"spawn_agent",
|
||||
function_payload(json!({"message": " "})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SpawnAgentHandler.handle(invocation).await else {
|
||||
panic!("empty message should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -137,7 +119,7 @@ async fn spawn_agent_rejects_when_message_and_items_are_both_set() {
|
||||
"items": [{"type": "mention", "name": "drive", "path": "app://drive"}]
|
||||
})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SpawnAgentHandler.handle(invocation).await else {
|
||||
panic!("message+items should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -183,7 +165,7 @@ async fn spawn_agent_uses_explorer_role_and_preserves_approval_policy() {
|
||||
"agent_type": "explorer"
|
||||
})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = SpawnAgentHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("spawn_agent should succeed");
|
||||
@@ -216,7 +198,7 @@ async fn spawn_agent_errors_when_manager_dropped() {
|
||||
"spawn_agent",
|
||||
function_payload(json!({"message": "hello"})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SpawnAgentHandler.handle(invocation).await else {
|
||||
panic!("spawn should fail without a manager");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -276,7 +258,7 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
|
||||
"agent_type": "explorer"
|
||||
})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = SpawnAgentHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("spawn_agent should succeed");
|
||||
@@ -321,7 +303,7 @@ async fn spawn_agent_rejects_when_depth_limit_exceeded() {
|
||||
"spawn_agent",
|
||||
function_payload(json!({"message": "hello"})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SpawnAgentHandler.handle(invocation).await else {
|
||||
panic!("spawn should fail when depth limit exceeded");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -360,7 +342,7 @@ async fn spawn_agent_allows_depth_up_to_configured_max_depth() {
|
||||
"spawn_agent",
|
||||
function_payload(json!({"message": "hello"})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = SpawnAgentHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("spawn should succeed within configured depth");
|
||||
@@ -386,7 +368,7 @@ async fn send_input_rejects_empty_message() {
|
||||
"send_input",
|
||||
function_payload(json!({"id": ThreadId::new().to_string(), "message": ""})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SendInputHandler.handle(invocation).await else {
|
||||
panic!("empty message should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -408,7 +390,7 @@ async fn send_input_rejects_when_message_and_items_are_both_set() {
|
||||
"items": [{"type": "mention", "name": "drive", "path": "app://drive"}]
|
||||
})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SendInputHandler.handle(invocation).await else {
|
||||
panic!("message+items should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -428,7 +410,7 @@ async fn send_input_rejects_invalid_id() {
|
||||
"send_input",
|
||||
function_payload(json!({"id": "not-a-uuid", "message": "hi"})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SendInputHandler.handle(invocation).await else {
|
||||
panic!("invalid id should be rejected");
|
||||
};
|
||||
let FunctionCallError::RespondToModel(msg) = err else {
|
||||
@@ -449,7 +431,7 @@ async fn send_input_reports_missing_agent() {
|
||||
"send_input",
|
||||
function_payload(json!({"id": agent_id.to_string(), "message": "hi"})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = SendInputHandler.handle(invocation).await else {
|
||||
panic!("missing agent should be reported");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -476,7 +458,7 @@ async fn send_input_interrupts_before_prompt() {
|
||||
"interrupt": true
|
||||
})),
|
||||
);
|
||||
MultiAgentHandler
|
||||
SendInputHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("send_input should succeed");
|
||||
@@ -517,7 +499,7 @@ async fn send_input_accepts_structured_items() {
|
||||
]
|
||||
})),
|
||||
);
|
||||
MultiAgentHandler
|
||||
SendInputHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("send_input should succeed");
|
||||
@@ -557,7 +539,7 @@ async fn resume_agent_rejects_invalid_id() {
|
||||
"resume_agent",
|
||||
function_payload(json!({"id": "not-a-uuid"})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = ResumeAgentHandler.handle(invocation).await else {
|
||||
panic!("invalid id should be rejected");
|
||||
};
|
||||
let FunctionCallError::RespondToModel(msg) = err else {
|
||||
@@ -578,7 +560,7 @@ async fn resume_agent_reports_missing_agent() {
|
||||
"resume_agent",
|
||||
function_payload(json!({"id": agent_id.to_string()})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = ResumeAgentHandler.handle(invocation).await else {
|
||||
panic!("missing agent should be reported");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -603,7 +585,7 @@ async fn resume_agent_noops_for_active_agent() {
|
||||
function_payload(json!({"id": agent_id.to_string()})),
|
||||
);
|
||||
|
||||
let output = MultiAgentHandler
|
||||
let output = ResumeAgentHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("resume_agent should succeed");
|
||||
@@ -666,7 +648,7 @@ async fn resume_agent_restores_closed_agent_and_accepts_send_input() {
|
||||
"resume_agent",
|
||||
function_payload(json!({"id": agent_id.to_string()})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = ResumeAgentHandler
|
||||
.handle(resume_invocation)
|
||||
.await
|
||||
.expect("resume_agent should succeed");
|
||||
@@ -682,7 +664,7 @@ async fn resume_agent_restores_closed_agent_and_accepts_send_input() {
|
||||
"send_input",
|
||||
function_payload(json!({"id": agent_id.to_string(), "message": "hello"})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = SendInputHandler
|
||||
.handle(send_invocation)
|
||||
.await
|
||||
.expect("send_input should succeed after resume");
|
||||
@@ -723,7 +705,7 @@ async fn resume_agent_rejects_when_depth_limit_exceeded() {
|
||||
"resume_agent",
|
||||
function_payload(json!({"id": ThreadId::new().to_string()})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = ResumeAgentHandler.handle(invocation).await else {
|
||||
panic!("resume should fail when depth limit exceeded");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -746,7 +728,7 @@ async fn wait_rejects_non_positive_timeout() {
|
||||
"timeout_ms": 0
|
||||
})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = WaitHandler.handle(invocation).await else {
|
||||
panic!("non-positive timeout should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -764,7 +746,7 @@ async fn wait_rejects_invalid_id() {
|
||||
"wait",
|
||||
function_payload(json!({"ids": ["invalid"]})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = WaitHandler.handle(invocation).await else {
|
||||
panic!("invalid id should be rejected");
|
||||
};
|
||||
let FunctionCallError::RespondToModel(msg) = err else {
|
||||
@@ -782,7 +764,7 @@ async fn wait_rejects_empty_ids() {
|
||||
"wait",
|
||||
function_payload(json!({"ids": []})),
|
||||
);
|
||||
let Err(err) = MultiAgentHandler.handle(invocation).await else {
|
||||
let Err(err) = WaitHandler.handle(invocation).await else {
|
||||
panic!("empty ids should be rejected");
|
||||
};
|
||||
assert_eq!(
|
||||
@@ -807,7 +789,7 @@ async fn wait_returns_not_found_for_missing_agents() {
|
||||
"timeout_ms": 1000
|
||||
})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = WaitHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("wait should succeed");
|
||||
@@ -841,7 +823,7 @@ async fn wait_times_out_when_status_is_not_final() {
|
||||
"timeout_ms": MIN_WAIT_TIMEOUT_MS
|
||||
})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = WaitHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("wait should succeed");
|
||||
@@ -882,11 +864,7 @@ async fn wait_clamps_short_timeouts_to_minimum() {
|
||||
})),
|
||||
);
|
||||
|
||||
let early = timeout(
|
||||
Duration::from_millis(50),
|
||||
MultiAgentHandler.handle(invocation),
|
||||
)
|
||||
.await;
|
||||
let early = timeout(Duration::from_millis(50), WaitHandler.handle(invocation)).await;
|
||||
assert!(
|
||||
early.is_err(),
|
||||
"wait should not return before the minimum timeout clamp"
|
||||
@@ -931,7 +909,7 @@ async fn wait_returns_final_status_without_timeout() {
|
||||
"timeout_ms": 1000
|
||||
})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = WaitHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("wait should succeed");
|
||||
@@ -964,7 +942,7 @@ async fn close_agent_submits_shutdown_and_returns_status() {
|
||||
"close_agent",
|
||||
function_payload(json!({"id": agent_id.to_string()})),
|
||||
);
|
||||
let output = MultiAgentHandler
|
||||
let output = CloseAgentHandler
|
||||
.handle(invocation)
|
||||
.await
|
||||
.expect("close_agent should succeed");
|
||||
|
||||
@@ -2335,7 +2335,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
use crate::tools::handlers::ListDirHandler;
|
||||
use crate::tools::handlers::McpHandler;
|
||||
use crate::tools::handlers::McpResourceHandler;
|
||||
use crate::tools::handlers::MultiAgentHandler;
|
||||
use crate::tools::handlers::PlanHandler;
|
||||
use crate::tools::handlers::ReadFileHandler;
|
||||
use crate::tools::handlers::RequestPermissionsHandler;
|
||||
@@ -2347,6 +2346,11 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
use crate::tools::handlers::ToolSuggestHandler;
|
||||
use crate::tools::handlers::UnifiedExecHandler;
|
||||
use crate::tools::handlers::ViewImageHandler;
|
||||
use crate::tools::handlers::multi_agents::CloseAgentHandler;
|
||||
use crate::tools::handlers::multi_agents::ResumeAgentHandler;
|
||||
use crate::tools::handlers::multi_agents::SendInputHandler;
|
||||
use crate::tools::handlers::multi_agents::SpawnAgentHandler;
|
||||
use crate::tools::handlers::multi_agents::WaitHandler;
|
||||
use std::sync::Arc;
|
||||
|
||||
let mut builder = ToolRegistryBuilder::new();
|
||||
@@ -2715,7 +2719,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
}
|
||||
|
||||
if config.collab_tools {
|
||||
let multi_agent_handler = Arc::new(MultiAgentHandler);
|
||||
push_tool_spec(
|
||||
&mut builder,
|
||||
create_spawn_agent_tool(config),
|
||||
@@ -2746,11 +2749,11 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
builder.register_handler("spawn_agent", multi_agent_handler.clone());
|
||||
builder.register_handler("send_input", multi_agent_handler.clone());
|
||||
builder.register_handler("resume_agent", multi_agent_handler.clone());
|
||||
builder.register_handler("wait", multi_agent_handler.clone());
|
||||
builder.register_handler("close_agent", multi_agent_handler);
|
||||
builder.register_handler("spawn_agent", Arc::new(SpawnAgentHandler));
|
||||
builder.register_handler("send_input", Arc::new(SendInputHandler));
|
||||
builder.register_handler("resume_agent", Arc::new(ResumeAgentHandler));
|
||||
builder.register_handler("wait", Arc::new(WaitHandler));
|
||||
builder.register_handler("close_agent", Arc::new(CloseAgentHandler));
|
||||
}
|
||||
|
||||
if config.agent_jobs_tools {
|
||||
|
||||
Reference in New Issue
Block a user