| 1 | use super::*; |
| 2 | use tempfile::tempdir; |
| 3 | |
| 4 | fn make_assignment() -> SubAgentAssignment { |
| 5 | SubAgentAssignment::new("prompt".to_string(), Some("worker".to_string())) |
| 6 | } |
| 7 | |
| 8 | fn make_snapshot(status: SubAgentStatus) -> SubAgentResult { |
| 9 | SubAgentResult { |
| 10 | agent_id: "agent_test".to_string(), |
| 11 | agent_type: SubAgentType::General, |
| 12 | assignment: make_assignment(), |
| 13 | model: "deepseek-v4-flash".to_string(), |
| 14 | nickname: None, |
| 15 | status, |
| 16 | result: None, |
| 17 | steps_taken: 0, |
| 18 | duration_ms: 0, |
| 19 | from_prior_session: false, |
| 20 | } |
| 21 | } |
| 22 | |
| 23 | #[test] |
| 24 | fn test_agent_type_from_str() { |
| 25 | assert_eq!( |
| 26 | SubAgentType::from_str("general"), |
| 27 | Some(SubAgentType::General) |
| 28 | ); |
| 29 | assert_eq!( |
| 30 | SubAgentType::from_str("explore"), |
| 31 | Some(SubAgentType::Explore) |
| 32 | ); |
| 33 | assert_eq!(SubAgentType::from_str("PLAN"), Some(SubAgentType::Plan)); |
| 34 | assert_eq!( |
| 35 | SubAgentType::from_str("code-review"), |
| 36 | Some(SubAgentType::Review) |
| 37 | ); |
| 38 | assert_eq!( |
| 39 | SubAgentType::from_str("worker"), |
| 40 | Some(SubAgentType::General) |
| 41 | ); |
| 42 | assert_eq!( |
| 43 | SubAgentType::from_str("default"), |
| 44 | Some(SubAgentType::General) |
| 45 | ); |
| 46 | assert_eq!( |
| 47 | SubAgentType::from_str("explorer"), |
| 48 | Some(SubAgentType::Explore) |
| 49 | ); |
| 50 | assert_eq!(SubAgentType::from_str("awaiter"), Some(SubAgentType::Plan)); |
| 51 | assert_eq!(SubAgentType::from_str("invalid"), None); |
| 52 | } |
| 53 | |
| 54 | #[test] |
| 55 | fn test_agent_type_implementer_aliases() { |
| 56 | // #404 — Implementer accepts the obvious aliases the model is |
| 57 | // likely to reach for when the user says "build this". |
| 58 | for alias in ["implementer", "implement", "implementation", "builder"] { |
| 59 | assert_eq!( |
| 60 | SubAgentType::from_str(alias), |
| 61 | Some(SubAgentType::Implementer), |
| 62 | "alias {alias} should resolve to Implementer" |
| 63 | ); |
| 64 | } |
| 65 | // Case-insensitive. |
| 66 | assert_eq!( |
| 67 | SubAgentType::from_str("IMPLEMENTER"), |
| 68 | Some(SubAgentType::Implementer) |
| 69 | ); |
| 70 | } |
| 71 | |
| 72 | #[test] |
| 73 | fn test_agent_type_verifier_aliases() { |
| 74 | // #404 — Verifier accepts test/validate aliases distinct from |
| 75 | // Reviewer, which is for *grading* code rather than *running* it. |
| 76 | for alias in ["verifier", "verify", "verification", "validator", "tester"] { |
| 77 | assert_eq!( |
| 78 | SubAgentType::from_str(alias), |
| 79 | Some(SubAgentType::Verifier), |
| 80 | "alias {alias} should resolve to Verifier" |
| 81 | ); |
| 82 | } |
| 83 | assert_eq!( |
| 84 | SubAgentType::from_str("VERIFY"), |
| 85 | Some(SubAgentType::Verifier) |
| 86 | ); |
| 87 | } |
| 88 | |
| 89 | #[test] |
| 90 | fn test_agent_type_round_trips_via_as_str() { |
| 91 | // Every type should serialize to a string that round-trips back |
| 92 | // through `from_str`. Catches missed variants when adding a new |
| 93 | // role. |
| 94 | for t in [ |
| 95 | SubAgentType::General, |
| 96 | SubAgentType::Explore, |
| 97 | SubAgentType::Plan, |
| 98 | SubAgentType::Review, |
| 99 | SubAgentType::Implementer, |
| 100 | SubAgentType::Verifier, |
| 101 | SubAgentType::Custom, |
| 102 | ] { |
| 103 | let label = t.as_str(); |
| 104 | let back = SubAgentType::from_str(label) |
| 105 | .unwrap_or_else(|| panic!("as_str label {label:?} doesn't round-trip via from_str")); |
| 106 | assert_eq!(back, t, "round-trip failed for {t:?} via {label:?}"); |
| 107 | } |
| 108 | } |
| 109 | |
| 110 | #[test] |
| 111 | fn test_implementer_and_verifier_have_distinct_prompts() { |
| 112 | // The whole point of adding the types is that they carry distinct |
| 113 | // posture. Defensive guard: catch the easy bug where copy-paste |
| 114 | // leaves two new variants with the same prompt as `General`. |
| 115 | let implementer = SubAgentType::Implementer.system_prompt(); |
| 116 | let verifier = SubAgentType::Verifier.system_prompt(); |
| 117 | let general = SubAgentType::General.system_prompt(); |
| 118 | assert_ne!( |
| 119 | implementer, general, |
| 120 | "Implementer prompt must differ from General" |
| 121 | ); |
| 122 | assert_ne!( |
| 123 | verifier, general, |
| 124 | "Verifier prompt must differ from General" |
| 125 | ); |
| 126 | assert_ne!( |
| 127 | implementer, verifier, |
| 128 | "Implementer and Verifier must differ" |
| 129 | ); |
| 130 | // Sanity: each prompt mentions the role's defining verb so the |
| 131 | // model has clear direction. |
| 132 | assert!( |
| 133 | implementer.to_lowercase().contains("implement") |
| 134 | || implementer.to_lowercase().contains("write the code"), |
| 135 | "Implementer prompt should reference its role: {implementer}" |
| 136 | ); |
| 137 | assert!( |
| 138 | verifier.to_lowercase().contains("verif") |
| 139 | || verifier.to_lowercase().contains("test suite") |
| 140 | || verifier.to_lowercase().contains("validation"), |
| 141 | "Verifier prompt should reference its role: {verifier}" |
| 142 | ); |
| 143 | } |
| 144 | |
| 145 | #[test] |
| 146 | fn test_implementer_allowed_tools_include_writes() { |
| 147 | // Implementer is the write-heavy role; the deprecated |
| 148 | // `allowed_tools()` advisory list should reflect that the role |
| 149 | // can write/edit/patch even if today's runtime grants full |
| 150 | // inheritance. |
| 151 | #[allow(deprecated)] |
| 152 | let tools = SubAgentType::Implementer.allowed_tools(); |
| 153 | assert!(tools.contains(&"write_file")); |
| 154 | assert!(tools.contains(&"edit_file")); |
| 155 | assert!(tools.contains(&"apply_patch")); |
| 156 | } |
| 157 | |
| 158 | #[test] |
| 159 | fn test_verifier_allowed_tools_include_test_runner_but_no_writes() { |
| 160 | // Verifier runs validation; it should not have write tools in |
| 161 | // its advisory list. The runtime will still gate writes through |
| 162 | // approval, but the advisory list signals intent. |
| 163 | #[allow(deprecated)] |
| 164 | let tools = SubAgentType::Verifier.allowed_tools(); |
| 165 | assert!(tools.contains(&"run_tests")); |
| 166 | assert!(tools.contains(&"diagnostics")); |
| 167 | assert!(!tools.contains(&"write_file")); |
| 168 | assert!(!tools.contains(&"apply_patch")); |
| 169 | } |
| 170 | |
| 171 | #[test] |
| 172 | fn test_parse_spawn_request_accepts_message_and_agent_type_aliases() { |
| 173 | let input = json!({ |
| 174 | "message": "Find references to Foo", |
| 175 | "agent_type": "explorer" |
| 176 | }); |
| 177 | let parsed = parse_spawn_request(&input).expect("spawn request should parse"); |
| 178 | assert_eq!(parsed.prompt, "Find references to Foo"); |
| 179 | assert_eq!(parsed.agent_type, SubAgentType::Explore); |
| 180 | assert_eq!(parsed.assignment.role.as_deref(), Some("explorer")); |
| 181 | } |
| 182 | |
| 183 | #[test] |
| 184 | fn test_parse_spawn_request_accepts_objective_and_role_alias() { |
| 185 | let input = json!({ |
| 186 | "objective": "Coordinate and wait", |
| 187 | "role": "awaiter" |
| 188 | }); |
| 189 | let parsed = parse_spawn_request(&input).expect("spawn request should parse"); |
| 190 | assert_eq!(parsed.prompt, "Coordinate and wait"); |
| 191 | assert_eq!(parsed.agent_type, SubAgentType::Plan); |
| 192 | assert_eq!(parsed.assignment.role.as_deref(), Some("awaiter")); |
| 193 | } |
| 194 | |
| 195 | #[test] |
| 196 | fn test_parse_spawn_request_accepts_items_payload() { |
| 197 | let input = json!({ |
| 198 | "items": [ |
| 199 | {"type": "text", "text": "Analyze module"}, |
| 200 | {"type": "mention", "name": "drive", "path": "app://drive"} |
| 201 | ], |
| 202 | "agent_name": "explorer" |
| 203 | }); |
| 204 | let parsed = parse_spawn_request(&input).expect("spawn request should parse"); |
| 205 | assert!(parsed.prompt.contains("Analyze module")); |
| 206 | assert!(parsed.prompt.contains("[mention:$drive](app://drive)")); |
| 207 | assert_eq!(parsed.agent_type, SubAgentType::Explore); |
| 208 | } |
| 209 | |
| 210 | #[test] |
| 211 | fn test_parse_spawn_request_rejects_text_and_items_together() { |
| 212 | let input = json!({ |
| 213 | "prompt": "Analyze module", |
| 214 | "items": [{"type": "text", "text": "dup"}] |
| 215 | }); |
| 216 | let err = parse_spawn_request(&input).expect_err("text+items should fail"); |
| 217 | assert!(err.to_string().contains("either prompt text or items")); |
| 218 | } |
| 219 | |
| 220 | #[test] |
| 221 | fn test_parse_spawn_request_rejects_invalid_role() { |
| 222 | let input = json!({ |
| 223 | "prompt": "do work", |
| 224 | "role": "unknown_role" |
| 225 | }); |
| 226 | let err = parse_spawn_request(&input).expect_err("invalid role should fail"); |
| 227 | assert!(err.to_string().contains("Invalid role alias")); |
| 228 | } |
| 229 | |
| 230 | #[test] |
| 231 | fn test_parse_spawn_request_rejects_conflicting_type_and_role() { |
| 232 | let input = json!({ |
| 233 | "prompt": "inspect internals", |
| 234 | "type": "explore", |
| 235 | "role": "worker" |
| 236 | }); |
| 237 | let err = parse_spawn_request(&input).expect_err("conflicting type+role should fail"); |
| 238 | assert!( |
| 239 | err.to_string() |
| 240 | .contains("Conflicting type/agent_type and role/agent_role") |
| 241 | ); |
| 242 | } |
| 243 | |
| 244 | #[test] |
| 245 | fn test_parse_assign_request_accepts_aliases() { |
| 246 | let input = json!({ |
| 247 | "id": "agent_1234", |
| 248 | "objective": "re-check failing tests", |
| 249 | "agent_role": "explorer", |
| 250 | "input": "focus on tests only", |
| 251 | "interrupt": false |
| 252 | }); |
| 253 | let request = parse_assign_request(&input).expect("assign request should parse"); |
| 254 | assert_eq!(request.agent_id, "agent_1234"); |
| 255 | assert_eq!(request.objective.as_deref(), Some("re-check failing tests")); |
| 256 | assert_eq!(request.role.as_deref(), Some("explorer")); |
| 257 | assert_eq!(request.message.as_deref(), Some("focus on tests only")); |
| 258 | assert!(!request.interrupt); |
| 259 | } |
| 260 | |
| 261 | #[test] |
| 262 | fn test_parse_assign_request_rejects_invalid_role() { |
| 263 | let input = json!({ |
| 264 | "agent_id": "agent_1234", |
| 265 | "role": "unknown" |
| 266 | }); |
| 267 | let err = parse_assign_request(&input).expect_err("invalid role should fail"); |
| 268 | assert!(err.to_string().contains("Invalid role alias")); |
| 269 | } |
| 270 | |
| 271 | #[test] |
| 272 | fn test_parse_assign_request_requires_update_fields() { |
| 273 | let input = json!({ |
| 274 | "agent_id": "agent_1234" |
| 275 | }); |
| 276 | let err = parse_assign_request(&input).expect_err("missing update fields should fail"); |
| 277 | assert!( |
| 278 | err.to_string().contains( |
| 279 | "Provide at least one of objective, role/agent_role, message/input, or items" |
| 280 | ) |
| 281 | ); |
| 282 | } |
| 283 | |
| 284 | #[test] |
| 285 | fn test_send_input_schema_does_not_require_message_field() { |
| 286 | let manager = Arc::new(RwLock::new(SubAgentManager::new(PathBuf::from("."), 1))); |
| 287 | let schema = AgentSendInputTool::new(manager, "send_input").input_schema(); |
| 288 | let required = schema |
| 289 | .get("required") |
| 290 | .and_then(Value::as_array) |
| 291 | .cloned() |
| 292 | .unwrap_or_default(); |
| 293 | assert!( |
| 294 | !required |
| 295 | .iter() |
| 296 | .any(|entry| entry.as_str().is_some_and(|name| name == "message")), |
| 297 | "send_input schema should allow items-only payloads" |
| 298 | ); |
| 299 | } |
| 300 | |
| 301 | #[test] |
| 302 | fn test_build_allowed_tools_independent_of_allow_shell() { |
| 303 | // v0.6.6: allow_shell no longer filters at the build_allowed_tools |
| 304 | // level — the registry builder controls shell-tool registration. |
| 305 | // Both calls return None (full inheritance) for a default General |
| 306 | // agent. |
| 307 | let with_shell = build_allowed_tools(&SubAgentType::General, None, true).unwrap(); |
| 308 | let without_shell = build_allowed_tools(&SubAgentType::General, None, false).unwrap(); |
| 309 | assert!(with_shell.is_none()); |
| 310 | assert!(without_shell.is_none()); |
| 311 | } |
| 312 | |
| 313 | #[test] |
| 314 | fn test_allowed_tools_are_deduplicated() { |
| 315 | let tools = build_allowed_tools( |
| 316 | &SubAgentType::Custom, |
| 317 | Some(vec![ |
| 318 | "read_file".to_string(), |
| 319 | "read_file".to_string(), |
| 320 | " ".to_string(), |
| 321 | "grep_files".to_string(), |
| 322 | ]), |
| 323 | true, |
| 324 | ) |
| 325 | .unwrap(); |
| 326 | assert_eq!( |
| 327 | tools, |
| 328 | Some(vec!["read_file".to_string(), "grep_files".to_string()]) |
| 329 | ); |
| 330 | } |
| 331 | |
| 332 | #[test] |
| 333 | fn test_custom_agent_requires_allowed_tools() { |
| 334 | let err = build_allowed_tools(&SubAgentType::Custom, None, true).unwrap_err(); |
| 335 | assert!(err.to_string().contains("requires")); |
| 336 | } |
| 337 | |
| 338 | #[test] |
| 339 | fn test_wait_mode_condition_any_and_all() { |
| 340 | let one_done = vec![ |
| 341 | make_snapshot(SubAgentStatus::Running), |
| 342 | make_snapshot(SubAgentStatus::Completed), |
| 343 | ]; |
| 344 | let all_done = vec![ |
| 345 | make_snapshot(SubAgentStatus::Completed), |
| 346 | make_snapshot(SubAgentStatus::Cancelled), |
| 347 | ]; |
| 348 | |
| 349 | assert!(WaitMode::Any.condition_met(&one_done)); |
| 350 | assert!(!WaitMode::All.condition_met(&one_done)); |
| 351 | assert!(WaitMode::All.condition_met(&all_done)); |
| 352 | } |
| 353 | |
| 354 | #[test] |
| 355 | fn test_parse_wait_mode() { |
| 356 | assert_eq!(parse_wait_mode(&json!({})).unwrap(), WaitMode::Any); |
| 357 | assert_eq!( |
| 358 | parse_wait_mode(&json!({"wait_mode": "all"})).unwrap(), |
| 359 | WaitMode::All |
| 360 | ); |
| 361 | assert_eq!( |
| 362 | parse_wait_mode(&json!({"wait_mode": "first"})).unwrap(), |
| 363 | WaitMode::Any |
| 364 | ); |
| 365 | assert!(parse_wait_mode(&json!({"wait_mode": "invalid"})).is_err()); |
| 366 | } |
| 367 | |
| 368 | #[test] |
| 369 | fn test_parse_wait_ids_accepts_aliases() { |
| 370 | let ids = parse_wait_ids(&json!({ |
| 371 | "ids": ["agent_a", "agent_b"], |
| 372 | "agent_id": "agent_c", |
| 373 | "id": "agent_a" |
| 374 | })); |
| 375 | |
| 376 | assert_eq!(ids, vec!["agent_a", "agent_b", "agent_c"]); |
| 377 | } |
| 378 | |
| 379 | #[test] |
| 380 | fn test_parse_wait_ids_empty_when_omitted() { |
| 381 | let ids = parse_wait_ids(&json!({})); |
| 382 | assert!(ids.is_empty()); |
| 383 | } |
| 384 | |
| 385 | #[test] |
| 386 | fn test_build_assignment_prompt_includes_metadata() { |
| 387 | let assignment = SubAgentAssignment::new( |
| 388 | "Inspect parser behavior".to_string(), |
| 389 | Some("explorer".to_string()), |
| 390 | ); |
| 391 | let prompt = build_assignment_prompt( |
| 392 | "Inspect parser behavior", |
| 393 | &assignment, |
| 394 | &SubAgentType::Explore, |
| 395 | ); |
| 396 | assert!(prompt.contains("Assignment metadata")); |
| 397 | assert!(prompt.contains("resolved_type: explore")); |
| 398 | assert!(prompt.contains("role: explorer")); |
| 399 | } |
| 400 | |
| 401 | #[test] |
| 402 | fn subagent_auto_model_routes_unconfigured_assignments() { |
| 403 | let runtime = stub_runtime().with_auto_model(true); |
| 404 | |
| 405 | assert_eq!( |
| 406 | fallback_subagent_assignment_route(&runtime, None, "implement the release fix").model, |
| 407 | "deepseek-v4-pro" |
| 408 | ); |
| 409 | assert_eq!( |
| 410 | fallback_subagent_assignment_route(&runtime, None, "say hello").model, |
| 411 | "deepseek-v4-flash" |
| 412 | ); |
| 413 | } |
| 414 | |
| 415 | #[test] |
| 416 | fn subagent_auto_route_respects_explicit_or_role_model() { |
| 417 | let runtime = stub_runtime().with_auto_model(true); |
| 418 | |
| 419 | assert_eq!( |
| 420 | fallback_subagent_assignment_route( |
| 421 | &runtime, |
| 422 | Some("deepseek-v4-flash".to_string()), |
| 423 | "implement the release fix" |
| 424 | ) |
| 425 | .model, |
| 426 | "deepseek-v4-flash" |
| 427 | ); |
| 428 | } |
| 429 | |
| 430 | #[test] |
| 431 | fn subagent_auto_reasoning_resolves_to_distinct_v4_tiers() { |
| 432 | let runtime = stub_runtime().with_reasoning_effort(Some("high".to_string()), true); |
| 433 | |
| 434 | assert_eq!( |
| 435 | fallback_subagent_assignment_route(&runtime, None, "quick lookup").reasoning_effort, |
| 436 | Some("high".to_string()) |
| 437 | ); |
| 438 | assert_eq!( |
| 439 | fallback_subagent_assignment_route(&runtime, None, "debug this release failure") |
| 440 | .reasoning_effort, |
| 441 | Some("max".to_string()) |
| 442 | ); |
| 443 | } |
| 444 | |
| 445 | #[test] |
| 446 | fn subagent_router_prompt_frames_assignment_as_auto_routing() { |
| 447 | let runtime = stub_runtime() |
| 448 | .with_auto_model(true) |
| 449 | .with_reasoning_effort(Some("high".to_string()), true); |
| 450 | let prompt = subagent_router_prompt(&runtime, "inspect one file"); |
| 451 | |
| 452 | assert!(prompt.contains("Parent selected model mode: auto")); |
| 453 | assert!(prompt.contains("Parent selected thinking mode: auto")); |
| 454 | assert!(prompt.contains("inspect one file")); |
| 455 | } |
| 456 | |
| 457 | #[test] |
| 458 | fn test_subagent_tool_registry_reports_unavailable_tools() { |
| 459 | let tmp = tempdir().expect("tempdir"); |
| 460 | let mut runtime = stub_runtime(); |
| 461 | runtime.context = ToolContext::new(tmp.path().to_path_buf()); |
| 462 | runtime.allow_shell = false; |
| 463 | let registry = SubAgentToolRegistry::new( |
| 464 | runtime, |
| 465 | Some(vec!["read_file".to_string(), "missing_tool".to_string()]), |
| 466 | Arc::new(Mutex::new(TodoList::new())), |
| 467 | Arc::new(Mutex::new(PlanState::default())), |
| 468 | ); |
| 469 | assert_eq!( |
| 470 | registry.unavailable_allowed_tools(), |
| 471 | vec!["missing_tool".to_string()] |
| 472 | ); |
| 473 | } |
| 474 | |
| 475 | #[tokio::test] |
| 476 | async fn test_wait_for_result_reports_timeout_when_still_running() { |
| 477 | let manager = Arc::new(RwLock::new(SubAgentManager::new(PathBuf::from("."), 2))); |
| 478 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 479 | let agent = SubAgent::new( |
| 480 | SubAgentType::Explore, |
| 481 | "prompt".to_string(), |
| 482 | make_assignment(), |
| 483 | "deepseek-v4-flash".to_string(), |
| 484 | Some("Blue".to_string()), |
| 485 | Some(vec!["read_file".to_string()]), |
| 486 | input_tx, |
| 487 | "boot_test".to_string(), |
| 488 | ); |
| 489 | let agent_id = agent.id.clone(); |
| 490 | { |
| 491 | let mut guard = manager.write().await; |
| 492 | guard.agents.insert(agent_id.clone(), agent); |
| 493 | } |
| 494 | |
| 495 | let (snapshot, timed_out) = wait_for_result(&manager, &agent_id, Duration::from_millis(10)) |
| 496 | .await |
| 497 | .expect("wait_for_result should succeed"); |
| 498 | assert!(timed_out); |
| 499 | assert_eq!(snapshot.status, SubAgentStatus::Running); |
| 500 | } |
| 501 | |
| 502 | #[tokio::test] |
| 503 | async fn test_running_count_counts_only_agents_with_live_task_handles() { |
| 504 | let mut manager = SubAgentManager::new(PathBuf::from("."), 1); |
| 505 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 506 | let mut agent = SubAgent::new( |
| 507 | SubAgentType::Explore, |
| 508 | "prompt".to_string(), |
| 509 | make_assignment(), |
| 510 | "deepseek-v4-flash".to_string(), |
| 511 | Some("Blue".to_string()), |
| 512 | Some(vec!["read_file".to_string()]), |
| 513 | input_tx, |
| 514 | "boot_test".to_string(), |
| 515 | ); |
| 516 | agent.status = SubAgentStatus::Running; |
| 517 | let handle = tokio::spawn(async { |
| 518 | tokio::time::sleep(Duration::from_secs(60)).await; |
| 519 | }); |
| 520 | agent.task_handle = Some(handle); |
| 521 | let agent_id = agent.id.clone(); |
| 522 | manager.agents.insert(agent.id.clone(), agent); |
| 523 | |
| 524 | assert_eq!(manager.running_count(), 1); |
| 525 | manager |
| 526 | .agents |
| 527 | .get_mut(&agent_id) |
| 528 | .and_then(|agent| agent.task_handle.take()) |
| 529 | .expect("live task handle") |
| 530 | .abort(); |
| 531 | } |
| 532 | |
| 533 | #[test] |
| 534 | fn test_running_count_ignores_running_status_without_task_handle() { |
| 535 | let mut manager = SubAgentManager::new(PathBuf::from("."), 1); |
| 536 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 537 | let mut agent = SubAgent::new( |
| 538 | SubAgentType::Explore, |
| 539 | "prompt".to_string(), |
| 540 | make_assignment(), |
| 541 | "deepseek-v4-flash".to_string(), |
| 542 | Some("Blue".to_string()), |
| 543 | Some(vec!["read_file".to_string()]), |
| 544 | input_tx, |
| 545 | "boot_test".to_string(), |
| 546 | ); |
| 547 | agent.status = SubAgentStatus::Running; |
| 548 | manager.agents.insert(agent.id.clone(), agent); |
| 549 | |
| 550 | assert_eq!(manager.running_count(), 0); |
| 551 | } |
| 552 | |
| 553 | #[tokio::test] |
| 554 | async fn test_running_count_ignores_finished_task_handles() { |
| 555 | let mut manager = SubAgentManager::new(PathBuf::from("."), 1); |
| 556 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 557 | let mut agent = SubAgent::new( |
| 558 | SubAgentType::Explore, |
| 559 | "prompt".to_string(), |
| 560 | make_assignment(), |
| 561 | "deepseek-v4-flash".to_string(), |
| 562 | Some("Blue".to_string()), |
| 563 | Some(vec!["read_file".to_string()]), |
| 564 | input_tx, |
| 565 | "boot_test".to_string(), |
| 566 | ); |
| 567 | agent.status = SubAgentStatus::Running; |
| 568 | let handle = tokio::spawn(async {}); |
| 569 | handle.await.expect("dummy task should finish immediately"); |
| 570 | agent.task_handle = Some(tokio::spawn(async {})); |
| 571 | if let Some(handle) = agent.task_handle.as_ref() { |
| 572 | while !handle.is_finished() { |
| 573 | tokio::task::yield_now().await; |
| 574 | } |
| 575 | } |
| 576 | manager.agents.insert(agent.id.clone(), agent); |
| 577 | |
| 578 | assert_eq!(manager.running_count(), 0); |
| 579 | } |
| 580 | |
| 581 | #[test] |
| 582 | fn test_assign_updates_running_agent_and_sends_message() { |
| 583 | let mut manager = SubAgentManager::new(PathBuf::from("."), 2); |
| 584 | let (input_tx, mut input_rx) = mpsc::unbounded_channel(); |
| 585 | let agent = SubAgent::new( |
| 586 | SubAgentType::General, |
| 587 | "work".to_string(), |
| 588 | make_assignment(), |
| 589 | "deepseek-v4-flash".to_string(), |
| 590 | Some("Blue".to_string()), |
| 591 | Some(vec!["read_file".to_string()]), |
| 592 | input_tx, |
| 593 | "boot_test".to_string(), |
| 594 | ); |
| 595 | let agent_id = agent.id.clone(); |
| 596 | manager.agents.insert(agent_id.clone(), agent); |
| 597 | |
| 598 | let snapshot = manager |
| 599 | .assign( |
| 600 | &agent_id, |
| 601 | Some("Re-check module boundaries".to_string()), |
| 602 | Some("explorer".to_string()), |
| 603 | None, |
| 604 | true, |
| 605 | ) |
| 606 | .expect("assignment should succeed"); |
| 607 | assert_eq!(snapshot.assignment.objective, "Re-check module boundaries"); |
| 608 | assert_eq!(snapshot.assignment.role.as_deref(), Some("explorer")); |
| 609 | |
| 610 | let dispatched = input_rx |
| 611 | .try_recv() |
| 612 | .expect("running agent should receive assignment update"); |
| 613 | assert!(dispatched.interrupt); |
| 614 | assert!(dispatched.text.contains("Assignment updated")); |
| 615 | assert!(dispatched.text.contains("objective")); |
| 616 | } |
| 617 | |
| 618 | #[test] |
| 619 | fn test_assign_rejects_message_for_non_running_agent() { |
| 620 | let mut manager = SubAgentManager::new(PathBuf::from("."), 1); |
| 621 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 622 | let mut agent = SubAgent::new( |
| 623 | SubAgentType::Explore, |
| 624 | "prompt".to_string(), |
| 625 | make_assignment(), |
| 626 | "deepseek-v4-flash".to_string(), |
| 627 | Some("Blue".to_string()), |
| 628 | Some(vec!["read_file".to_string()]), |
| 629 | input_tx, |
| 630 | "boot_test".to_string(), |
| 631 | ); |
| 632 | agent.status = SubAgentStatus::Completed; |
| 633 | let agent_id = agent.id.clone(); |
| 634 | manager.agents.insert(agent_id.clone(), agent); |
| 635 | |
| 636 | let err = manager |
| 637 | .assign(&agent_id, None, None, Some("keep going".to_string()), true) |
| 638 | .expect_err("non-running agent cannot receive assignment message"); |
| 639 | assert!(err.to_string().contains("is not running")); |
| 640 | } |
| 641 | |
| 642 | #[test] |
| 643 | fn test_assign_updates_non_running_metadata_without_message() { |
| 644 | let mut manager = SubAgentManager::new(PathBuf::from("."), 1); |
| 645 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 646 | let mut agent = SubAgent::new( |
| 647 | SubAgentType::Plan, |
| 648 | "prompt".to_string(), |
| 649 | make_assignment(), |
| 650 | "deepseek-v4-flash".to_string(), |
| 651 | Some("Blue".to_string()), |
| 652 | Some(vec!["read_file".to_string()]), |
| 653 | input_tx, |
| 654 | "boot_test".to_string(), |
| 655 | ); |
| 656 | agent.status = SubAgentStatus::Completed; |
| 657 | let agent_id = agent.id.clone(); |
| 658 | manager.agents.insert(agent_id.clone(), agent); |
| 659 | |
| 660 | let snapshot = manager |
| 661 | .assign( |
| 662 | &agent_id, |
| 663 | Some("Draft retry plan".to_string()), |
| 664 | Some("awaiter".to_string()), |
| 665 | None, |
| 666 | true, |
| 667 | ) |
| 668 | .expect("metadata update should succeed"); |
| 669 | assert_eq!(snapshot.assignment.objective, "Draft retry plan"); |
| 670 | assert_eq!(snapshot.assignment.role.as_deref(), Some("awaiter")); |
| 671 | } |
| 672 | |
| 673 | #[test] |
| 674 | fn test_persist_and_reload_marks_running_agent_as_interrupted() { |
| 675 | let tmp = tempdir().expect("tempdir"); |
| 676 | let workspace = tmp.path().to_path_buf(); |
| 677 | let state_path = default_state_path(tmp.path()); |
| 678 | |
| 679 | let mut manager = SubAgentManager::new(workspace.clone(), 2).with_state_path(state_path); |
| 680 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 681 | let running = SubAgent::new( |
| 682 | SubAgentType::General, |
| 683 | "work".to_string(), |
| 684 | make_assignment(), |
| 685 | "deepseek-v4-flash".to_string(), |
| 686 | Some("Blue".to_string()), |
| 687 | Some(vec!["read_file".to_string()]), |
| 688 | input_tx, |
| 689 | "boot_test".to_string(), |
| 690 | ); |
| 691 | let running_id = running.id.clone(); |
| 692 | manager.agents.insert(running_id.clone(), running); |
| 693 | manager.persist_state().expect("persist state"); |
| 694 | |
| 695 | let mut reloaded = |
| 696 | SubAgentManager::new(workspace, 2).with_state_path(default_state_path(tmp.path())); |
| 697 | reloaded.load_state().expect("load state"); |
| 698 | let snapshot = reloaded |
| 699 | .get_result(&running_id) |
| 700 | .expect("reloaded agent should exist"); |
| 701 | assert!(matches!( |
| 702 | snapshot.status, |
| 703 | SubAgentStatus::Interrupted(ref message) |
| 704 | if message.contains(SUBAGENT_RESTART_REASON) |
| 705 | )); |
| 706 | } |
| 707 | |
| 708 | #[test] |
| 709 | fn test_interrupted_status_name_and_summary() { |
| 710 | let snapshot = make_snapshot(SubAgentStatus::Interrupted( |
| 711 | SUBAGENT_RESTART_REASON.to_string(), |
| 712 | )); |
| 713 | assert_eq!(subagent_status_name(&snapshot.status), "interrupted"); |
| 714 | assert!(summarize_subagent_result(&snapshot).contains(SUBAGENT_RESTART_REASON)); |
| 715 | } |
| 716 | |
| 717 | // === Deprecation notice tests === |
| 718 | |
| 719 | /// Helper: build a plain ToolResult with a JSON payload. |
| 720 | fn make_plain_result(payload: serde_json::Value) -> crate::tools::spec::ToolResult { |
| 721 | crate::tools::spec::ToolResult::json(&payload).expect("json result") |
| 722 | } |
| 723 | |
| 724 | #[test] |
| 725 | fn test_wrap_with_deprecation_notice_adds_deprecation_block() { |
| 726 | let result = make_plain_result(json!({"agent_id": "abc"})); |
| 727 | let wrapped = wrap_with_deprecation_notice(result, "spawn_agent", "agent_spawn"); |
| 728 | |
| 729 | let meta = wrapped.metadata.expect("metadata should be present"); |
| 730 | let dep = &meta["_deprecation"]; |
| 731 | assert_eq!(dep["this_tool"], "spawn_agent"); |
| 732 | assert_eq!(dep["use_instead"], "agent_spawn"); |
| 733 | assert_eq!(dep["removed_in"], DEPRECATION_REMOVAL_VERSION); |
| 734 | assert!( |
| 735 | dep["message"] |
| 736 | .as_str() |
| 737 | .unwrap_or("") |
| 738 | .contains("spawn_agent") |
| 739 | ); |
| 740 | } |
| 741 | |
| 742 | #[test] |
| 743 | fn test_wrap_with_deprecation_notice_preserves_existing_metadata() { |
| 744 | let result = make_plain_result(json!({"agent_id": "abc"})) |
| 745 | .with_metadata(json!({"status": "Running", "snapshot": {}})); |
| 746 | let wrapped = wrap_with_deprecation_notice(result, "close_agent", "agent_cancel"); |
| 747 | |
| 748 | let meta = wrapped.metadata.expect("metadata should be present"); |
| 749 | // Existing metadata key must survive. |
| 750 | assert_eq!(meta["status"], "Running"); |
| 751 | // Deprecation block must be present alongside. |
| 752 | assert_eq!(meta["_deprecation"]["this_tool"], "close_agent"); |
| 753 | assert_eq!(meta["_deprecation"]["use_instead"], "agent_cancel"); |
| 754 | } |
| 755 | |
| 756 | #[test] |
| 757 | fn test_canonical_agent_send_input_has_no_deprecation() { |
| 758 | let manager = Arc::new(RwLock::new(SubAgentManager::new(PathBuf::from("."), 1))); |
| 759 | // The canonical name "agent_send_input" must NOT receive a deprecation notice. |
| 760 | // We verify this by inspecting the tool's name — the deprecation branch |
| 761 | // only fires when name == "send_input". |
| 762 | let tool = AgentSendInputTool::new(manager.clone(), "agent_send_input"); |
| 763 | assert_eq!(tool.name(), "agent_send_input"); |
| 764 | |
| 765 | let alias = AgentSendInputTool::new(manager, "send_input"); |
| 766 | assert_eq!(alias.name(), "send_input"); |
| 767 | } |
| 768 | |
| 769 | #[test] |
| 770 | fn test_wrap_with_deprecation_notice_all_alias_mappings() { |
| 771 | let cases = [ |
| 772 | ("spawn_agent", "agent_spawn"), |
| 773 | ("delegate_to_agent", "agent_spawn"), |
| 774 | ("close_agent", "agent_cancel"), |
| 775 | ("send_input", "agent_send_input"), |
| 776 | ]; |
| 777 | |
| 778 | for (alias, canonical) in cases { |
| 779 | let result = make_plain_result(json!({"ok": true})); |
| 780 | let wrapped = wrap_with_deprecation_notice(result, alias, canonical); |
| 781 | let meta = wrapped.metadata.expect("metadata for alias {alias}"); |
| 782 | assert_eq!(meta["_deprecation"]["this_tool"], alias, "alias={alias}"); |
| 783 | assert_eq!( |
| 784 | meta["_deprecation"]["use_instead"], canonical, |
| 785 | "alias={alias}" |
| 786 | ); |
| 787 | assert_eq!( |
| 788 | meta["_deprecation"]["removed_in"], DEPRECATION_REMOVAL_VERSION, |
| 789 | "alias={alias}" |
| 790 | ); |
| 791 | } |
| 792 | } |
| 793 | |
| 794 | // === v0.6.6 — sub-agent authority unification === |
| 795 | |
| 796 | #[test] |
| 797 | fn build_allowed_tools_general_returns_none_for_full_inheritance() { |
| 798 | // Default behavior: General agent with no explicit list inherits the |
| 799 | // parent's full registry (None signals no narrowing). |
| 800 | let result = build_allowed_tools(&SubAgentType::General, None, true).unwrap(); |
| 801 | assert!( |
| 802 | result.is_none(), |
| 803 | "General with no explicit_tools should default to full inheritance (None), got {result:?}" |
| 804 | ); |
| 805 | } |
| 806 | |
| 807 | #[test] |
| 808 | fn build_allowed_tools_explore_returns_none_for_full_inheritance() { |
| 809 | // Per-type allowlists are now advisory — Explore also gets the full |
| 810 | // surface unless an explicit list is passed. |
| 811 | let result = build_allowed_tools(&SubAgentType::Explore, None, true).unwrap(); |
| 812 | assert!( |
| 813 | result.is_none(), |
| 814 | "Explore with no explicit_tools should default to full inheritance" |
| 815 | ); |
| 816 | } |
| 817 | |
| 818 | #[test] |
| 819 | fn build_allowed_tools_custom_requires_explicit_list() { |
| 820 | // Custom is the one type that REQUIRES explicit allowed_tools. |
| 821 | let err = build_allowed_tools(&SubAgentType::Custom, None, true).unwrap_err(); |
| 822 | assert!( |
| 823 | err.to_string().contains("Custom sub-agent requires"), |
| 824 | "got: {err}" |
| 825 | ); |
| 826 | } |
| 827 | |
| 828 | #[test] |
| 829 | fn build_allowed_tools_explicit_list_returned_as_some() { |
| 830 | let explicit = vec!["read_file".to_string(), "list_dir".to_string()]; |
| 831 | let result = build_allowed_tools(&SubAgentType::Custom, Some(explicit.clone()), true).unwrap(); |
| 832 | assert_eq!(result, Some(explicit)); |
| 833 | } |
| 834 | |
| 835 | #[test] |
| 836 | fn build_allowed_tools_explicit_list_dedupes_and_trims() { |
| 837 | let explicit = vec![ |
| 838 | "read_file".to_string(), |
| 839 | " read_file ".to_string(), // trim + dedupe |
| 840 | "list_dir".to_string(), |
| 841 | "".to_string(), // skip empty |
| 842 | ]; |
| 843 | let result = build_allowed_tools(&SubAgentType::Custom, Some(explicit), true).unwrap(); |
| 844 | assert_eq!( |
| 845 | result, |
| 846 | Some(vec!["read_file".to_string(), "list_dir".to_string()]) |
| 847 | ); |
| 848 | } |
| 849 | |
| 850 | #[test] |
| 851 | fn parse_spawn_request_extracts_cwd_when_present() { |
| 852 | let input = json!({ |
| 853 | "prompt": "build feature A", |
| 854 | "cwd": ".worktrees/feature-a" |
| 855 | }); |
| 856 | let parsed = parse_spawn_request(&input).expect("spawn request should parse"); |
| 857 | assert_eq!( |
| 858 | parsed.cwd.as_ref().map(|p| p.to_string_lossy().to_string()), |
| 859 | Some(".worktrees/feature-a".to_string()) |
| 860 | ); |
| 861 | } |
| 862 | |
| 863 | #[test] |
| 864 | fn parse_spawn_request_cwd_absent_yields_none() { |
| 865 | let input = json!({ "prompt": "no cwd" }); |
| 866 | let parsed = parse_spawn_request(&input).expect("spawn request should parse"); |
| 867 | assert!(parsed.cwd.is_none()); |
| 868 | } |
| 869 | |
| 870 | #[test] |
| 871 | fn parse_spawn_request_cwd_empty_string_yields_none() { |
| 872 | let input = json!({ "prompt": "empty cwd", "cwd": " " }); |
| 873 | let parsed = parse_spawn_request(&input).expect("spawn request should parse"); |
| 874 | assert!(parsed.cwd.is_none(), "whitespace-only cwd should be None"); |
| 875 | } |
| 876 | |
| 877 | #[test] |
| 878 | fn build_subagent_system_prompt_appends_role_when_set() { |
| 879 | let assignment = SubAgentAssignment::new("p".to_string(), Some("worker".to_string())); |
| 880 | let prompt = build_subagent_system_prompt(&SubAgentType::General, &assignment); |
| 881 | assert!( |
| 882 | prompt.ends_with("You are operating in the role of `worker`."), |
| 883 | "expected role line at end, got: {}", |
| 884 | &prompt[prompt.len().saturating_sub(80)..] |
| 885 | ); |
| 886 | } |
| 887 | |
| 888 | #[test] |
| 889 | fn build_subagent_system_prompt_skips_role_when_none() { |
| 890 | let assignment = SubAgentAssignment::new("p".to_string(), None); |
| 891 | let prompt = build_subagent_system_prompt(&SubAgentType::General, &assignment); |
| 892 | assert!(!prompt.contains("You are operating in the role of")); |
| 893 | } |
| 894 | |
| 895 | #[test] |
| 896 | fn build_subagent_system_prompt_skips_role_when_blank() { |
| 897 | let assignment = SubAgentAssignment::new("p".to_string(), Some(" ".to_string())); |
| 898 | let prompt = build_subagent_system_prompt(&SubAgentType::General, &assignment); |
| 899 | assert!(!prompt.contains("You are operating in the role of")); |
| 900 | } |
| 901 | |
| 902 | #[test] |
| 903 | fn subagent_done_sentinel_format_is_well_formed() { |
| 904 | let res = make_snapshot(SubAgentStatus::Completed); |
| 905 | let sentinel = subagent_done_sentinel("agent_xyz", &res); |
| 906 | assert!(sentinel.starts_with("<deepseek:subagent.done>")); |
| 907 | assert!(sentinel.ends_with("</deepseek:subagent.done>")); |
| 908 | |
| 909 | // The inner JSON parses and carries the expected fields. |
| 910 | let inner = sentinel |
| 911 | .trim_start_matches("<deepseek:subagent.done>") |
| 912 | .trim_end_matches("</deepseek:subagent.done>"); |
| 913 | let parsed: serde_json::Value = serde_json::from_str(inner).expect("inner JSON parses"); |
| 914 | assert_eq!(parsed["agent_id"], "agent_xyz"); |
| 915 | assert_eq!(parsed["status"], "completed"); |
| 916 | assert_eq!(parsed["agent_type"], "general"); |
| 917 | } |
| 918 | |
| 919 | #[test] |
| 920 | fn subagent_failed_sentinel_format_is_well_formed() { |
| 921 | let sentinel = subagent_failed_sentinel("agent_zzz", "boom"); |
| 922 | let inner = sentinel |
| 923 | .trim_start_matches("<deepseek:subagent.done>") |
| 924 | .trim_end_matches("</deepseek:subagent.done>"); |
| 925 | let parsed: serde_json::Value = serde_json::from_str(inner).expect("inner JSON parses"); |
| 926 | assert_eq!(parsed["agent_id"], "agent_zzz"); |
| 927 | assert_eq!(parsed["status"], "failed"); |
| 928 | assert_eq!(parsed["error"], "boom"); |
| 929 | } |
| 930 | |
| 931 | #[test] |
| 932 | fn subagent_runtime_default_max_depth_is_three() { |
| 933 | // Sanity-check the constant — bumping it without a test means stale docs. |
| 934 | assert_eq!(DEFAULT_MAX_SPAWN_DEPTH, 3); |
| 935 | } |
| 936 | |
| 937 | #[test] |
| 938 | fn would_exceed_depth_at_boundary() { |
| 939 | // depth=2, max=3 → next spawn (depth 3) is allowed (allow-equal). |
| 940 | // depth=3, max=3 → next spawn (depth 4) exceeds. |
| 941 | let runtime = stub_runtime(); |
| 942 | let mut at_max = runtime.clone(); |
| 943 | at_max.spawn_depth = 3; |
| 944 | at_max.max_spawn_depth = 3; |
| 945 | assert!( |
| 946 | at_max.would_exceed_depth(), |
| 947 | "depth 3 + max 3 → next would be 4, exceeds" |
| 948 | ); |
| 949 | |
| 950 | let mut below_max = runtime; |
| 951 | below_max.spawn_depth = 2; |
| 952 | below_max.max_spawn_depth = 3; |
| 953 | assert!( |
| 954 | !below_max.would_exceed_depth(), |
| 955 | "depth 2 + max 3 → next is 3, allowed" |
| 956 | ); |
| 957 | } |
| 958 | |
| 959 | #[test] |
| 960 | fn child_runtime_increments_depth_and_forces_auto_approve() { |
| 961 | let mut parent = stub_runtime(); |
| 962 | parent.spawn_depth = 1; |
| 963 | parent.context.auto_approve = false; // parent in suggest mode |
| 964 | let child = parent.child_runtime(); |
| 965 | assert_eq!(child.spawn_depth, 2, "child depth = parent + 1"); |
| 966 | assert!( |
| 967 | child.context.auto_approve, |
| 968 | "child must auto-approve regardless of parent mode (spawning IS the approval)" |
| 969 | ); |
| 970 | // Parent mode is unchanged — the override is on the child only. |
| 971 | assert!(!parent.context.auto_approve); |
| 972 | } |
| 973 | |
| 974 | #[test] |
| 975 | fn child_cancellation_cascades_from_parent() { |
| 976 | let parent = stub_runtime(); |
| 977 | let child = parent.child_runtime(); |
| 978 | assert!(!child.cancel_token.is_cancelled()); |
| 979 | parent.cancel_token.cancel(); |
| 980 | assert!( |
| 981 | child.cancel_token.is_cancelled(), |
| 982 | "parent cancel() must propagate to child via child_token()" |
| 983 | ); |
| 984 | } |
| 985 | |
| 986 | #[test] |
| 987 | fn mailbox_propagates_through_child_runtime_chain() { |
| 988 | use crate::tools::subagent::mailbox::Mailbox; |
| 989 | let parent_token = CancellationToken::new(); |
| 990 | let (mailbox, _rx) = Mailbox::new(parent_token.clone()); |
| 991 | |
| 992 | let mut parent = stub_runtime(); |
| 993 | parent.cancel_token = parent_token; |
| 994 | parent.mailbox = Some(mailbox); |
| 995 | |
| 996 | let child = parent.child_runtime(); |
| 997 | let grandchild = child.child_runtime(); |
| 998 | assert!(parent.mailbox.is_some()); |
| 999 | assert!(child.mailbox.is_some(), "child inherits parent mailbox"); |
| 1000 | assert!( |
| 1001 | grandchild.mailbox.is_some(), |
| 1002 | "grandchild inherits via the cloned Arc inside Mailbox" |
| 1003 | ); |
| 1004 | } |
| 1005 | |
| 1006 | #[tokio::test] |
| 1007 | async fn mailbox_close_as_cancel_propagates_to_grandchild_runtime() { |
| 1008 | use crate::tools::subagent::mailbox::Mailbox; |
| 1009 | let parent_token = CancellationToken::new(); |
| 1010 | let (mailbox, _rx) = Mailbox::new(parent_token.clone()); |
| 1011 | |
| 1012 | let mut parent = stub_runtime(); |
| 1013 | parent.cancel_token = parent_token; |
| 1014 | parent.mailbox = Some(mailbox.clone()); |
| 1015 | |
| 1016 | let child = parent.child_runtime(); |
| 1017 | let grandchild = child.child_runtime(); |
| 1018 | assert!(!grandchild.cancel_token.is_cancelled()); |
| 1019 | |
| 1020 | // Close the mailbox via *any* clone — the original or the one stored on |
| 1021 | // the runtime. Cancellation must reach all the way to the grandchild. |
| 1022 | mailbox.close(); |
| 1023 | assert!(parent.cancel_token.is_cancelled()); |
| 1024 | assert!(child.cancel_token.is_cancelled()); |
| 1025 | assert!( |
| 1026 | grandchild.cancel_token.is_cancelled(), |
| 1027 | "close-as-cancel must propagate across max_spawn_depth=3" |
| 1028 | ); |
| 1029 | } |
| 1030 | |
| 1031 | #[tokio::test] |
| 1032 | async fn mailbox_orders_messages_from_parent_and_child_runtimes() { |
| 1033 | use crate::tools::subagent::mailbox::{Mailbox, MailboxMessage}; |
| 1034 | let parent_token = CancellationToken::new(); |
| 1035 | let (mailbox, mut rx) = Mailbox::new(parent_token.clone()); |
| 1036 | |
| 1037 | let mut parent = stub_runtime(); |
| 1038 | parent.cancel_token = parent_token; |
| 1039 | parent.mailbox = Some(mailbox); |
| 1040 | let child = parent.child_runtime(); |
| 1041 | |
| 1042 | // Interleave sends from both runtimes; sequence numbers stay monotonic. |
| 1043 | parent |
| 1044 | .mailbox |
| 1045 | .as_ref() |
| 1046 | .unwrap() |
| 1047 | .send(MailboxMessage::progress("parent_a", "step 1")); |
| 1048 | child |
| 1049 | .mailbox |
| 1050 | .as_ref() |
| 1051 | .unwrap() |
| 1052 | .send(MailboxMessage::progress("child_b", "step 1")); |
| 1053 | parent |
| 1054 | .mailbox |
| 1055 | .as_ref() |
| 1056 | .unwrap() |
| 1057 | .send(MailboxMessage::progress("parent_a", "step 2")); |
| 1058 | |
| 1059 | let drained = rx.drain(); |
| 1060 | assert_eq!(drained.len(), 3); |
| 1061 | assert_eq!(drained[0].seq, 1); |
| 1062 | assert_eq!(drained[1].seq, 2); |
| 1063 | assert_eq!(drained[2].seq, 3); |
| 1064 | // Verify ordering is preserved across publishers. |
| 1065 | match ( |
| 1066 | &drained[0].message, |
| 1067 | &drained[1].message, |
| 1068 | &drained[2].message, |
| 1069 | ) { |
| 1070 | ( |
| 1071 | MailboxMessage::Progress { agent_id: a, .. }, |
| 1072 | MailboxMessage::Progress { agent_id: b, .. }, |
| 1073 | MailboxMessage::Progress { agent_id: c, .. }, |
| 1074 | ) => { |
| 1075 | assert_eq!(a, "parent_a"); |
| 1076 | assert_eq!(b, "child_b"); |
| 1077 | assert_eq!(c, "parent_a"); |
| 1078 | } |
| 1079 | other => panic!("unexpected message order: {other:?}"), |
| 1080 | } |
| 1081 | } |
| 1082 | |
| 1083 | #[test] |
| 1084 | fn persisted_empty_allowed_tools_loads_as_full_inheritance() { |
| 1085 | // Backward-compat: a v0.6.5 session that persisted with an empty Vec |
| 1086 | // (or a v0.6.6 session with no narrowing) should load as None on |
| 1087 | // restart, meaning full inheritance. |
| 1088 | let dir = tempdir().unwrap(); |
| 1089 | let state_path = dir.path().join("subagents.v1.json"); |
| 1090 | let payload = serde_json::json!({ |
| 1091 | "schema_version": SUBAGENT_STATE_SCHEMA_VERSION, |
| 1092 | "agents": [{ |
| 1093 | "id": "agent_test", |
| 1094 | "agent_type": "general", |
| 1095 | "prompt": "p", |
| 1096 | "assignment": { "objective": "p" }, |
| 1097 | "status": "Completed", |
| 1098 | "result": null, |
| 1099 | "steps_taken": 0, |
| 1100 | "duration_ms": 0, |
| 1101 | "allowed_tools": [], |
| 1102 | "updated_at_ms": 0 |
| 1103 | }] |
| 1104 | }); |
| 1105 | std::fs::write(&state_path, payload.to_string()).unwrap(); |
| 1106 | |
| 1107 | let mut manager = SubAgentManager::new(dir.path().to_path_buf(), 5).with_state_path(state_path); |
| 1108 | manager.load_state().expect("load should succeed"); |
| 1109 | let agent = manager.agents.get("agent_test").expect("loaded agent"); |
| 1110 | assert!( |
| 1111 | agent.allowed_tools.is_none(), |
| 1112 | "empty Vec on disk → None (full inheritance)" |
| 1113 | ); |
| 1114 | } |
| 1115 | |
| 1116 | #[test] |
| 1117 | fn persisted_non_empty_allowed_tools_loads_as_narrow() { |
| 1118 | // Backward-compat the other way: a v0.6.5 session that persisted with |
| 1119 | // an explicit narrow list keeps that list on reload. |
| 1120 | let dir = tempdir().unwrap(); |
| 1121 | let state_path = dir.path().join("subagents.v1.json"); |
| 1122 | let payload = serde_json::json!({ |
| 1123 | "schema_version": SUBAGENT_STATE_SCHEMA_VERSION, |
| 1124 | "agents": [{ |
| 1125 | "id": "agent_narrow", |
| 1126 | "agent_type": "custom", |
| 1127 | "prompt": "p", |
| 1128 | "assignment": { "objective": "p" }, |
| 1129 | "status": "Completed", |
| 1130 | "result": null, |
| 1131 | "steps_taken": 0, |
| 1132 | "duration_ms": 0, |
| 1133 | "allowed_tools": ["read_file", "list_dir"], |
| 1134 | "updated_at_ms": 0 |
| 1135 | }] |
| 1136 | }); |
| 1137 | std::fs::write(&state_path, payload.to_string()).unwrap(); |
| 1138 | |
| 1139 | let mut manager = SubAgentManager::new(dir.path().to_path_buf(), 5).with_state_path(state_path); |
| 1140 | manager.load_state().expect("load should succeed"); |
| 1141 | let agent = manager.agents.get("agent_narrow").expect("loaded agent"); |
| 1142 | assert_eq!( |
| 1143 | agent.allowed_tools.as_deref(), |
| 1144 | Some(&["read_file".to_string(), "list_dir".to_string()][..]), |
| 1145 | "non-empty Vec → Some(list), narrow scope preserved" |
| 1146 | ); |
| 1147 | } |
| 1148 | |
| 1149 | /// Build a minimal `SubAgentRuntime` for tests that exercise pure runtime |
| 1150 | /// helpers (depth, cancellation, child_runtime). Doesn't construct a real |
| 1151 | /// HTTP client — calls that hit `runtime.client` would fail, but the |
| 1152 | /// helpers we test here don't. |
| 1153 | fn stub_runtime() -> SubAgentRuntime { |
| 1154 | use tokio_util::sync::CancellationToken; |
| 1155 | |
| 1156 | let workspace = std::env::temp_dir().join("deepseek-test-stub"); |
| 1157 | let context = ToolContext::new(workspace.clone()); |
| 1158 | SubAgentRuntime { |
| 1159 | client: stub_client(), |
| 1160 | model: "deepseek-v4-flash".to_string(), |
| 1161 | auto_model: false, |
| 1162 | reasoning_effort: None, |
| 1163 | reasoning_effort_auto: false, |
| 1164 | role_models: std::collections::HashMap::new(), |
| 1165 | context, |
| 1166 | allow_shell: true, |
| 1167 | event_tx: None, |
| 1168 | manager: new_shared_subagent_manager(workspace, 5), |
| 1169 | spawn_depth: 0, |
| 1170 | max_spawn_depth: DEFAULT_MAX_SPAWN_DEPTH, |
| 1171 | cancel_token: CancellationToken::new(), |
| 1172 | mailbox: None, |
| 1173 | parent_completion_tx: None, |
| 1174 | } |
| 1175 | } |
| 1176 | |
| 1177 | /// A minimal stub client. Test helpers below only ever check struct fields |
| 1178 | /// (depth, cancel_token, context); they don't call the network. We need a |
| 1179 | /// *some* `DeepSeekClient` because `SubAgentRuntime.client` isn't |
| 1180 | /// `Option<...>`. `Config::default()` is enough — `DeepSeekClient::new` |
| 1181 | /// only validates that an API key field exists, not that the key works. |
| 1182 | fn stub_client() -> DeepSeekClient { |
| 1183 | let config = crate::config::Config { |
| 1184 | api_key: Some("test-key".to_string()), |
| 1185 | ..crate::config::Config::default() |
| 1186 | }; |
| 1187 | DeepSeekClient::new(&config).expect("stub client should construct") |
| 1188 | } |
| 1189 | |
| 1190 | // ---- #405 session-boundary classification ---- |
| 1191 | // |
| 1192 | // Each manager assigns a fresh session_boot_id; agents stamp the id at |
| 1193 | // spawn time. After persist + reload by a *new* manager, those agents |
| 1194 | // carry the prior boot id and are classified as `from_prior_session`. |
| 1195 | // `agent_list` defaults to current-session only; `include_archived=true` |
| 1196 | // surfaces the prior-session records with the flag set. |
| 1197 | |
| 1198 | fn insert_prior_session_agent( |
| 1199 | manager: &mut SubAgentManager, |
| 1200 | id: &str, |
| 1201 | status: SubAgentStatus, |
| 1202 | boot_id: &str, |
| 1203 | ) { |
| 1204 | let (input_tx, _input_rx) = mpsc::unbounded_channel(); |
| 1205 | let mut agent = SubAgent::new( |
| 1206 | SubAgentType::General, |
| 1207 | "old prompt".to_string(), |
| 1208 | make_assignment(), |
| 1209 | "deepseek-v4-flash".to_string(), |
| 1210 | None, |
| 1211 | None, |
| 1212 | input_tx, |
| 1213 | boot_id.to_string(), |
| 1214 | ); |
| 1215 | agent.status = status; |
| 1216 | agent.id = id.to_string(); |
| 1217 | manager.agents.insert(id.to_string(), agent); |
| 1218 | } |
| 1219 | |
| 1220 | #[test] |
| 1221 | fn session_boot_ids_are_unique_per_manager() { |
| 1222 | let a = SubAgentManager::new(PathBuf::from("."), 1); |
| 1223 | let b = SubAgentManager::new(PathBuf::from("."), 1); |
| 1224 | assert_ne!(a.session_boot_id(), b.session_boot_id()); |
| 1225 | } |
| 1226 | |
| 1227 | #[test] |
| 1228 | fn list_filtered_drops_prior_session_terminals_by_default() { |
| 1229 | let mut manager = SubAgentManager::new(PathBuf::from("."), 5); |
| 1230 | let current_boot = manager.session_boot_id().to_string(); |
| 1231 | insert_prior_session_agent( |
| 1232 | &mut manager, |
| 1233 | "current_running", |
| 1234 | SubAgentStatus::Running, |
| 1235 | ¤t_boot, |
| 1236 | ); |
| 1237 | insert_prior_session_agent( |
| 1238 | &mut manager, |
| 1239 | "prior_completed", |
| 1240 | SubAgentStatus::Completed, |
| 1241 | "boot_old_session", |
| 1242 | ); |
| 1243 | insert_prior_session_agent( |
| 1244 | &mut manager, |
| 1245 | "prior_running", |
| 1246 | SubAgentStatus::Running, |
| 1247 | "boot_old_session", |
| 1248 | ); |
| 1249 | |
| 1250 | let listed = manager.list_filtered(false); |
| 1251 | let ids: Vec<&str> = listed.iter().map(|s| s.agent_id.as_str()).collect(); |
| 1252 | assert!(ids.contains(&"current_running"), "{ids:?}"); |
| 1253 | assert!( |
| 1254 | ids.contains(&"prior_running"), |
| 1255 | "still-running prior-session agents stay visible: {ids:?}" |
| 1256 | ); |
| 1257 | assert!( |
| 1258 | !ids.contains(&"prior_completed"), |
| 1259 | "completed prior-session agents are hidden by default: {ids:?}" |
| 1260 | ); |
| 1261 | |
| 1262 | let prior = listed |
| 1263 | .iter() |
| 1264 | .find(|s| s.agent_id == "prior_running") |
| 1265 | .unwrap(); |
| 1266 | assert!(prior.from_prior_session); |
| 1267 | let current = listed |
| 1268 | .iter() |
| 1269 | .find(|s| s.agent_id == "current_running") |
| 1270 | .unwrap(); |
| 1271 | assert!(!current.from_prior_session); |
| 1272 | } |
| 1273 | |
| 1274 | #[test] |
| 1275 | fn list_filtered_with_include_archived_returns_everything() { |
| 1276 | let mut manager = SubAgentManager::new(PathBuf::from("."), 5); |
| 1277 | let current_boot = manager.session_boot_id().to_string(); |
| 1278 | insert_prior_session_agent( |
| 1279 | &mut manager, |
| 1280 | "current_done", |
| 1281 | SubAgentStatus::Completed, |
| 1282 | ¤t_boot, |
| 1283 | ); |
| 1284 | insert_prior_session_agent( |
| 1285 | &mut manager, |
| 1286 | "prior_done", |
| 1287 | SubAgentStatus::Completed, |
| 1288 | "boot_old", |
| 1289 | ); |
| 1290 | insert_prior_session_agent( |
| 1291 | &mut manager, |
| 1292 | "prior_failed", |
| 1293 | SubAgentStatus::Failed("boom".to_string()), |
| 1294 | "boot_old", |
| 1295 | ); |
| 1296 | |
| 1297 | let listed = manager.list_filtered(true); |
| 1298 | assert_eq!(listed.len(), 3, "{listed:?}"); |
| 1299 | let prior = listed.iter().find(|s| s.agent_id == "prior_done").unwrap(); |
| 1300 | assert!(prior.from_prior_session); |
| 1301 | let current = listed |
| 1302 | .iter() |
| 1303 | .find(|s| s.agent_id == "current_done") |
| 1304 | .unwrap(); |
| 1305 | assert!(!current.from_prior_session); |
| 1306 | } |
| 1307 | |
| 1308 | #[test] |
| 1309 | fn agents_with_empty_boot_id_classify_as_prior_session() { |
| 1310 | // Records persisted before #405 land with an empty `session_boot_id` |
| 1311 | // due to `#[serde(default)]`. The manager treats those the same as |
| 1312 | // a non-matching id — i.e. prior session. |
| 1313 | let mut manager = SubAgentManager::new(PathBuf::from("."), 5); |
| 1314 | insert_prior_session_agent(&mut manager, "legacy", SubAgentStatus::Completed, ""); |
| 1315 | |
| 1316 | let listed_default = manager.list_filtered(false); |
| 1317 | assert!( |
| 1318 | listed_default.iter().all(|s| s.agent_id != "legacy"), |
| 1319 | "legacy completed agents are hidden by default" |
| 1320 | ); |
| 1321 | |
| 1322 | let listed_archived = manager.list_filtered(true); |
| 1323 | let legacy = listed_archived |
| 1324 | .iter() |
| 1325 | .find(|s| s.agent_id == "legacy") |
| 1326 | .unwrap(); |
| 1327 | assert!(legacy.from_prior_session); |
| 1328 | } |
| 1329 | |
| 1330 | #[test] |
| 1331 | fn persist_round_trip_preserves_session_boot_id() { |
| 1332 | let dir = tempdir().expect("tempdir"); |
| 1333 | let state_path = dir.path().join(SUBAGENT_STATE_FILE); |
| 1334 | |
| 1335 | let original_boot; |
| 1336 | { |
| 1337 | let mut writer = |
| 1338 | SubAgentManager::new(dir.path().to_path_buf(), 2).with_state_path(state_path.clone()); |
| 1339 | original_boot = writer.session_boot_id().to_string(); |
| 1340 | insert_prior_session_agent( |
| 1341 | &mut writer, |
| 1342 | "agent_persist", |
| 1343 | SubAgentStatus::Completed, |
| 1344 | &original_boot, |
| 1345 | ); |
| 1346 | writer |
| 1347 | .persist_state() |
| 1348 | .expect("persist round-trip should write"); |
| 1349 | } |
| 1350 | |
| 1351 | // A fresh manager comes up with a *different* boot id and reloads |
| 1352 | // the persisted state; the agent should now be classified prior. |
| 1353 | let mut reader = |
| 1354 | SubAgentManager::new(dir.path().to_path_buf(), 2).with_state_path(state_path.clone()); |
| 1355 | reader.load_state().expect("reload should succeed"); |
| 1356 | assert_ne!(reader.session_boot_id(), original_boot); |
| 1357 | |
| 1358 | let listed_default = reader.list_filtered(false); |
| 1359 | assert!( |
| 1360 | !listed_default.iter().any(|s| s.agent_id == "agent_persist"), |
| 1361 | "completed prior-session agent hidden after reload: {listed_default:?}" |
| 1362 | ); |
| 1363 | let listed_all = reader.list_filtered(true); |
| 1364 | let snap = listed_all |
| 1365 | .iter() |
| 1366 | .find(|s| s.agent_id == "agent_persist") |
| 1367 | .unwrap(); |
| 1368 | assert!(snap.from_prior_session); |
| 1369 | } |
| 1370 | |
| 1371 | // === Issue #756: parent-completion wakeup === |
| 1372 | // |
| 1373 | // When a direct child of the engine finishes, `run_subagent_task` emits |
| 1374 | // a `SubAgentCompletion` on the runtime's `parent_completion_tx`. The |
| 1375 | // engine's turn loop drains that channel before deciding to end the turn. |
| 1376 | // These tests cover the gating logic in `emit_parent_completion` so the |
| 1377 | // parent isn't flooded with grandchild completions and so the function |
| 1378 | // is safe when no channel is wired. |
| 1379 | |
| 1380 | fn runtime_with_depth( |
| 1381 | spawn_depth: u32, |
| 1382 | parent_completion_tx: Option<mpsc::UnboundedSender<SubAgentCompletion>>, |
| 1383 | ) -> SubAgentRuntime { |
| 1384 | let mut rt = stub_runtime(); |
| 1385 | rt.spawn_depth = spawn_depth; |
| 1386 | rt.parent_completion_tx = parent_completion_tx; |
| 1387 | rt |
| 1388 | } |
| 1389 | |
| 1390 | #[test] |
| 1391 | fn emit_parent_completion_fires_for_direct_child() { |
| 1392 | let (tx, mut rx) = mpsc::unbounded_channel::<SubAgentCompletion>(); |
| 1393 | let runtime = runtime_with_depth(1, Some(tx)); |
| 1394 | |
| 1395 | let sent = emit_parent_completion(&runtime, "agent_abc", "summary line\n<sentinel/>"); |
| 1396 | |
| 1397 | assert!(sent, "depth=1 with channel wired should send"); |
| 1398 | let received = rx.try_recv().expect("channel should have one message"); |
| 1399 | assert_eq!(received.agent_id, "agent_abc"); |
| 1400 | assert_eq!(received.payload, "summary line\n<sentinel/>"); |
| 1401 | assert!(rx.try_recv().is_err(), "should be exactly one message"); |
| 1402 | } |
| 1403 | |
| 1404 | #[test] |
| 1405 | fn emit_parent_completion_skips_grandchildren() { |
| 1406 | let (tx, mut rx) = mpsc::unbounded_channel::<SubAgentCompletion>(); |
| 1407 | let runtime = runtime_with_depth(2, Some(tx)); |
| 1408 | |
| 1409 | let sent = emit_parent_completion(&runtime, "agent_grandchild", "ignored"); |
| 1410 | |
| 1411 | assert!( |
| 1412 | !sent, |
| 1413 | "depth=2 grandchild must not fire on the parent channel" |
| 1414 | ); |
| 1415 | assert!( |
| 1416 | rx.try_recv().is_err(), |
| 1417 | "channel should remain empty for grandchildren" |
| 1418 | ); |
| 1419 | } |
| 1420 | |
| 1421 | #[test] |
| 1422 | fn emit_parent_completion_skips_engine_self() { |
| 1423 | // depth 0 is the engine itself — the engine never spawns a task at |
| 1424 | // depth 0, but defend against accidental misuse. |
| 1425 | let (tx, mut rx) = mpsc::unbounded_channel::<SubAgentCompletion>(); |
| 1426 | let runtime = runtime_with_depth(0, Some(tx)); |
| 1427 | |
| 1428 | let sent = emit_parent_completion(&runtime, "agent_root", "ignored"); |
| 1429 | |
| 1430 | assert!( |
| 1431 | !sent, |
| 1432 | "depth=0 must not fire (only depth=1 direct children)" |
| 1433 | ); |
| 1434 | assert!(rx.try_recv().is_err()); |
| 1435 | } |
| 1436 | |
| 1437 | #[test] |
| 1438 | fn emit_parent_completion_no_channel_is_noop() { |
| 1439 | let runtime = runtime_with_depth(1, None); |
| 1440 | |
| 1441 | let sent = emit_parent_completion(&runtime, "agent_no_chan", "anything"); |
| 1442 | |
| 1443 | assert!( |
| 1444 | !sent, |
| 1445 | "missing channel should be a silent no-op, not a panic" |
| 1446 | ); |
| 1447 | } |
| 1448 | |
| 1449 | #[test] |
| 1450 | fn emit_parent_completion_dropped_receiver_does_not_panic() { |
| 1451 | let (tx, rx) = mpsc::unbounded_channel::<SubAgentCompletion>(); |
| 1452 | drop(rx); |
| 1453 | let runtime = runtime_with_depth(1, Some(tx)); |
| 1454 | |
| 1455 | // The send returns an error internally but we discard it — the |
| 1456 | // caller's run_subagent_task does not care whether the engine is |
| 1457 | // still listening (it might be shutting down). |
| 1458 | let sent = emit_parent_completion(&runtime, "agent_orphan", "after-rx-drop"); |
| 1459 | |
| 1460 | assert!( |
| 1461 | sent, |
| 1462 | "we still attempt the send; the engine being gone is not our problem" |
| 1463 | ); |
| 1464 | } |
| 1465 | |
| 1466 | #[test] |
| 1467 | fn child_runtime_propagates_completion_tx_for_gating() { |
| 1468 | // The channel is cloned through `child_runtime()` so descendants carry |
| 1469 | // it. The gate at the send site (`spawn_depth == 1`) is what limits |
| 1470 | // who actually fires — `child_runtime` simply must not strand it. |
| 1471 | let (tx, _rx) = mpsc::unbounded_channel::<SubAgentCompletion>(); |
| 1472 | let parent = runtime_with_depth(0, Some(tx)); |
| 1473 | |
| 1474 | let child = parent.child_runtime(); |
| 1475 | |
| 1476 | assert_eq!(child.spawn_depth, 1, "child increments depth"); |
| 1477 | assert!( |
| 1478 | child.parent_completion_tx.is_some(), |
| 1479 | "child carries the wakeup channel forward" |
| 1480 | ); |
| 1481 | } |
| 1482 | |
| 1483 | #[test] |
| 1484 | fn subagent_completion_payload_carries_existing_sentinel_format() { |
| 1485 | // The payload format is the same one already documented in |
| 1486 | // prompts/base.md: human summary on line 1, `<deepseek:subagent.done>` |
| 1487 | // sentinel on line 2. This test pins the format so future refactors |
| 1488 | // don't silently break the model's parsing contract. |
| 1489 | let mut snap = make_snapshot(SubAgentStatus::Completed); |
| 1490 | snap.result = Some("Found three errors.".to_string()); |
| 1491 | |
| 1492 | let summary = summarize_subagent_result(&snap); |
| 1493 | let sentinel = subagent_done_sentinel("agent_test", &snap); |
| 1494 | let payload = format!("{summary}\n{sentinel}"); |
| 1495 | |
| 1496 | let mut lines = payload.lines(); |
| 1497 | let first = lines.next().expect("first line is summary"); |
| 1498 | let second = lines.next().expect("second line is sentinel"); |
| 1499 | assert!( |
| 1500 | !first.starts_with("<deepseek:subagent.done>"), |
| 1501 | "summary should not be the sentinel itself" |
| 1502 | ); |
| 1503 | assert!( |
| 1504 | second.starts_with("<deepseek:subagent.done>"), |
| 1505 | "second line is the sentinel" |
| 1506 | ); |
| 1507 | assert!(second.ends_with("</deepseek:subagent.done>")); |
| 1508 | assert!( |
| 1509 | second.contains("\"agent_id\":\"agent_test\""), |
| 1510 | "sentinel JSON includes agent_id" |
| 1511 | ); |
| 1512 | } |
| 1513 |