pm: honor NUB_CACHE_DIR for the engine cache, and report the env tier in nub config by colinhacks · Pull Request #740 · nubjs/nub

colinhacks · 2026-08-15T18:13:54Z

Closes #654.

Env-set cacheDir was inert in 0.6.0: an .npmrc-only gate ran ahead of the settings resolver. #621 dropped it and v0.7.0 shipped the fix, with nothing pinning it. Two related defects were still live.

resolved_cache_dir reads config_env("CACHE_DIR") ahead of the settings chain, so NUB_CACHE_DIR moves the engine cache, not just the resolver primer. Byte-identical for standalone aube.
nub config get / config list gained the env tier. They read files only, so an install moved the cache while config get cache-dir printed undefined. Only config-carrying vars surface; ambient ones (CI, http_proxy) do not, matching npm. Scoped reads unchanged.
New pm_env_matrix test pins both. Four stale comments corrected; the cache directory is now documented.

Copilot

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

vercel · 2026-08-15T18:14:01Z

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

Project	Deployment	Actions	Updated (UTC)
nub	Ready	Preview	Aug 15, 2026 10:23pm

pullfrog

Important

The two behavior changes hold up under scrutiny, but the new test that pins them is not hermetic — it reads the dev box's real ~/.npmrc, which can falsify three of its own controls.

Reviewed changes — full initial review of the single commit on cachedir-env-654.

resolved_cache_dir reads the host cache knob first — settings_context.rs returns config_env("CACHE_DIR") (NUB_CACHE_DIR under nub, AUBE_CACHE_DIR standalone) ahead of the settings chain, so the override reaches the packument caches and the global virtual store rather than only the resolver primer.
config get / config list gained the env tier — read_merged appends read_env_entries(), so the command reports the value the install will actually use instead of undefined.
env_source provenance helper — env_alias_hit is factored out of raw_from_env so the reporter and the resolver walk the same alias order under the same brand gate.
is_config_env_alias allow-list — only npm_config_* / NPM_CONFIG_* / pnpm_config_* / PNPM_CONFIG_* and the embedder's own prefix read as configuration; ambient vars (CI, both spellings of the proxy vars) do not.
primary_entry_key hoisted from list.rs::primary_list_key into config/mod.rs so env entries key the same way file entries do.
New pm_env_matrix integration test — four behavioral rows through the real binary, each with a negative control, using cache list as the read-side proof of which directory resolved.
Docs and comments — a new cache-directory section in the install docs, plus corrections to four stale comments that described the removed set_cache_root seam and the long-gone .npmrc-only gate.

I verified the two load-bearing claims in the new comments independently: cacheDir declares sources.cli = [] and carries no managedPolicy, so the early return neither inverts a CLI tier nor skips an enforcement pass; and AUBE_CACHE_DIR already sat first in the reversed alias walk, so standalone aube's precedence is genuinely preserved. config set / delete / explain, protected-key redaction, and --all default-marking are all unaffected by the read_merged change.

ℹ️ Documenting `NUB_CACHE_DIR` on the site is a brand-boundary decision, not just a doc addition

The install docs now publish NUB_CACHE_DIR and list it above the neutral npm_config_cache_dir. wiki/agents.md sanctions the knob's existence, but the brand-boundary section in the same file says "never a brand env var as a documented USER knob" and prefers neutral spellings on user-facing surfaces. The PR resolves that tension in favour of publishing, which is defensible — the wiki line it edits already claimed the knob was publicly documented — but it is the maintainer's call rather than a consequence of the bug fix.

Technical details

# Publishing a `NUB_*` knob as documented user surface

## Affected sites
- `site/content/docs/install/index.mdx:604-607` — `NUB_CACHE_DIR` listed first, `npm_config_cache_dir` second.
- `wiki/agents.md:396` — edited to record that `NUB_CACHE_DIR` is now documented and the other two knobs are not.
- `wiki/agents.md` brand-boundary section — "Internal `NUB_*` env vars are fine; never a brand env var as a documented USER knob."

## Required outcome
- A deliberate answer to: does Nub publish `NUB_CACHE_DIR` as supported user surface, or keep it live-but-undocumented and publish only the neutral `npm_config_cache_dir` spelling?

## Suggested approach (optional)
- If publishing: lead the docs block with the neutral `npm_config_cache_dir` and present `NUB_CACHE_DIR` as the Nub-branded equivalent, matching the stated preference for neutral names.
- If not: drop the `NUB_CACHE_DIR` line from the mdx and leave the `wiki/agents.md` correction, which is accurate either way.

## Open questions for the human
- The `resolved_cache_dir` fix is orthogonal to this and should land regardless.

ℹ️ Nitpicks

