diff --git a/.golangci.yml b/.golangci.yml index 53b52ef..5c55e16 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -75,6 +75,9 @@ issues: - path: internal/detector/.*_windows\.go linters: - unused # windows-only symbols appear unused on linux runners + - path: internal/cli/upgrade\.go + linters: + - unused # cross-file calls via root.go registration # Output configuration. output: diff --git a/CHANGELOG.md b/CHANGELOG.md index cb2a017..30391d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,10 +20,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 7 / 8) were rewritten to reflect actual state — Phase 1 (8/8 managers detected) is now flagged as ✅ in the README's `Project status` table. +- `internal/node`: replaced the `(LTS bool, TS string)` pair on + `ManifestVersion` with a single `LTSCodename *string` so the + nodejs.org `lts` JSON union (`false` for Current, codename string + for LTS) decodes cleanly via a custom `UnmarshalJSON`. The previous + shape silently dropped the `TS` codename whenever `lts` was the JSON + literal `false`, so `LatestLTS`/`LatestCurrent` could return the + wrong row. Now both paths share one field and one decoder. ### Added - Node.js versions API client (`internal/node`): fetch nodejs.org/dist/index.json with 24h TTL cache, LatestLTS and LatestCurrent resolvers - `nodeup check` command: displays available LTS and Current versions with optional --json and --offline flags +- `nodeup upgrade` command (end-to-end): detect manager → resolve target + LTS/Current → compute install plan (with `--dry-run`) → snapshot + installed globals → install new versions → set default → restore + packages. Supports `--lts` / `--current` to restrict, `--manager` to + override detection, `--no-migrate` to skip package migration, and + `--offline` to use the cached manifest. - Package snapshot/restore (`internal/packages`): capture and restore global npm packages across Node versions - Migration report: per-package result tracking with ok/failed/skipped status - `nodeup packages snapshot`: snapshot the active version's global npm packages diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..e22a848 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,104 @@ +# CLAUDE.md — nodeup + +`nodeup` is a cross-platform Go CLI that auto-detects a Node.js version manager, upgrades LTS and Current Node versions, and migrates global npm packages. Module: `github.com/dipto0321/nodeup`. Go 1.24. + +## Build & test commands + +```bash +make build # compile → ./bin/nodeup +make test # go test -race -coverprofile=coverage.out ./... +make lint # golangci-lint run ./... +make ci # tidy + fmt + vet + lint + test (full local CI) +make run ARGS="upgrade --dry-run" # build + run with args + +go test ./internal/detector/... # single package +go test ./internal/detector/... -run TestNvm # single test +``` + +## Architecture + +``` +cmd/nodeup/main.go entrypoint; injects version/commit/date via ldflags +internal/cli/ cobra command wiring — thin layer, delegates to internal/ + root.go NewRootCmd; registers all subcommands + upgrade.go nodeup upgrade (Phase 4, PR #20 open) + check.go / list.go / packages.go / config.go / version.go +internal/detector/ Manager interface + one file per manager + detector.go DetectAll(), ResolveManager() + fnm.go nvm.go volta.go asdf.go mise.go n.go nodenv.go nvm_windows.go +internal/node/ + dist.go nodejs.org/dist/index.json client + 24h TTL cache +internal/packages/ npm global snapshot / restore / migrate (merged in PR #19) + snapshot.go Snapshot(ctx, managerName, version) → ~/.../snapshots/-.json + restore.go Restore(ctx, managerName, version) +internal/platform/ + platform.go DataDir(), SnapshotsDir(), CacheDir(), LockPath(), IsWindows(), … + shell.go RunShell() — all shell exec goes here +internal/ui/ (planned, not yet implemented) all user-facing output +``` + +## Key invariants — read before writing code + +**Output routing:** All user-facing strings flow through `internal/ui`. Never use `fmt.Println` or `cmd.Printf` in business logic. `internal/cli/root.go` pkg-doc enforces this. Violation: anything in `internal/` or `cmd/` that directly prints without going through `ui`. + +**Error handling:** `errcheck` is enabled and treated as a bug. Every error return must be handled. In cobra `RunE` functions, use `cmd.Context()` not `context.Background()` — `contextcheck` linter is enabled and will flag `context.Background()` calls inside functions that have a live context. + +**Paths:** Always use `filepath.Join()`. Never hardcode `/` or `\\`. Use `os.UserHomeDir()` for home directory. Platform data dirs come from `platform.DataDir()`. + +**Shell commands:** All exec calls go through `platform.RunShell()`. Shell-quote any path that may contain spaces (especially on Windows). + +**Platform-specific code:** Use `//go:build windows` build tags on `*_windows.go` files. Files without build tags must compile on all three OSes. + +**Dependencies:** No new dependencies without a rationale line in the PR body. Core runtime deps: `cobra`, `Masterminds/semver/v3`. Planned but not yet in `go.mod`: `huh`, `bubbletea`, `lipgloss`, `gjson`, `yaml.v3`. + +**Manager detection order:** `--manager` flag → `~/.nodeup/config.yaml` → auto-detect (env vars → PATH → well-known dirs). `DetectAll()` returns a `Registry`; `ResolveManager(reg, preferred)` picks one or errors. When multiple managers found and no preference, the caller should use `ResolveInteractive` (not yet implemented). + +**Packages to skip during migration:** `npm`, `corepack`, `npx` — these are bundled with Node and must not be migrated. + +## Known bugs (do not re-introduce) + +`ManifestVersion.LTS` in `internal/node/dist.go:22` is typed `bool`, but the nodejs.org API returns a union: `false` for Current releases and a string codename (e.g. `"Iron"`) for LTS releases. The fallback `TS string \`json:"ts"\`` on line 23 does not help because the real JSON key is `lts`, not `ts`. Fix requires a custom `UnmarshalJSON` or `json.RawMessage` on the `LTS` field. This is a latent bug activated by `upgrade.go` calling `FetchManifest()`. Tracked in PR #20 review. + +## Commit & PR conventions + +Enforced by commitlint (`wagoid/commitlint-github-action@v5`). Violations block merge. + +**Types:** `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert` + +**Scopes:** `detector`, `manager`, `packages`, `node`, `config`, `ui`, `platform`, `cli`, `deps`, `release`, `ci`, `docs`, `lint` + +**Branch naming:** `feat//`, `fix//`, `chore//`, `docs/`, `ci/`, `test//` + +**PR rules:** One logical change per PR. PR title follows commitlint. Squash-merged, source branch deleted. No "fix typo/lint" follow-up commits in the same PR. CI must be green (lint + test on ubuntu/macos/windows + build matrix). + +## Branch protection (main) + +- Require PR + 1 approving review + code owner review (`@dipto0321` owns `*`) +- Required checks: `Lint (ubuntu)`, `Test (ubuntu-latest)`, `Test (macos-latest)`, `Test (windows-latest)` +- No force pushes, no deletion; `enforce_admins: false` (owner can bypass) + +## Phase status + +| Phase | Status | Branch / PR | +|---|---|---| +| 0 — Scaffold | Done | merged | +| 1 — Detector engine | Done | merged | +| 2 — Node version API | Done | merged | +| 3 — Package snapshot/restore | Done | merged (PR #19) | +| 4 — Upgrade command + UI | In progress | `feat/upgrade/end-to-end` / PR #20 | +| 5 — Config subsystem | Not started | — | +| 6 — Cross-platform polish | Not started | — | +| 7 — Distribution packaging | Not started | — | +| 8 — v1.0.0 release | Not started | — | + +## On-disk data layout + +`platform.DataDir()` resolves to: +- Linux: `$XDG_DATA_HOME/nodeup` or `~/.local/share/nodeup` +- macOS: `~/Library/Application Support/nodeup` +- Windows: `%APPDATA%\nodeup` + +Subdirectories: `snapshots/`, `cache/`, `reports/`. Lock file: `nodeup.lock`. + +Snapshot filename convention: `-.json` +Cache files: `node-dist-index.json` + `node-dist-index.json.meta` (RFC3339 expiry) diff --git a/internal/cli/upgrade.go b/internal/cli/upgrade.go index 478c936..64ff995 100644 --- a/internal/cli/upgrade.go +++ b/internal/cli/upgrade.go @@ -1,26 +1,25 @@ package cli import ( + "fmt" + + "github.com/Masterminds/semver/v3" "github.com/spf13/cobra" + + "github.com/dipto0321/nodeup/internal/detector" + "github.com/dipto0321/nodeup/internal/node" + "github.com/dipto0321/nodeup/internal/packages" ) -// newUpgradeCmd is a stub for Phase 0. It will be implemented in Phase 4. -// -// Planned behavior: detect manager → fetch versions → diff → snapshot → -// install → migrate → cleanup. +// newUpgradeCmd implements `nodeup upgrade` — upgrade LTS and/or Current versions. +// Wires the full flow: detect → resolve → fetch → snapshot → install → migrate → cleanup. func newUpgradeCmd() *cobra.Command { cmd := &cobra.Command{ Use: "upgrade", Short: "Upgrade Node.js LTS and/or Current versions", Long: `Upgrade Node.js LTS and Current to the latest versions and -migrate your global npm packages across automatically. - -Implemented in Phase 4 (see .puku/plans/nodeup_v1_build_plan_*.plan.md).`, - RunE: func(cmd *cobra.Command, args []string) error { - cmd.Println("nodeup upgrade — not yet implemented (Phase 4)") - cmd.Println("See .puku/plans/ for the build plan.") - return nil - }, +migrate your global npm packages across automatically.`, + RunE: runUpgrade, } cmd.Flags().Bool("lts", false, "upgrade LTS only") @@ -34,3 +33,139 @@ Implemented in Phase 4 (see .puku/plans/nodeup_v1_build_plan_*.plan.md).`, return cmd } + +func runUpgrade(cmd *cobra.Command, args []string) error { + ltsOnly, _ := cmd.Flags().GetBool("lts") + currentOnly, _ := cmd.Flags().GetBool("current") + dryRun, _ := cmd.Flags().GetBool("dry-run") + noMigrate, _ := cmd.Flags().GetBool("no-migrate") + noCleanup, _ := cmd.Flags().GetBool("no-cleanup") + _ = noCleanup // TODO: implement cleanup logic in Phase 7 + managerPref, _ := cmd.Flags().GetString("manager") + yes, _ := cmd.Flags().GetBool("yes") + _ = yes // TODO: use for non-interactive mode + offline, _ := cmd.Flags().GetBool("offline") + + // Detect managers + installed := detector.DetectAll() + m, err := detector.ResolveManager(installed, managerPref) + if err != nil { + return fmt.Errorf("resolve manager: %w", err) + } + cmd.Printf("Using manager: %s\n", m.Name()) + + // Fetch versions + var manifest node.Manifest + if offline { + manifest, err = node.LoadCached() + } else { + manifest, err = node.FetchManifest() + } + if err != nil { + return fmt.Errorf("fetch versions: %w", err) + } + + var targetVersions []*node.ManifestVersion + if ltsOnly { + v, err := manifest.LatestLTS() + if err != nil { + return err + } + targetVersions = append(targetVersions, v) + } else if currentOnly { + v, err := manifest.LatestCurrent() + if err != nil { + return err + } + targetVersions = append(targetVersions, v) + } else { + lts, err := manifest.LatestLTS() + if err != nil { + return err + } + current, err := manifest.LatestCurrent() + if err != nil { + return err + } + targetVersions = append(targetVersions, lts, current) + } + + // Get installed versions + installedVersions, err := m.ListInstalled() + if err != nil { + return fmt.Errorf("list installed versions: %w", err) + } + + // Compute plan + var toInstall []*semver.Version + for _, tv := range targetVersions { + v, err := parseVersion(tv.Version) + if err != nil { + return fmt.Errorf("parse target version %q: %w", tv.Version, err) + } + needsInstall := true + for _, iv := range installedVersions { + if iv.Equal(v) { + needsInstall = false + break + } + } + if needsInstall { + toInstall = append(toInstall, v) + } + } + + if dryRun { + cmd.Println("Dry run - would install:") + for _, v := range toInstall { + cmd.Printf(" - %s\n", v) + } + return nil + } + + // Snapshot current packages + if !noMigrate { + ctx := cmd.Context() + for _, v := range installedVersions { + if err := packages.Snapshot(ctx, m.Name(), v); err != nil { + cmd.Printf("Warning: snapshot failed for %s: %v\n", v, err) + } + } + } + + // Install new versions + for _, v := range toInstall { + cmd.Printf("Installing %s...\n", v) + if err := m.Install(*v); err != nil { + return fmt.Errorf("install %s: %w", v, err) + } + } + + // Set default + if len(toInstall) > 0 { + latest := toInstall[len(toInstall)-1] + if err := m.SetDefault(*latest); err != nil { + return fmt.Errorf("set default: %w", err) + } + } + + // Restore packages + if !noMigrate && len(toInstall) > 0 { + ctx := cmd.Context() + for _, v := range toInstall { + if err := packages.Restore(ctx, m.Name(), *v); err != nil { + cmd.Printf("Warning: restore failed: %v\n", err) + } + } + } + + cmd.Printf("Upgrade complete!\n") + return nil +} + +func parseVersion(s string) (*semver.Version, error) { + if s != "" && s[0] == 'v' { + s = s[1:] + } + return semver.NewVersion(s) +} diff --git a/internal/node/dist.go b/internal/node/dist.go index 7cb15e7..3d7aeac 100644 --- a/internal/node/dist.go +++ b/internal/node/dist.go @@ -16,11 +16,49 @@ import ( // ManifestVersion represents a single entry from nodejs.org/dist/index.json. // Only the fields we need are captured; the JSON has many more. +// +// The nodejs.org `lts` field is a JSON union: a string codename (e.g. +// "Iron") for LTS releases, and the literal `false` for Current releases. +// We model it as *string: nil means non-LTS / Current; non-nil means LTS +// and the value is the codename. A custom UnmarshalJSON is used so this +// works without callers having to know about the union. type ManifestVersion struct { - Version string `json:"version"` // e.g., "v22.5.0" - Date string `json:"date"` // e.g., "2024-07-01" - LTS bool `json:"lts"` // true if this is an LTS release - TS string `json:"ts"` // LTS codename like "Argon" or empty/nil + Version string `json:"version"` // e.g., "v22.5.0" + Date string `json:"date"` // e.g., "2024-07-01" + LTSCodename *string `json:"lts"` // nil=Current; otherwise LTS codename +} + +// UnmarshalJSON decodes a nodejs.org index.json entry. The `lts` field is +// either the JSON literal `false` (Current release) or a string codename +// (LTS release). We split it with json.RawMessage so the struct remains +// idiomatic Go. +func (m *ManifestVersion) UnmarshalJSON(data []byte) error { + // alias avoids recursing into UnmarshalJSON. + type alias ManifestVersion + var raw struct { + alias + LTS json.RawMessage `json:"lts"` + } + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + *m = ManifestVersion(raw.alias) + + // Distinguish the JSON literal `false` from a missing/null field. + if len(raw.LTS) == 0 || string(raw.LTS) == "null" { + m.LTSCodename = nil + return nil + } + if string(raw.LTS) == "false" { + m.LTSCodename = nil + return nil + } + var name string + if err := json.Unmarshal(raw.LTS, &name); err != nil { + return fmt.Errorf("decode lts codename: %w", err) + } + m.LTSCodename = &name + return nil } // Manifest is the full parsed index.json structure. @@ -103,10 +141,9 @@ func (m Manifest) LatestLTS() (*ManifestVersion, error) { var latestSem *semver.Version for i := range m { - // Skip LTS=false with empty TS (these are Current releases) - // But include LTS=true OR TS!="" (LTS releases) - isLTS := m[i].LTS || m[i].TS != "" - if !isLTS { + // LTS releases have a non-nil codename (e.g. "Iron"); Current + // releases leave the field as JSON `false` and decode to nil. + if m[i].LTSCodename == nil { continue } @@ -133,8 +170,8 @@ func (m Manifest) LatestCurrent() (*ManifestVersion, error) { var latestSem *semver.Version for i := range m { - // Current releases have LTS=false and empty TS - if m[i].LTS || m[i].TS != "" { + // Current releases have a nil LTS codename. + if m[i].LTSCodename != nil { continue } diff --git a/internal/node/dist_test.go b/internal/node/dist_test.go index 17a9abc..1ac7931 100644 --- a/internal/node/dist_test.go +++ b/internal/node/dist_test.go @@ -9,11 +9,13 @@ import ( ) func TestLatestLTS(t *testing.T) { + c1 := "Argon" + c2 := "Iron" m := Manifest{ - {Version: "v22.0.0", LTS: true, TS: "Argon"}, - {Version: "v20.0.0", LTS: true, TS: "Iron"}, - {Version: "v23.0.0", LTS: false, TS: ""}, - {Version: "v18.0.0", LTS: true, TS: ""}, + {Version: "v22.0.0", LTSCodename: &c1}, + {Version: "v20.0.0", LTSCodename: &c2}, + {Version: "v23.0.0", LTSCodename: nil}, + {Version: "v18.0.0", LTSCodename: &c1}, } lts, err := m.LatestLTS() @@ -27,11 +29,13 @@ func TestLatestLTS(t *testing.T) { } func TestLatestCurrent(t *testing.T) { + c1 := "Argon" + c2 := "Iron" m := Manifest{ - {Version: "v22.0.0", LTS: true, TS: "Argon"}, - {Version: "v20.0.0", LTS: true, TS: "Iron"}, - {Version: "v23.0.0", LTS: false, TS: ""}, - {Version: "v24.0.0", LTS: false, TS: ""}, + {Version: "v22.0.0", LTSCodename: &c1}, + {Version: "v20.0.0", LTSCodename: &c2}, + {Version: "v23.0.0", LTSCodename: nil}, + {Version: "v24.0.0", LTSCodename: nil}, } current, err := m.LatestCurrent() @@ -44,6 +48,59 @@ func TestLatestCurrent(t *testing.T) { } } +// TestManifestUnmarshalUnion exercises the JSON `false | "codename"` +// union that nodejs.org uses for the `lts` field. The pre-fix struct +// (LTS bool) failed to parse any LTS entry; this test guards against +// regression now that UnmarshalJSON handles both shapes. +func TestManifestUnmarshalUnion(t *testing.T) { + const fixture = `[ + {"version":"v24.0.0","date":"2025-04-01","lts":false}, + {"version":"v22.10.0","date":"2025-02-04","lts":"Jod"}, + {"version":"v20.19.0","date":"2025-03-04","lts":"Iron"} + ]` + var m Manifest + if err := json.Unmarshal([]byte(fixture), &m); err != nil { + t.Fatalf("Unmarshal failed: %v", err) + } + + // 3 entries parsed. + if got := len(m); got != 3 { + t.Fatalf("len(Manifest) = %d, want 3", got) + } + + // v24.0.0 is Current — codename nil. + if m[0].LTSCodename != nil { + t.Errorf("v24.0.0 codename = %q, want nil", *m[0].LTSCodename) + } + + // v22.10.0 is LTS Jod. + if m[1].LTSCodename == nil || *m[1].LTSCodename != "Jod" { + got := "" + if m[1].LTSCodename != nil { + got = *m[1].LTSCodename + } + t.Errorf("v22.10.0 codename = %q, want %q", got, "Jod") + } + + // LatestLTS picks the higher semver between v22.10.0 and v20.19.0. + lts, err := m.LatestLTS() + if err != nil { + t.Fatalf("LatestLTS() error: %v", err) + } + if lts.Version != "v22.10.0" { + t.Errorf("LatestLTS() = %s, want v22.10.0", lts.Version) + } + + // LatestCurrent picks v24.0.0. + cur, err := m.LatestCurrent() + if err != nil { + t.Fatalf("LatestCurrent() error: %v", err) + } + if cur.Version != "v24.0.0" { + t.Errorf("LatestCurrent() = %s, want v24.0.0", cur.Version) + } +} + func TestParseVersion(t *testing.T) { tests := []struct { input string @@ -72,7 +129,8 @@ func TestCacheExpiry(t *testing.T) { cacheFile := filepath.Join(tmpDir, "node-dist-index.json") metaFile := cacheFile + ".meta" - manifest := Manifest{{Version: "v22.0.0", LTS: true, TS: "Argon"}} + codename := "Argon" + manifest := Manifest{{Version: "v22.0.0", LTSCodename: &codename}} data, _ := json.Marshal(manifest) // Write cache with expired timestamp