From 6c935e774c4c14d660ec240d1ab582433c367c6d Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Fri, 10 Jul 2026 08:10:21 +0000 Subject: [PATCH] fix(storage): recover untracked branch graphs --- src/branch.rs | 11 ++- src/branch/tests.rs | 19 ++++ src/branch_meta.rs | 15 +++ src/migrate/consolidate/evidence.rs | 1 + src/migrate/consolidate/mod.rs | 99 +++++++++++++++++-- src/migrate/consolidate/prepare.rs | 2 + src/migrate/consolidate/tests.rs | 144 ++++++++++++++++++++++++++++ 7 files changed, 276 insertions(+), 15 deletions(-) diff --git a/src/branch.rs b/src/branch.rs index b8e97a54f..925a22276 100644 --- a/src/branch.rs +++ b/src/branch.rs @@ -663,10 +663,11 @@ fn now_unix_secs() -> u64 { /// Two independent sweeps, both age-gated so an in-flight branch that has not /// yet synced or a just-deleted-then-recreated ref is never collected: /// -/// (a) **Tracked, ref-gone branches** — for each tracked non-default branch -/// whose git ref no longer exists AND whose `last_synced_at` is older than -/// `branch_gc_days`, remove its DB files and metadata entry. The default -/// branch is never removed. +/// (a) **Tracked, ref-gone branches** — for each unprotected tracked +/// non-default branch whose git ref no longer exists AND whose +/// `last_synced_at` is older than `branch_gc_days`, remove its DB files and +/// metadata entry. The default branch and GC-protected entries are never +/// removed. /// (b) **Orphan DBs** — `branches/*.db` files not referenced by any meta entry /// whose mtime is older than `orphan_db_gc_days` are deleted along with /// their `-wal`/`-shm` sidecars. @@ -698,7 +699,7 @@ pub fn gc_dead_branch_stores( let candidates: Vec<(String, PathBuf, u64)> = meta .branches .iter() - .filter(|(name, _)| **name != default_branch) + .filter(|(name, entry)| **name != default_branch && !entry.gc_protected) .map(|(name, entry)| { ( name.clone(), diff --git a/src/branch/tests.rs b/src/branch/tests.rs index ff308309d..d388adb62 100644 --- a/src/branch/tests.rs +++ b/src/branch/tests.rs @@ -233,6 +233,25 @@ fn gc_keeps_fresh_dead_branch() { assert!(db.exists()); } +#[test] +fn gc_keeps_protected_ref_gone_stale_branch() { + let (_base, project_root, td) = setup_repo_with_meta(); + let db = add_tracked_branch(&project_root, &td, "recovered/orphan", 0, false); + let mut meta = crate::branch_meta::load_branch_meta(&td).unwrap(); + meta.branches + .get_mut("recovered/orphan") + .unwrap() + .gc_protected = true; + crate::branch_meta::save_branch_meta(&td, &meta).unwrap(); + + let report = gc_dead_branch_stores(&project_root, &td, 0, 0); + + assert!(report.removed_tracked.is_empty()); + assert!(db.exists()); + let reloaded = crate::branch_meta::load_branch_meta(&td).unwrap(); + assert!(reloaded.branches["recovered/orphan"].gc_protected); +} + #[test] fn gc_keeps_branch_with_live_ref() { let (_base, project_root, td) = setup_repo_with_meta(); diff --git a/src/branch_meta.rs b/src/branch_meta.rs index b5ad95139..c5f5fc732 100644 --- a/src/branch_meta.rs +++ b/src/branch_meta.rs @@ -23,6 +23,10 @@ pub struct BranchEntry { pub created_at: String, /// UNIX timestamp (seconds) of last successful sync. pub last_synced_at: String, + /// Whether automatic branch-store GC must retain this entry even when it + /// has no matching git ref. + #[serde(default)] + pub gc_protected: bool, } /// Top-level branch metadata for a project. @@ -57,6 +61,7 @@ impl BranchMeta { parent: None, created_at: now.clone(), last_synced_at: now, + gc_protected: false, }, ); Self { @@ -75,6 +80,7 @@ impl BranchMeta { parent: Some(parent.to_string()), created_at: now.clone(), last_synced_at: now, + gc_protected: false, }, ); } @@ -259,6 +265,15 @@ mod tests { assert!(parse("[]").is_err()); } + #[test] + fn parse_old_entry_defaults_gc_protected_to_false() { + let meta = parse( + r#"{"default_branch":"main","branches":{"main":{"db_file":"tracedecay.db","created_at":"1","last_synced_at":"1"}}}"#, + ) + .unwrap(); + assert!(!meta.branches["main"].gc_protected); + } + #[test] fn update_synced_timestamp_advances_tracked_branch() { let dir = tempfile::tempdir().unwrap(); diff --git a/src/migrate/consolidate/evidence.rs b/src/migrate/consolidate/evidence.rs index 242f8016b..d0e80730e 100644 --- a/src/migrate/consolidate/evidence.rs +++ b/src/migrate/consolidate/evidence.rs @@ -177,6 +177,7 @@ async fn capture_graph_evidence( { peak_scratch_bytes = peak_scratch_bytes.max(snapshot.copied_bytes()); } + sqlite::quick_check_connection(snapshot.connection(), path).await?; sqlite::extend_graph_identities(snapshot.connection(), &mut identities).await?; let fingerprint = crate::sqlite_read_snapshot::family_fingerprint(path).map_err(io_error)?; diff --git a/src/migrate/consolidate/mod.rs b/src/migrate/consolidate/mod.rs index 61724e98a..757a527b6 100644 --- a/src/migrate/consolidate/mod.rs +++ b/src/migrate/consolidate/mod.rs @@ -34,7 +34,7 @@ use finalize::{cut_over_markers, register_destination, verify_destination}; use preflight::{acquire_store_locks, ensure_profile_offline, preflight_disk_space}; use prepare::prepare_destination; -use crate::branch_meta::{self, BranchMeta}; +use crate::branch_meta::{self, BranchEntry, BranchMeta}; use crate::errors::{Result, TraceDecayError}; use crate::global_db::{GlobalDb, GraphScopeUpsert, StoreArtifactUpsert, StoreInstanceUpsert}; use crate::storage::{ @@ -377,8 +377,10 @@ async fn resolve_plan_inner( &destination_project_id, )?; - let source_meta = load_required_branch_meta(&source_layout)?; - let target_meta = load_required_branch_meta(&target_layout)?; + let mut source_meta = load_required_branch_meta(&source_layout)?; + let mut target_meta = load_required_branch_meta(&target_layout)?; + recover_untracked_branch_graphs(&source_layout, &mut source_meta)?; + recover_untracked_branch_graphs(&target_layout, &mut target_meta)?; let source_graphs = graph_db_paths(&source_layout, &source_meta)?; let target_graphs = graph_db_paths(&target_layout, &target_meta)?; let session_paths = vec![ @@ -583,6 +585,53 @@ fn load_required_branch_meta(layout: &StoreLayout) -> Result { }) } +fn recover_untracked_branch_graphs(layout: &StoreLayout, meta: &mut BranchMeta) -> Result<()> { + for (relative, path) in relative_file_map(&layout.data_root)? { + if !relative.starts_with("branches") || !is_sqlite_database(&relative) { + continue; + } + if meta + .branches + .values() + .any(|entry| same_path(&layout.data_root.join(&entry.db_file), &path)) + { + continue; + } + let db_file = relative.to_str().ok_or_else(|| { + config_error(format!( + "untracked branch graph '{}' cannot be represented in branch metadata", + path.display() + )) + })?; + let base = relative + .file_stem() + .and_then(|value| value.to_str()) + .map(crate::branch::sanitize_branch_name) + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| "branch".to_string()); + let mut hash = Sha256::new(); + hash.update(relative.as_os_str().as_encoded_bytes()); + let name = format!("recovered/{base}-{}", &hex::encode(hash.finalize())[..16]); + if meta.branches.contains_key(&name) { + return Err(config_error(format!( + "recovered branch name collision for '{}'", + path.display() + ))); + } + meta.branches.insert( + name, + BranchEntry { + db_file: db_file.to_string(), + parent: Some(meta.default_branch.clone()), + created_at: "0".to_string(), + last_synced_at: "0".to_string(), + gc_protected: true, + }, + ); + } + Ok(()) +} + async fn inventory_store( graph: &GraphStoreEvidence, sessions: &crate::sqlite_read_snapshot::SnapshotSet, @@ -927,13 +976,7 @@ fn graph_db_paths_for_root(root: &Path, meta: &BranchMeta) -> Result Result Result { + let relative = Path::new(db_file); + if relative.as_os_str().is_empty() + || relative.is_absolute() + || relative + .components() + .any(|component| !matches!(component, std::path::Component::Normal(_))) + { + return Err(config_error(format!( + "branch graph path '{db_file}' is not a normalized store-relative path" + ))); + } + let path = root.join(relative); + let canonical_root = root.canonicalize().map_err(io_error)?; + let canonical_path = path.canonicalize().map_err(|error| { + config_error(format!( + "branch graph '{}' is missing: {error}", + path.display() + )) + })?; + if !canonical_path.starts_with(&canonical_root) { + return Err(config_error(format!( + "branch graph '{}' escapes profile shard '{}'", + path.display(), + root.display() + ))); + } + if !canonical_path.is_file() { + return Err(config_error(format!( + "branch graph '{}' is not a file", + path.display() + ))); + } + Ok(path) +} + fn same_path(left: &Path, right: &Path) -> bool { canonical_or_original(left) == canonical_or_original(right) } diff --git a/src/migrate/consolidate/prepare.rs b/src/migrate/consolidate/prepare.rs index 7c2682aab..0f4c0ade6 100644 --- a/src/migrate/consolidate/prepare.rs +++ b/src/migrate/consolidate/prepare.rs @@ -93,6 +93,7 @@ fn preserve_source_branch_graphs( .map(|parent| format!("{prefix}{parent}")), created_at: entry.created_at.clone(), last_synced_at: entry.last_synced_at.clone(), + gc_protected: true, }, ); maybe_stop(stop, PrepareStop::SourceBranch(index + 1))?; @@ -177,6 +178,7 @@ fn expected_prepared_files( .map(|parent| format!("{prefix}{parent}")), created_at: entry.created_at.clone(), last_synced_at: entry.last_synced_at.clone(), + gc_protected: true, }, ); } diff --git a/src/migrate/consolidate/tests.rs b/src/migrate/consolidate/tests.rs index a78e5fa7c..d6a2f98d5 100644 --- a/src/migrate/consolidate/tests.rs +++ b/src/migrate/consolidate/tests.rs @@ -669,6 +669,125 @@ async fn identity_survives_symlink_and_repository_move() { )); } +#[tokio::test] +async fn untracked_branch_databases_are_recovered_into_the_destination() { + const SOURCE_ORPHAN_FACT: &str = "fact unique to the source orphan branch"; + const TARGET_ORPHAN_FACT: &str = "fact unique to the target orphan branch"; + let fixture = fixture().await; + let source = layout_for_id(&fixture.project, &fixture.profile, &fixture.source_id).unwrap(); + let target = layout_for_id(&fixture.project, &fixture.profile, &fixture.target_id).unwrap(); + add_untracked_branch(&source, "orphan-source", SOURCE_ORPHAN_FACT).await; + add_untracked_branch(&target, "orphan-target", TARGET_ORPHAN_FACT).await; + + let options = fixture.options(); + let planned = plan(&options).await.unwrap(); + assert_eq!(planned.source.graph_databases, 2); + assert_eq!(planned.target.graph_databases, 2); + assert_eq!(planned.source.branches, 2); + assert_eq!(planned.target.branches, 2); + + let applied = apply(&options, &planned.confirmation_token).await.unwrap(); + let meta = branch_meta::load_branch_meta(&applied.destination_data_root).unwrap(); + let recovered = meta + .branches + .iter() + .filter(|(name, _)| name.contains("orphan-source-") || name.contains("orphan-target-")) + .map(|(name, entry)| { + assert!(entry.gc_protected); + (name.clone(), entry.db_file.clone()) + }) + .collect::>(); + assert_eq!(recovered.len(), 2); + assert!(recovered.iter().any(|(name, db_file)| { + name.starts_with("recovered/orphan-target-") && db_file == "branches/orphan-target.db" + })); + assert!(recovered.iter().any(|(name, db_file)| { + name.starts_with(&format!( + "consolidated/{}/recovered/orphan-source-", + fixture.source_id + )) && applied.destination_data_root.join(db_file).is_file() + })); + + let gc = crate::branch::gc_dead_branch_stores( + &fixture.project, + &applied.destination_data_root, + 0, + 0, + ); + assert!(gc.removed_tracked.is_empty()); + let reloaded = branch_meta::load_branch_meta(&applied.destination_data_root).unwrap(); + for (name, db_file) in recovered { + assert!(reloaded.branches[&name].gc_protected); + let path = applied.destination_data_root.join(db_file); + assert!(path.is_file()); + let expected = if name.contains("orphan-source-") { + SOURCE_ORPHAN_FACT + } else { + TARGET_ORPHAN_FACT + }; + let (db, _) = Database::open_read_only(&path).await.unwrap(); + let facts = MemoryStore::new(db.conn()) + .list_facts(None, Some(0.0), 100) + .await + .unwrap(); + assert!( + facts.iter().any(|fact| fact.content == expected), + "recovered branch '{name}' lost its unique fact" + ); + db.close(); + } +} + +#[tokio::test] +async fn corrupt_untracked_branch_database_is_rejected_before_mutation() { + let fixture = fixture().await; + let source = layout_for_id(&fixture.project, &fixture.profile, &fixture.source_id).unwrap(); + add_untracked_branch(&source, "corrupt-orphan", "fact in corrupt orphan").await; + let corrupt = source.data_root.join("branches/corrupt-orphan.db"); + let file = fs::OpenOptions::new().write(true).open(&corrupt).unwrap(); + file.set_len(fs::metadata(&corrupt).unwrap().len() / 2) + .unwrap(); + drop(file); + let before = full_tree_snapshot(&fixture.profile); + + let error = plan(&fixture.options()).await.unwrap_err(); + + assert!(error.to_string().contains("quick_check"), "{error}"); + assert_eq!( + full_tree_snapshot(&fixture.profile), + before, + "corrupt-input planning mutated the profile" + ); +} + +#[tokio::test] +async fn branch_metadata_paths_cannot_escape_the_profile_shard() { + let fixture = fixture().await; + let source = layout_for_id(&fixture.project, &fixture.profile, &fixture.source_id).unwrap(); + let outside = fixture.profile.join("outside.db"); + fs::copy(&source.graph_db_path, &outside).unwrap(); + let mut meta = branch_meta::load_branch_meta(&source.data_root).unwrap(); + + for db_file in [ + "../outside.db".to_string(), + outside.to_string_lossy().to_string(), + ] { + meta.branches.insert( + "unsafe".to_string(), + BranchEntry { + db_file, + parent: Some(meta.default_branch.clone()), + created_at: "0".to_string(), + last_synced_at: "0".to_string(), + gc_protected: false, + }, + ); + branch_meta::save_branch_meta(&source.data_root, &meta).unwrap(); + let error = plan(&fixture.options()).await.unwrap_err(); + assert!(error.to_string().contains("store-relative path"), "{error}"); + } +} + #[tokio::test] async fn a_third_matching_shard_is_rejected_as_ambiguous() { let fixture = fixture().await; @@ -1160,6 +1279,31 @@ fn add_branch_links(fixture: &Fixture, project_id: &str, count: usize) { branch_meta::save_branch_meta(&layout.data_root, &meta).unwrap(); } +async fn add_untracked_branch(layout: &StoreLayout, name: &str, fact_content: &str) { + let branches = layout.data_root.join("branches"); + fs::create_dir_all(&branches).unwrap(); + let path = branches.join(format!("{name}.db")); + fs::copy(&layout.graph_db_path, &path).unwrap(); + let (db, _) = Database::open(&path).await.unwrap(); + MemoryStore::new(db.conn()) + .add_fact( + AddFactRequest { + content: fact_content.to_string(), + category: MemoryCategory::Project, + source: Some("untracked-branch-test".to_string()), + tags: vec![name.to_string()], + entities: vec!["TraceDecay".to_string()], + trust: Some(0.8), + metadata: json!({"branch": name}), + }, + 0.5, + ) + .await + .unwrap(); + db.checkpoint().await.unwrap(); + db.close(); +} + fn sqlite_family_bytes(path: &Path) -> u64 { [ path.to_path_buf(),