site/content/docs/install/index.mdx:616 — "split them and every install degrades to a per-file copy" holds only while the global virtual store is enabled. enableGlobalVirtualStore is off under CI, and a CI cache mount is one of the two motivating examples on this page. The settings.toml cacheDir docs carry the qualifier ("on installs with the global virtual store enabled"); the mdx drops it.
vendor/aube/crates/aube/src/commands/settings_context.rs:527-529 — the comment correctly notes that a cacheDir that grows a managedPolicy must not be short-circuited here, but nothing enforces it. A debug_assert! on meta.managed_policy.is_empty() would turn the note into a tripwire, if that feels proportionate.

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

…ng .npmrc Review round on #740. `pm_env_matrix` pinned the cache and data dirs but not `HOME`, so every row read the developer's real user `.npmrc`. Reproduced: a `cache-dir` line there falsifies three of the file's own controls — `cache list` names the leaked directory instead of nothing, and both the plain and `--global` reads of `cache-dir` report it instead of `undefined`. CI has a clean `HOME`, so this would have gone red on one machine and nowhere else. Pin `HOME`, `USERPROFILE` and `XDG_CONFIG_HOME` to fixture paths, and scrub both `userconfig` spellings, which relocate the user `.npmrc` even once `HOME` is pinned. Add the row for both cache-dir spellings set at once. That combination is where the precedence rule is encoded twice — the early return in `resolved_cache_dir` and the push order of the config env tier — so a drift between them makes `config get` report a cache the install is not using, which is the invariant the change exists to restore. The config TUI rendered the merged read under the label `Effective .npmrc value:`. That read spans every source, and since the env tier joined it the value shown is routinely one no file holds, so the label names the wrong source; it is now `Effective value:`. The edit path is unaffected — it reads project scope. A `debug_assert` turns the note about `cacheDir` carrying no `managedPolicy` into a tripwire, since the enforcement it would skip runs at the end of the generated accessor. Docs: the per-file-copy warning applies only while the shared virtual store is enabled, which is off in CI — one of the two motivating examples on that page. The `nub link -g` registry is a fourth cache that stays at the platform default. Lead the environment block with the neutral `npm_config_cache_dir`, matching the preference for neutral spellings on user-facing surfaces.

pullfrog

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes — the delta since the 79656035 review, which is the single follow-up commit fb98c351. It takes every point from the prior review; all four inline threads are resolved.

pm_env_matrix::run is now isolated from host config — HOME, USERPROFILE and XDG_CONFIG_HOME are pinned to a fixture-owned home/, and both userconfig spellings joined SCRUBBED. aube_store::dirs::cache_dir() consults XDG_CACHE_HOME ahead of the %LOCALAPPDATA% branch on every platform, so the existing cache pin holds on Windows as well.
Added both_cache_dir_spellings_agree_across_surfaces — sets NUB_CACHE_DIR and npm_config_cache_dir to two differently-seeded directories and asserts cache list and config get cache-dir both name the branded one. This is a real test rather than a tautology: config get resolves through read_merged, a code path independent of resolved_cache_dir, so inverting either encoding of the precedence rule turns it red.
debug_assert! guarding the early return — meta::find("cacheDir").is_none_or(|m| m.managed_policy.is_empty()). cacheDir declares no managedPolicy in settings.toml, and SettingMeta::managed_policy is a &'static str, so the assert holds today and fires the moment the setting grows a policy the early return would skip.
TUI detail pane relabeled Effective value: — the merged read now spans the env tier, so the old .npmrc wording named the wrong source.
Install docs corrected — the neutral npm_config_cache_dir now leads the env pair, the cross-volume warning is scoped to installs with the shared virtual store on (which settings.toml confirms is off under CI), and the global-links registry joins the list of caches that stay at the platform default.

I re-verified the load-bearing claims in the new copy against the code rather than the prior review: global_links_dir() is cache_dir().join("global-links") off the platform default, so it genuinely does not follow a cacheDir override; and enableGlobalVirtualStore is documented as on outside CI and off under it.

ℹ️ Nitpicks

vendor/aube/crates/aube-settings/settings.toml:2874-2876 — the cacheDir setting's own docs still list three caches that stay at the platform default (OSV mirror, node-gyp, git clones). That is the same list the mdx just gained a fourth entry for, and it is the copy nub config and the TUI render, so the two surfaces now disagree.
vendor/aube/crates/aube/src/commands/config/tui.rs:454 — the local npmrc_effective kept its name after the label became Effective value:, which reads slightly against the new comment directly below it explaining that the value routinely comes from no file.

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

