diff --git a/server/src/db/mod.rs b/server/src/db/mod.rs index d91faaf3..8bc902c0 100644 --- a/server/src/db/mod.rs +++ b/server/src/db/mod.rs @@ -19,11 +19,9 @@ 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. +/// Legacy YAML helpers — used 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}; @@ -384,6 +382,35 @@ mod tests { ); } + /// Regression (story 894): startup with no YAML migration call leaves + /// existing clean content readable and unmodified. Verifies that removing + /// `db::yaml_migration::run()` from the startup path does not break reads. + #[test] + fn startup_reads_clean_content_unchanged() { + crate::crdt_state::init_for_test(); + ensure_content_store(); + let story_id = "9894_story_clean_content"; + + // Plain body — no YAML block, representing post-migration state. + let body = "# Story Heading\n\nSome content.\n"; + write_item_with_content( + story_id, + "2_current", + body, + ItemMeta { + name: Some("Clean".into()), + ..ItemMeta::default() + }, + ); + + let read_back = read_content(story_id).expect("content present"); + assert_eq!(read_back, body, "plain content must be readable as-is"); + assert!( + !read_back.trim_start().starts_with("---"), + "no YAML header should appear" + ); + } + /// Bug 780: stage transitions must reset retry_count to 0 in the CRDT. /// Carryover from prior-stage retries was tripping the auto-assigner's /// deterministic-merge skip logic. diff --git a/server/src/db/yaml_migration.rs b/server/src/db/yaml_migration.rs deleted file mode 100644 index 7a2c2326..00000000 --- a/server/src/db/yaml_migration.rs +++ /dev/null @@ -1,316 +0,0 @@ -//! 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, - agent: Option, - retry_count: Option, - blocked: Option, - depends_on: Option>, - qa: Option, - mergemaster_attempted: Option, -} - -/// 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, 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::(&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); - } -} diff --git a/server/src/startup/project.rs b/server/src/startup/project.rs index 425f7a05..856f36f6 100644 --- a/server/src/startup/project.rs +++ b/server/src/startup/project.rs @@ -157,10 +157,6 @@ pub(crate) async fn init_subsystems(app_state: &Arc, cwd: &Path) { { worktree::migrate_slug_paths(project_root, &id_migrations); } - // Story 865: one-shot strip of legacy YAML front-matter from - // every stored body. Idempotent — bodies without `---` are - // skipped on subsequent runs. - db::yaml_migration::run(); } } }