merge(cycle-2): land Stream B — Plan B Phases 4+5+6 (session/manifest discipline)
4 commits from feature/cli-tail-stream-b-session-manifest: -2e41e0brefactor(cli): single canonical ParamsFile in session.rs (Phase 5) -7901c27refactor(cli): Vault::after_manifest_change wrapper (Phase 4) -4b657e7refactor(cli): batched purge in cmd_purge and cmd_trash_empty (Phase 6) -c4777ccrefactor(cli): apply simplify findings (Phases 4-6 polish) Phase 4 complete: Vault::after_manifest_change wrapper funnels NINE manifest- mutation sites (not 7 as the spec/notes flagged -- attach.rs add+detach, import.rs LastPass, and trash.rs cmd_trash_empty all previously SKIPPED refresh_groups_cache; the wrapper now refreshes them as a side-effect). save_manifest was DROPPED entirely (rather than just demoted to pub(crate) as the spec said) -- the simplify pass found no escape hatch was needed, so the only path to write the manifest now goes through the wrapper. Stronger than spec. Phase 5 complete: single pub(crate) struct ParamsFile in session.rs at module level with Serialize+Deserialize. Constructors for_new_vault and to_kdf_params (simplify pass changed into_kdf_params(self) to to_kdf_params(&self) for ergonomics). commands/init.rs uses ParamsFile::for_new_vault. On-disk JSON schema verified BYTE-STABLE via fixture-string round-trip test (session::tests::params_file_round_trips_current_layout + for_new_vault_produces_expected_shape) -- same fields, same ordering, same rename_all placement. Existing vaults read with no migration. Phase 6 complete: purge_item renamed purge_item_filesystem, mutates only filesystem + manifest, returns Vec<String> of paths. cmd_purge and cmd_trash_empty both follow after_manifest_change -> git_rm -> git add -> git commit. New helpers::git_rm extracted to DRY the pattern. Strict invariant locked: tests/basic_flows.rs::trash_empty_batches_into_one_commit counts commits via git rev-list --count HEAD before/after and asserts delta == 1. A 50-item trash empty now fires 3 git invocations, not 52. Simplify polish (c4777cc): all 5 findings legitimate, none rationale-skipped: - Dropped redundant save_manifest_raw escape hatch - Value-vs-self ergonomic fix (to_kdf_params(&self)) - DRY git_rm helper - TOCTOU pre-check dropped from purge_item_filesystem - Comment trim 3-way merge with stream-a (3dd1e1b) and stream-c (e69b347) clean: git auto-resolved commands/add.rs (stream-a prompt_or_flag changes interleaved with stream-b after_manifest_change call at the manifest-mutation site). Verified semantic correctness via post-merge cargo test. Pre-merge checklist on tipc4777cc+ post-merge verification: - cargo test --workspace standalone: 260 tests, 0 failures - cargo test --workspace post-merge: 281 tests, 0 failures - cargo clippy --workspace --all-targets -- -D warnings: silent - cargo build -p relicario-wasm --target wasm32-unknown-unknown: clean - Independent fresh-subagent code review: APPROVE - grep refresh_groups_cache crates/relicario-cli/src/: zero matches outside session.rs/helpers.rs (per spec done-criteria) - grep struct ParamsFile crates/relicario-cli/src/: ONE match (per spec done-criteria) Plan B COMPLETE. With Phase 3 (Stream A) merged at3dd1e1band Phases 7+8 (Stream C) merged ate69b347, all eight Plan B phases are now on main. One nit deferred (per subagent review): trash empty partial-failure recovery -- if git_rm fails after after_manifest_change succeeds, manifest.enc is rewritten in-tree and items are removed from disk but no commit is made. Pre-existing behavior was strictly worse (per-item interleaved partial-commit risk); current state is a net improvement. Tree-cleanup-on-failure belongs in a follow-up plan, not this PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -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<String> = vec![
|
||||
format!("items/{}.enc", item.id.as_str()),
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -41,8 +41,7 @@ pub fn cmd_edit(query: String, totp_qr: Option<PathBuf>) -> 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());
|
||||
|
||||
@@ -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)?;
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -65,17 +65,7 @@ pub fn cmd_init(image: PathBuf, output: PathBuf) -> Result<()> {
|
||||
fs::write(relicario_dir.join("salt"), salt)?;
|
||||
fs::write(
|
||||
relicario_dir.join("params.json"),
|
||||
serde_json::to_string_pretty(&ParamsFile {
|
||||
format_version: 2,
|
||||
kdf: ParamsKdf {
|
||||
algorithm: "argon2id-v0x13".into(),
|
||||
argon2_m: params.argon2_m,
|
||||
argon2_t: params.argon2_t,
|
||||
argon2_p: params.argon2_p,
|
||||
},
|
||||
aead: "xchacha20poly1305".into(),
|
||||
salt_path: ".relicario/salt".into(),
|
||||
})?,
|
||||
serde_json::to_string_pretty(&crate::session::ParamsFile::for_new_vault(¶ms))?,
|
||||
)?;
|
||||
let manifest = Manifest::new();
|
||||
fs::write(root.join("manifest.enc"), encrypt_manifest(&manifest, &master_key)?)?;
|
||||
@@ -106,20 +96,3 @@ pub fn cmd_init(image: PathBuf, output: PathBuf) -> Result<()> {
|
||||
eprintln!(" \u{2192} back this file up somewhere safe; it is your second factor.");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(serde::Serialize)]
|
||||
struct ParamsFile {
|
||||
format_version: u32,
|
||||
kdf: ParamsKdf,
|
||||
aead: String,
|
||||
salt_path: String,
|
||||
}
|
||||
|
||||
#[derive(serde::Serialize)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
struct ParamsKdf {
|
||||
algorithm: String,
|
||||
argon2_m: u32,
|
||||
argon2_t: u32,
|
||||
argon2_p: u32,
|
||||
}
|
||||
|
||||
@@ -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<ItemType> = match type_filter.as_deref() {
|
||||
None => None,
|
||||
|
||||
@@ -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,37 +32,41 @@ 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);
|
||||
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/<id>/, 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<()> {
|
||||
use std::fs;
|
||||
) -> Result<Vec<String>> {
|
||||
use std::{fs, io::ErrorKind};
|
||||
|
||||
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 item_rel = format!("items/{}.enc", id.as_str());
|
||||
let att_rel = format!("attachments/{}", id.as_str());
|
||||
|
||||
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);
|
||||
|
||||
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<()> {
|
||||
@@ -74,12 +77,16 @@ pub fn cmd_purge(query: String) -> Result<()> {
|
||||
let title = entry.title.clone();
|
||||
let _ = entry;
|
||||
|
||||
purge_item(&vault, &mut manifest, &id, &title)?;
|
||||
vault.save_manifest(&manifest)?;
|
||||
crate::refresh_groups_cache(vault.root(), &manifest);
|
||||
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"))?;
|
||||
crate::helpers::git_rm(vault.root(), &paths, &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())],
|
||||
@@ -116,13 +123,16 @@ pub fn cmd_trash_empty() -> Result<()> {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let mut purged_titles = Vec::new();
|
||||
let mut all_paths: Vec<String> = 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.save_manifest(&manifest)?;
|
||||
vault.after_manifest_change(&manifest)?;
|
||||
|
||||
crate::helpers::git_rm(vault.root(), &all_paths, "trash empty: git rm")?;
|
||||
crate::helpers::git_run(
|
||||
vault.root(),
|
||||
&["add", "manifest.enc"],
|
||||
@@ -130,10 +140,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(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user