huskies: merge 865

server/src/db/mod.rs

@@ -19,6 +19,11 @@ pub mod content_store;
 pub mod ops;
 /// Background shadow-write task — persists pipeline items to SQLite asynchronously.
 pub mod shadow_write;
+/// Legacy YAML helpers — used by the migration and by callers reading the
+/// small set of fields not yet mirrored into the CRDT.
+pub(crate) mod yaml_legacy;
+/// One-shot migration that strips legacy YAML front-matter from stored content (story 865).
+pub mod yaml_migration;
 
 pub use content_store::{all_content_ids, delete_content, read_content, write_content};
 pub use ops::{ItemMeta, delete_item, move_item_stage, next_item_number, write_item_with_content};
@@ -30,7 +35,7 @@ pub use content_store::ensure_content_store;
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::io::story_metadata::parse_front_matter;
+    use crate::db::yaml_legacy::parse_front_matter;
     use std::fs;
 
     /// Helper: write a minimal story .md file with front matter.

server/src/db/ops.rs

@@ -7,7 +7,7 @@ use super::content_store::{
     all_content_ids, delete_content, ensure_content_store, read_content, write_content,
 };
 use super::shadow_write::{PIPELINE_DB, PipelineWriteMsg};
-use crate::io::story_metadata::parse_front_matter;
+use super::yaml_legacy::parse_front_matter;
 
 /// Typed metadata for a pipeline item write.
 ///

server/src/db/yaml_legacy.rs

