From f5c8d7e5c5b932057ce06ffb6fd1c5ca0b1f3f06 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Wed, 3 Jun 2026 12:37:39 -0700 Subject: [PATCH] fix(subagent): align advertised role aliases --- crates/tui/src/tools/subagent/mod.rs | 35 ++++--- crates/tui/src/tools/subagent/tests.rs | 121 ++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 14 deletions(-) diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 4275fa57..b9b48e75 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -94,13 +94,26 @@ const SUBAGENT_STATE_SCHEMA_VERSION: u32 = 1; const SUBAGENT_STATE_FILE: &str = "subagents.v1.json"; const SUBAGENT_RESTART_REASON: &str = "Interrupted by process restart"; -const VALID_SUBAGENT_TYPES: &str = "general, explore, plan, review, implementer, verifier, tool_agent, custom, \ - worker, explorer, awaiter, default, implement, builder, verify, validator, tester, tool-agent, executor, fin"; +const VALID_SUBAGENT_TYPES: &str = "general (aliases: general-purpose, general_purpose, worker, default), \ + explore (aliases: exploration, explorer), plan (aliases: planning, planner, awaiter), \ + review (aliases: code-review, code_review, reviewer), implementer (aliases: implement, implementation, builder), \ + verifier (aliases: verify, verification, validator, tester), tool_agent (aliases: tool-agent, toolagent, executor, execution, fin), custom"; /// Role aliases accepted by `normalize_role_alias`. Kept in sync with the /// match arms below so every input that `SubAgentType::from_str` accepts also /// resolves to a canonical role (avoids the dual-validation rejection in #2649). -const VALID_ROLE_ALIASES: &str = "default, worker, explorer, awaiter, reviewer, implementer, verifier, tool_agent \ - (aliases: general, explore, plan/planner, review, implement/builder, verify/validator/tester, executor/fin)"; +const VALID_ROLE_ALIASES: &str = "default; worker (aliases: general, general-purpose, general_purpose); \ + explorer (aliases: explore, exploration); awaiter (aliases: plan, planning, planner); \ + reviewer (aliases: review, code-review, code_review); implementer (aliases: implement, implementation, builder); \ + verifier (aliases: verify, verification, validator, tester); tool_agent (aliases: tool-agent, toolagent, executor, execution, fin); custom"; +const SUBAGENT_TYPE_DESCRIPTION: &str = "Sub-agent type. Accepted vocabulary: general (aliases: general-purpose, general_purpose, worker, default), \ + explore (aliases: exploration, explorer), plan (aliases: planning, planner, awaiter), \ + review (aliases: code-review, code_review, reviewer), implementer (aliases: implement, implementation, builder), \ + verifier (aliases: verify, verification, validator, tester), tool_agent (aliases: tool-agent, toolagent, executor, execution, fin), custom."; +const SUBAGENT_ROLE_DESCRIPTION: &str = "Role alias. Accepted vocabulary: default; worker (aliases: general, general-purpose, general_purpose); \ + explorer (aliases: explore, exploration); awaiter (aliases: plan, planning, planner); \ + reviewer (aliases: review, code-review, code_review); implementer (aliases: implement, implementation, builder); \ + verifier (aliases: verify, verification, validator, tester); tool_agent (aliases: tool-agent, toolagent, executor, execution, fin); custom. \ + Must match `type` if both are given."; /// Whale species used as friendly names for sub-agents in the UI. The full /// Cetacea infraorder — baleen whales (Mysticeti), toothed whales /// (Odontoceti), plus select dolphin species (family Delphinidae) that @@ -2085,7 +2098,7 @@ impl ToolSpec for AgentOpenTool { }, "type": { "type": "string", - "description": "Sub-agent type: general, explore, plan, review, implementer, verifier, custom" + "description": SUBAGENT_TYPE_DESCRIPTION }, "agent_type": { "type": "string", @@ -2093,7 +2106,7 @@ impl ToolSpec for AgentOpenTool { }, "role": { "type": "string", - "description": "Role alias (canonical: default, worker, explorer, awaiter, reviewer, implementer, verifier, tool_agent). Must match `type` if both are given." + "description": SUBAGENT_ROLE_DESCRIPTION }, "agent_role": { "type": "string", @@ -2348,7 +2361,7 @@ impl ToolSpec for AgentSpawnTool { }, "type": { "type": "string", - "description": "Sub-agent type: general, explore, plan, review, implementer, verifier, custom. See docs/SUBAGENTS.md for posture per role." + "description": SUBAGENT_TYPE_DESCRIPTION }, "agent_type": { "type": "string", @@ -2360,7 +2373,7 @@ impl ToolSpec for AgentSpawnTool { }, "role": { "type": "string", - "description": "Role alias (canonical: default, worker, explorer, awaiter, reviewer, implementer, verifier, tool_agent). Must match `type` if both are given." + "description": SUBAGENT_ROLE_DESCRIPTION }, "agent_role": { "type": "string", @@ -3235,7 +3248,7 @@ impl ToolSpec for AgentAssignTool { }, "role": { "type": "string", - "description": "Updated role alias (canonical: default, worker, explorer, awaiter, reviewer, implementer, verifier, tool_agent)." + "description": SUBAGENT_ROLE_DESCRIPTION }, "agent_role": { "type": "string", @@ -3467,7 +3480,7 @@ impl ToolSpec for DelegateToAgentTool { "properties": { "agent_name": { "type": "string", - "description": "Name/type alias for the agent (general, explore, plan, review, implementer, verifier, worker, explorer, awaiter, builder, validator, tester)" + "description": SUBAGENT_TYPE_DESCRIPTION }, "type": { "type": "string", @@ -3479,7 +3492,7 @@ impl ToolSpec for DelegateToAgentTool { }, "role": { "type": "string", - "description": "Role alias (canonical: default, worker, explorer, awaiter, reviewer, implementer, verifier, tool_agent). Must match `type` if both are given." + "description": SUBAGENT_ROLE_DESCRIPTION }, "agent_role": { "type": "string", diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index d49833aa..72d50215 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -513,16 +513,52 @@ fn test_parse_spawn_request_rejects_invalid_role() { fn test_parse_spawn_request_accepts_full_role_vocabulary() { // Regression for #2649: roles that `SubAgentType::from_str` accepts must // also pass the second `normalize_role_alias` validation pass instead of - // being rejected with a stale four-value hint. + // being rejected with a stale hint. for (role, expected_type, expected_role) in [ - ("reviewer", SubAgentType::Review, "reviewer"), - ("review", SubAgentType::Review, "reviewer"), + ("general", SubAgentType::General, "worker"), + ("general-purpose", SubAgentType::General, "worker"), + ("general_purpose", SubAgentType::General, "worker"), + ("worker", SubAgentType::General, "worker"), + ("default", SubAgentType::General, "default"), + ("explore", SubAgentType::Explore, "explorer"), + ("exploration", SubAgentType::Explore, "explorer"), + ("explorer", SubAgentType::Explore, "explorer"), + ("plan", SubAgentType::Plan, "awaiter"), + ("planning", SubAgentType::Plan, "awaiter"), ("planner", SubAgentType::Plan, "awaiter"), + ("awaiter", SubAgentType::Plan, "awaiter"), + ("review", SubAgentType::Review, "reviewer"), + ("code-review", SubAgentType::Review, "reviewer"), + ("code_review", SubAgentType::Review, "reviewer"), + ("reviewer", SubAgentType::Review, "reviewer"), ("implementer", SubAgentType::Implementer, "implementer"), + ("implement", SubAgentType::Implementer, "implementer"), + ("implementation", SubAgentType::Implementer, "implementer"), ("builder", SubAgentType::Implementer, "implementer"), ("verifier", SubAgentType::Verifier, "verifier"), + ("verify", SubAgentType::Verifier, "verifier"), + ("verification", SubAgentType::Verifier, "verifier"), + ("validator", SubAgentType::Verifier, "verifier"), ("tester", SubAgentType::Verifier, "verifier"), + ("tool-agent", SubAgentType::ToolAgent, "tool_agent"), + ("tool_agent", SubAgentType::ToolAgent, "tool_agent"), + ("toolagent", SubAgentType::ToolAgent, "tool_agent"), + ("executor", SubAgentType::ToolAgent, "tool_agent"), + ("execution", SubAgentType::ToolAgent, "tool_agent"), + ("fin", SubAgentType::ToolAgent, "tool_agent"), + ("custom", SubAgentType::Custom, "custom"), ] { + assert_eq!( + SubAgentType::from_str(role), + Some(expected_type.clone()), + "from_str should accept role alias {role:?}" + ); + assert_eq!( + normalize_role_alias(role), + Some(expected_role), + "normalize_role_alias should accept role alias {role:?}" + ); + let input = json!({ "prompt": "do work", "role": role }); let parsed = parse_spawn_request(&input) .unwrap_or_else(|e| panic!("role {role:?} should parse, got {e}")); @@ -544,6 +580,85 @@ fn test_invalid_role_error_lists_real_aliases() { .to_string(); assert!(err.contains("reviewer"), "hint should list reviewer: {err}"); assert!(err.contains("verifier"), "hint should list verifier: {err}"); + assert!(err.contains("custom"), "hint should list custom: {err}"); + assert!( + err.contains("general-purpose"), + "hint should list general-purpose: {err}" + ); + assert!( + err.contains("code_review"), + "hint should list code_review: {err}" + ); + assert!( + err.contains("toolagent"), + "hint should list toolagent: {err}" + ); + assert!( + err.contains("execution"), + "hint should list execution: {err}" + ); +} + +fn schema_property_description<'a>(schema: &'a Value, property: &str) -> &'a str { + schema["properties"][property]["description"] + .as_str() + .unwrap_or_else(|| panic!("missing description for schema property {property:?}")) +} + +#[test] +fn subagent_tool_schemas_advertise_real_type_and_role_vocabulary() { + let tmp = tempdir().expect("tempdir"); + let manager = new_shared_subagent_manager(tmp.path().to_path_buf(), 1); + let agent_open_schema = AgentOpenTool::new(manager.clone(), stub_runtime()).input_schema(); + let agent_spawn_schema = AgentSpawnTool::new(manager.clone(), stub_runtime()).input_schema(); + let delegate_schema = DelegateToAgentTool::new(manager, stub_runtime()).input_schema(); + + for (schema, property) in [ + (&agent_open_schema, "type"), + (&agent_spawn_schema, "type"), + (&delegate_schema, "agent_name"), + ] { + let description = schema_property_description(schema, property); + for alias in [ + "general", + "explore", + "plan", + "review", + "implementer", + "verifier", + "tool_agent", + "custom", + ] { + assert!( + description.contains(alias), + "{property} description should list accepted type {alias:?}: {description}" + ); + } + } + + for (schema, property) in [ + (&agent_open_schema, "role"), + (&agent_spawn_schema, "role"), + (&delegate_schema, "role"), + ] { + let description = schema_property_description(schema, property); + for alias in [ + "default", + "worker", + "explorer", + "awaiter", + "reviewer", + "implementer", + "verifier", + "tool_agent", + "custom", + ] { + assert!( + description.contains(alias), + "{property} description should list accepted role {alias:?}: {description}" + ); + } + } } #[test]