story-kit: merge 176_bug_stories_moved_to_current_get_supervisor_instead_of_coder
This commit is contained in:
@@ -351,8 +351,10 @@ impl AgentPool {
|
|||||||
name.to_string()
|
name.to_string()
|
||||||
}
|
}
|
||||||
None => config
|
None => config
|
||||||
.default_agent()
|
.default_coder_agent()
|
||||||
.ok_or_else(|| "No agents configured".to_string())?
|
.ok_or_else(|| {
|
||||||
|
"No coder agent configured. Specify an agent_name explicitly.".to_string()
|
||||||
|
})?
|
||||||
.name
|
.name
|
||||||
.clone(),
|
.clone(),
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -57,6 +57,20 @@ pub struct AgentConfig {
|
|||||||
pub inactivity_timeout_secs: u64,
|
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 {
|
fn default_path() -> String {
|
||||||
".".to_string()
|
".".to_string()
|
||||||
}
|
}
|
||||||
@@ -189,6 +203,15 @@ impl ProjectConfig {
|
|||||||
self.agent.first()
|
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.
|
/// Render template variables in agent args and prompt for the given agent.
|
||||||
/// If `agent_name` is None, uses the first (default) agent.
|
/// If `agent_name` is None, uses the first (default) agent.
|
||||||
pub fn render_agent_args(
|
pub fn render_agent_args(
|
||||||
@@ -480,6 +503,89 @@ name = "second"
|
|||||||
assert!(config.find_agent("missing").is_none());
|
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]
|
#[test]
|
||||||
fn parse_project_toml_from_file() {
|
fn parse_project_toml_from_file() {
|
||||||
let tmp = tempfile::tempdir().unwrap();
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
|||||||
@@ -442,7 +442,7 @@ fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
|
|||||||
},
|
},
|
||||||
"agent_name": {
|
"agent_name": {
|
||||||
"type": "string",
|
"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"]
|
"required": ["story_id"]
|
||||||
@@ -2890,6 +2890,71 @@ mod tests {
|
|||||||
assert!(result.unwrap_err().contains("story_id"));
|
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]
|
#[tokio::test]
|
||||||
async fn tool_create_worktree_missing_story_id() {
|
async fn tool_create_worktree_missing_story_id() {
|
||||||
let tmp = tempfile::tempdir().unwrap();
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
|||||||
Reference in New Issue
Block a user