huskies: merge 962

This commit is contained in:
dave
2026-05-13 11:58:50 +00:00
parent 658e02c9b2
commit 184c214c34
19 changed files with 204 additions and 44 deletions
@@ -16,9 +16,7 @@ pub(super) fn read_story_front_matter_agent(
// Story 929: agent name comes from the CRDT register. The previous // Story 929: agent name comes from the CRDT register. The previous
// YAML fallback is gone — post-891 every story has its CRDT entry, // YAML fallback is gone — post-891 every story has its CRDT entry,
// and any story without one is treated as having no pinned agent. // and any story without one is treated as having no pinned agent.
crate::crdt_state::read_item(story_id) crate::crdt_state::read_item(story_id).and_then(|w| w.agent().map(|a| a.to_string()))
.and_then(|w| w.agent().map(str::to_string))
.filter(|s| !s.is_empty())
} }
/// Return `true` if the story is in `Stage::ReviewHold`. /// Return `true` if the story is in `Stage::ReviewHold`.
+5 -4
View File
@@ -130,7 +130,8 @@ impl AgentPool {
// Read the preferred agent from the story's front matter before acquiring // Read the preferred agent from the story's front matter before acquiring
// the lock. (See validation::read_front_matter_agent.) // the lock. (See validation::read_front_matter_agent.)
let front_matter_agent: Option<String> = read_front_matter_agent(story_id, agent_name); let front_matter_agent: Option<crate::config::AgentName> =
read_front_matter_agent(story_id, agent_name);
// Atomically resolve agent name, check availability, and register as // Atomically resolve agent name, check availability, and register as
// Pending. When `agent_name` is `None` the first idle coder is // Pending. When `agent_name` is `None` the first idle coder is
@@ -158,12 +159,12 @@ impl AgentPool {
// (bug 379). Mirrors the auto_assign selection logic. // (bug 379). Mirrors the auto_assign selection logic.
if let Some(ref pref) = front_matter_agent { if let Some(ref pref) = front_matter_agent {
let stage_matches = config let stage_matches = config
.find_agent(pref) .find_agent(pref.as_str())
.map(|cfg| agent_config_stage(cfg) == PipelineStage::Coder) .map(|cfg| agent_config_stage(cfg) == PipelineStage::Coder)
.unwrap_or(false); .unwrap_or(false);
if stage_matches { if stage_matches {
if auto_assign::is_agent_free(&agents, pref) { if auto_assign::is_agent_free(&agents, pref.as_str()) {
pref.clone() pref.to_string()
} else { } else {
return Err(format!( return Err(format!(
"Preferred agent '{pref}' from story front matter is busy; \ "Preferred agent '{pref}' from story front matter is busy; \
+5 -4
View File
@@ -56,14 +56,15 @@ pub(super) fn validate_agent_stage(
/// `start_agent` honour an explicit `agent: coder-opus` written by the /// `start_agent` honour an explicit `agent: coder-opus` written by the
/// `assign` command (bug 379). Returns `None` when an explicit agent_name /// `assign` command (bug 379). Returns `None` when an explicit agent_name
/// was already supplied or when the story has no front-matter preference. /// was already supplied or when the story has no front-matter preference.
pub(super) fn read_front_matter_agent(story_id: &str, agent_name: Option<&str>) -> Option<String> { pub(super) fn read_front_matter_agent(
story_id: &str,
agent_name: Option<&str>,
) -> Option<crate::config::AgentName> {
if agent_name.is_some() { if agent_name.is_some() {
return None; return None;
} }
// Story 929: the agent pin lives in the CRDT typed register; the // Story 929: the agent pin lives in the CRDT typed register; the
// legacy YAML fallback is gone — post-891 every story has its CRDT // legacy YAML fallback is gone — post-891 every story has its CRDT
// entry and any story without one has no pinned agent. // entry and any story without one has no pinned agent.
crate::crdt_state::read_item(story_id) crate::crdt_state::read_item(story_id).and_then(|w| w.agent())
.and_then(|w| w.agent().map(str::to_string))
.filter(|s| !s.is_empty())
} }
+4 -1
View File
@@ -419,7 +419,10 @@ mod tests {
"", "",
Some("Agent Story"), Some("Agent Story"),
); );
crate::crdt_state::set_agent("55_story_agent_story", Some("coder-1")); crate::crdt_state::set_agent(
"55_story_agent_story",
Some(crate::config::AgentName::Coder1),
);
let output = status_triage_cmd(tmp.path(), "55").unwrap(); let output = status_triage_cmd(tmp.path(), "55").unwrap();
assert!( assert!(
output.contains("coder-1"), output.contains("coder-1"),
+4 -1
View File
@@ -122,7 +122,10 @@ pub async fn handle_assign(
if running_coders.is_empty() { if running_coders.is_empty() {
// No coder running — persist the CRDT agent pin for the future start. // No coder running — persist the CRDT agent pin for the future start.
crate::crdt_state::set_agent(&story_id, Some(&agent_name)); crate::crdt_state::set_agent(
&story_id,
agent_name.parse::<crate::config::AgentName>().ok(),
);
return format!( return format!(
"Assigned **{agent_name}** to **{story_name}** (story {story_number}). \ "Assigned **{agent_name}** to **{story_name}** (story {story_number}). \
The model will be used when the story starts." The model will be used when the story starts."
+125
View File
@@ -0,0 +1,125 @@
//! Typed agent-name enum mirroring the `agents.toml` roster.
//!
//! `AgentName` is the single source of truth for valid agent identifiers.
//! Adding a new agent to `agents.toml` without a corresponding variant here
//! causes server startup to fail — `validate_agents` calls `AgentName::from_str`
//! for every configured agent name and returns an error for any unknown name.
use std::fmt;
use std::str::FromStr;
/// The set of named agents defined in `agents.toml`.
///
/// Variants mirror the `name` field of each `[[agent]]` entry. Serialises to
/// the canonical dash-form string (e.g. `"coder-1"`).
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum AgentName {
/// `name = "coder-1"`
Coder1,
/// `name = "coder-2"`
Coder2,
/// `name = "coder-3"`
Coder3,
/// `name = "coder-opus"`
CoderOpus,
/// `name = "qa"`
Qa,
/// `name = "qa-2"`
Qa2,
/// `name = "mergemaster"`
Mergemaster,
}
impl AgentName {
/// The canonical string form, matching the `name` field in `agents.toml`.
pub fn as_str(self) -> &'static str {
match self {
Self::Coder1 => "coder-1",
Self::Coder2 => "coder-2",
Self::Coder3 => "coder-3",
Self::CoderOpus => "coder-opus",
Self::Qa => "qa",
Self::Qa2 => "qa-2",
Self::Mergemaster => "mergemaster",
}
}
/// All known variants, in agents.toml declaration order.
pub fn all() -> &'static [AgentName] {
&[
Self::Coder1,
Self::Coder2,
Self::Coder3,
Self::CoderOpus,
Self::Qa,
Self::Qa2,
Self::Mergemaster,
]
}
}
impl fmt::Display for AgentName {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(self.as_str())
}
}
impl FromStr for AgentName {
type Err = String;
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"coder-1" => Ok(Self::Coder1),
"coder-2" => Ok(Self::Coder2),
"coder-3" => Ok(Self::Coder3),
"coder-opus" => Ok(Self::CoderOpus),
"qa" => Ok(Self::Qa),
"qa-2" => Ok(Self::Qa2),
"mergemaster" => Ok(Self::Mergemaster),
other => Err(format!(
"unknown agent name '{other}'; add a variant to AgentName or update agents.toml"
)),
}
}
}
impl serde::Serialize for AgentName {
fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
s.serialize_str(self.as_str())
}
}
impl<'de> serde::Deserialize<'de> for AgentName {
fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
let s = String::deserialize(d)?;
Self::from_str(&s).map_err(serde::de::Error::custom)
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn round_trip_all_variants() {
for &name in AgentName::all() {
let s = name.as_str();
let parsed: AgentName = s.parse().expect("parse should succeed");
assert_eq!(parsed, name);
assert_eq!(parsed.to_string(), s);
}
}
#[test]
fn unknown_name_returns_err() {
assert!("unknown-agent".parse::<AgentName>().is_err());
assert!("".parse::<AgentName>().is_err());
}
#[test]
fn display_matches_as_str() {
assert_eq!(AgentName::Coder1.to_string(), "coder-1");
assert_eq!(AgentName::CoderOpus.to_string(), "coder-opus");
assert_eq!(AgentName::Mergemaster.to_string(), "mergemaster");
}
}
+16
View File
@@ -1,4 +1,9 @@
//! Project configuration — parses `project.toml` for agents, components, and server settings. //! Project configuration — parses `project.toml` for agents, components, and server settings.
/// Typed agent name (mirrors the agents.toml roster).
pub mod agent_name;
pub use agent_name::AgentName;
use crate::slog; use crate::slog;
use serde::Deserialize; use serde::Deserialize;
use std::collections::HashSet; use std::collections::HashSet;
@@ -646,6 +651,17 @@ fn validate_agents(agents: &[AgentConfig]) -> Result<(), String> {
} }
} }
} }
// Skip the AgentName roster check in test mode — unit tests use
// synthetic agent names (e.g. "first", "second") that are not in the
// production enum. The check is meaningful only at server startup.
#[cfg(not(test))]
agent.name.parse::<AgentName>().map_err(|_| {
format!(
"Agent '{}': name is not in the AgentName roster; \
add a variant to `config::AgentName` or remove this agent from agents.toml",
agent.name
)
})?;
} }
Ok(()) Ok(())
} }
+2 -2
View File
@@ -312,7 +312,7 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option<PipelineItemV
_ => return None, _ => return None,
}; };
let agent = match item.agent.view() { let agent = match item.agent.view() {
JsonValue::String(s) if !s.is_empty() => Some(s), JsonValue::String(s) if !s.is_empty() => s.parse::<crate::config::AgentName>().ok(),
_ => None, _ => None,
}; };
let retry_count = match item.retry_count.view() { let retry_count = match item.retry_count.view() {
@@ -584,7 +584,7 @@ mod tests {
assert_eq!(view.story_id, "40_story_view"); assert_eq!(view.story_id, "40_story_view");
assert!(matches!(view.stage, crate::pipeline_state::Stage::Qa)); assert!(matches!(view.stage, crate::pipeline_state::Stage::Qa));
assert_eq!(view.name, "View Test"); assert_eq!(view.name, "View Test");
assert_eq!(view.agent.as_deref(), Some("coder-1")); assert_eq!(view.agent.map(|a| a.as_str()), Some("coder-1"));
assert_eq!(view.retry_count, 2u32); assert_eq!(view.retry_count, 2u32);
assert_eq!(view.depends_on, vec![10u32, 20u32]); assert_eq!(view.depends_on, vec![10u32, 20u32]);
} }
+5 -5
View File
@@ -175,7 +175,7 @@ pub struct WorkItem {
/// out of all read paths) when this register is empty — a nameless item is /// out of all read paths) when this register is empty — a nameless item is
/// treated as malformed, not surfaced with an empty string. /// treated as malformed, not surfaced with an empty string.
pub(super) name: String, pub(super) name: String,
pub(super) agent: Option<String>, pub(super) agent: Option<crate::config::AgentName>,
/// Retry counter — `0` when the CRDT register is unset. /// Retry counter — `0` when the CRDT register is unset.
pub(super) retry_count: u32, pub(super) retry_count: u32,
/// Dependency story numbers — empty `Vec` when the register is unset. /// Dependency story numbers — empty `Vec` when the register is unset.
@@ -210,9 +210,9 @@ impl WorkItem {
&self.name &self.name
} }
/// Agent name pinned to this item, or `None` when unset. /// Typed agent pinned to this item, or `None` when unset.
pub fn agent(&self) -> Option<&str> { pub fn agent(&self) -> Option<crate::config::AgentName> {
self.agent.as_deref() self.agent
} }
/// Retry counter. Returns `0` when the register is unset. /// Retry counter. Returns `0` when the register is unset.
@@ -255,7 +255,7 @@ impl WorkItem {
story_id: impl Into<String>, story_id: impl Into<String>,
stage: crate::pipeline_state::Stage, stage: crate::pipeline_state::Stage,
name: impl Into<String>, name: impl Into<String>,
agent: Option<String>, agent: Option<crate::config::AgentName>,
retry_count: u32, retry_count: u32,
depends_on: Vec<u32>, depends_on: Vec<u32>,
claim: Option<Claim>, claim: Option<Claim>,
+2 -2
View File
@@ -147,7 +147,7 @@ pub fn set_name(story_id: &str, name: Option<&str>) -> bool {
/// other fields to be known. /// other fields to be known.
/// ///
/// Returns `true` if the item was found and the write was performed. /// Returns `true` if the item was found and the write was performed.
pub fn set_agent(story_id: &str, agent: Option<&str>) -> bool { pub fn set_agent(story_id: &str, agent: Option<crate::config::AgentName>) -> bool {
let Some(state_mutex) = get_crdt() else { let Some(state_mutex) = get_crdt() else {
return false; return false;
}; };
@@ -157,7 +157,7 @@ pub fn set_agent(story_id: &str, agent: Option<&str>) -> bool {
let Some(&idx) = state.index.get(story_id) else { let Some(&idx) = state.index.get(story_id) else {
return false; return false;
}; };
let value = agent.unwrap_or("").to_string(); let value = agent.map(|a| a.as_str().to_string()).unwrap_or_default();
apply_and_persist(&mut state, |s| { apply_and_persist(&mut state, |s| {
s.crdt.doc.items[idx].agent.set(value.clone()) s.crdt.doc.items[idx].agent.set(value.clone())
}); });
+13 -7
View File
@@ -213,7 +213,7 @@ fn migrate_story_ids_to_numeric_preserves_stage_and_name() {
let item = read_item("45").expect("item must be accessible by numeric ID"); let item = read_item("45").expect("item must be accessible by numeric ID");
assert!(matches!(item.stage, crate::pipeline_state::Stage::Coding)); assert!(matches!(item.stage, crate::pipeline_state::Stage::Coding));
assert_eq!(item.name, "Crash Bug"); assert_eq!(item.name, "Crash Bug");
assert_eq!(item.agent.as_deref(), Some("coder-1")); assert_eq!(item.agent.map(|a| a.as_str()), Some("coder-1"));
} }
#[test] #[test]
@@ -356,12 +356,15 @@ fn set_agent_some_writes_name() {
None, None,
); );
let found = set_agent("871_story_set_agent_write", Some("coder-1")); let found = set_agent(
"871_story_set_agent_write",
Some(crate::config::AgentName::Coder1),
);
assert!(found, "set_agent should return true for an existing item"); assert!(found, "set_agent should return true for an existing item");
let item = read_item("871_story_set_agent_write").expect("item must exist"); let item = read_item("871_story_set_agent_write").expect("item must exist");
assert_eq!( assert_eq!(
item.agent.as_deref(), item.agent.map(|a| a.as_str()),
Some("coder-1"), Some("coder-1"),
"agent should be written to CRDT register" "agent should be written to CRDT register"
); );
@@ -385,7 +388,7 @@ fn set_agent_none_clears_register() {
// Confirm agent is set. // Confirm agent is set.
let before = read_item("871_story_set_agent_clear").expect("item must exist"); let before = read_item("871_story_set_agent_clear").expect("item must exist");
assert_eq!(before.agent.as_deref(), Some("coder-2")); assert_eq!(before.agent.map(|a| a.as_str()), Some("coder-2"));
// Clear it. // Clear it.
let found = set_agent("871_story_set_agent_clear", None); let found = set_agent("871_story_set_agent_clear", None);
@@ -393,8 +396,8 @@ fn set_agent_none_clears_register() {
let after = read_item("871_story_set_agent_clear").expect("item must exist"); let after = read_item("871_story_set_agent_clear").expect("item must exist");
assert!( assert!(
after.agent.as_deref().unwrap_or("").is_empty(), after.agent.is_none(),
"agent should be cleared (empty string) after set_agent(None)" "agent should be cleared after set_agent(None)"
); );
} }
@@ -402,7 +405,10 @@ fn set_agent_none_clears_register() {
fn set_agent_returns_false_for_unknown_story() { fn set_agent_returns_false_for_unknown_story() {
init_for_test(); init_for_test();
let found = set_agent("999_story_nonexistent", Some("coder-1")); let found = set_agent(
"999_story_nonexistent",
Some(crate::config::AgentName::Coder1),
);
assert!( assert!(
!found, !found,
"set_agent should return false when story is not in the CRDT" "set_agent should return false when story is not in the CRDT"
+1 -1
View File
@@ -328,7 +328,7 @@ mod tests {
let view = crate::crdt_state::read_item(story_id).expect("story exists in CRDT"); let view = crate::crdt_state::read_item(story_id).expect("story exists in CRDT");
assert_eq!(view.stage().dir_name(), "coding"); assert_eq!(view.stage().dir_name(), "coding");
assert_eq!(view.name(), "Typed Name"); assert_eq!(view.name(), "Typed Name");
assert_eq!(view.agent(), Some("coder-1")); assert_eq!(view.agent(), Some(crate::config::AgentName::Coder1));
assert_eq!(view.retry_count(), 2); assert_eq!(view.retry_count(), 2);
assert_eq!(view.depends_on(), &[100, 200]); assert_eq!(view.depends_on(), &[100, 200]);
+1 -1
View File
@@ -171,7 +171,7 @@ pub fn move_item_stage(
if let Some(db) = PIPELINE_DB.get() { if let Some(db) = PIPELINE_DB.get() {
let view = crate::crdt_state::read_item(story_id); let view = crate::crdt_state::read_item(story_id);
let name = view.as_ref().map(|v| v.name().to_string()); let name = view.as_ref().map(|v| v.name().to_string());
let agent = view.as_ref().and_then(|v| v.agent().map(str::to_string)); let agent = view.as_ref().and_then(|v| v.agent().map(|a| a.to_string()));
let depends_on = view let depends_on = view
.as_ref() .as_ref()
.map(|v| v.depends_on()) .map(|v| v.depends_on())
+2 -3
View File
@@ -141,9 +141,8 @@ pub(super) async fn tool_reject_qa(args: &Value, ctx: &AppContext) -> Result<Str
// Restart the coder agent with rejection context. // Restart the coder agent with rejection context.
// Agent name comes from the CRDT WorkItem register (story 929). // Agent name comes from the CRDT WorkItem register (story 929).
let agent_name = let agent_name = crate::crdt_state::read_item(story_id).and_then(|w| w.agent());
crate::crdt_state::read_item(story_id).and_then(|w| w.agent().map(str::to_string)); let agent_name = agent_name.map_or("coder-opus", |a| a.as_str());
let agent_name = agent_name.as_deref().unwrap_or("coder-opus");
let context = format!( let context = format!(
"\n\n---\n## QA Rejection\n\ "\n\n---\n## QA Rejection\n\
+4 -1
View File
@@ -378,7 +378,10 @@ mod tests {
story_content, story_content,
crate::db::ItemMeta::named("My Test Story"), crate::db::ItemMeta::named("My Test Story"),
); );
crate::crdt_state::set_agent("9886_story_status_test", Some("coder-1")); crate::crdt_state::set_agent(
"9886_story_status_test",
Some(crate::config::AgentName::Coder1),
);
let ctx = crate::http::context::AppContext::new_test(tmp.path().to_path_buf()); let ctx = crate::http::context::AppContext::new_test(tmp.path().to_path_buf());
let result = tool_status(&json!({"story_id": "9886_story_status_test"}), &ctx) let result = tool_status(&json!({"story_id": "9886_story_status_test"}), &ctx)
@@ -21,7 +21,7 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
crate::crdt_state::set_name(story_id, Some(name)); crate::crdt_state::set_name(story_id, Some(name));
} }
if let Some(agent) = args.get("agent").and_then(|v| v.as_str()) { if let Some(agent) = args.get("agent").and_then(|v| v.as_str()) {
crate::crdt_state::set_agent(story_id, Some(agent)); crate::crdt_state::set_agent(story_id, agent.parse::<crate::config::AgentName>().ok());
} }
if let Some(epic) = args.get("epic").and_then(|v| v.as_str()) { if let Some(epic) = args.get("epic").and_then(|v| v.as_str()) {
crate::crdt_state::set_epic(story_id, crate::crdt_state::EpicId::from_crdt_str(epic)); crate::crdt_state::set_epic(story_id, crate::crdt_state::EpicId::from_crdt_str(epic));
@@ -42,8 +42,10 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
crate::crdt_state::set_name(story_id, s); crate::crdt_state::set_name(story_id, s);
} }
"agent" => { "agent" => {
let s = value.as_str().filter(|s| !s.is_empty()); let parsed = value
crate::crdt_state::set_agent(story_id, s); .as_str()
.and_then(|s| s.parse::<crate::config::AgentName>().ok());
crate::crdt_state::set_agent(story_id, parsed);
} }
"qa" => { "qa" => {
let mode = value let mode = value
+1 -1
View File
@@ -161,7 +161,7 @@ mod tests {
"42_story_test", "42_story_test",
Stage::Coding, Stage::Coding,
"Test", "Test",
Some("coder-1".to_string()), Some(crate::config::AgentName::Coder1),
2u32, 2u32,
vec![], vec![],
None, None,
+1 -1
View File
@@ -165,7 +165,7 @@ pub fn get_work_item_content(
let crdt_name = crdt_view.as_ref().map(|v| v.name().to_string()); let crdt_name = crdt_view.as_ref().map(|v| v.name().to_string());
let crdt_agent = crdt_view let crdt_agent = crdt_view
.as_ref() .as_ref()
.and_then(|v| v.agent().map(str::to_string)); .and_then(|v| v.agent().map(|a| a.to_string()));
for (stage_dir, stage) in &stages { for (stage_dir, stage) in &stages {
if let Some(content) = io::read_work_item_from_stage(&work_dir, stage_dir, &filename)? { if let Some(content) = io::read_work_item_from_stage(&work_dir, stage_dir, &filename)? {
+7 -4
View File
@@ -25,7 +25,10 @@ pub async fn assign_and_start(
project_root: &Path, project_root: &Path,
agents: &AgentPool, agents: &AgentPool,
) -> Result<AgentInfo, String> { ) -> Result<AgentInfo, String> {
crate::crdt_state::set_agent(story_id, Some(agent_name)); crate::crdt_state::set_agent(
story_id,
agent_name.parse::<crate::config::AgentName>().ok(),
);
agents agents
.start_agent(project_root, story_id, Some(agent_name), None, None) .start_agent(project_root, story_id, Some(agent_name), None, None)
.await .await
@@ -114,10 +117,10 @@ mod tests {
let agents = make_agents(); let agents = make_agents();
// Simulate Matrix path: call assign_and_start directly (as handle_assign does). // Simulate Matrix path: call assign_and_start directly (as handle_assign does).
let _ = assign_and_start(story_id_a, "coder-sonnet", tmp.path(), &agents).await; let _ = assign_and_start(story_id_a, "coder-opus", tmp.path(), &agents).await;
// Simulate MCP path: call assign_and_start directly (as tool_start_agent does). // Simulate MCP path: call assign_and_start directly (as tool_start_agent does).
let _ = assign_and_start(story_id_b, "coder-sonnet", tmp.path(), &agents).await; let _ = assign_and_start(story_id_b, "coder-opus", tmp.path(), &agents).await;
// Both must leave the same CRDT agent register value. // Both must leave the same CRDT agent register value.
for sid in &[story_id_a, story_id_b] { for sid in &[story_id_a, story_id_b] {
@@ -129,7 +132,7 @@ mod tests {
.expect("item must be in CRDT"); .expect("item must be in CRDT");
assert_eq!( assert_eq!(
item.agent.as_deref(), item.agent.as_deref(),
Some("coder-sonnet"), Some("coder-opus"),
"CRDT agent register must match for story {sid}" "CRDT agent register must match for story {sid}"
); );
} }