From 2a24a4cc8529e8421f54585e034bec0efbf3121e Mon Sep 17 00:00:00 2001 From: dave Date: Fri, 10 Apr 2026 10:33:35 +0000 Subject: [PATCH] huskies: merge 522_story_migrate_status_command_pipeline_view_from_filesystem_to_pipeline_state_read_all_typed --- server/src/chat/commands/status.rs | 462 +++++++++++++++++------------ 1 file changed, 265 insertions(+), 197 deletions(-) diff --git a/server/src/chat/commands/status.rs b/server/src/chat/commands/status.rs index f047408b..368185a5 100644 --- a/server/src/chat/commands/status.rs +++ b/server/src/chat/commands/status.rs @@ -2,6 +2,7 @@ use crate::agents::{AgentPool, AgentStatus}; use crate::config::ProjectConfig; +use crate::pipeline_state::{PipelineItem, Stage}; use std::collections::{HashMap, HashSet}; use super::CommandContext; @@ -49,22 +50,6 @@ pub(super) fn story_short_label(stem: &str, name: Option<&str>) -> String { } } -/// Read the `blocked` flag from a story file's YAML front matter. -/// -/// Returns `true` when the story has `blocked: true` set (retry limit reached). -fn read_story_blocked(project_root: &std::path::Path, stage_dir: &str, stem: &str) -> bool { - let path = project_root - .join(".huskies") - .join("work") - .join(stage_dir) - .join(format!("{stem}.md")); - std::fs::read_to_string(path) - .ok() - .and_then(|c| crate::io::story_metadata::parse_front_matter(&c).ok()) - .and_then(|m| m.blocked) - .unwrap_or(false) -} - /// Choose the traffic-light indicator for a work item. /// /// Priority: blocked > throttled > running > idle. @@ -87,43 +72,44 @@ pub(super) fn traffic_light_dot(blocked: bool, throttled: bool, has_agent: bool) } } -/// Read all story IDs and names from a pipeline stage directory. -fn read_stage_items( - project_root: &std::path::Path, - stage_dir: &str, -) -> Vec<(String, Option)> { - let dir = project_root - .join(".huskies") - .join("work") - .join(stage_dir); - if !dir.exists() { - return Vec::new(); - } - let mut items = Vec::new(); - if let Ok(entries) = std::fs::read_dir(&dir) { - for entry in entries.flatten() { - let path = entry.path(); - if path.extension().and_then(|e| e.to_str()) != Some("md") { - continue; +/// Check which dependency numbers from `item.depends_on` are unmet. +/// +/// A dependency is considered met if the dep is in `Done` or `Archived` stage +/// in `all_items`. If the dep is not found in `all_items` at all (e.g. it was +/// archived before the CRDT migration and has no row), it is treated as met. +fn unmet_deps_from_items(item: &PipelineItem, all_items: &[PipelineItem]) -> Vec { + item.depends_on + .iter() + .filter_map(|dep_id| { + // dep_id.0 is the raw number string (e.g. "999") as projected + // from PipelineItemView.depends_on: Vec. + let dep_num: u32 = dep_id.0.parse().ok()?; + // Find the dep by matching the numeric prefix of its story_id. + let dep = all_items.iter().find(|i| { + i.story_id.0 == dep_id.0 + || i.story_id.0.split('_').next() == Some(dep_id.0.as_str()) + }); + match dep { + Some(d) if matches!(d.stage, Stage::Done { .. } | Stage::Archived { .. }) => None, + Some(_) => Some(dep_num), // Found but not done = unmet + None => None, // Not in CRDT; treat as met } - if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) { - let name = std::fs::read_to_string(&path) - .ok() - .and_then(|contents| { - crate::io::story_metadata::parse_front_matter(&contents) - .ok() - .and_then(|m| m.name) - }); - items.push((stem.to_string(), name)); - } - } - } - items.sort_by(|a, b| a.0.cmp(&b.0)); - items + }) + .collect() } /// Build the full pipeline status text formatted for Matrix (markdown). pub(super) fn build_pipeline_status(project_root: &std::path::Path, agents: &AgentPool) -> String { + let items = crate::pipeline_state::read_all_typed(); + build_status_from_items(project_root, agents, &items) +} + +/// Inner implementation that accepts pre-loaded items for testability. +fn build_status_from_items( + project_root: &std::path::Path, + agents: &AgentPool, + items: &[PipelineItem], +) -> String { // Build a map from story_id → active AgentInfo for quick lookup. let active_agents = agents.list_agents().unwrap_or_default(); let active_map: HashMap = active_agents @@ -146,62 +132,58 @@ pub(super) fn build_pipeline_status(project_root: &std::path::Path, agents: &Age let mut out = String::from("**Pipeline Status**\n\n"); - let stages = [ - ("1_backlog", "Backlog"), - ("2_current", "In Progress"), - ("3_qa", "QA"), - ("4_merge", "Merge"), - ("5_done", "Done"), + // Active pipeline stages to display (Archived is handled separately below). + type StagePredicate = fn(&Stage) -> bool; + let stage_filters: &[(&str, StagePredicate)] = &[ + ("Backlog", |s| matches!(s, Stage::Backlog)), + ("In Progress", |s| matches!(s, Stage::Coding)), + ("QA", |s| matches!(s, Stage::Qa)), + ("Merge", |s| matches!(s, Stage::Merge { .. })), + ("Done", |s| matches!(s, Stage::Done { .. })), ]; - for (dir, label) in &stages { - let items = read_stage_items(project_root, dir); - let count = items.len(); + for (label, filter) in stage_filters { + let mut stage_items: Vec<&PipelineItem> = + items.iter().filter(|i| filter(&i.stage)).collect(); + stage_items.sort_by(|a, b| a.story_id.0.cmp(&b.story_id.0)); + let count = stage_items.len(); out.push_str(&format!("**{label}** ({count})\n")); - if items.is_empty() { + if stage_items.is_empty() { out.push_str(" *(none)*\n"); } else { - for (story_id, name) in &items { - let display = story_short_label(story_id, name.as_deref()); - let cost_suffix = cost_by_story - .get(story_id) - .filter(|&&c| c > 0.0) - .map(|c| format!(" — ${c:.2}")) - .unwrap_or_default(); - let blocked = read_story_blocked(project_root, dir, story_id); - let agent = active_map.get(story_id); - let throttled = agent.map(|a| a.throttled).unwrap_or(false); - let dot = traffic_light_dot(blocked, throttled, agent.is_some()); - // Check for unmet dependencies and append a note when present. - let unmet = crate::io::story_metadata::check_unmet_deps( - project_root, - dir, - story_id, - ); - let dep_suffix = if unmet.is_empty() { - String::new() - } else { - let nums: Vec = unmet.iter().map(|n| n.to_string()).collect(); - format!(" *(waiting on: {})*", nums.join(", ")) - }; - if let Some(agent) = agent { - let model_str = config - .as_ref() - .and_then(|cfg| cfg.find_agent(&agent.agent_name)) - .and_then(|ac| ac.model.as_deref()) - .unwrap_or("?"); - out.push_str(&format!( - " {dot}{display}{cost_suffix}{dep_suffix} — {} ({model_str})\n", - agent.agent_name - )); - } else { - out.push_str(&format!(" {dot}{display}{cost_suffix}{dep_suffix}\n")); - } + for item in &stage_items { + out.push_str(&render_item_line( + item, + items, + &active_map, + &cost_by_story, + &config, + )); } } out.push('\n'); } + // Blocked items: Archived { reason: Blocked } shown with 🔴 indicator. + let mut blocked_items: Vec<&PipelineItem> = items + .iter() + .filter(|i| i.stage.is_blocked()) + .collect(); + blocked_items.sort_by(|a, b| a.story_id.0.cmp(&b.story_id.0)); + if !blocked_items.is_empty() { + out.push_str(&format!("**Blocked** ({})\n", blocked_items.len())); + for item in &blocked_items { + out.push_str(&render_item_line( + item, + items, + &active_map, + &cost_by_story, + &config, + )); + } + out.push('\n'); + } + // Free agents: configured agents not currently running or pending. out.push_str("**Free Agents**\n"); if let Some(cfg) = &config { @@ -235,10 +217,80 @@ pub(super) fn build_pipeline_status(project_root: &std::path::Path, agents: &Age out } +/// Render a single status line for one pipeline item. +fn render_item_line( + item: &PipelineItem, + all_items: &[PipelineItem], + active_map: &HashMap, + cost_by_story: &HashMap, + config: &Option, +) -> String { + let story_id = &item.story_id.0; + let name_opt = if item.name.is_empty() { + None + } else { + Some(item.name.as_str()) + }; + let display = story_short_label(story_id, name_opt); + let cost_suffix = cost_by_story + .get(story_id) + .filter(|&&c| c > 0.0) + .map(|c| format!(" — ${c:.2}")) + .unwrap_or_default(); + let blocked = item.stage.is_blocked(); + let agent = active_map.get(story_id); + let throttled = agent.map(|a| a.throttled).unwrap_or(false); + let dot = traffic_light_dot(blocked, throttled, agent.is_some()); + let unmet = unmet_deps_from_items(item, all_items); + let dep_suffix = if unmet.is_empty() { + String::new() + } else { + let nums: Vec = unmet.iter().map(|n| n.to_string()).collect(); + format!(" *(waiting on: {})*", nums.join(", ")) + }; + if let Some(agent) = agent { + let model_str = config + .as_ref() + .and_then(|cfg| cfg.find_agent(&agent.agent_name)) + .and_then(|ac| ac.model.as_deref()) + .unwrap_or("?"); + format!( + " {dot}{display}{cost_suffix}{dep_suffix} — {} ({model_str})\n", + agent.agent_name + ) + } else { + format!(" {dot}{display}{cost_suffix}{dep_suffix}\n") + } +} + #[cfg(test)] mod tests { use super::*; use crate::agents::AgentPool; + use crate::pipeline_state::{ArchiveReason, PipelineItem, Stage, StoryId}; + use chrono::Utc; + + /// Build a minimal PipelineItem for tests. + fn make_item(id: &str, name: &str, stage: Stage) -> PipelineItem { + PipelineItem { + story_id: StoryId(id.to_string()), + name: name.to_string(), + stage, + depends_on: Vec::new(), + retry_count: 0, + } + } + + /// Build a PipelineItem with dependencies for tests. + fn make_item_with_deps(id: &str, name: &str, stage: Stage, deps: Vec) -> PipelineItem { + PipelineItem { + story_id: StoryId(id.to_string()), + name: name.to_string(), + stage, + depends_on: deps.iter().map(|n| StoryId(n.to_string())).collect(), + retry_count: 0, + } + } #[test] fn status_command_matches() { @@ -322,24 +374,21 @@ mod tests { assert_eq!(label, "42 — Some item"); } - // -- build_pipeline_status formatting ----------------------------------- + // -- build_status_from_items formatting ----------------------------------- #[test] fn status_does_not_show_full_filename_stem() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - // Write a story file with a front-matter name - let story_path = stage_dir.join("293_story_register_all_bot_commands.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Register all bot commands\n---\n").unwrap(); + let items = vec![make_item( + "293_story_register_all_bot_commands", + "Register all bot commands", + Stage::Coding, + )]; let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( !output.contains("293_story_register_all_bot_commands"), @@ -355,16 +404,14 @@ mod tests { #[test] fn status_shows_cost_when_token_usage_exists() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - let story_path = stage_dir.join("293_story_register_all_bot_commands.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Register all bot commands\n---\n").unwrap(); + let items = vec![make_item( + "293_story_register_all_bot_commands", + "Register all bot commands", + Stage::Coding, + )]; // Write token usage for this story. let usage = crate::agents::TokenUsage { @@ -383,7 +430,7 @@ mod tests { crate::agents::token_usage::append_record(tmp.path(), &record).unwrap(); let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( output.contains("293 [story] — Register all bot commands — $0.29"), @@ -393,20 +440,17 @@ mod tests { #[test] fn status_no_cost_when_no_usage() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - let story_path = stage_dir.join("293_story_register_all_bot_commands.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Register all bot commands\n---\n").unwrap(); + let items = vec![make_item( + "293_story_register_all_bot_commands", + "Register all bot commands", + Stage::Coding, + )]; - // No token usage written. let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( !output.contains("$"), @@ -416,16 +460,14 @@ mod tests { #[test] fn status_aggregates_multiple_records_per_story() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - let story_path = stage_dir.join("293_story_register_all_bot_commands.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Register all bot commands\n---\n").unwrap(); + let items = vec![make_item( + "293_story_register_all_bot_commands", + "Register all bot commands", + Stage::Coding, + )]; // Write two records for the same story — costs should be summed. for cost in [0.10, 0.19] { @@ -446,7 +488,7 @@ mod tests { } let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( output.contains("293 [story] — Register all bot commands — $0.29"), @@ -458,20 +500,18 @@ mod tests { #[test] fn status_shows_waiting_on_for_story_with_unmet_deps() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - // Dep 999 is NOT done — no 5_done directory at all. - let story_path = stage_dir.join("10_story_waiting.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Waiting Story\ndepends_on: [999]\n---\n").unwrap(); + // Story 10 depends on story 999, which is NOT in all_items (treated as met) + // OR present in backlog (unmet). Let's add dep 999 in Backlog stage (unmet). + let items = vec![ + make_item_with_deps("10_story_waiting", "Waiting Story", Stage::Coding, vec![999]), + make_item("999_story_dep", "Dep Story", Stage::Backlog), + ]; let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( output.contains("waiting on: 999"), @@ -481,23 +521,20 @@ mod tests { #[test] fn status_does_not_show_waiting_on_when_dep_is_done() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - let done_dir = tmp.path().join(".huskies/work/5_done"); - std::fs::create_dir_all(&stage_dir).unwrap(); - std::fs::create_dir_all(&done_dir).unwrap(); - // Dep 999 is done. - std::fs::write(done_dir.join("999_story_dep.md"), "---\nname: Dep\n---\n").unwrap(); - let story_path = stage_dir.join("10_story_unblocked.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Unblocked Story\ndepends_on: [999]\n---\n").unwrap(); + // Dep 999 is in Done stage — met. + let items = vec![ + make_item_with_deps("10_story_unblocked", "Unblocked Story", Stage::Coding, vec![999]), + make_item("999_story_dep", "Dep Story", Stage::Done { + merged_at: Utc::now(), + merge_commit: crate::pipeline_state::GitSha("abc123".to_string()), + }), + ]; let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( !output.contains("waiting on"), @@ -507,19 +544,13 @@ mod tests { #[test] fn status_shows_no_waiting_info_when_no_deps() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - let story_path = stage_dir.join("42_story_nodeps.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: No Deps Story\n---\n").unwrap(); + let items = vec![make_item("42_story_nodeps", "No Deps Story", Stage::Coding)]; let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( !output.contains("waiting on"), @@ -554,53 +585,44 @@ mod tests { assert_eq!(traffic_light_dot(true, false, false), "\u{1F534} "); // 🔴 } - // -- read_story_blocked -------------------------------------------------- + // -- Stage::is_blocked() replaces read_story_blocked -------------------- #[test] - fn read_story_blocked_returns_true_when_blocked() { - use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - std::fs::write( - stage_dir.join("42_story_foo.md"), - "---\nname: Foo\nblocked: true\n---\n", - ) - .unwrap(); - assert!(read_story_blocked(tmp.path(), "2_current", "42_story_foo")); + fn stage_is_blocked_returns_true_for_archived_blocked() { + let stage = Stage::Archived { + archived_at: Utc::now(), + reason: ArchiveReason::Blocked { + reason: "too many retries".to_string(), + }, + }; + assert!(stage.is_blocked()); } #[test] - fn read_story_blocked_returns_false_when_not_blocked() { - use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - std::fs::write( - stage_dir.join("42_story_foo.md"), - "---\nname: Foo\n---\n", - ) - .unwrap(); - assert!(!read_story_blocked(tmp.path(), "2_current", "42_story_foo")); + fn stage_is_blocked_returns_false_for_coding() { + assert!(!Stage::Coding.is_blocked()); + } + + #[test] + fn stage_is_blocked_returns_false_for_archived_completed() { + let stage = Stage::Archived { + archived_at: Utc::now(), + reason: ArchiveReason::Completed, + }; + assert!(!stage.is_blocked()); } // -- status output shows idle dot for items with no active agent -------- #[test] fn status_shows_idle_dot_for_unassigned_story() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - let story_path = stage_dir.join("42_story_idle.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Idle Story\n---\n").unwrap(); + let items = vec![make_item("42_story_idle", "Idle Story", Stage::Coding)]; let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( output.contains("\u{26AA} "), // ⚪ @@ -610,23 +632,69 @@ mod tests { #[test] fn status_shows_blocked_dot_for_blocked_story() { - use std::io::Write; use tempfile::TempDir; - let tmp = TempDir::new().unwrap(); - let stage_dir = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(&stage_dir).unwrap(); - let story_path = stage_dir.join("42_story_blocked.md"); - let mut f = std::fs::File::create(&story_path).unwrap(); - writeln!(f, "---\nname: Blocked Story\nblocked: true\n---\n").unwrap(); + let items = vec![make_item( + "42_story_blocked", + "Blocked Story", + Stage::Archived { + archived_at: Utc::now(), + reason: ArchiveReason::Blocked { + reason: "retry limit exceeded".to_string(), + }, + }, + )]; let agents = AgentPool::new_test(3000); - let output = build_pipeline_status(tmp.path(), &agents); + let output = build_status_from_items(tmp.path(), &agents, &items); assert!( output.contains("\u{1F534} "), // 🔴 "blocked story should show red circle emoji: {output}" ); } + + // -- Regression: CRDT-only story appears in correct stage --------------- + + #[test] + fn status_shows_crdt_done_story_in_done_not_backlog() { + use tempfile::TempDir; + let tmp = TempDir::new().unwrap(); + + // Story is Done in the typed API — even if a stale 1_backlog/ shadow existed + // on the filesystem, the status must show it in Done, not Backlog. + let items = vec![make_item( + "503_story_some_feature", + "Some Feature", + Stage::Done { + merged_at: Utc::now(), + merge_commit: crate::pipeline_state::GitSha("deadbeef".to_string()), + }, + )]; + + let agents = AgentPool::new_test(3000); + let output = build_status_from_items(tmp.path(), &agents, &items); + + // Must appear under Done, not Backlog. + let done_pos = output.find("**Done**").expect("Done section must exist"); + let backlog_pos = output.find("**Backlog**").expect("Backlog section must exist"); + let story_pos = output.find("503 [story]").expect("story must appear in output"); + + assert!( + story_pos > done_pos, + "story should appear after Done header: backlog={backlog_pos} done={done_pos} story={story_pos}\n{output}" + ); + assert!( + story_pos > backlog_pos, + "story should not be in the Backlog section: {output}" + ); + + // Verify it's not in Backlog section specifically. + let backlog_section = &output[backlog_pos..done_pos]; + assert!( + !backlog_section.contains("503"), + "503 must not appear in Backlog section: {backlog_section}" + ); + } }