diff --git a/server/src/agents.rs b/server/src/agents.rs index 79c3fa5..0f0eb09 100644 --- a/server/src/agents.rs +++ b/server/src/agents.rs @@ -351,8 +351,10 @@ impl AgentPool { name.to_string() } None => config - .default_agent() - .ok_or_else(|| "No agents configured".to_string())? + .default_coder_agent() + .ok_or_else(|| { + "No coder agent configured. Specify an agent_name explicitly.".to_string() + })? .name .clone(), }; diff --git a/server/src/config.rs b/server/src/config.rs index 6e634e1..826483a 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -57,6 +57,20 @@ pub struct AgentConfig { pub inactivity_timeout_secs: u64, } +impl AgentConfig { + /// Returns true if this agent is a coder. + /// + /// Prefers the explicit `stage` field; falls back to checking whether the + /// agent name starts with `"coder"` for backwards compatibility. + pub fn is_coder(&self) -> bool { + match self.stage.as_deref() { + Some("coder") => true, + Some(_) => false, + None => self.name.starts_with("coder"), + } + } +} + fn default_path() -> String { ".".to_string() } @@ -189,6 +203,15 @@ impl ProjectConfig { self.agent.first() } + /// Get the first coder agent config. + /// + /// Prefers the explicit `stage = "coder"` field over the legacy name-based + /// heuristic (names starting with "coder"). Returns `None` if no coder + /// agent is configured. + pub fn default_coder_agent(&self) -> Option<&AgentConfig> { + self.agent.iter().find(|a| a.is_coder()) + } + /// Render template variables in agent args and prompt for the given agent. /// If `agent_name` is None, uses the first (default) agent. pub fn render_agent_args( @@ -480,6 +503,89 @@ name = "second" assert!(config.find_agent("missing").is_none()); } + #[test] + fn default_coder_agent_skips_supervisor() { + let toml_str = r#" +[[agent]] +name = "supervisor" +stage = "other" + +[[agent]] +name = "coder-1" +stage = "coder" +"#; + let config = ProjectConfig::parse(toml_str).unwrap(); + assert_eq!(config.default_coder_agent().unwrap().name, "coder-1"); + } + + #[test] + fn default_coder_agent_uses_name_heuristic_when_no_stage() { + let toml_str = r#" +[[agent]] +name = "supervisor" + +[[agent]] +name = "coder-1" +"#; + let config = ProjectConfig::parse(toml_str).unwrap(); + assert_eq!(config.default_coder_agent().unwrap().name, "coder-1"); + } + + #[test] + fn default_coder_agent_returns_none_when_no_coders() { + let toml_str = r#" +[[agent]] +name = "supervisor" +stage = "other" +"#; + let config = ProjectConfig::parse(toml_str).unwrap(); + assert!(config.default_coder_agent().is_none()); + } + + #[test] + fn is_coder_explicit_stage() { + let mut agent = AgentConfig { + name: "my-agent".to_string(), + role: String::new(), + command: "claude".to_string(), + args: vec![], + prompt: String::new(), + model: None, + allowed_tools: None, + max_turns: None, + max_budget_usd: None, + system_prompt: None, + stage: Some("coder".to_string()), + inactivity_timeout_secs: 300, + }; + assert!(agent.is_coder()); + agent.stage = Some("other".to_string()); + assert!(!agent.is_coder()); + } + + #[test] + fn is_coder_name_heuristic() { + let make = |name: &str| AgentConfig { + name: name.to_string(), + role: String::new(), + command: "claude".to_string(), + args: vec![], + prompt: String::new(), + model: None, + allowed_tools: None, + max_turns: None, + max_budget_usd: None, + system_prompt: None, + stage: None, + inactivity_timeout_secs: 300, + }; + assert!(make("coder-1").is_coder()); + assert!(make("coder-opus").is_coder()); + assert!(!make("supervisor").is_coder()); + assert!(!make("qa").is_coder()); + assert!(!make("mergemaster").is_coder()); + } + #[test] fn parse_project_toml_from_file() { let tmp = tempfile::tempdir().unwrap(); diff --git a/server/src/http/mcp.rs b/server/src/http/mcp.rs index d1dce4d..c1e8d5e 100644 --- a/server/src/http/mcp.rs +++ b/server/src/http/mcp.rs @@ -442,7 +442,7 @@ fn handle_tools_list(id: Option) -> JsonRpcResponse { }, "agent_name": { "type": "string", - "description": "Agent name from project.toml config. If omitted, uses the first configured agent." + "description": "Agent name from project.toml config. If omitted, uses the first coder agent (stage = \"coder\"). Supervisor must be requested explicitly by name." } }, "required": ["story_id"] @@ -2890,6 +2890,71 @@ mod tests { assert!(result.unwrap_err().contains("story_id")); } + #[tokio::test] + async fn tool_start_agent_no_agent_name_no_coder_returns_clear_error() { + // Config has only a supervisor — start_agent without agent_name should + // refuse rather than silently assigning supervisor. + let tmp = tempfile::tempdir().unwrap(); + let sk = tmp.path().join(".story_kit"); + std::fs::create_dir_all(&sk).unwrap(); + std::fs::write( + sk.join("project.toml"), + r#" +[[agent]] +name = "supervisor" +stage = "other" +"#, + ) + .unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_start_agent(&json!({"story_id": "42_my_story"}), &ctx).await; + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + err.contains("coder"), + "error should mention 'coder', got: {err}" + ); + } + + #[tokio::test] + async fn tool_start_agent_no_agent_name_picks_coder_not_supervisor() { + // Config has supervisor first, then coder-1. Without agent_name the + // coder should be selected, not supervisor. The call will fail due to + // missing git repo / worktree, but the error must NOT be about + // "No coder agent configured". + let tmp = tempfile::tempdir().unwrap(); + let sk = tmp.path().join(".story_kit"); + std::fs::create_dir_all(&sk).unwrap(); + std::fs::write( + sk.join("project.toml"), + r#" +[[agent]] +name = "supervisor" +stage = "other" + +[[agent]] +name = "coder-1" +stage = "coder" +"#, + ) + .unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_start_agent(&json!({"story_id": "42_my_story"}), &ctx).await; + // May succeed or fail for infrastructure reasons (no git repo), but + // must NOT fail with "No coder agent configured". + if let Err(err) = result { + assert!( + !err.contains("No coder agent configured"), + "should not fail on agent selection, got: {err}" + ); + // Should also not complain about supervisor being absent. + assert!( + !err.contains("supervisor"), + "should not select supervisor, got: {err}" + ); + } + } + #[tokio::test] async fn tool_create_worktree_missing_story_id() { let tmp = tempfile::tempdir().unwrap();