From 4b657e71f127b0141d424efce5d42f9d4702b1e3 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 9 May 2026 11:39:03 -0400 Subject: [PATCH] refactor(cli): batched purge in cmd_purge and cmd_trash_empty (Plan B Phase 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renames purge_item to purge_item_filesystem — body becomes filesystem-only (remove item.enc, remove attachments//, manifest.remove). Returns the relative paths it removed. cmd_purge and cmd_trash_empty accumulate the paths and fire ONE git rm + ONE git add + ONE git commit per invocation. A 50-item trash empty now produces 3 git subprocesses regardless of N (was N+2). New regression test trash_empty_batches_into_one_commit asserts the one-commit invariant via git rev-list --count. --- crates/relicario-cli/src/commands/trash.rs | 48 ++++++++++------ crates/relicario-cli/tests/basic_flows.rs | 66 ++++++++++++++++++++++ 2 files changed, 97 insertions(+), 17 deletions(-) diff --git a/crates/relicario-cli/src/commands/trash.rs b/crates/relicario-cli/src/commands/trash.rs index 1f13c36..c9af462 100644 --- a/crates/relicario-cli/src/commands/trash.rs +++ b/crates/relicario-cli/src/commands/trash.rs @@ -39,29 +39,29 @@ pub fn cmd_restore(query: String) -> Result<()> { Ok(()) } -/// Inner purge: assumes vault is already unlocked and manifest is loaded. -/// Caller is responsible for saving the manifest and committing afterwards. -pub(super) fn purge_item( +/// Filesystem-only purge: removes the item.enc, attachments//, and updates +/// the manifest in memory. Returns the relative paths the caller must stage +/// via `git rm` after the loop. Does NOT invoke any git commands — the caller +/// batches them. +pub(super) fn purge_item_filesystem( vault: &crate::session::UnlockedVault, manifest: &mut relicario_core::Manifest, id: &relicario_core::ItemId, title: &str, -) -> Result<()> { +) -> Result> { use std::fs; + let item_rel = format!("items/{}.enc", id.as_str()); + let att_rel = format!("attachments/{}", id.as_str()); + let item_path = vault.item_path(id); if item_path.exists() { fs::remove_file(&item_path)?; } let att_dir = vault.root().join("attachments").join(id.as_str()); if att_dir.exists() { fs::remove_dir_all(&att_dir)?; } manifest.remove(id); - let _ = crate::helpers::git_command(vault.root(), &["rm", "-rf", "--ignore-unmatch", - &format!("items/{}.enc", id.as_str()), - &format!("attachments/{}", id.as_str()), - ]).status()?; - // Note: caller adds+commits manifest.enc after processing all purges. eprintln!("Purged: {title}"); - Ok(()) + Ok(vec![item_rel, att_rel]) } pub fn cmd_purge(query: String) -> Result<()> { @@ -72,11 +72,19 @@ pub fn cmd_purge(query: String) -> Result<()> { let title = entry.title.clone(); let _ = entry; - purge_item(&vault, &mut manifest, &id, &title)?; + let paths = purge_item_filesystem(&vault, &mut manifest, &id, &title)?; vault.after_manifest_change(&manifest)?; let purge_ctx = format!("purge \"{}\" ({})", title, id.as_str()); - crate::helpers::git_run(vault.root(), &["add", "manifest.enc"], &format!("{purge_ctx}: git add manifest.enc"))?; + let mut rm_args: Vec<&str> = vec!["rm", "-rf", "--ignore-unmatch"]; + let path_refs: Vec<&str> = paths.iter().map(String::as_str).collect(); + rm_args.extend(path_refs.iter().copied()); + crate::helpers::git_run(vault.root(), &rm_args, &format!("{purge_ctx}: git rm"))?; + crate::helpers::git_run( + vault.root(), + &["add", "manifest.enc"], + &format!("{purge_ctx}: git add manifest.enc"), + )?; crate::helpers::git_run( vault.root(), &["commit", "-m", &format!("purge: {} ({})", title, id.as_str())], @@ -113,13 +121,19 @@ pub fn cmd_trash_empty() -> Result<()> { return Ok(()); } - let mut purged_titles = Vec::new(); + let mut all_paths: Vec = Vec::new(); + let purged_count = purgeable.len(); for (id, title) in purgeable { - purge_item(&vault, &mut manifest, &id, &title)?; - purged_titles.push(title); + let mut paths = purge_item_filesystem(&vault, &mut manifest, &id, &title)?; + all_paths.append(&mut paths); } vault.after_manifest_change(&manifest)?; + + let mut rm_args: Vec<&str> = vec!["rm", "-rf", "--ignore-unmatch"]; + let path_refs: Vec<&str> = all_paths.iter().map(String::as_str).collect(); + rm_args.extend(path_refs.iter().copied()); + crate::helpers::git_run(vault.root(), &rm_args, "trash empty: git rm")?; crate::helpers::git_run( vault.root(), &["add", "manifest.enc"], @@ -127,10 +141,10 @@ pub fn cmd_trash_empty() -> Result<()> { )?; crate::helpers::git_run( vault.root(), - &["commit", "-m", &format!("trash empty: purged {} item(s)", purged_titles.len())], + &["commit", "-m", &format!("trash empty: purged {} item(s)", purged_count)], "trash empty: git commit", )?; - eprintln!("Emptied trash: {} item(s)", purged_titles.len()); + eprintln!("Emptied trash: {} item(s)", purged_count); Ok(()) } diff --git a/crates/relicario-cli/tests/basic_flows.rs b/crates/relicario-cli/tests/basic_flows.rs index 8f85095..832e066 100644 --- a/crates/relicario-cli/tests/basic_flows.rs +++ b/crates/relicario-cli/tests/basic_flows.rs @@ -109,6 +109,72 @@ fn rm_restore_purge_cycle() { assert!(!String::from_utf8(out.stdout).unwrap().contains("target")); } +#[test] +fn trash_empty_batches_into_one_commit() { + let v = TestVault::init(); + + // Add 3 items. + for title in ["alpha", "bravo", "charlie"] { + let out = v.run(&[ + "add", "login", + "--title", title, + "--username", "u", + "--password", "p", + ]); + assert!(out.status.success(), "add {title} failed"); + } + + // Soft-delete all 3. + for title in ["alpha", "bravo", "charlie"] { + let out = v.run(&["rm", title]); + assert!(out.status.success(), "rm {title} failed"); + } + + // Set retention to 0 days so the recently-trashed items become purgeable + // (should_purge: now - trashed_at > 0 * 86400 = 0). + let out = v.run(&["settings", "trash-retention", "--days", "0"]); + assert!(out.status.success(), "settings trash-retention failed"); + + // Brief sleep to ensure now > trashed_at by at least 1 second + // (otherwise should_purge returns false at strict-greater-than equality). + std::thread::sleep(std::time::Duration::from_secs(1)); + + // Count commits before. + let before = std::process::Command::new("git") + .args(["rev-list", "--count", "HEAD"]) + .current_dir(v.path()) + .output() + .unwrap(); + let before_count: u32 = String::from_utf8(before.stdout).unwrap().trim().parse().unwrap(); + + // Run trash empty. + let out = v.run(&["trash", "empty"]); + assert!(out.status.success(), "trash empty failed: stderr={}", + String::from_utf8_lossy(&out.stderr)); + + // Count commits after. + let after = std::process::Command::new("git") + .args(["rev-list", "--count", "HEAD"]) + .current_dir(v.path()) + .output() + .unwrap(); + let after_count: u32 = String::from_utf8(after.stdout).unwrap().trim().parse().unwrap(); + + assert_eq!( + after_count - before_count, 1, + "trash empty should fire exactly one commit; before={before_count} after={after_count}" + ); + + // The remaining `list --trashed` should be empty. + let out = v.run(&["list", "--trashed"]); + let stdout = String::from_utf8(out.stdout).unwrap(); + let stderr = String::from_utf8(out.stderr).unwrap(); + assert!( + !stdout.contains("alpha") && !stdout.contains("bravo") && !stdout.contains("charlie"), + "items still in trashed list: stdout={stdout} stderr={stderr}" + ); +} + #[test] fn generate_random_and_bip39() { let dir = tempfile::TempDir::new().unwrap();