pm: garbage-collect the virtual store and extracted-tree tiers

[colinhacks]

store prune only walked the CAS. Nothing collected the global virtual store or <store>/v1/trees/. Both key on the dep path plus its graph hash, and calc_deps_hash folds each child's hash into every ancestor, so one dep bump strands that package and every ancestor reaching it. One dev machine held 5,999 and 26,143 entries, 6.8%/11.7% of them churn duplicates.

Installs now record the project at <virtual-store>/.projects/<hash>; prune marks what registered projects reach and sweeps the rest, as pnpm's pruneGlobalVirtualStore does. An empty registry prunes nothing. Also splits out the CAS sweep, which returned early when the CAS root was absent and skipped the tiers.

6 unit tests, each proven to fail when its guard is removed, plus 11 e2e cases in tests/store-prune/.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
nub	Ready	Preview	Aug 17, 2026 11:44pm

[pullfrog]

Important

store prune can delete a global-virtual-store entry that a concurrently running install has already materialized but not yet symlinked. The install still exits 0.

Reviewed changes — the full diff at 5dc26e8 (6 files, 1 commit), plus the linker's two-phase GVS write path, the registration call site's gating, dep_path_to_filename's output space, the CI wiring for vendor/aube tests, and pnpm's actual pruneGlobalVirtualStore/projectRegistry sources.

