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 <noreply@anthropic.com>
This commit is contained in:
adlee-was-taken
2026-05-30 21:26:34 -04:00
parent d2d11a4c9f
commit d717f0d4a1
3 changed files with 7 additions and 3 deletions

View File

@@ -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)?;

View File

@@ -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<ItemType> = match type_filter.as_deref() {
None => None,

View File

@@ -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::<String>::new();
for entry in manifest.items.values() {
if let Some(g) = entry.group.as_ref() {