Skip to content

fix(node): cache manifest under platform.CacheDir per documented layout#112

Merged
dipto0321 merged 1 commit into
mainfrom
fix/node/manifest-cache-platform-cachedir
Jul 3, 2026
Merged

fix(node): cache manifest under platform.CacheDir per documented layout#112
dipto0321 merged 1 commit into
mainfrom
fix/node/manifest-cache-platform-cachedir

Conversation

@dipto0321

Copy link
Copy Markdown
Owner

Summary

Move the nodejs.org manifest cache from os.UserCacheDir()/nodeup to platform.CacheDir() (<DataDir>/cache), matching the documented on-disk layout, and add a package-level TestMain guard so no test in internal/node can ever write the real user's cache again. Closes #110

Linked issues

Closes #110

Type of change

  • feat — new feature (MINOR bump)
  • fix — bug fix (PATCH bump)
  • refactor — no behavior change
  • perf — performance improvement
  • docs — documentation only
  • test — tests only
  • chore — maintenance / dependency bump
  • ci — CI/CD only
  • build — build system only
  • Breaking change (MAJOR bump) — explain below

Checklist

  • Title follows Conventional Commits (feat(scope): subject)
  • I ran make ci locally and it passes
  • I added or updated tests for the change
  • I updated relevant docs (README, docs/, inline godoc)
  • No new linter warnings
  • If breaking: I documented the migration path in the PR body and updated CHANGELOG.md

Scope notes

  • No migration of the legacy cache dir. Worst case after upgrading is one cold manifest fetch; deleting os.UserCacheDir()/nodeup on sight felt too aggressive for a cache. Documented in the cachePath() godoc.
  • The incident that surfaced this (a pre-fix(node): timeout, retry, context, and atomicity on nodejs.org manifest fetch #82 test run persisting dist_test.go's fixture manifest into the real cache, then nodeup check reporting LTS v22.10.0 / Current v24.0.0 for up to 24h) is prevented two ways: TestMain redirects HOME/XDG/APPDATA for the whole package, and withEmptyCache now overrides the platform.DataDir()-relevant vars per test. TestCachePathUnderPlatformCacheDir pins the location.

Screenshots / output

Before (with polluted legacy cache): nodeup checkLTS: v22.10.0 / Current: v24.0.0 (test fixture data).
After (fresh fetch, correct location): LTS: v24.18.0 (released 2026-06-23) / Current: v26.4.0 (released 2026-06-24).

🤖 Generated with Claude Code

cachePath() built its own location from os.UserCacheDir()/nodeup,
diverging from platform.CacheDir() (<DataDir>/cache) — the layout
CLAUDE.md documents and every other persistent artifact uses. The
documented cache dir sat empty while real data accumulated in
~/Library/Caches/nodeup (macOS). Point cachePath() at
platform.CacheDir(); filenames unchanged. No migration: worst case
is one cold manifest fetch after upgrading.

Also add a TestMain to internal/node that points HOME/XDG/APPDATA at
a throwaway dir for the whole package, so no test can ever write the
real user cache again (the incident behind this issue: a pre-#82
test run persisted its 2-entry fixture manifest into the real cache,
and nodeup check served fixture versions until the TTL lapsed).
withEmptyCache now overrides the DataDir-relevant vars for per-test
isolation, and a new test pins cachePath() to platform.CacheDir().

Refs #110

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cocogitto-bot

cocogitto-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

✔️ d17d9d2 - Conventional commits check succeeded.

@dipto0321 dipto0321 merged commit aef6851 into main Jul 3, 2026
10 checks passed
@dipto0321 dipto0321 deleted the fix/node/manifest-cache-platform-cachedir branch July 3, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(node): manifest cache written to os.UserCacheDir, not platform.CacheDir — stale test fixture served to nodeup check

1 participant