Skip to content

bug(packages): restore aborts on first failed install; migration report never written (re-files #46) #103

Description

@dipto0321

Summary

Re-files #46, which was closed on 2026-07-03 with no fix commit, no closing PR, and no comment (verified via the issue timeline — the closed event has a null commit_id). Both halves of the original finding are still present on main (e37b326):

  1. Restore aborts on the first failed install. internal/packages/snapshot.go:171-179installPackages returns on the first npm install -g error, so one broken/renamed/yanked package kills the migration of every package after it. A user upgrading with 30 globals where feat(detector): implement Volta detection with mocked tests #3 fails loses 27 packages silently.

  2. Migration report is never written. internal/packages/report.go (MigrationReport, PackageResult, Save()) has zero call sites anywhere in the repo — the entire file is dead code. platform.DataDir()/reports/ exists as a documented data dir but nothing ever writes to it.

How to fix

Part 1 — continue past failures, collect results

Rework installPackages to loop all packages, recording a PackageResult per package instead of returning early:

func installPackages(ctx context.Context, pkgs []Package) ([]PackageResult, error) {
	results := make([]PackageResult, 0, len(pkgs))
	var failed int
	for _, pkg := range pkgs {
		if ctx.Err() != nil {
			return results, ctx.Err() // Ctrl-C still aborts promptly
		}
		_, err := platform.RunShell(ctx, "npm", "install", "-g", pkgSpec(pkg))
		r := PackageResult{Name: pkg.Name, AttemptedVersion: pkg.Version, Status: "ok"}
		if err != nil {
			r.Status = "failed"
			r.Error = err.Error()
			failed++
		}
		results = append(results, r)
	}
	if failed > 0 {
		return results, fmt.Errorf("%d of %d packages failed to install", failed, len(pkgs))
	}
	return results, nil
}

Context cancellation must still abort the loop (the ctx.Err() check) — only per-package npm failures continue.

Part 2 — wire up the migration report

  • Restore / RestoreFromSnapshot return the []PackageResult (or accept a callback) so the caller can build the report.
  • In internal/cli/upgrade.go's restore step (~line 309-317), construct a MigrationReport{Timestamp, Manager, FromVersion (oldVersion), ToVersion (new default), Results} and Save() it to platform.DataDir()/reports/ — on both success and partial failure.
  • Print the report path in the post-restore summary, especially when some packages failed, so the user knows what to re-install by hand.

Sentinel semantics (decide + document)

upgrade.go:100-103 removes the sentinel only when restoreSucceeded. With partial failures now possible, define: partial failure keeps the sentinel (restoreSucceeded = false), so the interrupted-upgrade replay path stays available — but the report still gets written either way. Document this in the sentinel arming comment.

Tests

  • Table-driven test with the runShell seam failing on package N of M → asserts packages N+1..M were still attempted and the returned results carry the right statuses.
  • Test that a report file lands in reports/ with correct Results on partial failure.
  • Test that context cancellation aborts mid-loop (results truncated, error is ctx.Err()).

Acceptance criteria

  • One failing global package no longer prevents the rest from migrating.
  • Every nodeup upgrade migration writes a report file to platform.DataDir()/reports/; path surfaced to the user on partial failure.
  • internal/packages/report.go has real call sites (no longer dead code).
  • Sentinel behavior on partial failure is deliberate and documented.

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