Skip to content

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

Description

@dipto0321

Symptom (live user report, 2026-07-04)

nodeup check reported LTS: v22.10.0 (released 2025-02-04) / Current: v24.0.0 (released 2025-04-01) while the live nodejs.org index has LTS v24.18.0 / Current v26.4.0 — and the user's own fnm had 26.4.0 installed. Reproduced with a fresh build of main (c5b4301).

Root cause

~/Library/Caches/nodeup/node-dist-index.json contained a 115-byte copy of dist_test.go's validManifestBody fixture (the exact two entries above), with a fresh .meta expiry. A test run from before the withEmptyCache isolation helper landed (#82 fix-wave) executed FetchManifestCtx against an httptest server and saveToCache wrote the fixture to the real user cache. nodeup check then faithfully served the fixture until the 24h TTL lapsed. Current tests were verified clean — running go test ./internal/node/ today does not touch the real cache — so this specific incident self-heals and the immediate remedy is just deleting the two files.

The bugs worth fixing

  1. Cache location contradicts the platform layer and the docs. internal/node/dist.go cachePath() builds os.UserCacheDir()/nodeup (macOS: ~/Library/Caches/nodeup). But platform.CacheDir() — and CLAUDE.md's on-disk layout section — say the cache lives at <DataDir>/cache (macOS: ~/Library/Application Support/nodeup/cache). Both directories exist on disk; the platform one sits empty while the real cache accumulates elsewhere. Pick one (presumably platform.CacheDir(), since it's the documented invariant and the platform package exists precisely so paths aren't derived ad-hoc) and migrate.

  2. Nothing stops a future test from re-polluting. withEmptyCache(t) is opt-in per test. One new FetchManifestCtx-calling test that forgets it silently writes fixtures into the developer's real cache again. Consider a package-level guard — e.g. TestMain in internal/node that sets the cache-redirect env vars once for the whole package, or making defaultCachePaths a swappable seam that tests must set.

Suggested fix shape

  • cachePath() → delegate to platform.CacheDir(); keep the filenames (node-dist-index.json + .meta).
  • One-time migration is unnecessary (worst case: one cold fetch after upgrade); optionally delete the legacy os.UserCacheDir()/nodeup dir when found.
  • Add TestMain cache isolation to internal/node so pollution is structurally impossible.
  • Tests: assert cachePath() lives under platform.DataDir().

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions