huskies: merge 731_refactor_migrate_existing_stories_from_slug_based_ids_to_numeric_only
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -333,12 +333,15 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option<PipelineItemV
|
||||
/// according to CRDT state.
|
||||
///
|
||||
/// Returns `true` if the dependency is satisfied (item found in a done stage).
|
||||
/// Matches both legacy slug-form IDs (`"664_story_foo"`) and numeric-only IDs
|
||||
/// (`"664"`) so the check remains correct after the slug→numeric migration.
|
||||
/// See `dep_is_archived_crdt` to distinguish archive-satisfied from cleanly-done.
|
||||
pub fn dep_is_done_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)
|
||||
(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 { .. })
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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<String> {
|
||||
// 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<String> = 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();
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<PathBuf> {
|
||||
|
||||
Reference in New Issue
Block a user