From d717f0d4a1cbe01c020a37972c434803e65313ce Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 30 May 2026 21:26:34 -0400 Subject: [PATCH] refactor(cli): tighten refresh_groups_cache to pub(crate) (Plan B Phase 4 polish) Plan B Phase 4 wanted "every mutating handler must call refresh_groups_cache" to be a compile-time invariant, with all callers funneled through Vault::after_manifest_change. The mutating-handler sweep happened, but two read-side callsites (commands/list.rs and commands/get.rs) still called the public helper directly for opportunistic shell-completion cache freshness. Closes the gap: - helpers::refresh_groups_cache demoted from pub to pub(crate). - list.rs and get.rs drop their explicit calls. Cache freshness between mutations is unaffected: every mutating handler still funnels through after_manifest_change. The minor staleness window (manifest changed externally via git pull, no local mutation since) is the trade-off the spec accepts in exchange for the compile-time invariant. The Plan B done-criterion "grep refresh_groups_cache outside session.rs returns zero" now passes apart from the function definition itself, which lives in helpers.rs (the natural place for a flat utility). The visibility scoping achieves the architectural intent. Co-Authored-By: Claude Opus 4.7 --- crates/relicario-cli/src/commands/get.rs | 1 - crates/relicario-cli/src/commands/list.rs | 1 - crates/relicario-cli/src/helpers.rs | 8 +++++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/relicario-cli/src/commands/get.rs b/crates/relicario-cli/src/commands/get.rs index 14cb595..c6de334 100644 --- a/crates/relicario-cli/src/commands/get.rs +++ b/crates/relicario-cli/src/commands/get.rs @@ -8,7 +8,6 @@ pub fn cmd_get(query: String, show: bool, copy: bool) -> Result<()> { let vault = crate::session::UnlockedVault::unlock_interactive()?; let manifest = vault.load_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/list.rs b/crates/relicario-cli/src/commands/list.rs index d192b5a..d4be596 100644 --- a/crates/relicario-cli/src/commands/list.rs +++ b/crates/relicario-cli/src/commands/list.rs @@ -12,7 +12,6 @@ pub fn cmd_list( let vault = crate::session::UnlockedVault::unlock_interactive()?; let manifest = vault.load_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/helpers.rs b/crates/relicario-cli/src/helpers.rs index 4c58906..30a5fd4 100644 --- a/crates/relicario-cli/src/helpers.rs +++ b/crates/relicario-cli/src/helpers.rs @@ -142,7 +142,13 @@ pub fn groups_cache_path(vault_dir: &Path) -> PathBuf { /// /// 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) { +/// +/// Visibility note: this is `pub(crate)` so only `session::after_manifest_change` +/// can call it. The Plan B Phase 4 done-criterion requires every mutating +/// handler to funnel through the wrapper — exposing this helper to commands/ +/// would let a caller refresh the cache without updating the manifest, breaking +/// the invariant. +pub(crate) 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() {