From 7901c2758d4358448e1848a0a6f27d3e1ad62beb Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 9 May 2026 11:29:52 -0400 Subject: [PATCH] refactor(cli): Vault::after_manifest_change wrapper (Plan B Phase 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the canonical post-mutation funnel: save_manifest_raw + groups.cache refresh in one method. Converts nine commands/*.rs mutation callsites from the manual save_manifest + refresh_groups_cache pair to a single vault.after_manifest_change(&manifest)?. save_manifest renamed to save_manifest_raw (pub(crate)) so future commands cannot accidentally bypass the cache refresh. Four of the nine sites (attach.rs add/detach, import.rs LastPass, trash.rs cmd_trash_empty's per-item save) previously skipped the cache refresh — the wrapper fixes them. refresh_groups_cache moves from main.rs to helpers.rs so the read-side warmup callers in get.rs/list.rs still reach it. --- crates/relicario-cli/src/commands/add.rs | 3 +- crates/relicario-cli/src/commands/attach.rs | 4 +-- crates/relicario-cli/src/commands/edit.rs | 3 +- crates/relicario-cli/src/commands/get.rs | 2 +- crates/relicario-cli/src/commands/import.rs | 2 +- crates/relicario-cli/src/commands/list.rs | 2 +- crates/relicario-cli/src/commands/trash.rs | 11 +++---- crates/relicario-cli/src/helpers.rs | 18 ++++++++++++ crates/relicario-cli/src/main.rs | 18 ------------ crates/relicario-cli/src/session.rs | 32 ++++++++++++++++++++- 10 files changed, 60 insertions(+), 35 deletions(-) diff --git a/crates/relicario-cli/src/commands/add.rs b/crates/relicario-cli/src/commands/add.rs index 6d58154..f9c72a5 100644 --- a/crates/relicario-cli/src/commands/add.rs +++ b/crates/relicario-cli/src/commands/add.rs @@ -36,8 +36,7 @@ pub fn cmd_add(kind: AddKind) -> Result<()> { vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; let mut paths: Vec = vec![ format!("items/{}.enc", item.id.as_str()), diff --git a/crates/relicario-cli/src/commands/attach.rs b/crates/relicario-cli/src/commands/attach.rs index 8b99496..8185334 100644 --- a/crates/relicario-cli/src/commands/attach.rs +++ b/crates/relicario-cli/src/commands/attach.rs @@ -72,7 +72,7 @@ pub fn cmd_attach(query: String, file: PathBuf) -> Result<()> { item.modified = now_unix(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; let paths = [ format!("items/{}.enc", item.id.as_str()), @@ -161,7 +161,7 @@ pub fn cmd_detach(query: String, aid: String) -> Result<()> { item.modified = now_unix(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; let item_path = format!("items/{}.enc", item.id.as_str()); let blob_relpath = format!("attachments/{}/{}.enc", item.id.as_str(), removed.id.as_str()); diff --git a/crates/relicario-cli/src/commands/edit.rs b/crates/relicario-cli/src/commands/edit.rs index a480021..798aa52 100644 --- a/crates/relicario-cli/src/commands/edit.rs +++ b/crates/relicario-cli/src/commands/edit.rs @@ -41,8 +41,7 @@ pub fn cmd_edit(query: String, totp_qr: Option) -> Result<()> { item.modified = now_unix(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; super::commit_paths(&vault, &format!("edit: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?; eprintln!("Updated {}", item.id.as_str()); diff --git a/crates/relicario-cli/src/commands/get.rs b/crates/relicario-cli/src/commands/get.rs index 4618a3a..14cb595 100644 --- a/crates/relicario-cli/src/commands/get.rs +++ b/crates/relicario-cli/src/commands/get.rs @@ -8,7 +8,7 @@ pub fn cmd_get(query: String, show: bool, copy: bool) -> Result<()> { let vault = crate::session::UnlockedVault::unlock_interactive()?; let manifest = vault.load_manifest()?; - crate::refresh_groups_cache(vault.root(), &manifest); + crate::helpers::refresh_groups_cache(vault.root(), &manifest); let entry = super::resolve_query(&manifest, &query)?; let item = vault.load_item(&entry.id)?; diff --git a/crates/relicario-cli/src/commands/import.rs b/crates/relicario-cli/src/commands/import.rs index f4b5713..5353305 100644 --- a/crates/relicario-cli/src/commands/import.rs +++ b/crates/relicario-cli/src/commands/import.rs @@ -49,7 +49,7 @@ fn cmd_import_lastpass(csv_path: PathBuf) -> Result<()> { } } - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; written_paths.push("manifest.enc".into()); let path_refs: Vec<&str> = written_paths.iter().map(String::as_str).collect(); diff --git a/crates/relicario-cli/src/commands/list.rs b/crates/relicario-cli/src/commands/list.rs index 1c153a5..d192b5a 100644 --- a/crates/relicario-cli/src/commands/list.rs +++ b/crates/relicario-cli/src/commands/list.rs @@ -12,7 +12,7 @@ pub fn cmd_list( let vault = crate::session::UnlockedVault::unlock_interactive()?; let manifest = vault.load_manifest()?; - crate::refresh_groups_cache(vault.root(), &manifest); + crate::helpers::refresh_groups_cache(vault.root(), &manifest); let parsed_type: Option = match type_filter.as_deref() { None => None, diff --git a/crates/relicario-cli/src/commands/trash.rs b/crates/relicario-cli/src/commands/trash.rs index 65b41cd..1f13c36 100644 --- a/crates/relicario-cli/src/commands/trash.rs +++ b/crates/relicario-cli/src/commands/trash.rs @@ -15,8 +15,7 @@ pub fn cmd_rm(query: String) -> Result<()> { item.soft_delete(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; super::commit_paths(&vault, &format!("trash: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?; eprintln!("Moved to trash: {}", item.title); @@ -33,8 +32,7 @@ pub fn cmd_restore(query: String) -> Result<()> { item.restore(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; super::commit_paths(&vault, &format!("restore: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?; eprintln!("Restored: {}", item.title); @@ -75,8 +73,7 @@ pub fn cmd_purge(query: String) -> Result<()> { let _ = entry; purge_item(&vault, &mut manifest, &id, &title)?; - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + 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"))?; @@ -122,7 +119,7 @@ pub fn cmd_trash_empty() -> Result<()> { purged_titles.push(title); } - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; crate::helpers::git_run( vault.root(), &["add", "manifest.enc"], diff --git a/crates/relicario-cli/src/helpers.rs b/crates/relicario-cli/src/helpers.rs index f07b266..a1f356a 100644 --- a/crates/relicario-cli/src/helpers.rs +++ b/crates/relicario-cli/src/helpers.rs @@ -126,6 +126,24 @@ pub fn groups_cache_path(vault_dir: &Path) -> PathBuf { vault_dir.join(".relicario").join("groups.cache") } +/// Collect all non-empty group names from the manifest and write them to the +/// plaintext `groups.cache` file so shell completion can enumerate `--group` +/// candidates without prompting for the vault passphrase. +/// +/// Failures are silently swallowed — a missing cache is merely a UX degradation, +/// not a correctness problem. +pub fn refresh_groups_cache(vault_dir: &Path, manifest: &relicario_core::Manifest) { + let mut set = std::collections::BTreeSet::::new(); + for entry in manifest.items.values() { + if let Some(g) = entry.group.as_ref() { + if !g.is_empty() { + set.insert(g.clone()); + } + } + } + let _ = write_groups_cache(vault_dir, &set); +} + /// Write the sorted set of group names to `/.relicario/groups.cache`, /// one name per line. In debug builds, setting `RELICARIO_NO_GROUPS_CACHE` /// suppresses the write (developer debugging tool). In release builds the env diff --git a/crates/relicario-cli/src/main.rs b/crates/relicario-cli/src/main.rs index 8a32f5a..ab59cdb 100644 --- a/crates/relicario-cli/src/main.rs +++ b/crates/relicario-cli/src/main.rs @@ -457,24 +457,6 @@ fn main() -> Result<()> { } } -/// Collect all non-empty group names from the manifest and write them to the -/// plaintext `groups.cache` file so shell completion can enumerate `--group` -/// candidates without prompting for the vault passphrase. -/// -/// Failures are silently swallowed — a missing cache is merely a UX degradation, -/// not a correctness problem. -pub(crate) fn refresh_groups_cache(vault_dir: &std::path::Path, manifest: &relicario_core::Manifest) { - let mut set = std::collections::BTreeSet::::new(); - for entry in manifest.items.values() { - if let Some(g) = entry.group.as_ref() { - if !g.is_empty() { - set.insert(g.clone()); - } - } - } - let _ = helpers::write_groups_cache(vault_dir, &set); -} - /// Check for test passphrase override (debug builds only; stripped from release). #[cfg(debug_assertions)] pub(crate) fn test_passphrase_override() -> Option { diff --git a/crates/relicario-cli/src/session.rs b/crates/relicario-cli/src/session.rs index 7bbf68e..f58c6df 100644 --- a/crates/relicario-cli/src/session.rs +++ b/crates/relicario-cli/src/session.rs @@ -69,7 +69,20 @@ impl UnlockedVault { Ok(decrypt_manifest(&bytes, &self.master_key)?) } - pub fn save_manifest(&self, manifest: &Manifest) -> Result<()> { + /// Save the manifest and refresh the plaintext groups.cache. This is the + /// canonical "I just mutated the manifest" funnel — every command that + /// changes the manifest goes through this method, so cache freshness is + /// a compile-time invariant rather than a discipline rule. + pub fn after_manifest_change(&self, manifest: &Manifest) -> Result<()> { + self.save_manifest_raw(manifest)?; + crate::helpers::refresh_groups_cache(&self.root, manifest); + Ok(()) + } + + /// Encrypt the manifest and atomically write it. Most callers want + /// `after_manifest_change` instead — this method skips the groups.cache + /// refresh, leaving shell completion stale until the next mutation. + pub(crate) fn save_manifest_raw(&self, manifest: &Manifest) -> Result<()> { let bytes = encrypt_manifest(manifest, &self.master_key)?; atomic_write(&self.manifest_path(), &bytes) } @@ -252,4 +265,21 @@ mod tests { assert_eq!(v["aead"], "xchacha20poly1305"); assert_eq!(v["salt_path"], ".relicario/salt"); } + + #[test] + fn after_manifest_change_writes_manifest_and_groups_cache() { + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path().to_path_buf(); + std::fs::create_dir_all(root.join(".relicario")).unwrap(); + std::fs::create_dir_all(root.join("items")).unwrap(); + let vault = UnlockedVault { + root: root.clone(), + master_key: Zeroizing::new([0u8; 32]), + }; + let manifest = Manifest::new(); + + vault.after_manifest_change(&manifest).unwrap(); + assert!(root.join("manifest.enc").exists()); + assert!(root.join(".relicario/groups.cache").exists()); + } }