huskies: merge 894
This commit is contained in:
dave
+31
-4
@@ -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.
|
||||
|
||||
@@ -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<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);
|
||||
}
|
||||
}
|
||||
@@ -157,10 +157,6 @@ pub(crate) async fn init_subsystems(app_state: &Arc<SessionState>, 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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user