…cache list Review round 2 on #740, all three accuracy points. `pm_env_matrix`'s new doc comment claimed every config root was pinned, but the global and builtin `.npmrc` paths derive from `NPM_CONFIG_PREFIX` / `npm_config_prefix` / `PREFIX` (`resolve_global_npmrc_paths`), with `globalconfig` and `builtin_config` as direct overrides in the same family. None were scrubbed, so running the suite from an npm script — or in any shell exporting `PREFIX` — put `<prefix>/etc/npmrc` back in the chain. A `cache-dir` line there is unlikely, so this was the claim being inexact rather than a live falsifier; scrubbing the family makes it exact. The `cacheDir` setting's own docs listed three caches that stay at the platform default while the install docs now list four. That copy is what `config explain` and the TUI render, so the two surfaces disagreed; the global-links registry joins the list. Rename the TUI's `npmrc_effective` to `effective`, which is what the relabelled line and the comment above it now describe.

pullfrog

ℹ️ No critical issues — one minor consistency nit inline.

Reviewed changes — the delta since the fb98c351 review, which is the single follow-up commit 96567771. It takes all three accuracy points from that round; the one inline thread is resolved.

SCRUBBED gained the npmrc path-redirect family — npm_config_prefix / NPM_CONFIG_PREFIX / PREFIX plus the globalconfig and builtin_config spellings, with the rewritten doc comment naming resolve_global_npmrc_paths as the reason. That makes run()'s "every config root is pinned" claim exact.
cacheDir's own docs name the global-links registry — settings.toml now lists four caches that stay at the platform default, matching the list the install docs gained last round.
TUI local renamed to effective — follows the Effective value: relabel and the comment below it explaining the value routinely comes from no file.

I checked the two load-bearing claims against the code rather than the commit message. resolve_global_npmrc_paths (aube-registry/src/config/load.rs:948-973) reads exactly the seven variables now scrubbed and nothing else, so the family is complete rather than merely longer. global_links_dir() is cache_dir().join("global-links") (aube-store/src/dirs.rs:20) with no cacheDir consultation, so it genuinely belongs on the stays-put list.

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

pullfrog · 2026-08-15T21:26:45Z

+///   non-settings consumers that deliberately stay on the platform default
+///   regardless — the OSV advisory mirror, the bootstrapped node-gyp, git
+///   clones — are documented as such on the setting itself.


This is the third copy of the platform-default cache list, and 96567771 updated the other two — settings.toml and the install docs both name the global-links registry now, so the comment that defers to "the setting itself" enumerates one fewer than the setting does.

Suggested change

/// non-settings consumers that deliberately stay on the platform default

/// regardless — the OSV advisory mirror, the bootstrapped node-gyp, git

/// clones — are documented as such on the setting itself.

/// non-settings consumers that deliberately stay on the platform default

/// regardless — the OSV advisory mirror, the bootstrapped node-gyp, git

/// clones, and the global-links registry behind `link -g` — are documented

/// as such on the setting itself.

… in nub config An env-set `cacheDir` was inert in 0.6.0: a hand-written `.npmrc`-only presence gate ran ahead of the settings resolver, so `npm_config_cache_dir` and `NPM_CONFIG_CACHE_DIR` were accepted and discarded while the `.npmrc` spelling worked. The v1.35.0 engine sync dropped that gate and v0.7.0 shipped the fix, but nothing pinned it and two related defects were still live. `resolved_cache_dir` now reads the host's first-class `config_env("CACHE_DIR")` knob ahead of the settings chain. Only the resolver primer read it before, so the one spelling under nub's own brand moved a subdirectory of the cache rather than the cache. Standalone aube is unchanged: `config_env` composes the same `AUBE_CACHE_DIR` the settings table declares, which already outranked the `npm_config_*` forms, so this preserves that order rather than introducing one. `config get` and `config list` gained the env tier. `read_merged` composed embedder defaults plus files, so an env-set setting was invisible to both — an install moved the cache while `config get cache-dir` printed `undefined`. The tier is appended last, matching `cli > env > files`. Only config-carrying variables are surfaced: the `npm_config_*` family, its pnpm sibling, and the active brand's prefix. An ambient variable a setting merely observes is not configuration and stays out, which is where npm draws the line. That test is an allow-list because several settings declare both spellings of a proxy var, so a deny-list naming the uppercase forms would hide one and report the other. Scoped reads are unchanged: `--local` and `--global` ask about a file. The new `pm_env_matrix` test pins every spelling, the precedence, and both exclusions, offline, each row against a control that fails when the behavior is absent. Four comments describing the removed gate and a since-deleted `set_cache_root` registration are corrected, and the cache directory is now documented. Closes #654