@@ -0,0 +1,254 @@
+//! Legacy YAML front-matter helpers — kept ONLY for the one-shot migration
+//! and for the small set of fields not yet mirrored into the CRDT
+//! (`item_type`, `epic`, `review_hold`, `merge_failure` reason text, etc.).
+//!
+//! After the migration runs, every body in the content store is YAML-free, so
+//! every helper here returns `Ok(None)` / a no-op on the next read.  Callers
+//! should treat this module as a deprecated escape hatch — new code should
+//! read typed CRDT registers instead.
+
+use crate::io::story_metadata::QaMode;
+use serde::Deserialize;
+use std::fs;
+use std::path::Path;
+
+/// Front-matter fields used by the legacy `parse_front_matter` API.  Mirrors
+/// the original `io::story_metadata::FrontMatter`.
+#[derive(Debug, Default, Deserialize)]
+pub(crate) struct FrontMatter {
+    pub name: Option<String>,
+    pub coverage_baseline: Option<String>,
+    pub merge_failure: Option<String>,
+    pub agent: Option<String>,
+    pub review_hold: Option<bool>,
+    pub qa: Option<String>,
+    pub retry_count: Option<u32>,
+    pub blocked: Option<bool>,
+    pub depends_on: Option<Vec<u32>>,
+    pub frozen: Option<bool>,
+    pub resume_to_stage: Option<String>,
+    pub run_tests_passed: Option<bool>,
+    #[serde(rename = "type")]
+    pub item_type: Option<String>,
+    pub mergemaster_attempted: Option<bool>,
+    pub epic: Option<String>,
+}
+
+/// Parsed metadata view returned by [`parse_front_matter`].
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
+pub(crate) struct StoryMetadata {
+    pub name: Option<String>,
+    pub coverage_baseline: Option<String>,
+    pub merge_failure: Option<String>,
+    pub agent: Option<String>,
+    pub review_hold: Option<bool>,
+    pub qa: Option<QaMode>,
+    pub retry_count: Option<u32>,
+    pub blocked: Option<bool>,
+    pub depends_on: Option<Vec<u32>>,
+    pub frozen: Option<bool>,
+    pub resume_to_stage: Option<String>,
+    pub run_tests_passed: Option<bool>,
+    pub item_type: Option<String>,
+    pub mergemaster_attempted: Option<bool>,
+    pub epic: Option<String>,
+}
+
+/// Errors that can occur when parsing legacy YAML front matter.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) enum StoryMetaError {
+    MissingFrontMatter,
+    InvalidFrontMatter(String),
+}
+
+impl std::fmt::Display for StoryMetaError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::MissingFrontMatter => write!(f, "Missing front matter"),
+            Self::InvalidFrontMatter(m) => write!(f, "Invalid front matter: {m}"),
+        }
+    }
+}
+
+/// Parse the YAML front-matter block from a markdown body.
+///
+/// Post-migration this returns `Err(StoryMetaError::MissingFrontMatter)` for
+/// every body since the front matter has been stripped.  Callers that need
+/// fields not stored in the CRDT (`item_type`, `epic`, …) should treat the
+/// missing-front-matter case as "default value".
+pub(crate) fn parse_front_matter(contents: &str) -> Result<StoryMetadata, StoryMetaError> {
+    let mut lines = contents.lines();
+    let first = lines.next().unwrap_or_default().trim();
+    if first != "---" {
+        return Err(StoryMetaError::MissingFrontMatter);
+    }
+    let mut front_lines = Vec::new();
+    for line in &mut lines {
+        if line.trim() == "---" {
+            let raw = front_lines.join("\n");
+            let front: FrontMatter = serde_yaml::from_str(&raw)
+                .map_err(|e| StoryMetaError::InvalidFrontMatter(e.to_string()))?;
+            return Ok(StoryMetadata {
+                qa: front.qa.as_deref().and_then(QaMode::from_str),
+                name: front.name,
+                coverage_baseline: front.coverage_baseline,
+                merge_failure: front.merge_failure,
+                agent: front.agent,
+                review_hold: front.review_hold,
+                retry_count: front.retry_count,
+                blocked: front.blocked,
+                depends_on: front.depends_on,
+                frozen: front.frozen,
+                resume_to_stage: front.resume_to_stage,
+                run_tests_passed: front.run_tests_passed,
+                item_type: front.item_type,
+                mergemaster_attempted: front.mergemaster_attempted,
+                epic: front.epic,
+            });
+        }
+        front_lines.push(line);
+    }
+    Err(StoryMetaError::InvalidFrontMatter(
+        "Missing closing front matter delimiter".to_string(),
+    ))
+}
+
+/// Insert or update a `key: value` line in the YAML front matter of a
+/// markdown string.  Returns the input unchanged if no `---` block is found.
+pub(crate) fn set_front_matter_field(contents: &str, key: &str, value: &str) -> String {
+    let mut lines: Vec<String> = contents.lines().map(String::from).collect();
+    if lines.is_empty() || lines[0].trim() != "---" {
+        return contents.to_string();
+    }
+    let close_idx = match lines[1..].iter().position(|l| l.trim() == "---") {
+        Some(i) => i + 1,
+        None => return contents.to_string(),
+    };
+    let key_prefix = format!("{key}:");
+    let existing_idx = lines[1..close_idx]
+        .iter()
+        .position(|l| l.trim_start().starts_with(&key_prefix))
+        .map(|i| i + 1);
+    let new_line = format!("{key}: {value}");
+    if let Some(idx) = existing_idx {
+        lines[idx] = new_line;
+    } else {
+        lines.insert(close_idx, new_line);
+    }
+    let mut result = lines.join("\n");
+    if contents.ends_with('\n') {
+        result.push('\n');
+    }
+    result
+}
+
+/// Remove a `key: value` line from the YAML front matter of a markdown string.
+pub(crate) fn clear_front_matter_field_in_content(contents: &str, key: &str) -> String {
+    let mut lines: Vec<String> = contents.lines().map(String::from).collect();
+    if lines.is_empty() || lines[0].trim() != "---" {
+        return contents.to_string();
+    }
+    let close_idx = match lines[1..].iter().position(|l| l.trim() == "---") {
+        Some(i) => i + 1,
+        None => return contents.to_string(),
+    };
+    let key_prefix = format!("{key}:");
+    if let Some(idx) = lines[1..close_idx]
+        .iter()
+        .position(|l| l.trim_start().starts_with(&key_prefix))
+        .map(|i| i + 1)
+    {
+        lines.remove(idx);
+    } else {
+        return contents.to_string();
+    }
+    let mut result = lines.join("\n");
+    if contents.ends_with('\n') {
+        result.push('\n');
+    }
+    result
+}
+
+/// Append rejection notes to a markdown body.
+pub(crate) fn write_rejection_notes_to_content(contents: &str, notes: &str) -> String {
+    format!("{contents}\n\n## QA Rejection Notes\n\n{notes}\n")
+}
+
+/// Write or update `merge_failure` in story content.
+pub(crate) fn write_merge_failure_in_content(contents: &str, reason: &str) -> String {
+    let escaped = reason
+        .replace('"', "\\\"")
+        .replace('\n', " ")
+        .replace('\r', "");
+    set_front_matter_field(contents, "merge_failure", &format!("\"{escaped}\""))
+}
+
+/// Write `review_hold: true` to story content.
+pub(crate) fn write_review_hold_in_content(contents: &str) -> String {
+    set_front_matter_field(contents, "review_hold", "true")
+}
+
+/// Write `mergemaster_attempted: true` to story content.
+pub(crate) fn write_mergemaster_attempted_in_content(contents: &str) -> String {
+    set_front_matter_field(contents, "mergemaster_attempted", "true")
+}
+
+/// Remove a key from the YAML front matter of a story file on disk.
+///
+/// Legacy filesystem-backed wrapper around
+/// [`clear_front_matter_field_in_content`] for the small number of callers
+/// that still read story files directly.
+pub(crate) fn clear_front_matter_field(path: &Path, key: &str) -> Result<(), String> {
+    let contents =
+        fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?;
+    let updated = clear_front_matter_field_in_content(&contents, key);
+    if updated != contents {
+        fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?;
+    }
+    Ok(())
+}
+
+/// Write `review_hold: true` to the YAML front matter of a story file on disk.
+///
+/// Legacy filesystem-backed wrapper around [`write_review_hold_in_content`].
+pub(crate) fn write_review_hold(path: &Path) -> Result<(), String> {
+    let contents =
+        fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?;
+    let updated = write_review_hold_in_content(&contents);
+    fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?;
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parse_front_matter_round_trips_basic_fields() {
+        let input = "---\nname: Test\nagent: coder-1\n---\n# Body\n";
+        let meta = parse_front_matter(input).expect("parse");
+        assert_eq!(meta.name.as_deref(), Some("Test"));
+        assert_eq!(meta.agent.as_deref(), Some("coder-1"));
+    }
+
+    #[test]
+    fn parse_front_matter_returns_missing_when_no_yaml() {
+        let err = parse_front_matter("# Plain markdown\n").unwrap_err();
+        assert_eq!(err, StoryMetaError::MissingFrontMatter);
+    }
+
+    #[test]
+    fn set_front_matter_field_inserts_when_absent() {
+        let out = set_front_matter_field("---\nname: X\n---\n# B\n", "agent", "coder-1");
+        assert!(out.contains("agent: coder-1"));
+    }
+
+    #[test]
+    fn clear_front_matter_field_removes_key() {
+        let out = clear_front_matter_field_in_content(
+            "---\nname: X\nblocked: true\n---\n# B\n",
+            "blocked",
+        );
+        assert!(!out.contains("blocked"));
+    }
+}

