huskies: merge 946

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,
 };

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.

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<PipelineItemView> {
+    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<PipelineItemV
         JsonValue::String(s) if !s.is_empty() => 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::<Vec<u32>>(&s).ok(),
-        _ => None,
+        JsonValue::String(s) if !s.is_empty() => {
+            serde_json::from_str::<Vec<u32>>(&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<PipelineItemV
         _ => 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<PipelineItemV
         agent,
         retry_count,
         depends_on,
-        claimed_by,
-        claimed_at,
-        merged_at,
+        claim,
         qa_mode,
         item_type,
         epic,
@@ -571,10 +583,10 @@ mod tests {
         let view = extract_item_view(&crdt.doc.items[0]).unwrap();
         assert_eq!(view.story_id, "40_story_view");
         assert!(matches!(view.stage, crate::pipeline_state::Stage::Qa));
-        assert_eq!(view.name.as_deref(), Some("View Test"));
+        assert_eq!(view.name, "View Test");
         assert_eq!(view.agent.as_deref(), Some("coder-1"));
-        assert_eq!(view.retry_count, Some(2));
-        assert_eq!(view.depends_on, Some(vec![10, 20]));
+        assert_eq!(view.retry_count, 2u32);
+        assert_eq!(view.depends_on, vec![10u32, 20u32]);
     }
 
     #[test]

server/src/crdt_state/state/tests.rs

@@ -205,7 +205,7 @@ async fn init_and_write_read_roundtrip() {
     let view = extract_item_view(&crdt2.doc.items[0]).unwrap();
     assert_eq!(view.story_id, "50_story_roundtrip");
     assert!(matches!(view.stage, crate::pipeline_state::Stage::Backlog));
-    assert_eq!(view.name.as_deref(), Some("Roundtrip"));
+    assert_eq!(view.name, "Roundtrip");
 }
 
 #[test]

server/src/crdt_state/types.rs

@@ -117,6 +117,47 @@ pub struct NodePresenceCrdt {
 
 // ── Read-side view types ─────────────────────────────────────────────
 
+/// Active claim on a pipeline item — node that owns it and when the claim was written.
+///
+/// Both fields must be present for a claim to be valid; a partial claim (node
+/// but no timestamp, or vice versa) is treated as absent by `extract_item_view`.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct Claim {
+    /// Hex-encoded Ed25519 public key of the node that holds the claim.
+    pub node: String,
+    /// Unix timestamp (seconds, integer) when the claim was written.
+    pub at: u64,
+}
+
+/// Numeric identifier for an epic work item.
+///
+/// The numeric prefix of the epic's story_id (e.g. `EpicId(9990)` for the
+/// epic whose CRDT story_id register holds `"9990"`).  Epics are always
+/// created with a pure-numeric story_id by `create_epic_file`.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
+pub struct EpicId(pub u32);
+
+impl EpicId {
+    /// Parse a CRDT epic string (e.g. `"9990"`) into an `EpicId`.
+    ///
+    /// Accepts both pure-numeric strings (`"9990"`) and slug-prefixed strings
+    /// (`"9990_epic_foo"`) by stripping any non-digit suffix.
+    pub fn from_crdt_str(s: &str) -> Option<Self> {
+        // Try pure-numeric parse first.
+        if let Ok(n) = s.parse::<u32>() {
+            return Some(Self(n));
+        }
+        // Fall back: take the leading numeric prefix before the first `_`.
+        s.split('_').next()?.parse::<u32>().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<String>,
+    /// 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<String>,
-    pub(super) retry_count: Option<i64>,
-    pub(super) depends_on: Option<Vec<u32>>,
-    /// Node ID of the node that claimed this item (hex-encoded Ed25519 pubkey).
-    pub(super) claimed_by: Option<String>,
-    /// Unix timestamp (seconds) when the claim was written.
-    pub(super) claimed_at: Option<f64>,
-    /// Unix timestamp (seconds) when the item was merged to master.
-    pub(super) merged_at: Option<f64>,
-    /// QA mode override: `"server"`, `"agent"`, or `"human"`.
-    pub(super) qa_mode: Option<String>,
-    /// Item type (sub-story 933): `"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`.
-    pub(super) item_type: Option<String>,
-    /// Epic ID this item belongs to, or `None` (sub-story 933).
-    pub(super) epic: Option<String>,
+    /// 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<u32>,
+    /// 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<Claim>,
+    /// QA mode override. `None` means "use the project default".
+    pub(super) qa_mode: Option<crate::io::story_metadata::QaMode>,
+    /// Item type. `None` means "infer from the story_id slug prefix".
+    pub(super) item_type: Option<crate::io::story_metadata::ItemType>,
+    /// Epic this item belongs to. `None` when the item has no parent epic.
+    pub(super) epic: Option<EpicId>,
 }
 
 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<f64> {
-        self.claimed_at
+    /// QA mode override, or `None` when the register is unset (use project default).
+    pub fn qa_mode(&self) -> Option<crate::io::story_metadata::QaMode> {
+        self.qa_mode
     }
 
-    /// Unix timestamp (seconds) when the item was merged to master, or `None`.
-    pub fn merged_at(&self) -> Option<f64> {
-        self.merged_at
+    /// Item type, or `None` when the register is unset (infer from story_id prefix).
+    pub fn item_type(&self) -> Option<crate::io::story_metadata::ItemType> {
+        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<EpicId> {
+        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<String>,
         stage: crate::pipeline_state::Stage,
-        name: Option<String>,
+        name: impl Into<String>,
         agent: Option<String>,
-        retry_count: Option<i64>,
-        depends_on: Option<Vec<u32>>,
-        claimed_by: Option<String>,
-        claimed_at: Option<f64>,
-        merged_at: Option<f64>,
-        qa_mode: Option<String>,
-        item_type: Option<String>,
-        epic: Option<String>,
+        retry_count: u32,
+        depends_on: Vec<u32>,
+        claim: Option<Claim>,
+        qa_mode: Option<crate::io::story_metadata::QaMode>,
+        item_type: Option<crate::io::story_metadata::ItemType>,
+        epic: Option<EpicId>,
     ) -> 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,

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<crate::io::story_metadata::ItemType>,
+) -> 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<crate::crdt_state::types::EpicId>) -> 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
 }

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]