diff --git a/server/src/crdt_state/mod.rs b/server/src/crdt_state/mod.rs index af5a6574..f206c497 100644 --- a/server/src/crdt_state/mod.rs +++ b/server/src/crdt_state/mod.rs @@ -38,7 +38,9 @@ pub use types::{ CrdtEvent, NodePresenceCrdt, NodePresenceView, PipelineDoc, PipelineItemCrdt, PipelineItemView, subscribe, }; -pub use write::{migrate_names_from_slugs, name_from_story_id, write_item}; +pub use write::{ + migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id, write_item, +}; #[cfg(test)] pub use state::init_for_test; diff --git a/server/src/crdt_state/read.rs b/server/src/crdt_state/read.rs index 6f83cd57..5b3eb55b 100644 --- a/server/src/crdt_state/read.rs +++ b/server/src/crdt_state/read.rs @@ -333,12 +333,15 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option bool { use crate::pipeline_state::{Stage, read_all_typed}; + let exact = dep_number.to_string(); let prefix = format!("{dep_number}_"); read_all_typed().into_iter().any(|item| { - item.story_id.0.starts_with(&prefix) + (item.story_id.0 == exact || item.story_id.0.starts_with(&prefix)) && matches!(item.stage, Stage::Done { .. } | Stage::Archived { .. }) }) } @@ -348,11 +351,14 @@ pub fn dep_is_done_crdt(dep_number: u32) -> bool { /// /// Used to detect when a dependency is satisfied via archive rather than via a clean /// completion through `5_done`. Returns `false` when the CRDT layer is not initialised. +/// Matches both legacy slug-form IDs (`"664_story_foo"`) and numeric-only IDs (`"664"`). pub fn dep_is_archived_crdt(dep_number: u32) -> bool { use crate::pipeline_state::{Stage, read_all_typed}; + let exact = dep_number.to_string(); let prefix = format!("{dep_number}_"); read_all_typed().into_iter().any(|item| { - item.story_id.0.starts_with(&prefix) && matches!(item.stage, Stage::Archived { .. }) + (item.story_id.0 == exact || item.story_id.0.starts_with(&prefix)) + && matches!(item.stage, Stage::Archived { .. }) }) } diff --git a/server/src/crdt_state/write.rs b/server/src/crdt_state/write.rs index 00f84c12..483de3be 100644 --- a/server/src/crdt_state/write.rs +++ b/server/src/crdt_state/write.rs @@ -49,6 +49,88 @@ pub fn name_from_story_id(story_id: &str) -> String { } } +/// Extract the numeric-only ID from a slug-form story ID, if applicable. +/// +/// Returns `Some("664")` for `"664_story_my_feature"`, and `None` for IDs +/// that are already numeric-only (`"664"`) or have no valid numeric prefix. +fn numeric_id_from_slug(story_id: &str) -> Option { + // Already numeric-only — no migration needed. + if story_id.chars().all(|c: char| c.is_ascii_digit()) { + return None; + } + // Must have a non-empty numeric segment before the first underscore. + let idx = story_id.find('_')?; + let prefix = &story_id[..idx]; + if prefix.is_empty() || !prefix.chars().all(|c| c.is_ascii_digit()) { + return None; + } + Some(prefix.to_string()) +} + +/// Migrate existing story IDs from slug form (`664_story_my_feature`) to +/// numeric-only form (`664`) in the in-memory CRDT, persisting a signed op +/// for each updated register so the change survives restarts. +/// +/// Returns the list of `(old_id, new_id)` pairs that were actually migrated. +/// Callers should use this list to rename downstream filesystem artifacts +/// (worktree directories, git branches, log directories). +/// +/// Items whose `story_id` is already numeric-only are left untouched. +/// Items where the target numeric ID is already in use are skipped to avoid +/// conflicts. Running this migration repeatedly is safe — subsequent calls +/// on already-migrated state are no-ops. +pub fn migrate_story_ids_to_numeric() -> Vec<(String, String)> { + let Some(state_mutex) = get_crdt() else { + return Vec::new(); + }; + + // First pass: collect (index, old_id, new_id) while holding the lock. + let migrations: Vec<(usize, String, String)> = { + let Ok(state) = state_mutex.lock() else { + return Vec::new(); + }; + + let existing_ids: std::collections::HashSet = state.index.keys().cloned().collect(); + + state + .index + .iter() + .filter_map(|(story_id, &idx)| { + let numeric = numeric_id_from_slug(story_id)?; + // Skip if the target numeric ID is already occupied. + if existing_ids.contains(&numeric) { + return None; + } + Some((idx, story_id.clone(), numeric)) + }) + .collect() + }; + + if migrations.is_empty() { + return Vec::new(); + } + + // Second pass: apply story_id register updates. + let Ok(mut state) = state_mutex.lock() else { + return Vec::new(); + }; + + let mut result = Vec::new(); + for (idx, old_id, new_id) in migrations { + apply_and_persist(&mut state, |s| { + s.crdt.doc.items[idx].story_id.set(new_id.clone()) + }); + result.push((old_id, new_id)); + } + + // Rebuild the index so all downstream reads use the new numeric IDs. + state.index = rebuild_index(&state.crdt); + + let count = result.len(); + slog!("[crdt] Migrated {count} story IDs from slug form to numeric"); + result +} + /// Backfill the `name` CRDT field for pipeline items that have an empty name. /// /// Iterates over all items in the in-memory CRDT. For each item whose `name` @@ -292,6 +374,175 @@ mod tests { // ── migrate_names_from_slugs tests ─────────────────────────────────────── + // ── numeric_id_from_slug tests ──────────────────────────────────────────── + + #[test] + fn numeric_id_from_slug_extracts_prefix() { + assert_eq!( + numeric_id_from_slug("664_story_my_feature"), + Some("664".to_string()) + ); + assert_eq!( + numeric_id_from_slug("4_bug_login_crash"), + Some("4".to_string()) + ); + assert_eq!( + numeric_id_from_slug("730_refactor_foo_bar"), + Some("730".to_string()) + ); + } + + #[test] + fn numeric_id_from_slug_returns_none_for_numeric_only() { + assert_eq!(numeric_id_from_slug("664"), None); + assert_eq!(numeric_id_from_slug("1"), None); + assert_eq!(numeric_id_from_slug("730"), None); + } + + #[test] + fn numeric_id_from_slug_returns_none_for_non_numeric_prefix() { + assert_eq!(numeric_id_from_slug("story_no_number"), None); + assert_eq!(numeric_id_from_slug(""), None); + assert_eq!(numeric_id_from_slug("abc_story"), None); + } + + // ── migrate_story_ids_to_numeric tests ─────────────────────────────────── + + #[test] + fn migrate_story_ids_to_numeric_rewrites_slug_ids() { + init_for_test(); + + write_item( + "42_story_my_feature", + "1_backlog", + Some("My Feature"), + None, + None, + None, + None, + None, + None, + None, + ); + + let result = migrate_story_ids_to_numeric(); + assert_eq!(result.len(), 1, "exactly one item should be migrated"); + assert_eq!(result[0].0, "42_story_my_feature"); + assert_eq!(result[0].1, "42"); + + // After migration the item is accessible by the numeric ID. + let item = read_item("42").expect("item should be found by numeric ID"); + assert_eq!(item.story_id, "42"); + + // The slug-based ID is gone. + assert!( + read_item("42_story_my_feature").is_none(), + "slug ID should no longer be in the index" + ); + } + + #[test] + fn migrate_story_ids_to_numeric_is_idempotent() { + init_for_test(); + + write_item( + "43", + "1_backlog", + Some("Already Numeric"), + None, + None, + None, + None, + None, + None, + None, + ); + + // First call — nothing to migrate. + let r1 = migrate_story_ids_to_numeric(); + assert!(r1.is_empty(), "no migration for already-numeric ID"); + + // Second call — still nothing. + let r2 = migrate_story_ids_to_numeric(); + assert!(r2.is_empty(), "second call must be a no-op"); + + // Item is still there. + assert!(read_item("43").is_some()); + } + + #[test] + fn migrate_story_ids_to_numeric_skips_conflict() { + init_for_test(); + + // Both the slug form AND its numeric target exist. + write_item( + "44_story_foo", + "1_backlog", + None, + None, + None, + None, + None, + None, + None, + None, + ); + write_item( + "44", + "2_current", + None, + 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. + assert!( + result.is_empty(), + "conflicting slug must not overwrite existing numeric entry" + ); + + // Both items remain as they were. + assert!(read_item("44_story_foo").is_some()); + assert!(read_item("44").is_some()); + } + + #[test] + fn migrate_story_ids_to_numeric_noop_when_crdt_not_initialised() { + // Must not panic when called before init. + migrate_story_ids_to_numeric(); + } + + #[test] + fn migrate_story_ids_to_numeric_preserves_stage_and_name() { + init_for_test(); + + write_item( + "45_bug_crash", + "2_current", + Some("Crash Bug"), + Some("coder-1"), + None, + None, + None, + None, + None, + None, + ); + + migrate_story_ids_to_numeric(); + + let item = read_item("45").expect("item must be accessible by numeric ID"); + assert_eq!(item.stage, "2_current"); + assert_eq!(item.name.as_deref(), Some("Crash Bug")); + assert_eq!(item.agent.as_deref(), Some("coder-1")); + } + #[test] fn migrate_names_from_slugs_fills_empty_names() { init_for_test(); diff --git a/server/src/main.rs b/server/src/main.rs index ff9b5663..cbbbad6f 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -271,6 +271,18 @@ async fn main() -> Result<(), std::io::Error> { // Migrate items that have an empty name field: derive the name // from the story ID slug. No-op for items that already have a name. crdt_state::migrate_names_from_slugs(); + + // Migrate story IDs from slug form ("664_story_...") to numeric-only + // ("664"). Returns migrated pairs so we can rename filesystem artifacts. + // No-op when all IDs are already numeric. + let id_migrations = crdt_state::migrate_story_ids_to_numeric(); + if !id_migrations.is_empty() { + // Derive the project root from the db_path: .huskies/pipeline.db + // lives two levels below the project root. + if let Some(project_root) = db_path.parent().and_then(|p| p.parent()) { + worktree::migrate_slug_paths(project_root, &id_migrations); + } + } } } diff --git a/server/src/worktree.rs b/server/src/worktree.rs index afaaccbb..7a69639e 100644 --- a/server/src/worktree.rs +++ b/server/src/worktree.rs @@ -215,6 +215,89 @@ pub async fn remove_worktree_by_story_id( remove_worktree(project_root, &info, config).await } +/// Migrate filesystem artifacts for story IDs that were rewritten from slug form +/// (`664_story_my_feature`) to numeric-only form (`664`). +/// +/// For each `(old_id, new_id)` pair (as returned by +/// `crdt_state::migrate_story_ids_to_numeric`), this function: +/// 1. Moves the git worktree directory via `git worktree move` when it exists. +/// 2. Renames the git branch from `feature/story-{old_id}` to `feature/story-{new_id}`. +/// 3. Renames the log directory at `.huskies/logs/{old_id}` when it exists. +/// +/// All steps are best-effort: failures are logged but do not abort the migration. +/// Operations are skipped when the destination path already exists (idempotent). +pub fn migrate_slug_paths(project_root: &Path, migrations: &[(String, String)]) { + for (old_id, new_id) in migrations { + // ── Worktree directory ────────────────────────────────────────────── + let old_wt = worktree_path(project_root, old_id); + let new_wt = worktree_path(project_root, new_id); + + if old_wt.exists() && !new_wt.exists() { + let out = Command::new("git") + .args([ + "worktree", + "move", + &old_wt.to_string_lossy(), + &new_wt.to_string_lossy(), + ]) + .current_dir(project_root) + .output(); + match out { + Ok(o) if o.status.success() => { + slog!("[migrate] Moved worktree {old_id} → {new_id}"); + } + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr); + slog!( + "[migrate] git worktree move failed for {old_id}: {stderr}; \ + falling back to directory rename" + ); + if let Err(e) = std::fs::rename(&old_wt, &new_wt) { + slog!("[migrate] Directory rename for worktree {old_id} failed: {e}"); + } else { + slog!("[migrate] Renamed worktree directory {old_id} → {new_id}"); + } + } + Err(e) => { + slog!("[migrate] git worktree move error for {old_id}: {e}"); + } + } + } + + // ── Git branch ────────────────────────────────────────────────────── + let old_branch = branch_name(old_id); + let new_branch = branch_name(new_id); + let out = Command::new("git") + .args(["branch", "-m", &old_branch, &new_branch]) + .current_dir(project_root) + .output(); + match out { + Ok(o) if o.status.success() => { + slog!("[migrate] Renamed branch {old_branch} → {new_branch}"); + } + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr); + // Branch may not exist (story already merged/archived) — log at debug level. + slog!("[migrate] Branch rename skipped for {old_id}: {stderr}"); + } + Err(e) => { + slog!("[migrate] Branch rename error for {old_id}: {e}"); + } + } + + // ── Log directory ─────────────────────────────────────────────────── + let old_log = project_root.join(".huskies").join("logs").join(old_id); + let new_log = project_root.join(".huskies").join("logs").join(new_id); + if old_log.exists() && !new_log.exists() { + if let Err(e) = std::fs::rename(&old_log, &new_log) { + slog!("[migrate] Log directory rename for {old_id} failed: {e}"); + } else { + slog!("[migrate] Moved log directory {old_id} → {new_id}"); + } + } + } +} + /// List all worktrees under `{project_root}/.huskies/worktrees/`. /// Find the worktree path for a given story ID, if it exists. pub fn find_worktree_path(project_root: &Path, story_id: &str) -> Option {