From a7840ea4b04d275f83f80fc526b63df14eabf981 Mon Sep 17 00:00:00 2001 From: dave Date: Wed, 13 May 2026 07:54:50 +0000 Subject: [PATCH] huskies: merge 946 --- server/src/agent_mode/claim.rs | 9 +- server/src/agent_mode/loop_ops.rs | 28 ++-- server/src/agents/lifecycle.rs | 9 +- .../agents/pool/pipeline/advance/helpers.rs | 4 +- server/src/agents/pool/start/spawn.rs | 2 +- server/src/agents/pool/stop.rs | 2 +- server/src/chat/commands/depends.rs | 10 +- server/src/chat/commands/diff.rs | 2 +- server/src/chat/commands/freeze.rs | 4 +- server/src/chat/commands/logs.rs | 4 +- server/src/chat/commands/move_story.rs | 3 +- server/src/chat/commands/overview.rs | 2 +- server/src/chat/commands/triage.rs | 11 +- server/src/chat/commands/unblock.rs | 2 +- server/src/chat/commands/unreleased.rs | 2 +- server/src/chat/transport/matrix/assign.rs | 2 +- server/src/chat/transport/matrix/delete.rs | 2 +- server/src/chat/transport/matrix/start.rs | 2 +- server/src/crdt_state/mod.rs | 4 +- server/src/crdt_state/presence.rs | 2 +- server/src/crdt_state/read.rs | 54 ++++--- server/src/crdt_state/state/tests.rs | 2 +- server/src/crdt_state/types.rs | 143 +++++++++++------- server/src/crdt_state/write/item.rs | 18 ++- server/src/crdt_state/write/tests.rs | 67 ++++---- server/src/db/mod.rs | 17 +-- server/src/db/ops.rs | 2 +- server/src/http/mcp/diagnostics/permission.rs | 6 +- server/src/http/mcp/status_tools.rs | 18 +-- server/src/http/mcp/story_tools/bug.rs | 2 +- server/src/http/mcp/story_tools/criteria.rs | 3 +- server/src/http/mcp/story_tools/epic.rs | 29 ++-- .../src/http/mcp/story_tools/story/delete.rs | 2 +- .../src/http/mcp/story_tools/story/query.rs | 12 +- .../src/http/mcp/story_tools/story/update.rs | 14 +- server/src/http/workflow/bug_ops/bug.rs | 5 +- server/src/http/workflow/bug_ops/epic.rs | 2 +- server/src/http/workflow/bug_ops/refactor.rs | 5 +- server/src/http/workflow/bug_ops/tests.rs | 2 +- server/src/http/workflow/pipeline.rs | 64 ++++---- server/src/http/workflow/story_ops/create.rs | 4 +- .../src/http/workflow/story_ops/criterion.rs | 5 +- server/src/http/workflow/utils.rs | 5 +- server/src/io/story_metadata/mod.rs | 4 +- server/src/io/story_metadata/parser.rs | 4 +- server/src/io/story_metadata/types.rs | 49 +++++- server/src/pipeline_state/projection.rs | 42 ++--- server/src/service/agents/mod.rs | 4 +- server/src/service/notifications/io/mod.rs | 2 +- 49 files changed, 378 insertions(+), 314 deletions(-) diff --git a/server/src/agent_mode/claim.rs b/server/src/agent_mode/claim.rs index 5471ef38..922e9386 100644 --- a/server/src/agent_mode/claim.rs +++ b/server/src/agent_mode/claim.rs @@ -105,11 +105,12 @@ mod tests { // Confirm the stale claim is in place. let before = read_item(story_id).expect("item should exist"); assert_eq!( - before.claimed_by(), + before.claim().map(|c| c.node.as_str()), Some(stale_holder), "pre-condition: item should be claimed by the stale holder" ); - let age = chrono::Utc::now().timestamp() as f64 - before.claimed_at().unwrap_or(0.0); + let age = chrono::Utc::now().timestamp() as f64 + - before.claim().map(|c| c.at as f64).unwrap_or(0.0); assert!( age >= CLAIM_TIMEOUT_SECS, "pre-condition: claim age ({age}s) must exceed TTL ({CLAIM_TIMEOUT_SECS}s)" @@ -136,12 +137,12 @@ mod tests { let our_id = our_node_id().expect("node id should be available after init_for_test"); let after = read_item(story_id).expect("item should still exist"); assert_eq!( - after.claimed_by(), + after.claim().map(|c| c.node.as_str()), Some(our_id.as_str()), "new claim should have displaced the stale holder" ); assert_ne!( - after.claimed_by(), + after.claim().map(|c| c.node.as_str()), Some(stale_holder), "stale holder must no longer own the claim" ); diff --git a/server/src/agent_mode/loop_ops.rs b/server/src/agent_mode/loop_ops.rs index 033b6094..edccb414 100644 --- a/server/src/agent_mode/loop_ops.rs +++ b/server/src/agent_mode/loop_ops.rs @@ -52,20 +52,18 @@ pub(super) async fn scan_and_claim( } // If already claimed by us, skip. - if item.claimed_by() == Some(our_node.as_str()) { + if item.claim().is_some_and(|c| c.node == our_node) { continue; } // If claimed by another node, respect the claim while it is fresh. // Once the TTL expires the claim is considered stale regardless of // whether the holder appears alive — displacement is purely TTL-driven. - if let Some(claimer) = item.claimed_by() - && !claimer.is_empty() - && claimer != our_node.as_str() - && let Some(claimed_at) = item.claimed_at() + if let Some(claim) = item.claim() + && claim.node != our_node { - let now = chrono::Utc::now().timestamp() as f64; - let age = now - claimed_at; + let now = chrono::Utc::now().timestamp() as u64; + let age = now.saturating_sub(claim.at) as f64; if age < CLAIM_TIMEOUT_SECS { // Claim is still fresh — respect it. continue; @@ -75,7 +73,7 @@ pub(super) async fn scan_and_claim( "[agent-mode] Displacing stale claim on '{}' held by {:.12}… \ (age {}s > TTL {}s)", item.story_id(), - claimer, + claim.node, age as u64, CLAIM_TIMEOUT_SECS as u64, ); @@ -172,18 +170,14 @@ pub(super) fn reclaim_timed_out_work(_project_root: &Path) { // Release the claim if the TTL has expired — regardless of whether the // holder is still alive. A node actively working should refresh its // claim before the TTL window closes. - if let Some(claimer) = item.claimed_by() { - if claimer.is_empty() { - continue; - } - if let Some(claimed_at) = item.claimed_at() - && now - claimed_at >= CLAIM_TIMEOUT_SECS - { + if let Some(claim) = item.claim() { + let age = now as u64 - claim.at.min(now as u64); + if age as f64 >= CLAIM_TIMEOUT_SECS { slog!( "[agent-mode] Releasing stale claim on '{}' held by {:.12}… (age {}s)", item.story_id(), - claimer, - (now - claimed_at) as u64, + claim.node, + age, ); crdt_state::release_claim(item.story_id()); } diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 2cd934c6..1f3ca65f 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -35,10 +35,11 @@ pub(crate) fn item_type_from_id(item_id: &str) -> &'static str { && let Some(view) = crate::crdt_state::read_item(item_id) && let Some(t) = view.item_type() { + use crate::io::story_metadata::ItemType; return match t { - "bug" => "bug", - "spike" => "spike", - "refactor" => "refactor", + ItemType::Bug => "bug", + ItemType::Spike => "spike", + ItemType::Refactor => "refactor", _ => "story", }; } @@ -527,7 +528,7 @@ mod tests { &format!("# Test {t}\n"), crate::db::ItemMeta::named(format!("Test {t}")), ); - crate::crdt_state::set_item_type(id, Some(t)); + crate::crdt_state::set_item_type(id, crate::io::story_metadata::ItemType::from_str(t)); } assert_eq!(item_type_from_id("9999"), "bug"); diff --git a/server/src/agents/pool/pipeline/advance/helpers.rs b/server/src/agents/pool/pipeline/advance/helpers.rs index dc7b8ea0..4ee340a8 100644 --- a/server/src/agents/pool/pipeline/advance/helpers.rs +++ b/server/src/agents/pool/pipeline/advance/helpers.rs @@ -60,9 +60,7 @@ pub(super) fn resolve_qa_mode_from_store( default: crate::io::story_metadata::QaMode, ) -> crate::io::story_metadata::QaMode { crate::crdt_state::read_item(story_id) - .and_then(|view| view.qa_mode().map(str::to_string)) - .as_deref() - .and_then(crate::io::story_metadata::QaMode::from_str) + .and_then(|view| view.qa_mode()) .unwrap_or(default) } diff --git a/server/src/agents/pool/start/spawn.rs b/server/src/agents/pool/start/spawn.rs index 4dc32b3b..4e4367aa 100644 --- a/server/src/agents/pool/start/spawn.rs +++ b/server/src/agents/pool/start/spawn.rs @@ -230,7 +230,7 @@ pub(super) async fn run_agent_spawn( // Story 933: epic linkage is now a typed CRDT register on PipelineItemCrdt. if let Some(view) = crate::crdt_state::read_item(&sid) && let Some(epic_id) = view.epic() - && let Some(epic_content) = crate::db::read_content(epic_id) + && let Some(epic_content) = crate::db::read_content(&epic_id.to_string()) { let block = format!( "# Epic Context\n\nThis work item belongs to epic `{epic_id}`.\ diff --git a/server/src/agents/pool/stop.rs b/server/src/agents/pool/stop.rs index 4273bc55..8c41ad31 100644 --- a/server/src/agents/pool/stop.rs +++ b/server/src/agents/pool/stop.rs @@ -147,7 +147,7 @@ mod tests { "60_story_cleanup", "2_current", story_content, - crate::db::ItemMeta::default(), + crate::db::ItemMeta::named("Cleanup"), ); let pool = AgentPool::new_test(3001); diff --git a/server/src/chat/commands/depends.rs b/server/src/chat/commands/depends.rs index 93b9cb03..c4aeba16 100644 --- a/server/src/chat/commands/depends.rs +++ b/server/src/chat/commands/depends.rs @@ -63,7 +63,7 @@ pub(super) fn handle_depends(ctx: &CommandContext) -> Option { // Story name comes from the CRDT register, not the on-disk YAML // (story 929 — CRDT is the sole source of story metadata). let story_name = crate::crdt_state::read_item(&story_id) - .and_then(|w| w.name().map(str::to_string)) + .map(|w| w.name().to_string()) .unwrap_or_else(|| story_id.clone()); // Write depends_on to the typed CRDT register — single source of truth. @@ -183,7 +183,7 @@ mod tests { "1_backlog", "9910_story_foo.md", "---\nname: Foo\n---\n", - None, + Some("Foo"), ); let output = depends_cmd_with_root(tmp.path(), "9910 477 478").unwrap(); assert!( @@ -214,7 +214,7 @@ mod tests { "2_current", "9911_story_bar.md", "---\nname: Bar\n---\n", - None, + Some("Bar"), ); // Pre-seed CRDT with deps so we can verify clearing. crate::crdt_state::set_depends_on("9911_story_bar", &[477]); @@ -251,7 +251,7 @@ mod tests { "1_backlog", "8790_story_chat_dep.md", "---\nname: Chat Dep\n---\n", - None, + Some("Chat Dep"), ); let out = depends_cmd_with_root(tmp.path(), "8790 500 501").unwrap(); @@ -286,7 +286,7 @@ mod tests { "1_backlog", "9920_story_scr.md", "---\nname: SCR\n---\n", - None, + Some("SCR"), ); // Set to [1, 2, 3]. diff --git a/server/src/chat/commands/diff.rs b/server/src/chat/commands/diff.rs index c4b2b258..d5d9523b 100644 --- a/server/src/chat/commands/diff.rs +++ b/server/src/chat/commands/diff.rs @@ -212,7 +212,7 @@ mod tests { "2_current", "55551_story_no_worktree.md", "---\nname: No Worktree\n---\n", - None, + Some("No Worktree"), ); let output = diff_cmd(tmp.path(), "55551").unwrap(); assert!( diff --git a/server/src/chat/commands/freeze.rs b/server/src/chat/commands/freeze.rs index 73cfe88e..0e561a43 100644 --- a/server/src/chat/commands/freeze.rs +++ b/server/src/chat/commands/freeze.rs @@ -98,7 +98,7 @@ fn unfreeze_by_story_id(story_id: &str) -> String { /// Falls back to `story_id` if no CRDT entry exists. fn resolve_story_name(story_id: &str) -> String { crate::crdt_state::read_item(story_id) - .and_then(|w| w.name().map(str::to_string)) + .map(|w| w.name().to_string()) .unwrap_or_else(|| story_id.to_string()) } @@ -274,7 +274,7 @@ mod tests { "2_current", "9943_story_alreadyfrozen.md", "---\nname: Already Frozen\n---\n# Story\n", - None, + Some("Already Frozen"), ); // Freeze it first. freeze_cmd_with_root(tmp.path(), "9943").unwrap(); diff --git a/server/src/chat/commands/logs.rs b/server/src/chat/commands/logs.rs index 763b3cc9..de3dec37 100644 --- a/server/src/chat/commands/logs.rs +++ b/server/src/chat/commands/logs.rs @@ -202,7 +202,7 @@ mod tests { "2_current", "77_story_no_log.md", "---\nname: No Log\n---\n", - None, + Some("No Log"), ); let output = logs_cmd(tmp.path(), "77").unwrap(); assert!( @@ -222,7 +222,7 @@ mod tests { "2_current", "88_story_has_log.md", "---\nname: Has Log\n---\n", - None, + Some("Has Log"), ); // Write a log file in the expected location. let log_dir = tmp diff --git a/server/src/chat/commands/move_story.rs b/server/src/chat/commands/move_story.rs index 6bd3b056..3d92805b 100644 --- a/server/src/chat/commands/move_story.rs +++ b/server/src/chat/commands/move_story.rs @@ -57,8 +57,7 @@ pub(super) fn handle_move(ctx: &CommandContext) -> Option { }; // Display name comes from the CRDT name register (story 929). - let found_name = - crate::crdt_state::read_item(&story_id).and_then(|w| w.name().map(str::to_string)); + let found_name = crate::crdt_state::read_item(&story_id).map(|w| w.name().to_string()); let display_name = found_name.as_deref().unwrap_or(&story_id); diff --git a/server/src/chat/commands/overview.rs b/server/src/chat/commands/overview.rs index 790cf80d..477ed13f 100644 --- a/server/src/chat/commands/overview.rs +++ b/server/src/chat/commands/overview.rs @@ -110,7 +110,7 @@ fn find_story_merge_commit(root: &std::path::Path, num_str: &str) -> Option Option { let (story_id, _, _, _) = crate::chat::lookup::find_story_by_number(root, num_str)?; - crate::crdt_state::read_item(&story_id).and_then(|w| w.name().map(str::to_string)) + crate::crdt_state::read_item(&story_id).map(|w| w.name().to_string()) } /// Return the `git show --stat` output for a commit. diff --git a/server/src/chat/commands/triage.rs b/server/src/chat/commands/triage.rs index 847907e3..42701ad6 100644 --- a/server/src/chat/commands/triage.rs +++ b/server/src/chat/commands/triage.rs @@ -72,10 +72,7 @@ fn build_triage_dump( // Story metadata now comes from the CRDT registers and adjacent CRDT entries // (MergeJob.error), not from YAML front matter (story 929). let crdt_item = crate::crdt_state::read_item(story_id); - let name = crdt_item - .as_ref() - .and_then(|w| w.name()) - .unwrap_or("(unnamed)"); + let name = crdt_item.as_ref().map(|w| w.name()).unwrap_or("(unnamed)"); let mut out = String::new(); @@ -377,7 +374,7 @@ mod tests { "2_current", "99_story_criteria_test.md", "---\nname: Criteria Test\n---\n\n- [ ] First thing\n- [x] Done thing\n- [ ] Second thing\n", - None, + Some("Criteria Test"), ); let output = status_triage_cmd(tmp.path(), "99").unwrap(); assert!( @@ -403,7 +400,7 @@ mod tests { "2_current", "55_story_blocked_story.md", "---\nname: Blocked Story\nblocked: true\n---\n", - None, + Some("Blocked Story"), ); let output = status_triage_cmd(tmp.path(), "55").unwrap(); assert!( @@ -460,7 +457,7 @@ mod tests { "2_current", "77_story_no_worktree.md", "---\nname: No Worktree\n---\n", - None, + Some("No Worktree"), ); let output = status_triage_cmd(tmp.path(), "77").unwrap(); // Branch name should still appear diff --git a/server/src/chat/commands/unblock.rs b/server/src/chat/commands/unblock.rs index 8b801b32..0ad04796 100644 --- a/server/src/chat/commands/unblock.rs +++ b/server/src/chat/commands/unblock.rs @@ -55,7 +55,7 @@ fn unblock_by_story_id(story_id: &str) -> String { let crdt_item = crate::crdt_state::read_item(story_id); let story_name = crdt_item .as_ref() - .and_then(|i| i.name().map(str::to_string)) + .map(|i| i.name().to_string()) .unwrap_or_else(|| story_id.to_string()); // Story 945: `Stage::Blocked` / `Stage::MergeFailure` are the single diff --git a/server/src/chat/commands/unreleased.rs b/server/src/chat/commands/unreleased.rs index 7c62d7eb..ddd471ac 100644 --- a/server/src/chat/commands/unreleased.rs +++ b/server/src/chat/commands/unreleased.rs @@ -146,7 +146,7 @@ fn find_story_name(_root: &std::path::Path, num_str: &str) -> Option { let items = crate::crdt_state::read_all_items()?; for item in items { if item.story_id().split('_').next().unwrap_or("") == num_str { - return item.name().map(str::to_string); + return Some(item.name().to_string()); } } None diff --git a/server/src/chat/transport/matrix/assign.rs b/server/src/chat/transport/matrix/assign.rs index 114958ed..4b69ad49 100644 --- a/server/src/chat/transport/matrix/assign.rs +++ b/server/src/chat/transport/matrix/assign.rs @@ -103,7 +103,7 @@ pub async fn handle_assign( // Story name comes from the CRDT name register (story 929). let story_name = crate::crdt_state::read_item(&story_id) - .and_then(|w| w.name().map(str::to_string)) + .map(|w| w.name().to_string()) .unwrap_or_else(|| story_id.clone()); let agent_name = resolve_agent_name(model_str); diff --git a/server/src/chat/transport/matrix/delete.rs b/server/src/chat/transport/matrix/delete.rs index c1e1b583..619162b9 100644 --- a/server/src/chat/transport/matrix/delete.rs +++ b/server/src/chat/transport/matrix/delete.rs @@ -71,7 +71,7 @@ pub async fn handle_delete( // Story name comes from the CRDT name register (story 929). let story_name = crate::crdt_state::read_item(&story_id) - .and_then(|w| w.name().map(str::to_string)) + .map(|w| w.name().to_string()) .unwrap_or_else(|| story_id.clone()); let outcome = match crate::service::work_item::delete::delete_work_item( diff --git a/server/src/chat/transport/matrix/start.rs b/server/src/chat/transport/matrix/start.rs index 068a2041..4c13ba64 100644 --- a/server/src/chat/transport/matrix/start.rs +++ b/server/src/chat/transport/matrix/start.rs @@ -90,7 +90,7 @@ pub async fn handle_start( // Story name comes from the CRDT name register (story 929). let story_name = crate::crdt_state::read_item(&story_id) - .and_then(|w| w.name().map(str::to_string)) + .map(|w| w.name().to_string()) .unwrap_or_else(|| story_id.clone()); // Resolve agent name: try "coder-{hint}" first, then the hint as-is. diff --git a/server/src/crdt_state/mod.rs b/server/src/crdt_state/mod.rs index 380ae4ef..f9d6a607 100644 --- a/server/src/crdt_state/mod.rs +++ b/server/src/crdt_state/mod.rs @@ -46,8 +46,8 @@ pub use read::{ }; pub use state::{init, subscribe}; pub use types::{ - ActiveAgentCrdt, ActiveAgentView, AgentThrottleCrdt, AgentThrottleView, CrdtEvent, - GatewayConfigCrdt, GatewayProjectCrdt, GatewayProjectView, MergeJobCrdt, MergeJobView, + ActiveAgentCrdt, ActiveAgentView, AgentThrottleCrdt, AgentThrottleView, Claim, CrdtEvent, + EpicId, GatewayConfigCrdt, GatewayProjectCrdt, GatewayProjectView, MergeJobCrdt, MergeJobView, NodePresenceCrdt, NodePresenceView, PipelineDoc, PipelineItemCrdt, PipelineItemView, TestJobCrdt, TestJobView, TokenUsageCrdt, TokenUsageView, WorkItem, }; diff --git a/server/src/crdt_state/presence.rs b/server/src/crdt_state/presence.rs index b9671103..77b8551a 100644 --- a/server/src/crdt_state/presence.rs +++ b/server/src/crdt_state/presence.rs @@ -110,7 +110,7 @@ pub fn is_claimed_by_us(story_id: &str) -> bool { let Some(item) = read_item(story_id) else { return false; }; - item.claimed_by.as_deref() == Some(&node_id) + item.claim().is_some_and(|c| c.node == node_id) } /// Write or update a node presence entry in the CRDT. diff --git a/server/src/crdt_state/read.rs b/server/src/crdt_state/read.rs index 51d0e08d..578e4309 100644 --- a/server/src/crdt_state/read.rs +++ b/server/src/crdt_state/read.rs @@ -292,9 +292,12 @@ pub fn evict_item(story_id: &str) -> Result<(), String> { /// /// Projects the loose CRDT `stage` register into a typed /// [`crate::pipeline_state::Stage`]. Items with an unknown or missing stage -/// string are filtered out (`None`), so every `WorkItem` that escapes the -/// read path carries a valid typed stage. +/// string, or with no name set, are filtered out (`None`) — a nameless item +/// is treated as malformed and never surfaces to callers. pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option { + use super::types::{Claim, EpicId}; + use crate::io::story_metadata::{ItemType, QaMode}; + let story_id = match item.story_id.view() { JsonValue::String(s) if !s.is_empty() => s, _ => return None, @@ -303,48 +306,58 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option s, _ => return None, }; + // AC 5: nameless item = malformed; filter it out. let name = match item.name.view() { - JsonValue::String(s) if !s.is_empty() => Some(s), - _ => None, + JsonValue::String(s) if !s.is_empty() => s, + _ => return None, }; let agent = match item.agent.view() { JsonValue::String(s) if !s.is_empty() => Some(s), _ => None, }; let retry_count = match item.retry_count.view() { - JsonValue::Number(n) => Some(n as i64), - _ => None, + JsonValue::Number(n) if n >= 0.0 => n as u32, + _ => 0u32, }; let depends_on = match item.depends_on.view() { - JsonValue::String(s) if !s.is_empty() => serde_json::from_str::>(&s).ok(), - _ => None, + JsonValue::String(s) if !s.is_empty() => { + serde_json::from_str::>(&s).unwrap_or_default() + } + _ => Vec::new(), }; let claimed_by = match item.claimed_by.view() { JsonValue::String(s) if !s.is_empty() => Some(s), _ => None, }; - let claimed_at = match item.claimed_at.view() { - JsonValue::Number(n) if n > 0.0 => Some(n), + let claimed_at_secs = match item.claimed_at.view() { + JsonValue::Number(n) if n > 0.0 => Some(n as u64), _ => None, }; - let merged_at = match item.merged_at.view() { + let claim = match (claimed_by, claimed_at_secs) { + (Some(node), Some(at)) => Some(Claim { node, at }), + _ => None, + }; + + // `merged_at` is read only to project into `Stage::Done`; it is not + // stored on `WorkItem` (callers access it via `Stage::Done { merged_at }`). + let merged_at_float = match item.merged_at.view() { JsonValue::Number(n) if n > 0.0 => Some(n), _ => None, }; let qa_mode = match item.qa_mode.view() { - JsonValue::String(s) if !s.is_empty() => Some(s), + JsonValue::String(s) if !s.is_empty() => QaMode::from_str(&s), _ => None, }; let item_type = match item.item_type.view() { - JsonValue::String(s) if !s.is_empty() => Some(s), + JsonValue::String(s) if !s.is_empty() => ItemType::from_str(&s), _ => None, }; let epic = match item.epic.view() { - JsonValue::String(s) if !s.is_empty() => Some(s), + JsonValue::String(s) if !s.is_empty() => EpicId::from_crdt_str(&s), _ => None, }; @@ -353,7 +366,8 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option None, }; - let stage = project_stage_for_view(&stage_str, &story_id, merged_at, resume_to.as_deref())?; + let stage = + project_stage_for_view(&stage_str, &story_id, merged_at_float, resume_to.as_deref())?; Some(PipelineItemView { story_id, @@ -362,9 +376,7 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option Option { + // Try pure-numeric parse first. + if let Ok(n) = s.parse::() { + return Some(Self(n)); + } + // Fall back: take the leading numeric prefix before the first `_`. + s.split('_').next()?.parse::().ok().map(Self) + } +} + +impl std::fmt::Display for EpicId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + /// A typed snapshot of a single pipeline work item derived from the CRDT document. /// /// Access fields exclusively through the typed accessor methods — raw field access is @@ -130,22 +171,24 @@ pub struct NodePresenceCrdt { pub struct WorkItem { pub(super) story_id: String, pub(super) stage: crate::pipeline_state::Stage, - pub(super) name: Option, + /// Human-readable name. `extract_item_view` returns `None` (filtering the item + /// out of all read paths) when this register is empty — a nameless item is + /// treated as malformed, not surfaced with an empty string. + pub(super) name: String, pub(super) agent: Option, - pub(super) retry_count: Option, - pub(super) depends_on: Option>, - /// Node ID of the node that claimed this item (hex-encoded Ed25519 pubkey). - pub(super) claimed_by: Option, - /// Unix timestamp (seconds) when the claim was written. - pub(super) claimed_at: Option, - /// Unix timestamp (seconds) when the item was merged to master. - pub(super) merged_at: Option, - /// QA mode override: `"server"`, `"agent"`, or `"human"`. - pub(super) qa_mode: Option, - /// Item type (sub-story 933): `"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`. - pub(super) item_type: Option, - /// Epic ID this item belongs to, or `None` (sub-story 933). - pub(super) epic: Option, + /// Retry counter — `0` when the CRDT register is unset. + pub(super) retry_count: u32, + /// Dependency story numbers — empty `Vec` when the register is unset. + pub(super) depends_on: Vec, + /// Active claim (node + timestamp). `None` when the item is unclaimed or + /// when only one of the two companion registers is set. + pub(super) claim: Option, + /// QA mode override. `None` means "use the project default". + pub(super) qa_mode: Option, + /// Item type. `None` means "infer from the story_id slug prefix". + pub(super) item_type: Option, + /// Epic this item belongs to. `None` when the item has no parent epic. + pub(super) epic: Option, } impl WorkItem { @@ -159,9 +202,12 @@ impl WorkItem { &self.stage } - /// Human-readable story name, or `None` when unset. - pub fn name(&self) -> Option<&str> { - self.name.as_deref() + /// Human-readable story name. + /// + /// Items without a name never reach callers — `extract_item_view` returns + /// `None` for nameless items so they are filtered out of all read paths. + pub fn name(&self) -> &str { + &self.name } /// Agent name pinned to this item, or `None` when unset. @@ -171,43 +217,32 @@ impl WorkItem { /// Retry counter. Returns `0` when the register is unset. pub fn retry_count(&self) -> u32 { - self.retry_count.unwrap_or(0).max(0) as u32 + self.retry_count } /// Dependency story numbers. Returns an empty slice when unset. pub fn depends_on(&self) -> &[u32] { - self.depends_on.as_deref().unwrap_or(&[]) + &self.depends_on } - /// Node ID of the current claim holder, or `None` when unclaimed. - pub fn claimed_by(&self) -> Option<&str> { - self.claimed_by.as_deref() + /// Active claim on this item, or `None` when unclaimed. + pub fn claim(&self) -> Option<&Claim> { + self.claim.as_ref() } - /// Unix timestamp (seconds) when the current claim was written, or `None`. - pub fn claimed_at(&self) -> Option { - self.claimed_at + /// QA mode override, or `None` when the register is unset (use project default). + pub fn qa_mode(&self) -> Option { + self.qa_mode } - /// Unix timestamp (seconds) when the item was merged to master, or `None`. - pub fn merged_at(&self) -> Option { - self.merged_at + /// Item type, or `None` when the register is unset (infer from story_id prefix). + pub fn item_type(&self) -> Option { + self.item_type } - /// QA mode override (`"server"`, `"agent"`, or `"human"`), or `None` when unset. - pub fn qa_mode(&self) -> Option<&str> { - self.qa_mode.as_deref() - } - - /// Item type (`"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`), or - /// `None` when the register is unset. - pub fn item_type(&self) -> Option<&str> { - self.item_type.as_deref() - } - - /// Epic ID this item is a member of, or `None` when unset. - pub fn epic(&self) -> Option<&str> { - self.epic.as_deref() + /// Epic this item belongs to, or `None` when unset. + pub fn epic(&self) -> Option { + self.epic } /// Construct a `WorkItem` for use in tests outside `crdt_state::*`. @@ -219,27 +254,23 @@ impl WorkItem { pub fn for_test( story_id: impl Into, stage: crate::pipeline_state::Stage, - name: Option, + name: impl Into, agent: Option, - retry_count: Option, - depends_on: Option>, - claimed_by: Option, - claimed_at: Option, - merged_at: Option, - qa_mode: Option, - item_type: Option, - epic: Option, + retry_count: u32, + depends_on: Vec, + claim: Option, + qa_mode: Option, + item_type: Option, + epic: Option, ) -> Self { Self { story_id: story_id.into(), stage, - name, + name: name.into(), agent, retry_count, depends_on, - claimed_by, - claimed_at, - merged_at, + claim, qa_mode, item_type, epic, diff --git a/server/src/crdt_state/write/item.rs b/server/src/crdt_state/write/item.rs index eea5c1e9..25463b0a 100644 --- a/server/src/crdt_state/write/item.rs +++ b/server/src/crdt_state/write/item.rs @@ -41,12 +41,15 @@ pub fn set_depends_on(story_id: &str, deps: &[u32]) -> bool { /// Set the `item_type` CRDT register for a pipeline item (sub-story 933). /// -/// `Some(t)` writes the type string (e.g. `"story"`, `"epic"`, `"bug"`). +/// `Some(t)` writes the canonical type string (e.g. `"story"`, `"epic"`, `"bug"`). /// `None` clears the register to an empty string, which means "use the /// id-prefix heuristic" (see `item_type_from_id`). /// /// Returns `true` if the item was found and the op was applied, `false` otherwise. -pub fn set_item_type(story_id: &str, item_type: Option<&str>) -> bool { +pub fn set_item_type( + story_id: &str, + item_type: Option, +) -> bool { let Some(state_mutex) = get_crdt() else { return false; }; @@ -56,18 +59,21 @@ pub fn set_item_type(story_id: &str, item_type: Option<&str>) -> bool { let Some(&idx) = state.index.get(story_id) else { return false; }; - let value = item_type.unwrap_or("").to_string(); + let value = item_type + .map(|t| t.as_str().to_string()) + .unwrap_or_default(); apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].item_type.set(value)); true } /// Set the `epic` CRDT register for a pipeline item (sub-story 933). /// -/// `Some(epic_id)` links the item to its parent epic. +/// `Some(id)` links the item to its parent epic (stored as the numeric string, +/// e.g. `"9990"` for `EpicId(9990)`). /// `None` clears the register to an empty string (no epic membership). /// /// Returns `true` if the item was found and the op was applied, `false` otherwise. -pub fn set_epic(story_id: &str, epic_id: Option<&str>) -> bool { +pub fn set_epic(story_id: &str, epic_id: Option) -> bool { let Some(state_mutex) = get_crdt() else { return false; }; @@ -77,7 +83,7 @@ pub fn set_epic(story_id: &str, epic_id: Option<&str>) -> bool { let Some(&idx) = state.index.get(story_id) else { return false; }; - let value = epic_id.unwrap_or("").to_string(); + let value = epic_id.map(|e| e.to_string()).unwrap_or_default(); apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].epic.set(value)); true } diff --git a/server/src/crdt_state/write/tests.rs b/server/src/crdt_state/write/tests.rs index 6f4d2a20..50bddc65 100644 --- a/server/src/crdt_state/write/tests.rs +++ b/server/src/crdt_state/write/tests.rs @@ -154,7 +154,18 @@ fn migrate_story_ids_to_numeric_skips_conflict() { write_item_str( "44_story_foo", "1_backlog", - None, + Some("Foo slug"), + None, + None, + None, + None, + None, + None, + ); + write_item_str( + "44", + "2_current", + Some("Foo numeric"), None, None, None, @@ -162,7 +173,6 @@ fn migrate_story_ids_to_numeric_skips_conflict() { None, None, ); - write_item_str("44", "2_current", None, None, None, None, None, None, None); let result = migrate_story_ids_to_numeric(); // The slug entry must NOT be migrated because "44" is already occupied. @@ -202,7 +212,7 @@ fn migrate_story_ids_to_numeric_preserves_stage_and_name() { let item = read_item("45").expect("item must be accessible by numeric ID"); assert!(matches!(item.stage, crate::pipeline_state::Stage::Coding)); - assert_eq!(item.name.as_deref(), Some("Crash Bug")); + assert_eq!(item.name, "Crash Bug"); assert_eq!(item.agent.as_deref(), Some("coder-1")); } @@ -223,20 +233,18 @@ fn migrate_names_from_slugs_fills_empty_names() { None, ); - // Before migration the name should be empty. - let before = read_item("42_story_my_feature").unwrap(); + // Before migration: nameless item is filtered by read_item (AC 5). assert!( - before.name.as_deref().unwrap_or("").is_empty(), - "name should be empty before migration" + read_item("42_story_my_feature").is_none(), + "nameless item must not be returned by read_item before migration" ); migrate_names_from_slugs(); - // After migration the name should be derived from the slug. + // After migration the item has a name and is visible to read_item. let after = read_item("42_story_my_feature").unwrap(); assert_eq!( - after.name.as_deref(), - Some("My feature"), + after.name, "My feature", "name should be derived from slug after migration" ); } @@ -261,8 +269,7 @@ fn migrate_names_from_slugs_leaves_existing_names_unchanged() { let after = read_item("43_story_named_item").unwrap(); assert_eq!( - after.name.as_deref(), - Some("Already Named"), + after.name, "Already Named", "pre-existing name must not be overwritten" ); } @@ -300,7 +307,7 @@ fn set_depends_on_round_trip_and_clear() { let view = read_item("872_test_target").unwrap(); assert_eq!( view.depends_on, - Some(vec![837]), + vec![837u32], "CRDT register should hold [837]" ); @@ -308,8 +315,8 @@ fn set_depends_on_round_trip_and_clear() { let ok = set_depends_on("872_test_target", &[]); assert!(ok, "set_depends_on([]) should return true"); let view = read_item("872_test_target").unwrap(); - assert_eq!( - view.depends_on, None, + assert!( + view.depends_on.is_empty(), "clearing should leave register unset" ); @@ -412,7 +419,7 @@ fn set_qa_mode_round_trip_server_then_human() { write_item_str( "869_story_qa_roundtrip", "1_backlog", - None, + Some("Qa Roundtrip"), None, None, None, @@ -426,9 +433,9 @@ fn set_qa_mode_round_trip_server_then_human() { assert!(ok, "set_qa_mode should return true for known item"); let view = read_item("869_story_qa_roundtrip").unwrap(); assert_eq!( - view.qa_mode.as_deref(), - Some("server"), - "CRDT register should hold \"server\"" + view.qa_mode, + Some(QaMode::Server), + "CRDT register should hold Server" ); // Set qa=human via typed path and assert CRDT register is updated. @@ -436,9 +443,9 @@ fn set_qa_mode_round_trip_server_then_human() { assert!(ok, "set_qa_mode should return true for known item"); let view = read_item("869_story_qa_roundtrip").unwrap(); assert_eq!( - view.qa_mode.as_deref(), - Some("human"), - "CRDT register should hold \"human\"" + view.qa_mode, + Some(QaMode::Human), + "CRDT register should hold Human" ); // Clear via None — register goes back to unset. @@ -467,7 +474,7 @@ fn bump_retry_count_increments_by_one() { write_item_str( "9001_story_bump_test", "2_current", - None, + Some("Bump Test"), None, None, None, @@ -483,11 +490,7 @@ fn bump_retry_count_increments_by_one() { assert_eq!(v2, 2, "second bump should return 2"); let item = read_item("9001_story_bump_test").expect("item must exist"); - assert_eq!( - item.retry_count, - Some(2), - "CRDT must reflect final bump value" - ); + assert_eq!(item.retry_count, 2u32, "CRDT must reflect final bump value"); } #[test] @@ -496,7 +499,7 @@ fn set_retry_count_resets_to_zero() { write_item_str( "9002_story_set_test", "2_current", - None, + Some("Set Test"), None, Some(5), None, @@ -508,11 +511,7 @@ fn set_retry_count_resets_to_zero() { set_retry_count("9002_story_set_test", 0); let item = read_item("9002_story_set_test").expect("item must exist"); - assert_eq!( - item.retry_count, - Some(0), - "set_retry_count(0) must reset to 0" - ); + assert_eq!(item.retry_count, 0u32, "set_retry_count(0) must reset to 0"); } #[test] diff --git a/server/src/db/mod.rs b/server/src/db/mod.rs index 74e7c600..9093c8e9 100644 --- a/server/src/db/mod.rs +++ b/server/src/db/mod.rs @@ -321,7 +321,7 @@ mod tests { let view = crate::crdt_state::read_item(story_id).expect("story exists in CRDT"); assert_eq!(view.stage().dir_name(), "coding"); - assert_eq!(view.name(), Some("Typed Name")); + assert_eq!(view.name(), "Typed Name"); assert_eq!(view.agent(), Some("coder-1")); assert_eq!(view.retry_count(), 2); assert_eq!(view.depends_on(), &[100, 200]); @@ -343,17 +343,10 @@ mod tests { let content = "---\nname: Should Not Appear\nagent: ghost\n---\n# Body\n"; write_item_with_content(story_id, "2_current", content, ItemMeta::default()); - let view = crate::crdt_state::read_item(story_id).expect("story exists in CRDT"); - assert_eq!(view.stage().dir_name(), "coding"); - assert_eq!( - view.name(), - None, - "name must come from typed meta, not parsed YAML" - ); - assert_eq!( - view.agent(), - None, - "agent must come from typed meta, not parsed YAML" + // Nameless items are filtered out by read_item (AC 5: nameless = malformed). + assert!( + crate::crdt_state::read_item(story_id).is_none(), + "name must come from typed meta, not parsed YAML — nameless items must not be surfaced" ); } diff --git a/server/src/db/ops.rs b/server/src/db/ops.rs index 545af3b5..8f614c1c 100644 --- a/server/src/db/ops.rs +++ b/server/src/db/ops.rs @@ -170,7 +170,7 @@ pub fn move_item_stage( // mirror stays in sync. Always reset retry_count to 0 on stage transition. if let Some(db) = PIPELINE_DB.get() { let view = crate::crdt_state::read_item(story_id); - let name = view.as_ref().and_then(|v| v.name().map(str::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 depends_on = view .as_ref() diff --git a/server/src/http/mcp/diagnostics/permission.rs b/server/src/http/mcp/diagnostics/permission.rs index 0560ea79..d44d2dd2 100644 --- a/server/src/http/mcp/diagnostics/permission.rs +++ b/server/src/http/mcp/diagnostics/permission.rs @@ -453,7 +453,7 @@ mod tests { "5_story_test", "1_backlog", content, - crate::db::ItemMeta::default(), + crate::db::ItemMeta::named("Test"), ); let ctx = test_ctx(root); @@ -485,7 +485,7 @@ mod tests { "6_story_back", "2_current", content, - crate::db::ItemMeta::default(), + crate::db::ItemMeta::named("Back"), ); let ctx = test_ctx(root); @@ -517,7 +517,7 @@ mod tests { "9907_story_idem", "2_current", content, - crate::db::ItemMeta::default(), + crate::db::ItemMeta::named("Idem"), ); let ctx = test_ctx(root); diff --git a/server/src/http/mcp/status_tools.rs b/server/src/http/mcp/status_tools.rs index e46f918e..d8d9b23d 100644 --- a/server/src/http/mcp/status_tools.rs +++ b/server/src/http/mcp/status_tools.rs @@ -177,14 +177,12 @@ pub(super) async fn tool_status(args: &Value, ctx: &AppContext) -> Result 0 { @@ -194,15 +192,9 @@ pub(super) async fn tool_status(args: &Value, ctx: &AppContext) -> Result 0.0 - { - front_matter.insert("claimed_at".to_string(), json!(ca)); + if let Some(claim) = view.claim() { + front_matter.insert("claimed_by".to_string(), json!(claim.node)); + front_matter.insert("claimed_at".to_string(), json!(claim.at)); } } diff --git a/server/src/http/mcp/story_tools/bug.rs b/server/src/http/mcp/story_tools/bug.rs index 8e313ebc..eb59f0fa 100644 --- a/server/src/http/mcp/story_tools/bug.rs +++ b/server/src/http/mcp/story_tools/bug.rs @@ -464,7 +464,7 @@ mod tests { "9901_bug_crash", "1_backlog", content, - crate::db::ItemMeta::default(), + crate::db::ItemMeta::named("Crash"), ); // Stage the file so it's tracked std::process::Command::new("git") diff --git a/server/src/http/mcp/story_tools/criteria.rs b/server/src/http/mcp/story_tools/criteria.rs index d0c8bf14..79a15d49 100644 --- a/server/src/http/mcp/story_tools/criteria.rs +++ b/server/src/http/mcp/story_tools/criteria.rs @@ -37,8 +37,7 @@ pub(crate) fn tool_get_story_todos(args: &Value, ctx: &AppContext) -> Result Result { continue; }; - if view.item_type() == Some("epic") { + use crate::io::story_metadata::ItemType; + if view.item_type() == Some(ItemType::Epic) { epics.push((sid.clone(), item.name.clone())); } @@ -110,13 +111,17 @@ pub(crate) fn tool_show_epic(args: &Value, _ctx: &AppContext) -> Result = Vec::new(); @@ -126,7 +131,7 @@ pub(crate) fn tool_show_epic(args: &Value, _ctx: &AppContext) -> Result = serde_json::from_str(&result).unwrap(); let epic = parsed .iter() - .find(|e| e["epic_id"] == "9990_epic_rollup") + .find(|e| e["epic_id"] == "9990") .expect("expected rollup epic in list"); assert_eq!(epic["members_total"], 2, "two members expected"); assert_eq!(epic["members_done"], 1, "one done member expected"); diff --git a/server/src/http/mcp/story_tools/story/delete.rs b/server/src/http/mcp/story_tools/story/delete.rs index efe213d7..7ccc9ae8 100644 --- a/server/src/http/mcp/story_tools/story/delete.rs +++ b/server/src/http/mcp/story_tools/story/delete.rs @@ -230,7 +230,7 @@ mod tests { "51_story_no_branch", "2_current", content, - crate::db::ItemMeta::default(), + crate::db::ItemMeta::named("No Branch"), ); let ctx = test_ctx(tmp.path()); diff --git a/server/src/http/mcp/story_tools/story/query.rs b/server/src/http/mcp/story_tools/story/query.rs index c32403e9..4bf35c58 100644 --- a/server/src/http/mcp/story_tools/story/query.rs +++ b/server/src/http/mcp/story_tools/story/query.rs @@ -246,6 +246,9 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); crate::db::ensure_content_store(); + // Story 946 AC 5: nameless items are invisible at the CRDT layer. + // `extract_item_view` returns `None` for items with no name register set, + // so they are filtered from all read paths including `validate_story_dirs`. crate::db::write_item_with_content( "9908_test", "2_current", @@ -256,10 +259,9 @@ mod tests { let ctx = test_ctx(tmp.path()); let result = tool_validate_stories(&ctx).unwrap(); let parsed: Vec = serde_json::from_str(&result).unwrap(); - let item = parsed - .iter() - .find(|v| v["story_id"] == "9908_test") - .expect("expected 9908_test in validation results"); - assert_eq!(item["valid"], false); + assert!( + parsed.iter().all(|v| v["story_id"] != "9908_test"), + "nameless items must be invisible to tool_validate_stories" + ); } } diff --git a/server/src/http/mcp/story_tools/story/update.rs b/server/src/http/mcp/story_tools/story/update.rs index ad0f1a12..a06bf09c 100644 --- a/server/src/http/mcp/story_tools/story/update.rs +++ b/server/src/http/mcp/story_tools/story/update.rs @@ -24,7 +24,7 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result Result { - let s = value.as_str().filter(|s| !s.is_empty()); - crate::crdt_state::set_epic(story_id, s); + let parsed = value + .as_str() + .and_then(crate::crdt_state::EpicId::from_crdt_str); + crate::crdt_state::set_epic(story_id, parsed); } "type" => { - let s = value.as_str().filter(|s| !s.is_empty()); - crate::crdt_state::set_item_type(story_id, s); + let parsed = value + .as_str() + .and_then(crate::io::story_metadata::ItemType::from_str); + crate::crdt_state::set_item_type(story_id, parsed); } "depends_on" => { if let Some(arr) = value.as_array() { diff --git a/server/src/http/workflow/bug_ops/bug.rs b/server/src/http/workflow/bug_ops/bug.rs index 20bd2f4f..c28cd7bb 100644 --- a/server/src/http/workflow/bug_ops/bug.rs +++ b/server/src/http/workflow/bug_ops/bug.rs @@ -77,9 +77,8 @@ pub(super) fn is_bug_item(stem: &str) -> bool { } if after_num.is_empty() { return crate::crdt_state::read_item(stem) - .and_then(|v| v.item_type().map(str::to_string)) - .map(|t| t == "bug") - .unwrap_or(false); + .and_then(|v| v.item_type()) + .is_some_and(|t| t == crate::io::story_metadata::ItemType::Bug); } false } diff --git a/server/src/http/workflow/bug_ops/epic.rs b/server/src/http/workflow/bug_ops/epic.rs index 72eababc..a4255d4f 100644 --- a/server/src/http/workflow/bug_ops/epic.rs +++ b/server/src/http/workflow/bug_ops/epic.rs @@ -73,7 +73,7 @@ pub fn create_epic_file( write_story_content(root, &epic_id, "1_backlog", &content, Some(name)); // Story 933: typed CRDT register for item_type. - crate::crdt_state::set_item_type(&epic_id, Some("epic")); + crate::crdt_state::set_item_type(&epic_id, Some(crate::io::story_metadata::ItemType::Epic)); Ok(epic_id) } diff --git a/server/src/http/workflow/bug_ops/refactor.rs b/server/src/http/workflow/bug_ops/refactor.rs index 0269ea8c..4655d93e 100644 --- a/server/src/http/workflow/bug_ops/refactor.rs +++ b/server/src/http/workflow/bug_ops/refactor.rs @@ -71,9 +71,8 @@ pub(super) fn is_refactor_item(stem: &str) -> bool { } if after_num.is_empty() { return crate::crdt_state::read_item(stem) - .and_then(|v| v.item_type().map(str::to_string)) - .map(|t| t == "refactor") - .unwrap_or(false); + .and_then(|v| v.item_type()) + .is_some_and(|t| t == crate::io::story_metadata::ItemType::Refactor); } false } diff --git a/server/src/http/workflow/bug_ops/tests.rs b/server/src/http/workflow/bug_ops/tests.rs index c96d8bef..832c9d29 100644 --- a/server/src/http/workflow/bug_ops/tests.rs +++ b/server/src/http/workflow/bug_ops/tests.rs @@ -362,7 +362,7 @@ fn create_spike_file_with_special_chars_in_name_produces_valid_yaml() { let spike_id = result.unwrap(); let view = crate::crdt_state::read_item(&spike_id).expect("CRDT entry should exist"); - assert_eq!(view.name(), Some(name)); + assert_eq!(view.name(), name); } #[test] diff --git a/server/src/http/workflow/pipeline.rs b/server/src/http/workflow/pipeline.rs index e3006a29..ff74f4ee 100644 --- a/server/src/http/workflow/pipeline.rs +++ b/server/src/http/workflow/pipeline.rs @@ -103,8 +103,10 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { } else { None }; - let qa = view.as_ref().and_then(|v| v.qa_mode().map(str::to_string)); - let epic_id = view.as_ref().and_then(|v| v.epic().map(str::to_string)); + let qa = view + .as_ref() + .and_then(|v| v.qa_mode().map(|q| q.as_str().to_string())); + let epic_id = view.as_ref().and_then(|v| v.epic().map(|e| e.to_string())); let merge_failure = crate::crdt_state::read_merge_job(sid).and_then(|j| j.error); let story = UpcomingStory { @@ -219,7 +221,7 @@ pub fn load_upcoming_stories(_ctx: &AppContext) -> Result, St .map(|item| { let sid = &item.story_id.0; let epic_id = - crate::crdt_state::read_item(sid).and_then(|v| v.epic().map(str::to_string)); + crate::crdt_state::read_item(sid).and_then(|v| v.epic().map(|e| e.to_string())); UpcomingStory { story_id: item.story_id.0.clone(), name: if item.name.is_empty() { @@ -265,34 +267,25 @@ pub fn load_upcoming_stories(_ctx: &AppContext) -> Result, St /// /// Story 929: validation reads the typed CRDT `name` register; the legacy YAML /// front-matter parse is gone. +/// +/// Story 946: nameless items are filtered at the CRDT layer (`extract_item_view` +/// returns `None` for items with no name register set) and therefore never reach +/// this function. Every item in `read_all_typed()` is guaranteed to have a +/// non-empty name, so the only validation left here is stage filtering. pub fn validate_story_dirs(_root: &Path) -> Result, String> { use crate::pipeline_state::Stage; let mut results = Vec::new(); - let typed_items = crate::pipeline_state::read_all_typed(); - for item in typed_items { + for item in crate::pipeline_state::read_all_typed() { if !matches!(item.stage, Stage::Backlog | Stage::Coding) { continue; } - let story_id = item.story_id.0.clone(); - let name = crate::crdt_state::read_item(&story_id) - .and_then(|v| v.name().map(str::to_string)) - .filter(|s| !s.is_empty()); - - if name.is_some() { - results.push(StoryValidationResult { - story_id, - valid: true, - error: None, - }); - } else { - results.push(StoryValidationResult { - story_id, - valid: false, - error: Some("Missing 'name' field".to_string()), - }); - } + results.push(StoryValidationResult { + story_id: item.story_id.0.clone(), + valid: true, + error: None, + }); } results.sort_by(|a, b| a.story_id.cmp(&b.story_id)); @@ -591,12 +584,13 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let results = validate_story_dirs(tmp.path()).unwrap(); - let r = results - .iter() - .find(|r| r.story_id == "9875_story_no_fm") - .unwrap(); - assert!(!r.valid); - assert_eq!(r.error.as_deref(), Some("Missing 'name' field")); + // Story 946: nameless items are invisible at the CRDT layer (AC 5). + // `extract_item_view` returns `None` for items with no name register, + // so they never surface to `validate_story_dirs`. + assert!( + results.iter().all(|r| r.story_id != "9875_story_no_fm"), + "nameless items must be invisible to validate_story_dirs" + ); } #[test] @@ -611,13 +605,11 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let results = validate_story_dirs(tmp.path()).unwrap(); - let r = results - .iter() - .find(|r| r.story_id == "9876_story_no_name") - .unwrap(); - assert!(!r.valid); - let err = r.error.as_deref().unwrap(); - assert!(err.contains("Missing 'name' field")); + // Story 946: nameless items are invisible at the CRDT layer (AC 5). + assert!( + results.iter().all(|r| r.story_id != "9876_story_no_name"), + "nameless items must be invisible to validate_story_dirs" + ); } #[test] diff --git a/server/src/http/workflow/story_ops/create.rs b/server/src/http/workflow/story_ops/create.rs index 720fb8de..64ea78ff 100644 --- a/server/src/http/workflow/story_ops/create.rs +++ b/server/src/http/workflow/story_ops/create.rs @@ -185,7 +185,7 @@ mod tests { let story_id = result.unwrap(); let view = crate::crdt_state::read_item(&story_id).expect("CRDT entry should exist after create"); - assert_eq!(view.name(), Some(name)); + assert_eq!(view.name(), name); } // ── check_criterion_in_file tests ───────────────────────────────────────── @@ -242,7 +242,7 @@ mod tests { let view = crate::crdt_state::read_item(&story_id).expect("CRDT entry must exist"); assert_eq!( view.item_type(), - Some("story"), + Some(crate::io::story_metadata::ItemType::Story), "CRDT register must be set to story" ); } diff --git a/server/src/http/workflow/story_ops/criterion.rs b/server/src/http/workflow/story_ops/criterion.rs index aadf27d0..7a861c69 100644 --- a/server/src/http/workflow/story_ops/criterion.rs +++ b/server/src/http/workflow/story_ops/criterion.rs @@ -435,7 +435,10 @@ mod tests { setup_story_in_fs(tmp.path(), "100_spike_my_spike", spike_content); // Convert spike to story by updating the typed item_type CRDT register. - crate::crdt_state::set_item_type("100_spike_my_spike", Some("story")); + crate::crdt_state::set_item_type( + "100_spike_my_spike", + Some(crate::io::story_metadata::ItemType::Story), + ); // Add three acceptance criteria. add_criterion_to_file(tmp.path(), "100_spike_my_spike", "First criterion") diff --git a/server/src/http/workflow/utils.rs b/server/src/http/workflow/utils.rs index e87cb07b..e45ef634 100644 --- a/server/src/http/workflow/utils.rs +++ b/server/src/http/workflow/utils.rs @@ -266,7 +266,10 @@ pub(crate) fn create_item_in_backlog( write_story_content(root, &item_id, "1_backlog", &content, Some(name)); crate::crdt_state::set_depends_on(&item_id, depends_on.unwrap_or(&[])); - crate::crdt_state::set_item_type(&item_id, Some(item_type)); + crate::crdt_state::set_item_type( + &item_id, + crate::io::story_metadata::ItemType::from_str(item_type), + ); Ok(item_id) } diff --git a/server/src/io/story_metadata/mod.rs b/server/src/io/story_metadata/mod.rs index 9f71e14a..3f44864f 100644 --- a/server/src/io/story_metadata/mod.rs +++ b/server/src/io/story_metadata/mod.rs @@ -2,7 +2,7 @@ //! //! Story 865 stripped YAML front matter from the content store; this module //! no longer parses or writes YAML. What remains: -//! - `types` — `QaMode` enum. +//! - `types` — `QaMode` and `ItemType` enums. //! - `parser` — `parse_unchecked_todos`, `resolve_qa_mode`, `is_story_frozen_in_store`. //! - `deps` — dependency satisfaction checks (CRDT-backed). @@ -11,4 +11,4 @@ mod parser; mod types; pub use parser::{is_story_frozen_in_store, parse_unchecked_todos, resolve_qa_mode}; -pub use types::QaMode; +pub use types::{ItemType, QaMode}; diff --git a/server/src/io/story_metadata/parser.rs b/server/src/io/story_metadata/parser.rs index 7c66010a..e9bffa86 100644 --- a/server/src/io/story_metadata/parser.rs +++ b/server/src/io/story_metadata/parser.rs @@ -23,9 +23,7 @@ pub fn parse_unchecked_todos(contents: &str) -> Vec { /// spikes themselves. pub fn resolve_qa_mode(story_id: &str, default: QaMode) -> QaMode { crate::crdt_state::read_item(story_id) - .and_then(|view| view.qa_mode().map(str::to_string)) - .as_deref() - .and_then(QaMode::from_str) + .and_then(|view| view.qa_mode()) .unwrap_or(default) } diff --git a/server/src/io/story_metadata/types.rs b/server/src/io/story_metadata/types.rs index 66115ad4..3dc4f194 100644 --- a/server/src/io/story_metadata/types.rs +++ b/server/src/io/story_metadata/types.rs @@ -1,4 +1,4 @@ -//! Core data types for story metadata. +//! Core data types for story metadata: [`QaMode`] and [`ItemType`] enums. /// QA mode for a story: determines how the pipeline handles post-coder review. /// @@ -6,7 +6,7 @@ /// If gates pass, advance straight to merge. /// - `Agent` — spin up a QA agent (Claude session) to review code and run gates. /// - `Human` — hold in QA for human approval after server gates pass. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub enum QaMode { Server, Agent, @@ -39,3 +39,48 @@ impl std::fmt::Display for QaMode { f.write_str(self.as_str()) } } + +/// The type of a pipeline work item. +/// +/// Stored as a typed register in the CRDT (`"story"`, `"bug"`, `"spike"`, +/// `"refactor"`, `"epic"`). `None` in the view means "infer from the +/// story_id slug prefix". +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub enum ItemType { + Story, + Bug, + Spike, + Refactor, + Epic, +} + +impl ItemType { + /// Parse a string into an `ItemType`. Returns `None` for unrecognised values. + pub fn from_str(s: &str) -> Option { + match s.trim().to_lowercase().as_str() { + "story" => Some(Self::Story), + "bug" => Some(Self::Bug), + "spike" => Some(Self::Spike), + "refactor" => Some(Self::Refactor), + "epic" => Some(Self::Epic), + _ => None, + } + } + + /// Return the lowercase string representation of this item type. + pub fn as_str(&self) -> &'static str { + match self { + Self::Story => "story", + Self::Bug => "bug", + Self::Spike => "spike", + Self::Refactor => "refactor", + Self::Epic => "epic", + } + } +} + +impl std::fmt::Display for ItemType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index 66622cde..dd274cd8 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -44,7 +44,7 @@ impl TryFrom<&PipelineItemView> for PipelineItem { fn try_from(view: &PipelineItemView) -> Result { let story_id = StoryId(view.story_id().to_string()); - let name = view.name().unwrap_or("").to_string(); + let name = view.name().to_string(); let depends_on: Vec = view .depends_on() @@ -115,12 +115,10 @@ mod tests { PipelineItemView::for_test( story_id, stage, - name.map(str::to_string), - None, - None, - None, - None, + name.unwrap_or("(unnamed)"), None, + 0u32, + vec![], None, None, None, @@ -140,12 +138,10 @@ mod tests { let view = PipelineItemView::for_test( "42_story_test", Stage::Backlog, - Some("Test Story".to_string()), - None, - None, - Some(vec![10, 20]), - None, + "Test Story", None, + 0u32, + vec![10, 20], None, None, None, @@ -164,12 +160,10 @@ mod tests { let view = PipelineItemView::for_test( "42_story_test", Stage::Coding, - Some("Test".to_string()), + "Test", Some("coder-1".to_string()), - Some(2), - None, - None, - None, + 2u32, + vec![], None, None, None, @@ -225,12 +219,10 @@ mod tests { reason: "migrated from legacy blocked field".to_string(), }, }, - Some("Test".to_string()), - None, - None, - None, - None, + "Test", None, + 0u32, + vec![], None, None, None, @@ -254,12 +246,10 @@ mod tests { archived_at: Utc::now(), reason: ArchiveReason::Completed, }, - Some("Test".to_string()), - None, - None, - None, - None, + "Test", None, + 0u32, + vec![], None, None, None, diff --git a/server/src/service/agents/mod.rs b/server/src/service/agents/mod.rs index 9d51b841..e7161474 100644 --- a/server/src/service/agents/mod.rs +++ b/server/src/service/agents/mod.rs @@ -197,9 +197,7 @@ pub fn get_work_item_content( let filename = format!("{story_id}.md"); let crdt_view = crate::crdt_state::read_item(story_id); - let crdt_name = crdt_view - .as_ref() - .and_then(|v| v.name().map(str::to_string)); + let crdt_name = crdt_view.as_ref().map(|v| v.name().to_string()); let crdt_agent = crdt_view .as_ref() .and_then(|v| v.agent().map(str::to_string)); diff --git a/server/src/service/notifications/io/mod.rs b/server/src/service/notifications/io/mod.rs index 2700f5de..a4a8ff1e 100644 --- a/server/src/service/notifications/io/mod.rs +++ b/server/src/service/notifications/io/mod.rs @@ -20,7 +20,7 @@ mod tests_stage; /// /// Returns `None` if the item is not in the CRDT or has no name set. pub fn read_story_name(_project_root: &Path, _stage: &str, item_id: &str) -> Option { - crate::crdt_state::read_item(item_id).and_then(|v| v.name().map(str::to_string)) + crate::crdt_state::read_item(item_id).map(|v| v.name().to_string()) } /// Look up a story name from the CRDT content store regardless of stage.