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:
@@ -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)?;
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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() {
|
||||
|
||||
Reference in New Issue
Block a user