• Project registry inside the virtual store — PROJECTS_SUBDIR/projects_dir()/register_project()/registered_projects() in aube-store, each record a plain file named blake3(path)[..16] holding the absolute project path, published write-then-rename, with records for deleted projects swept on read.
• Registration on install — run_link_phase records cwd when the linker uses the global virtual store, best-effort so a registry failure can't fail an install.
• prune split into prune_cas + prune_virtual_store — the old return Ok(()) on an absent CAS root no longer skips the tier sweep, which is the bug that let an e2e case pass without ever reaching the guard it tested.
• Mark-and-sweep over the virtual store and <store>/v1/trees/ — walk each registered project's node_modules, follow symlinks landing in the store, recurse transitively; sweep unmarked, non-dot children. An empty registry prunes nothing.
• Tests and docs — 6 unit tests (which do run in CI via aube-parity.yml's cd vendor/aube && cargo test --workspace), a manual bash e2e harness under tests/store-prune/, and a "Reclaiming disk space" section in the virtual-store docs.

I verified the pnpm parity claim against pnpm's source rather than its docs: the algorithm, the <store>/projects/ location, and the empty-registry bail-out all match. Two differences worth knowing — pnpm's registry records are symlinks (nub's plain-file choice is deliberate and documented), and pnpm's walk skips every dot directory where this one skips only .git.

ℹ️ A store that predates the registry loses entries on the first prune after another project registers

The empty-registry guard only protects a store where nothing is registered. As soon as one project reinstalls, every project that hasn't is invisible to the mark phase and its entries are swept — the docs' <Callout> says as much, and pnpm has the identical exposure, so this reads as an accepted design rather than a defect. The gap is that the operator gets no signal at the moment it matters: the output names the count only after the fact, in the "fully referenced by N registered project(s)" branch, which is exactly the branch that does not fire when entries are being deleted.

Technical details

# Surface the reachability basis before the sweep, not after

## Affected sites
- `vendor/aube/crates/aube/src/commands/store.rs:350-385` — `projects.len()` is only
  printed on the "nothing was removed" path; the path that deletes entries prints
  counts but never says how many projects the marking was based on.

## Required outcome
- A user running the first prune after upgrading can tell, from the output alone,
  that the sweep was computed against a suspiciously small set of projects — before
  they discover it by way of `Cannot find module` in an unrelated checkout.

## Suggested approach (optional)
- Print the registered-project count on the sweep path too, e.g.
  `Marking against {n} registered project(s)` before `sweep_unreachable` runs.

ℹ️ Nitpicks

• the_project_scan_finds_workspace_node_modules_but_does_not_descend creates .git/objects, but nothing under .git would appear in found even if the .git arm were removed — that arm is not actually pinned by the assertion.
• find_node_modules_dirs uses entry.file_type(), which does not follow symlinks, so a symlinked node_modules or symlinked workspace member is never entered. That is an under-marking case, which sits awkwardly with the "every heuristic here fails toward over-marking" claim in prune_virtual_store's doc comment. pnpm behaves the same way, so this is a comment-accuracy point rather than a behavior ask.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Important

The docs' own remedy for an unregistered project — reinstall, then prune — is a no-op on the install warm path, so the projects most at risk never register. The new sweep lock is also taken after the fetch phase has already published entries under their final names.

Reviewed changes — the delta from 5dc26e8 to fb9a3b1, which reworks registration and adds a store-level advisory lock in response to the prior review. I also traced the install pipeline's ordering (install/mod.rs, install/startup.rs, install/materialize.rs) and the linker's publish path (aube-linker/src/materialize.rs) to check what the new lock actually covers.

• Added a .gc.lock advisory lock — gc_lock_file / lock_for_link (blocking lock_shared) / try_lock_for_sweep (non-blocking try_lock) at <store>/v1/.gc.lock, with prune_virtual_store declining rather than racing and the link phase holding it shared.
• Moved registration after the link and made it unconditional — store.register_project(cwd) now runs once link_all/link_workspace has succeeded, on every linker mode rather than only the shared store, so a project-local install's un-hashed tree entries are protected by its own registration.
• Widened the project scan to skip every dot directory — matching pnpm's findAllNodeModulesDirs, with the doc comment explaining why the project's own .aube/ is still reached.
• Tightened two tests — the registry test now reads the registry directory instead of re-calling the accessor, and the e2e sweep compares realpaths so macOS's /tmp → /private/tmp no longer reports a false mismatch.
• Added a lock unit test and e2e CASE 7 — a_held_link_lock_stops_the_sweep with a drop-the-guard positive control, and a hoisted-project registration case.

I checked the std API against the toolchain: CI pins Rust 1.93/1.95/stable and File::try_lock() returns Result<(), TryLockError> (stabilized 1.89), so try_lock().ok()? correctly maps contention to None. Per flock(2), two file descriptions on the same path do conflict within one process, so a_held_link_lock_stops_the_sweep pins the lock rather than passing vacuously.

I left the TOCTOU thread from the previous review open — the lock closes the link-phase half of that window, not the fetch-phase half.

ℹ️ Nitpicks

• prune_virtual_store's contended-lock message reads An install is using this store, but the exclusive lock is also what a second concurrent store prune holds, so the message names the wrong holder in that case.
• prune_cas never takes the lock, so half of the same prune() command still runs unsynchronized. referenced_hashes reads only <store>/v1/index/*.json, and import_verified_tarball writes CAS content (lifecycle.rs:921) before the index naming it (:948); a freshly imported file has nlink == 1 so the existing nlink > 1 guard does not cover it. Pre-existing rather than introduced here, and it fails loudly as MissingStoreFile, but it sits directly inside this commit's stated goal.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ Both blocking concerns from the last review are closed. Minor suggestions inline.

Reviewed changes — the delta from fb9a3b1 to aeadb33, which moves the sweep lock and adds warm-path registration in response to the prior review. I re-traced the install pipeline to check what the relocated guard now covers, and audited every command that can reach the linker's store-write paths.

• Moved the sweep lock to run_inner, ahead of the fetch phase — let _sweep_guard = store.lock_for_link() now sits at install/mod.rs:1356, before select_lockfile_result, both spawn_gvs_prewarm sites and run_link_phase, and is held to the end of the install. link.rs keeps a comment where the acquisition used to be.
• Registered the project on the install warm path — try_install_fast_path's return at install/mod.rs:789 now calls store.register_project(&cwd) before control::complete, so the docs' repair advice ("run an install, then prune again") actually registers the projects it was written for.
• Corrected prune_virtual_store's doc comment — it names the dot-directory skip and the lock decline instead of the stale .git-only wording.
• Added e2e CASE 8 — empties the registry, reinstalls, and asserts the record comes back plus that projA still resolves after a follow-up prune.

I verified the relocated guard actually covers everything: run_link_phase has exactly one caller, and add / remove / update / dedupe / ci / dlx / patch / deploy / auto_install all funnel through install::run → run_scoped → run_inner, so they inherit it. import and rebuild never write either tier, and aube-linker/src/quarantine.rs only strips xattrs off entries the linker already materialized. Both prewarm handles are joined on the ordinary success and error paths. The warm-path registration also cannot arm the sweep with a project that marks nothing — compatibility_metadata_is_current requires .modules.yaml currency, and that file lives inside node_modules.

ℹ️ Nitpicks

• Store::lock_for_link's doc at vendor/aube/crates/aube-store/src/lib.rs:384 still says "for the duration of a link phase". It now spans the whole install, which is the fact a future reader most needs; the method name says "link" too.
• The contention window widened from the link phase to the entire install, lifecycle scripts included, and try_lock_for_sweep never waits. So nub store prune now skips the tier sweep — and still exits 0 — for as long as any install is running. A bounded blocking acquire, or a non-zero exit when the sweep was skipped, would keep the command from looking like it collected nothing to collect.
• materialize_handle (install/mod.rs:2514) is not joined or aborted when the sibling fetch_handle task panics: the ? at :2525 fires on the outer JoinError and returns before the combine_install_pipeline_errors join at :2533, and MaterializeJoinHandle is a bare JoinHandle rather than an abort-on-drop JoinSet. A detached prewarm writer can then outlive _sweep_guard. Pre-existing and panic-only, but it bounds the guarantee the new comment states.
• The warm path's super::open_store(&cwd) costs four with_settings_ctx loads by default — resolved_store_dir, resolved_cache_dir, global_virtual_store_dir, and that function's own resolved_cache_dir fallback — and FileSources::load is not memoized. That is paid on every "Already up to date" install to rewrite a record that is almost always already there.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ The warm-path pin lands correctly. Two minor points — one factual, one about the pin's durability.

Reviewed changes — the delta from aeadb33 to 9487050, which is test-harness only; no Rust changed. I traced the RUST_LOG plumbing to confirm the new probe can actually fire, and checked the README's CI claim against aube-parity.yml.

• Pinned the warm-path precondition in e2e CASE 8 — the install now runs under RUST_LOG=debug and an ok asserts phase:link is absent, so a fall-through to the slow path fails the case instead of passing green via the link-phase registration.
• Documented the coverage situation in the harness README — a "Known gaps" paragraph and a table row for the warm-install case.

I verified the probe is live rather than assuming it: crates/nub-cli/src/main.rs:49 calls pm_engine::log::init() on every invocation, log.rs:81-92 hands RUST_LOG ownership of the EnvFilter, and RewriteLayer::on_event eprintln!s each enabled event as LEVEL <message>, so a slow install prints DEBUG phase:link 1.2ms (N files) and present::rewrite leaves that text alone. The trailing space is load-bearing exactly as the comment says — link.rs:542 emits phase:link_bins.

ℹ️ A command that deletes directories from the user's global store has no automated end-to-end coverage

The Rust unit tests exercise mark_from / sweep_unreachable / the registry as pure functions, but nothing automated drives nub store prune against a real store built by a real install — which is where the sweep's actual failure mode lives (deleting a directory a live project is symlinked at, then exiting 0). The new README paragraph accepts that as a known gap; whether manual-only is the right long-term posture for a destructive command is a call for the human, not something the diff can settle.

Technical details

# The GC's only end-to-end coverage is a manual script

## Affected sites
- `tests/store-prune/run-prune-sweep.sh` — 11 assertions over 8 cases, invoked by hand only.
- `tests/store-prune/README.md:34` — records the gap as accepted.
- `.github/workflows/aube-parity.yml` — already path-filtered to `vendor/aube/**` and already
  provisions a Rust toolchain, so it is the closest existing home if this becomes a CI job.

## Required outcome
- A decision on record: either the harness gets a CI trigger, or the manual-only posture is
  deliberate and the README says so as a choice rather than as a limitation.

## Open questions for the human
- Is `tests/store-prune/` intended to stay manual indefinitely, in the shape of
  `tests/pnp/`, or is wiring it up follow-up work worth an issue?
- The script needs network (`nub install` of `debug` / `ms`) and reassigns `HOME`; the
  registry-dependent cases would also need `CI` unset, which a CI job makes awkward. Is that
  what makes it manual, or just sequencing?

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

The unit tests do run in nub CI: aube-parity.yml runs cargo test --workspace inside vendor/aube on ubuntu and windows-latest, both gating pull_request into main, path-filtered on vendor/aube/** — which this PR touches. The --all-targets parenthetical is true of the root-workspace gates only, so as written the paragraph tells a future maintainer these tests are unprotected when only the bash script is.

Suggested change

**This harness is manual, and it is the only coverage the warm-path registration has.** The Rust unit tests reach the mark, sweep, and registry functions but not the install pipeline, and neither this script nor those tests run in nub CI — the unit tests live under `vendor/aube/**`, which `--all-targets` never builds from the root (the aube-workspace gate). Run it by hand when touching `store.rs`, `aube-store`, or the install fast path.

**This harness is manual, and it is the only coverage the warm-path registration has.** The Rust unit tests reach the mark, sweep, and registry functions but not the install pipeline. They do run in CI — `aube-parity.yml` runs `cargo test --workspace` inside `vendor/aube` on ubuntu and windows for any PR touching `vendor/aube/**` — but this script runs nowhere. Run it by hand when touching `store.rs`, `aube-store`, or the install fast path.

[pullfrog]

This is an absence assertion with nothing proving the string can appear, so if the label moves, the RUST_LOG plumbing changes, or the debug channel goes quiet, the grep stops matching, the case reports warm unconditionally, and a slow-path fall-through passes green again — the exact failure the pin exists to catch. A total install failure is still caught by the next assertion (registry count 0 ≠ 1); only the broken-instrument case slips through.

Technical details

# Give the `phase:link ` probe a positive control

## Affected sites
- `tests/store-prune/run-prune-sweep.sh:98-99` — asserts `phase:link ` is absent with no
  companion assertion that it is ever present.

## Required outcome
- A broken or renamed probe fails a case instead of making CASE 8 pass vacuously.

## Suggested approach (optional)
- CASE 7's `flat` install (`:78`) is a guaranteed-cold install that must reach
  `run_link_phase`. Run it under `RUST_LOG=debug` and assert `phase:link ` IS present there;
  two lines, and it validates both the pattern and the trailing-space distinction against
  `phase:link_bins`.
- Scoping the filter to `RUST_LOG=aube=debug` would also cut the log to the engine's own
  events while still matching, since `link.rs` lives in the `aube` crate.

[pullfrog]

Important

The new rule that an unresolvable project stops the sweep collides with nubx: nub dlx / nubx / nub x / nub create register a scratch project they delete on exit, so a fresh permanently-unresolvable record is minted on every invocation and the tier sweep this PR exists to add never runs.

Reviewed changes — the delta from 9487050 to 9eb810f, the first Rust change since aeadb33. I traced every caller that reaches Store::register_project, audited whether the new .gc-state file can be misread by any other consumer of the store roots, and derived pluralizer's actual output from its 0.5.0 rule tables rather than assuming it.

• Two-phase removal on a 30-day clock — GRACE, SweepOutcome, and a .gc-state file per tier recording when each entry was first seen unreferenced; sweep_unreachable defers on first sighting, clears the record when an entry becomes reachable again, and drops records for entries that have vanished.
• Reading the registry no longer destroys records — registered_projects returns RegisteredProject { record, dir, exists } and removal moved to an explicit Store::forget_project, with prune_virtual_store aging stale records out on the same clock.
• The sweep declines while any registered project is unresolvable — an unmounted disk and a deletion are indistinguishable, so it returns rather than marking against partial knowledge.
• Fixed two ways a live project read as dead — registered_projects skips every dot-prefixed name (its own state file was being parsed back as a registration) and strips only \r/\n instead of trim()ing a legitimate trailing space; find_node_modules_dirs now takes a symlinked node_modules.
• Announced the blocking acquire — lock_for_link_with(on_wait) prints Waiting for a store prune to finish... on the slow install path, prune takes the lock once for both sweeps, and store add takes it too.
• Extended both test layers — four unit tests (grace deferral with a positive control, clock reset on revival, state-file bounding, symlinked node_modules, trailing-space path) plus e2e CASE 9 for the upgrade sequence.

I confirmed the new .gc-state file and the .projects directory cannot be misread elsewhere: only sweep_unreachable and registered_projects enumerate the virtual-store and trees roots, both with an explicit dot-skip, and aube-linker/src/sweep.rs:16-43 matches only .tmp-<pid>-. Every other read_dir in the engine targets a project-local .aube, a project node_modules, or the CAS.

ℹ️ The 30-day hold has no override, so store prune cannot reclaim disk today

The hold is the right default and the reasoning in GRACE's doc comment is convincing. What is missing is the exit: a user who runs nub store prune because they are out of disk space now gets Holding N entries... and zero bytes back, with no flag to say "I know what I am doing" and no way to shorten the window. pnpm's store prune deletes immediately, so this is a deliberate divergence in the command's headline purpose rather than an oversight — worth being a decision on record.

Technical details

# `store prune` has no way to act on the first pass

## Affected sites
- `vendor/aube/crates/aube/src/commands/store.rs:588` — `GRACE` is a hard-coded constant with no
  setting, env var, or flag reaching it.
- `vendor/aube/crates/aube/src/commands/store.rs:451-464` — the deferral message tells the user
  how to KEEP entries, never how to remove them now.
- `site/content/docs/install/virtual-store.mdx:142-149` — documents the delay as unconditional.

## Required outcome
- A user who understands the tradeoff can reclaim the space in one command, or the docs state
  plainly that they cannot and why.

## Open questions for the human
- Is a `--force` / `--min-age=<duration>` on `store prune` in scope for this PR, or deliberately
  deferred? A duration knob generalizes better than a boolean and matches the `NUB_PRIMER_TTL`
  precedent in AGENTS.md.
- Should the deferral message name the remedy for the other direction ("run again after N days,
  or `--force` to remove now")?

ℹ️ Nitpicks

• pluralizer::pluralize("registered project is", n, true) does not produce English. Derived from pluralizer 0.5.0's constants.rs: at n == 1 to_singular falls through to the generic ("(?i)s$", "") rule and prints 1 registered project i not reachable right now; at n >= 2 to_plural hits ("(?i)s?$", "s"), which consumes the trailing s and re-emits it, printing 2 registered project is not reachable right now. The verb needs to sit outside the pluralized noun. The e2e case greps only not reachable right now, so it passes either way.
• unix_now()'s unwrap_or(0) (store.rs:603-608) means "already expired": a prune that runs while the clock is unreadable stamps every unreferenced entry 0, and the next prune with a healthy clock deletes all of them at once, bypassing the window entirely — the harness's own expire_all uses exactly 0 for that effect. It needs a pre-1970 clock to trigger, so this is cheap insurance rather than a live bug: skip recording instead of recording 0.
• lock_for_link's doc (aube-store/src/lib.rs:423-424) says it is "for callers whose critical section is a single file write, where a wait is imperceptible", but neither caller fits. store add holds it across the whole fetch loop, and the warm install path can stall for the length of a prune — the same 24 s of silence lock_for_link_with was added to fix, on the most common install of all. What the user feels is the wait, not the critical section.
• e2e CASE 9's rescue assertions (run-prune-sweep.sh:147-150) only check that two and three still resolve, which is also true if the second prune swept nothing at all. Asserting the entry count dropped, or that u-prune2.log reports a removal, would keep the case from passing vacuously.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Important

register_in_store is the right mechanism, but two more callers with the identical shape still register a directory they delete — so the tier sweep is still permanently blocked for most users.

50435c30 resolves all three threads from the last review: SweepLock now distinguishes Busy from Unsupported, virtual-store.mdx drops the stale "a later prune collects them" claim, and tests/store-prune/README.md no longer says the unit tests skip nub CI. CASE 10's grep -q '(oo)' precondition and the new positive control for the phase:link absence assertion both close the vacuous-pass holes. a0d11e0 is a re-export reorder only.

The open question from the last review — should deletion of a known-owned directory call forget_project at the point of deletion? — got answered for dlx and deploy, and the answer was "suppress registration at the source." That works, but the enumeration is incomplete. Verified by reading the code:

Caller	Registers	Deletes the same dir	Covered by 50435c30
dlx / nubx / nub create	—	scratch tempdir	yes
deploy	—	shipped or discarded target	yes
git dep with a prepare script	git_prepare.rs:111	ScratchDir::drop, git_prepare.rs:16-20	no
add -g upgrade, failure, remove -g	add/global.rs:38 → add/mod.rs:134	add/global.rs:90,324, global.rs:509	no

Both leave a record whose path is gone the moment the command exits, and forget_project has no caller outside store.rs:406's 30-day aging path (repo-wide grep). The git-dep one is the worse of the two: it needs no unusual user action, only a dependency installed from a git URL that ships a prepare script, and it fires on every cold install.

That a short audit found two more is the argument for the second suggestion from the last review rather than a third and fourth flag: let an unresolvable record disqualify only its own contribution to reachable and let the rest of the sweep proceed. Then a missed caller costs the entries that project reached, not the whole command, and correctness stops depending on enumerating every ephemeral-project site in the codebase — a list that grows every time someone adds a nested install.

If the per-caller approach stays, the two above need the flag (or a forget_project at each deletion site), and the e2e harness needs a case for each — CASE 10 pins dlx only.

Nothing else in the incremental delta raised a concern. The SweepLock match in prune is exhaustive and the Unsupported arm's "proceed unsynchronized, as the CAS sweep did before this lock existed" reasoning holds. The flag is threaded correctly: register_project has exactly two production call sites (link.rs:475, mod.rs:809) and both are gated. The unit tests carry their own positive controls (a_held_link_lock_stops_the_sweep re-acquires after drop(guard) and then proves the entry was collectable; an_unreferenced_entry_survives_its_first_sweep closes with a zero-grace sweep), which is what makes the absence assertions mean something.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — minor suggestions inline.

760eab8 implements the narrowing the last two reviews asked for: an unresolvable registry record now disqualifies only its own contribution to reachable, and the sweep proceeds. Verified in the code — all is filtered to p.exists at store.rs:434-438, so an unresolvable sibling removes only itself from the mark set and never removes a project that does resolve. That takes the worst case for a missed ephemeral-install caller from a permanently dead store prune down to a slow leak: one report line per prune, plus a 30-day hold on whatever that project alone reached. Correctness no longer depends on enumerating every ephemeral-project site in the codebase, which is the part that was never going to hold.

The deploy revert to register_in_store: true is right for the reason that thread gave — deploy does not override enableGlobalVirtualStore, so an in-place target's node_modules symlinks into the shared store and has to keep marking it.

Nothing in the delta is unsafe. What is left is four comments that still describe the old decline semantics, and one branch that became reachable for the first time with no test behind it.

The drop-and-continue path is not unit-tested

The 12 cases in mod virtual_store_prune_tests are the ones that actually run in CI (aube-parity.yml, cd vendor/aube && cargo test --workspace, ubuntu and windows). None of them uses a mixed registry: every case has either all-resolvable projects or an empty one, so the .filter(|p| p.exists) that is the whole point of this commit is covered by nothing automated. The new semantics are pinned only by CASE 4 of tests/store-prune/run-prune-sweep.sh, which is manual-only.

The assertion worth having is the one that would catch a future refactor restoring the return: with one live project and one unresolvable record, an entry only the live project reaches survives, an entry only the unresolvable record reached is held rather than removed on the first pass, and the unresolvable record is still on disk afterwards. That is a handful of lines on top of the existing sweep_keeps_reachable_entries_and_removes_the_rest fixture.

Review metadata

Mode: IncrementalReview
Files reviewed: 12
Base: main
Head: gvs-gc (760eab8)
Reviewed commits: 760eab8 — pm: drop an unreachable project from the mark set instead of declining
Prior pullfrog review: a0d11e0 (https://github.com/nubjs/nub/pull/720#pullrequestreview-4954323199)

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

nit, carried over from 9eb810f and still unaddressed in the rewritten string: pluralizer inflects the last word of the phrase, so "registered project is" becomes 1 registered project i at blocked == 1 and is left as 2 registered project is at 2. Deriving this from the crate's rules rather than a local run, so worth a one-line check — format!("{} registered project{} not reachable", blocked, if blocked == 1 { " is" } else { "s are" }) sidesteps it either way.

[pullfrog]

This branch is reachable in a new way now: a registry that is non-empty but entirely unresolvable used to hit the decline and return above, and now falls through to here. Both sentences are then wrong — projects are registered, and running an install in each is not the fix (remounting the disk, or waiting out the grace period, is). The early return is still the safe behavior; only the message misleads.

Worth distinguishing the two cases, e.g. keeping this text for a genuinely empty registry and printing something like no registered project is reachable right now; skipping the virtual store when all was non-empty.

[pullfrog]

ℹ️ No critical issues — one inline nit, one coverage observation.

Reviewed changes — the delta from 760eab8 to 02c1b9a (4 files), which adds the unit test the last review asked for and refreshes part of the decline-era comment prose. No behavior changed.

• Added an_unresolvable_record_disqualifies_only_itself — a mixed registry (one live project, one record whose directory was deleted after registration), asserting the live project's entry survives, an already-expired entry is collected, an in-window entry is held, and the unresolvable record is still on disk afterwards.
• Refreshed three decline-era comments — dlx.rs's register_in_store rationale, registered_projects's dot-skip explanation, and the first paragraph above the unresolvable-project loop in prune_virtual_store.
• Corrected the harness README's empty-registry row — it now distinguishes the whole-command decline from a single record dropping out of the mark set.

The new test is the one the last review asked for, and it is not vacuous: collectable@1.0.0-bbbb is pre-expired, so a refactor restoring the early return fails the second assertion, while fresh@1.0.0-cccc pins that the sweep ran selectively rather than deleting everything it could not mark. Every API it touches is pub and already exercised by sibling tests in the module, and it runs in CI through aube-parity.yml.

Four of the seven threads from the last review are untouched rather than disagreed with, so I left them open: install/args.rs:331-333 still says the sweep DECLINEs, both run-prune-sweep.sh labels still describe CASE 10's old premise, and the pluralizer nit is unchanged. The store.rs:392-402 thread is half-fixed — the rewritten first paragraph is right, but the second still reads "a genuinely deleted project stops blocking pruning once the window passes", which is the framing the drop-and-continue change removed.

ℹ️ The case .filter(|p| p.exists) actually guards has no test

The new test covers a mixed registry, where the filter is inert — a registered directory that does not resolve marks nothing whether or not it is filtered out. The filter is load-bearing only when every record is unresolvable: it is what leaves projects empty so the projects.is_empty() early return fires. Drop it and that case falls through to a sweep against an empty reachable, marking both tiers wholesale. The grace period makes that a delayed loss rather than an immediate one, but it is the outcome this whole design exists to prevent, and it is one line away with nothing automated in front of it.

Technical details

# Pin the all-unresolvable-registry path

## Affected sites
- `vendor/aube/crates/aube/src/commands/store.rs:433-437` — `.filter(|p| p.exists)` is the
  only thing routing an entirely-unresolvable registry into the `projects.is_empty()` early
  return at `:438`.
- `vendor/aube/crates/aube/src/commands/store.rs:968-1026` — the new test uses a mixed
  registry, where the filter cannot change the outcome.
- `vendor/aube/crates/aube/src/commands/store.rs:927` — `an_empty_registry_prunes_nothing`
  covers the empty registry, not this one.

## Required outcome
- A registry holding one or more records, none of which resolve, provably leaves both tiers
  untouched — no entry removed, and `.gc-state` not stamped, so the grace clock does not
  start on entries a remounted disk would re-mark.

## Suggested approach (optional)
- The same fixture shape as the new test with the live project omitted: register `gone`,
  delete it, `expire` one entry, run `prune_virtual_store`, assert the expired entry is
  still present. Removing the filter makes it fail.

## Open questions for the human (optional)
- The still-open thread on `store.rs:443-449` is about the message this same branch prints.
  If you reshape that branch to distinguish "empty registry" from "nothing resolves", this
  is the test that keeps the two apart.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

The new clause sits in the list of behaviors that fail toward over-marking, but dropping a project from the mark set is under-marking — its entries become unreferenced and are deleted once the window passes, just later. The sentence immediately below already carves out "the one place it does not over-mark"; there are now two, and this belongs with it rather than against it.

[pullfrog]

ℹ️ Nothing new to fix — the coverage gap from the last review is closed. One optional nitpick.

Reviewed changes — the delta from 02c1b9a to 9e69172 (3 files), which adds the unit test the last review asked for and finishes the decline-era comment sweep. No behavior changed.

• Added a_wholly_unresolvable_registry_prunes_nothing — a registry holding only unresolvable records (gone-a, gone-b, both registered then deleted), with a pre-expired entry in the virtual store, asserting that entry survives the sweep.
• Retired the last four decline-era comments — install/args.rs's register_in_store rationale, the registry-aging paragraph in prune_virtual_store, and both run-prune-sweep.sh CASE 10 strings (the premise block and the assertion label).

The new test is the one the last review asked for, and it is not vacuous. I traced the mutation by hand: drop .filter(|p| p.exists) and projects becomes [gone-a, gone-b], find_node_modules_dirs returns empty because read_dir fails on both, reachable stays empty, and still-needed@1.0.0-aaaa — whose first_seen is 0 from the expire call — clears GRACE and is deleted, failing the assertion. The fixture also cannot silently degenerate into the existing empty-registry test: with_dirs puts the virtual store under <tmp>/cache/, so register_project's starts_with early return never fires and both records really are written. It runs on Linux through aube-parity.yml.

Three threads from the earlier reviews are still open. All three sit on code these commits did not touch, so they are untouched rather than disagreed with: the store.rs:433 pluralizer nit, the store.rs:443-449 message that describes an empty registry when it can also mean "nothing resolves", and the store.rs:372-375 doc paragraph that still lists the drop-and-continue clause among behaviors that fail toward over-marking. That last one is the only substantive one left — it is prose, not behavior, but it is the paragraph a future reader will use to reason about the design.

ℹ️ Nitpicks

• #[cfg(unix)] on the new test is free coverage given away. a_wholly_unresolvable_registry_prunes_nothing uses no link() helper and no std::os::unix, and with reachable empty the sweep never reaches the symlink walk — so nothing in it is unix-specific. Its nearest sibling an_empty_registry_prunes_nothing is uncfg'd and does run on the Windows leg of aube-parity.yml. The module convention is genuinely mixed (3 of 14 tests are uncfg'd), so this is reasonable to decline; it just costs a platform.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}