fix(node): timeout, retry, context, and atomicity on nodejs.org manifest fetch

[dipto0321]

Summary

The nodejs.org manifest was fetched via http.Get(url) against http.DefaultClient, which has no timeout. A hung nodejs.org (or a corporate proxy that silently drops the connection) would block nodeup upgrade / nodeup check indefinitely, with no recovery path — and even Ctrl-C had no effect because no context was threaded into the HTTP call. The cache write path was likewise non-atomic: two os.WriteFile calls (data + meta) could leave the cache in a mismatched state if the meta write failed.

Replace with FetchManifestCtx(ctx) (the existing FetchManifest() becomes a context.Background() wrapper) which:

• Builds the request via http.NewRequestWithContext, so a caller-supplied cmd.Context() from the CLI layer propagates and Ctrl-C cancels an in-flight fetch.
• Uses a package-level http.Client{Timeout: 30s}, so the worst-case floor for an unresponsive host is 30s even when the caller has no parent deadline.
• Retries up to 3 times with exponential backoff (200ms → 400ms → 800ms, capped at 2s) for transient errors — 5xx responses and network errors (DNS, connection refused, ...) — plus the explicitly retryable 4xx (408 / 429). Permanent 4xx (404 etc.) short-circuit immediately.
• Honors context cancellation between attempts; no backoff sleep fires if the caller's ctx is already done.

saveToCache is now atomic via temp-file + rename on both data and meta, so two concurrent nodeup invocations refreshing the cache can no longer tear the on-disk state. The cache helpers take an explicit cachePaths struct so tests can drive them hermetically.

Both CLI callers (upgrade.go, check.go) now pass cmd.Context() through to the new FetchManifestCtx. New tests in internal/node/dist_test.go cover: success path, retry-on-503, no-retry-on-404, context cancellation mid-flight (regression pin for the Ctrl-C behavior), retry budget exhaustion, the cache round-trip, the freshness gate (expired meta is rejected), and the no-tmp-leak invariant after a successful save.

Closes #48.

Linked issues

• Closes bug(node): no HTTP timeout/retry/context on nodejs.org manifest fetch; cache write not atomic #48

Type of change

• fix — bug fix (PATCH bump)

Checklist

• Title follows Conventional Commits (fix(scope): subject)
• I ran make ci locally and it passes (fmt, vet, test pass locally; make lint fails on the v2→v1 schema mismatch tracked in build(lint): .golangci.yml v1 schema incompatible with golangci-lint v2 (brew installs v2, make lint fails to even parse config) #62 — pre-existing on main, independent of this fix; CI runs golangci-lint v1.64.8 per .github/workflows/ci.yml and parses the config fine)
• I added or updated tests for the change (8 new tests in internal/node/dist_test.go: TestCacheLoadSaveRoundTrip, TestCacheLoadFromCache_ExpiredMetaIsRejected, TestSaveToCache_NoTempFileLeakOnSuccess, TestFetchManifestCtx_Success, TestFetchManifestCtx_RetriesOn5xx, TestFetchManifestCtx_Permanent4xxReturnsFast, TestFetchManifestCtx_ContextCancelMidFlight, TestFetchManifestCtx_RequestRetriesAreBounded; existing TestLatestLTS/LatestCurrent/ManifestUnmarshalUnion/ParseVersion/CacheExpiry are preserved)
• 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 (N/A — no CLI surface breaking; existing FetchManifest() and FetchManifestForce() are kept as thin context.Background() wrappers around the new _Ctx variants so all current call sites compile unchanged)

Scope notes

• The package-level httpClient is a single instance with a 30s Timeout. We don't open a fresh client per call — a *http.Client is documented as safe for concurrent use and the timeout is the floor we care about, not a per-attempt budget.
• manifestURL is exposed as a var (not const) so tests can redirect it to an httptest.Server.URL. The pre-fix http.Get(const string) had no seam; the new code's httpClient.Do(req) does, by virtue of one indirection through a var instead of a literal.
• withEmptyCache in the test file redirects os.UserCacheDir() by setting XDG_CACHE_HOME (Linux), HOME (macOS), and APP/LOCALAPPDATA/USERPROFILE (Windows). Without this, the developer's real cache would short-circuit every fetch test with a stale-but-fresh manifest read — exactly the test pollution TestCacheExpiry used to dodge by not actually calling loadFromCache.
• writeFileAtomic writes the data file first, then the meta. Readers that race us during the window between the data rename and the meta rename see the new data paired with the old (expired) meta and refuse to serve the cache — the safe failure mode (treated as stale).
• I deliberately did NOT implement a "concurrent invocations should serialize" layer above the temp-file rename. Two nodeup invocations racing refresh-cache is rare in practice (the upgrade command takes a process-wide lock via platform.AcquireLock per bug(platform): lock file implemented but never wired into upgrade/config — concurrent runs not actually blocked #44, and nodeup check runs read-only against an externally-writable cache). If we wanted to harden that further we'd put a cache-level flock around saveToCache; not worth the complexity here.
• I considered wiring manifestURL through an OPTIONS env var (NODEUP_NODEJS_INDEX_URL) for users behind a self-hosted mirror. Out of scope; issue bug(node): no HTTP timeout/retry/context on nodejs.org manifest fetch; cache write not atomic #48 is specifically about resilience, not configurability.

Out of scope (separate issues)

• internal/cli/packages.go runSnapshot/runRestore still use context.Background() instead of cmd.Context() — this is the body of bug(packages): interrupted-upgrade sentinel lifecycle broken (deleted on failure, never cleared after manual restore) #47 but a title-mismatch means it lives filed as fix(cli): context.Background() used instead of cmd.Context() in packages.go (contextcheck violation) #49.
• --cleanup / --yes precedence conflict — bug(cli): --yes silently overrides --no-cleanup, auto-deletes with zero confirmation #57.
• Path traversal in packages restore/diff via unsanitized manager name — fix(packages): path traversal via unsanitized manager name in packages restore/diff #51.
• asdf ASDF_DIR vs ASDF_DATA_DIR inconsistency — fix(detector): asdf uses ASDF_DIR vs ASDF_DATA_DIR inconsistently between docs, detector, and system-node classifier #50.
• Current() failure leaves active Node unprotected from cleanup — bug(cli): Current() failure leaves the active Node version unprotected from cleanup deletion #58.

Screenshots / output

User-visible behavior change:

• Before: nodeup upgrade against a hung nodejs.org blocks indefinitely. Ctrl-C does nothing (no context threaded through the HTTP call). User has no recovery path except kill -9.
• After: Same scenario:
  • If the cancel reaches the in-flight request, the fetch returns context canceled within ~ms and the upgrade command fails with a clean error message.
  • If cancellation never lands (e.g., the kernel TCP buffer is full and the connection is wedged), the package-level 30s Timeout fires and the fetch fails the same way.
  • With --offline, no network is touched and the cached manifest is used immediately.

Verification path: go test -race ./internal/node/... -v -run TestFetchManifestCtx covers all five fetch scenarios; the cache tests exercise the round-trip and freshness-gate paths.

[cocogitto-bot]

✔️ 95ede99...1bb5042 - Conventional commits check succeeded.