server/src/db/yaml_migration.rs

@@ -0,0 +1,316 @@
+//! One-shot migration: strip YAML front-matter from stored story content.
+//!
+//! Story 865 finishes the move to CRDT registers as the single source of truth
+//! for pipeline metadata. At server startup we walk every entry in the content
+//! store, parse its (possibly-present) YAML block one last time, assert that
+//! every YAML field still agrees with the corresponding CRDT register, and
+//! rewrite the body without the `---` block. Divergence between YAML and the
+//! CRDT is logged as ERROR but does not stop the migration — the YAML is
+//! authoritative-of-history; the CRDT is authoritative-of-truth.
+//!
+//! After this runs once (idempotent — a second run finds no YAML headers),
+//! `parse_front_matter` and friends can be deleted.  The parsing logic now
+//! lives privately in this module so the rest of the codebase can drop the
+//! YAML helpers entirely.
+use crate::io::story_metadata::QaMode;
+use crate::slog;
+use serde::Deserialize;
+
+/// Snapshot of the legacy YAML front-matter fields we still need to read once
+/// during the migration. After this run completes, every body in the content
+/// store is YAML-free and this struct is unused.
+#[derive(Debug, Default, Deserialize)]
+struct LegacyFrontMatter {
+    name: Option<String>,
+    agent: Option<String>,
+    retry_count: Option<u32>,
+    blocked: Option<bool>,
+    depends_on: Option<Vec<u32>>,
+    qa: Option<String>,
+    mergemaster_attempted: Option<bool>,
+}
+
+/// Parse the YAML front-matter block from a markdown body.
+///
+/// Returns `Ok(None)` when the body has no opening `---`. Returns an error
+/// message when an opening `---` is present but parsing fails.
+fn parse_legacy_front_matter(contents: &str) -> Result<Option<LegacyFrontMatter>, String> {
+    let mut lines = contents.lines();
+    let first = lines.next().unwrap_or_default().trim();
+    if first != "---" {
+        return Ok(None);
+    }
+    let mut front_lines = Vec::new();
+    for line in &mut lines {
+        if line.trim() == "---" {
+            let raw = front_lines.join("\n");
+            return serde_yaml::from_str::<LegacyFrontMatter>(&raw)
+                .map(Some)
+                .map_err(|e| e.to_string());
+        }
+        front_lines.push(line);
+    }
+    Err("missing closing front-matter delimiter".to_string())
+}
+
+/// Strip the YAML front-matter block (`---\n...\n---\n`) from a markdown body.
+///
+/// Returns the body without the leading YAML block and its trailing delimiter,
+/// preserving any leading newline that follows the closing `---`. If the input
+/// has no opening `---` the body is returned unchanged.
+pub(super) fn strip_yaml_block(content: &str) -> String {
+    let mut lines = content.lines();
+    let first = match lines.next() {
+        Some(l) => l,
+        None => return String::new(),
+    };
+    if first.trim() != "---" {
+        return content.to_string();
+    }
+
+    let mut consumed = first.len() + 1; // +1 for the newline
+    for line in &mut lines {
+        consumed += line.len() + 1;
+        if line.trim() == "---" {
+            // Drop a single blank line that often follows the closing fence.
+            let rest = &content[consumed.min(content.len())..];
+            return rest.trim_start_matches('\n').to_string();
+        }
+    }
+    // No closing fence — leave content alone rather than mangle it.
+    content.to_string()
+}
+
+/// Run the one-shot YAML-strip migration over every story in the content store.
+///
+/// For each story:
+///   1. Read the markdown body from the content store.
+///   2. If it starts with a `---` block, parse the YAML one last time.
+///   3. Compare every CRDT-mirrored field against the YAML value; any
+///      mismatch is logged at ERROR level (the migration trusts the CRDT).
+///   4. Rewrite the body in the content store and shadow table without the
+///      YAML block.
+///
+/// Bodies that already lack a `---` opener are skipped (idempotent re-runs).
+pub fn run() {
+    let ids = super::all_content_ids();
+    let mut migrated = 0usize;
+    let mut diverged = 0usize;
+
+    for story_id in ids {
+        let Some(content) = super::read_content(&story_id) else {
+            continue;
+        };
+        if !content.trim_start().starts_with("---") {
+            continue;
+        }
+
+        // One last parse — this is the only remaining caller after AC2.
+        let yaml = match parse_legacy_front_matter(&content) {
+            Ok(Some(m)) => m,
+            Ok(None) => continue,
+            Err(e) => {
+                slog!(
+                    "[migrate-yaml] {}: parse failed ({}); leaving content untouched",
+                    story_id,
+                    e
+                );
+                continue;
+            }
+        };
+
+        let crdt = crate::crdt_state::read_item(&story_id);
+
+        if let Some(ref view) = crdt {
+            // Compare every YAML field to its CRDT counterpart and log
+            // divergence. The CRDT wins; YAML is dropped regardless.
+            let mut field_diverged = false;
+            macro_rules! cmp {
+                ($field:expr, $yaml:expr, $crdt:expr) => {
+                    let yv = $yaml;
+                    let cv = $crdt;
+                    if yv.is_some() && yv != cv {
+                        slog!(
+                            "[migrate-yaml][ERROR] {} field '{}' diverged: yaml={:?} crdt={:?}",
+                            story_id,
+                            $field,
+                            yv,
+                            cv,
+                        );
+                        field_diverged = true;
+                    }
+                };
+            }
+            cmp!("name", yaml.name.clone(), view.name.clone());
+            cmp!("agent", yaml.agent.clone(), view.agent.clone());
+            cmp!(
+                "retry_count",
+                yaml.retry_count.map(|n| n as i64),
+                view.retry_count
+            );
+            cmp!("blocked", yaml.blocked, view.blocked);
+            cmp!(
+                "depends_on",
+                yaml.depends_on.clone(),
+                view.depends_on.clone()
+            );
+            cmp!(
+                "qa",
+                yaml.qa
+                    .as_deref()
+                    .and_then(QaMode::from_str)
+                    .map(|q| q.as_str().to_string()),
+                view.qa_mode.clone()
+            );
+            cmp!(
+                "mergemaster_attempted",
+                yaml.mergemaster_attempted,
+                view.mergemaster_attempted
+            );
+            if field_diverged {
+                diverged += 1;
+            }
+        } else if yaml.name.is_some() || yaml.agent.is_some() {
+            // YAML had real fields but the CRDT has no entry. Log and proceed.
+            slog!(
+                "[migrate-yaml][ERROR] {} has YAML fields but no CRDT entry — yaml dropped",
+                story_id
+            );
+            diverged += 1;
+        }
+
+        let stripped = strip_yaml_block(&content);
+        if stripped == content {
+            continue;
+        }
+
+        // Persist the stripped body. We use `move_item_stage` with a transform
+        // so the shadow table stays in sync; if the CRDT has no entry we fall
+        // back to writing the content store directly.
+        if let Some(view) = crdt {
+            super::move_item_stage(&story_id, &view.stage, Some(&|_| stripped.clone()));
+        } else {
+            super::write_content(&story_id, &stripped);
+        }
+        migrated += 1;
+    }
+
+    if migrated > 0 || diverged > 0 {
+        slog!(
+            "[migrate-yaml] stripped YAML from {} stories ({} had divergent CRDT)",
+            migrated,
+            diverged
+        );
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::db::{ItemMeta, ensure_content_store, read_content, write_item_with_content};
+
+    #[test]
+    fn strip_yaml_block_removes_leading_block() {
+        let input = "---\nname: Test\nagent: coder-1\n---\n# Body\n";
+        assert_eq!(strip_yaml_block(input), "# Body\n");
+    }
+
+    #[test]
+    fn strip_yaml_block_preserves_body_without_yaml() {
+        let input = "# Just a body\n\nNo YAML here.\n";
+        assert_eq!(strip_yaml_block(input), input);
+    }
+
+    #[test]
+    fn strip_yaml_block_leaves_content_alone_when_unclosed() {
+        // No closing --- — we don't want to mangle the body.
+        let input = "---\nname: Test\n# Story\n";
+        assert_eq!(strip_yaml_block(input), input);
+    }
+
+    #[test]
+    fn strip_yaml_block_drops_only_one_blank_line_after_fence() {
+        let input = "---\nname: Test\n---\n\n# Body\n";
+        // The closing `---\n` is dropped plus one blank line; the actual body starts at `# Body`.
+        assert_eq!(strip_yaml_block(input), "# Body\n");
+    }
+
+    #[test]
+    fn run_migration_strips_yaml_when_crdt_matches() {
+        crate::crdt_state::init_for_test();
+        ensure_content_store();
+        let story_id = "9001_story_strip_match";
+
+        let body = "---\nname: Match Story\nagent: coder-1\n---\n# Body\n";
+        write_item_with_content(
+            story_id,
+            "2_current",
+            body,
+            ItemMeta {
+                name: Some("Match Story".into()),
+                agent: Some("coder-1".into()),
+                ..ItemMeta::default()
+            },
+        );
+
+        run();
+
+        let after = read_content(story_id).expect("content present");
+        assert!(
+            !after.trim_start().starts_with("---"),
+            "YAML block should be stripped: {after:?}"
+        );
+        assert!(after.contains("# Body"));
+    }
+
+    #[test]
+    fn run_migration_logs_divergence_but_still_strips() {
+        crate::crdt_state::init_for_test();
+        ensure_content_store();
+        let story_id = "9002_story_strip_diverge";
+
+        // YAML says name = "OLD"; CRDT says name = "NEW".
+        let body = "---\nname: OLD\n---\n# Body\n";
+        write_item_with_content(
+            story_id,
+            "2_current",
+            body,
+            ItemMeta {
+                name: Some("NEW".into()),
+                ..ItemMeta::default()
+            },
+        );
+
+        run();
+
+        let after = read_content(story_id).expect("content present");
+        assert!(!after.contains("---"));
+        // CRDT wins — name is still "NEW".
+        let view = crate::crdt_state::read_item(story_id).expect("crdt item");
+        assert_eq!(view.name.as_deref(), Some("NEW"));
+    }
+
+    #[test]
+    fn run_migration_is_idempotent_on_already_stripped_content() {
+        crate::crdt_state::init_for_test();
+        ensure_content_store();
+        let story_id = "9003_story_strip_idempotent";
+
+        let body = "# Already-stripped body\n";
+        write_item_with_content(
+            story_id,
+            "2_current",
+            body,
+            ItemMeta {
+                name: Some("Already".into()),
+                ..ItemMeta::default()
+            },
+        );
+
+        run();
+        run();
+
+        let after = read_content(story_id).expect("content present");
+        assert_eq!(after, body);
+    }
+}