…ng .npmrc Review round on #740. `pm_env_matrix` pinned the cache and data dirs but not `HOME`, so every row read the developer's real user `.npmrc`. Reproduced: a `cache-dir` line there falsifies three of the file's own controls — `cache list` names the leaked directory instead of nothing, and both the plain and `--global` reads of `cache-dir` report it instead of `undefined`. CI has a clean `HOME`, so this would have gone red on one machine and nowhere else. Pin `HOME`, `USERPROFILE` and `XDG_CONFIG_HOME` to fixture paths, and scrub both `userconfig` spellings, which relocate the user `.npmrc` even once `HOME` is pinned. Add the row for both cache-dir spellings set at once. That combination is where the precedence rule is encoded twice — the early return in `resolved_cache_dir` and the push order of the config env tier — so a drift between them makes `config get` report a cache the install is not using, which is the invariant the change exists to restore. The config TUI rendered the merged read under the label `Effective .npmrc value:`. That read spans every source, and since the env tier joined it the value shown is routinely one no file holds, so the label names the wrong source; it is now `Effective value:`. The edit path is unaffected — it reads project scope. A `debug_assert` turns the note about `cacheDir` carrying no `managedPolicy` into a tripwire, since the enforcement it would skip runs at the end of the generated accessor. Docs: the per-file-copy warning applies only while the shared virtual store is enabled, which is off in CI — one of the two motivating examples on that page. The `nub link -g` registry is a fourth cache that stays at the platform default. Lead the environment block with the neutral `npm_config_cache_dir`, matching the preference for neutral spellings on user-facing surfaces.

…cache list Review round 2 on #740, all three accuracy points. `pm_env_matrix`'s new doc comment claimed every config root was pinned, but the global and builtin `.npmrc` paths derive from `NPM_CONFIG_PREFIX` / `npm_config_prefix` / `PREFIX` (`resolve_global_npmrc_paths`), with `globalconfig` and `builtin_config` as direct overrides in the same family. None were scrubbed, so running the suite from an npm script — or in any shell exporting `PREFIX` — put `<prefix>/etc/npmrc` back in the chain. A `cache-dir` line there is unlikely, so this was the claim being inexact rather than a live falsifier; scrubbing the family makes it exact. The `cacheDir` setting's own docs listed three caches that stay at the platform default while the install docs now list four. That copy is what `config explain` and the TUI render, so the two surfaces disagreed; the global-links registry joins the list. Rename the TUI's `npmrc_effective` to `effective`, which is what the relabelled line and the comment above it now describe.

Rebasing onto #644 put "Relocating the store" directly above the new cache directory section, and it already gives the same move-both-together advice from the store side. Stop restating it and cross-reference instead, keeping only what that section does not say: the pairing matters solely while the shared virtual store is on, which is off under CI.

Copilot AI lite review requested due to automatic review settings August 15, 2026 18:13

Copilot AI reviewed Aug 15, 2026

vercel Bot deployed to Preview August 15, 2026 18:16 View deployment

pullfrog Bot reviewed Aug 15, 2026

View reviewed changes

Comment thread crates/nub-cli/tests/pm_env_matrix.rs

Comment thread vendor/aube/crates/aube/src/commands/config/mod.rs

Comment thread crates/nub-cli/tests/pm_env_matrix.rs

Comment thread site/content/docs/install/index.mdx Outdated

vercel Bot deployed to Preview August 15, 2026 20:02 View deployment

pullfrog Bot reviewed Aug 15, 2026

View reviewed changes

Comment thread crates/nub-cli/tests/pm_env_matrix.rs

vercel Bot deployed to Preview August 15, 2026 21:25 View deployment

pullfrog Bot reviewed Aug 15, 2026

View reviewed changes

colinhacks added 4 commits August 15, 2026 15:18

colinhacks force-pushed the cachedir-env-654 branch from 9656777 to c195627 Compare August 15, 2026 22:21

vercel Bot deployed to Preview August 15, 2026 22:23 View deployment

Navigation Menu

Uh oh!

pm: honor NUB_CACHE_DIR for the engine cache, and report the env tier in nub config - #740

pm: honor NUB_CACHE_DIR for the engine cache, and report the env tier in nub config#740
colinhacks wants to merge 4 commits into
mainfrom
cachedir-env-654

colinhacks commented Aug 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

vercel Bot commented Aug 15, 2026 •

edited

Loading

Uh oh!

pullfrog Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pullfrog Bot left a comment

Uh oh!

Uh oh!

pullfrog Bot left a comment

Uh oh!

pullfrog Bot Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

colinhacks commented Aug 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

vercel Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

ℹ️ Documenting NUB_CACHE_DIR on the site is a brand-boundary decision, not just a doc addition

ℹ️ Nitpicks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

ℹ️ Nitpicks

Uh oh!

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullfrog Bot Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel Bot commented Aug 15, 2026 •

edited

Loading

ℹ️ Documenting `NUB_CACHE_DIR` on the site is a brand-boundary decision, not just a doc addition