refactor(cli): apply simplify findings (Plan B Phases 4-6 polish)

- session.rs: drop save_manifest_raw — its only caller was
  after_manifest_change itself; the pub(crate) advertised the exact
  bypass-the-cache-refresh footgun the wrapper exists to eliminate.
  Inline the encrypt + atomic_write pair.
- session.rs: into_kdf_params(self) → to_kdf_params(&self). Body just
  copies three u32s; the consume-self had no ownership benefit and
  forced the round-trip test to rebuild a ParamsFile field-by-field.
- helpers.rs: add git_rm(repo, paths, context) wrapper around git_run
  + the load-bearing --ignore-unmatch flag. Replaces two near-identical
  three-line "build rm_args, extend, git_run" blocks in trash.rs.
- trash.rs: purge_item_filesystem drops the if x.exists() pre-checks
  (TOCTOU + redundant stat per item per trash-empty iteration). Uses
  ErrorKind::NotFound swallow on remove_file/remove_dir_all instead.
- basic_flows.rs: trim trash_empty_batches_into_one_commit's sleep
  comment to just the WHY.

crates/relicario-cli/src/commands/trash.rs

@@ -49,15 +49,20 @@ pub(super) fn purge_item_filesystem(
     id: &relicario_core::ItemId,
     title: &str,
 ) -> Result<Vec<String>> {
-    use std::fs;
+    use std::{fs, io::ErrorKind};
 
     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)?; }
+    let ignore_missing = |r: std::io::Result<()>| -> Result<()> {
+        match r {
+            Ok(()) => Ok(()),
+            Err(e) if e.kind() == ErrorKind::NotFound => Ok(()),
+            Err(e) => Err(e.into()),
+        }
+    };
+    ignore_missing(fs::remove_file(vault.item_path(id)))?;
+    ignore_missing(fs::remove_dir_all(vault.root().join("attachments").join(id.as_str())))?;
     manifest.remove(id);
 
     eprintln!("Purged: {title}");
@@ -76,10 +81,7 @@ pub fn cmd_purge(query: String) -> Result<()> {
     vault.after_manifest_change(&manifest)?;
 
     let purge_ctx = format!("purge \"{}\" ({})", title, id.as_str());
-    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_rm(vault.root(), &paths, &format!("{purge_ctx}: git rm"))?;
     crate::helpers::git_run(
         vault.root(),
         &["add", "manifest.enc"],
@@ -130,10 +132,7 @@ pub fn cmd_trash_empty() -> Result<()> {
 
     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_rm(vault.root(), &all_paths, "trash empty: git rm")?;
     crate::helpers::git_run(
         vault.root(),
         &["add", "manifest.enc"],

crates/relicario-cli/src/helpers.rs

@@ -86,6 +86,16 @@ pub fn git_run(repo: &Path, args: &[&str], context: &str) -> Result<()> {
     Ok(())
 }
 
+/// Stage `paths` for removal in one `git rm -rf --ignore-unmatch` invocation.
+/// `--ignore-unmatch` is load-bearing: a previous partial-write crash can
+/// leave the manifest entry without the corresponding `items/<id>.enc` on
+/// disk, and we want the rm to succeed regardless.
+pub fn git_rm(repo: &Path, paths: &[String], context: &str) -> Result<()> {
+    let mut args: Vec<&str> = vec!["rm", "-rf", "--ignore-unmatch"];
+    args.extend(paths.iter().map(String::as_str));
+    git_run(repo, &args, context)
+}
+
 /// Format a Unix-seconds timestamp as an ISO-8601 UTC string.
 /// Audit M11: replaces the old `now_iso8601` helper that actually returned
 /// a numeric string.

crates/relicario-cli/src/session.rs

@@ -74,19 +74,12 @@ impl UnlockedVault {
     /// 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)?;
+        let bytes = encrypt_manifest(manifest, &self.master_key)?;
+        atomic_write(&self.manifest_path(), &bytes)?;
         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)
-    }
-
     pub fn load_settings(&self) -> Result<VaultSettings> {
         let bytes = fs::read(self.settings_path()).context("failed to read settings.enc")?;
         Ok(decrypt_settings(&bytes, &self.master_key)?)
@@ -152,7 +145,7 @@ impl ParamsFile {
         }
     }
 
-    pub fn into_kdf_params(self) -> KdfParams {
+    pub fn to_kdf_params(&self) -> KdfParams {
         KdfParams {
             argon2_m: self.kdf.argon2_m,
             argon2_t: self.kdf.argon2_t,
@@ -165,7 +158,7 @@ fn read_params(root: &Path) -> Result<KdfParams> {
     let s = fs::read_to_string(root.join(".relicario").join("params.json"))
         .context("failed to read .relicario/params.json")?;
     let pf: ParamsFile = serde_json::from_str(&s).context("failed to parse params.json")?;
-    Ok(pf.into_kdf_params())
+    Ok(pf.to_kdf_params())
 }
 
 /// Locate the reference image path via `RELICARIO_IMAGE` env var or interactive prompt.
@@ -225,18 +218,7 @@ mod tests {
         assert_eq!(pf.aead, "xchacha20poly1305");
         assert_eq!(pf.salt_path, ".relicario/salt");
 
-        let kdf = ParamsFile {
-            format_version: pf.format_version,
-            kdf: ParamsKdf {
-                algorithm: pf.kdf.algorithm.clone(),
-                argon2_m: pf.kdf.argon2_m,
-                argon2_t: pf.kdf.argon2_t,
-                argon2_p: pf.kdf.argon2_p,
-            },
-            aead: pf.aead.clone(),
-            salt_path: pf.salt_path.clone(),
-        }
-        .into_kdf_params();
+        let kdf = pf.to_kdf_params();
         assert_eq!(kdf.argon2_m, 65536);
         assert_eq!(kdf.argon2_t, 3);
         assert_eq!(kdf.argon2_p, 4);

crates/relicario-cli/tests/basic_flows.rs

@@ -135,8 +135,8 @@ fn trash_empty_batches_into_one_commit() {
     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).
+    // should_purge uses strict > on (now - trashed_at), so equal-second
+    // timestamps don't qualify.
     std::thread::sleep(std::time::Duration::from_secs(1));
 
     // Count commits before.