fix(update): actionable EACCES + suppress stale nudge

cmd/ana/main.go

@@ -67,7 +67,7 @@ func run(args []string, stdio cli.IO, env func(string) string) error {
 	// can place `--profile`/`--endpoint`/`--token-file` after the verb and
 	// still have config resolution honour them — ParseGlobal stops at the
 	// first positional, which would leave those flags invisible here.
-	global, _, err := cli.StripGlobals(args)
+	global, verbArgs, err := cli.StripGlobals(args)
 	if err != nil {
 		// Don't return here — let Dispatch produce the canonical usage error
 		// (it prints root help + err to stderr). Fall through with zero
@@ -131,10 +131,19 @@ func run(args []string, stdio cli.IO, env func(string) string) error {
 	// nil flows through drainNudge as a no-op.
 	nudgeCh := startNudge(env, loaded, global)
 	err = cli.Dispatch(ctx, verbs, args, stdio)
-	drainNudge(nudgeCh, 500*time.Millisecond, err, stdio.Stderr)
+	drainNudge(nudgeCh, 500*time.Millisecond, err, firstVerb(verbArgs), stdio.Stderr)
 	return err
 }
 
+// firstVerb returns the leading positional from StripGlobals' verb-args slice,
+// or "" for an empty slice. Keeps the drainNudge call site readable.
+func firstVerb(verbArgs []string) string {
+	if len(verbArgs) == 0 {
+		return ""
+	}
+	return verbArgs[0]
+}
+
 // startNudge launches the passive update-check goroutine when enabled and
 // returns a buffered channel that drainNudge reads. Returns nil whenever the
 // check is skipped so drainNudge can short-circuit without touching the
@@ -180,14 +189,21 @@ func startNudge(env func(string) string, loaded config.Config, global cli.Global
 // it produces a non-empty message and the verb did not return a help/usage
 // error, the message is written to stderr — we intentionally suppress the
 // nudge on help/usage paths so help text doesn't get crowded by an upgrade
-// prompt. A nil ch (check was skipped) or a timeout is a clean no-op.
-func drainNudge(ch chan string, timeout time.Duration, verbErr error, stderr io.Writer) {
+// prompt. A successful `ana update` is also suppressed: the goroutine captured
+// the pre-swap version at process start and can't know the binary was just
+// replaced, so its "new version available" line would contradict the verb's
+// own success output. A failed `ana update` still nudges — the user needs the
+// retry hint. A nil ch (check was skipped) or a timeout is a clean no-op.
+func drainNudge(ch chan string, timeout time.Duration, verbErr error, verb string, stderr io.Writer) {
 	if ch == nil {
 		return
 	}
 	if errors.Is(verbErr, cli.ErrHelp) || errors.Is(verbErr, cli.ErrUsage) {
 		return
 	}
+	if verb == "update" && verbErr == nil {
+		return
+	}
 	timer := time.NewTimer(timeout)
 	defer timer.Stop()
 	select {

cmd/ana/main_test.go

@@ -576,13 +576,14 @@ func TestStartNudge_SkipConditions(t *testing.T) {
 	}
 }
 
-// TestDrainNudge covers the four branches: nil channel, help-err suppression,
-// non-empty message printed, and empty message (no print).
+// TestDrainNudge covers every branch: nil channel, help-err suppression,
+// successful `ana update` suppression, failed `ana update` still nudges,
+// non-empty message printed, empty message (no print), and the timeout.
 func TestDrainNudge(t *testing.T) {
 	t.Parallel()
 	t.Run("nil channel is a no-op", func(t *testing.T) {
 		var buf bytes.Buffer
-		drainNudge(nil, time.Millisecond, nil, &buf)
+		drainNudge(nil, time.Millisecond, nil, "", &buf)
 		if buf.Len() != 0 {
 			t.Fatalf("stderr: %q", buf.String())
 		}
@@ -591,16 +592,34 @@ func TestDrainNudge(t *testing.T) {
 		ch := make(chan string, 1)
 		ch <- "should not print"
 		var buf bytes.Buffer
-		drainNudge(ch, time.Millisecond, cli.ErrHelp, &buf)
+		drainNudge(ch, time.Millisecond, cli.ErrHelp, "", &buf)
 		if buf.Len() != 0 {
 			t.Fatalf("stderr: %q", buf.String())
 		}
 	})
+	t.Run("update success suppresses", func(t *testing.T) {
+		ch := make(chan string, 1)
+		ch <- "stale nudge from pre-swap version"
+		var buf bytes.Buffer
+		drainNudge(ch, time.Millisecond, nil, "update", &buf)
+		if buf.Len() != 0 {
+			t.Fatalf("stderr: %q", buf.String())
+		}
+	})
+	t.Run("update failure still nudges", func(t *testing.T) {
+		ch := make(chan string, 1)
+		ch <- "retry hint"
+		var buf bytes.Buffer
+		drainNudge(ch, time.Millisecond, errors.New("permission denied"), "update", &buf)
+		if !strings.Contains(buf.String(), "retry hint") {
+			t.Fatalf("stderr: %q", buf.String())
+		}
+	})
 	t.Run("message printed", func(t *testing.T) {
 		ch := make(chan string, 1)
 		ch <- "hello"
 		var buf bytes.Buffer
-		drainNudge(ch, time.Millisecond, nil, &buf)
+		drainNudge(ch, time.Millisecond, nil, "", &buf)
 		if !strings.Contains(buf.String(), "hello") {
 			t.Fatalf("stderr: %q", buf.String())
 		}
@@ -609,21 +628,35 @@ func TestDrainNudge(t *testing.T) {
 		ch := make(chan string, 1)
 		ch <- ""
 		var buf bytes.Buffer
-		drainNudge(ch, time.Millisecond, nil, &buf)
+		drainNudge(ch, time.Millisecond, nil, "", &buf)
 		if buf.Len() != 0 {
 			t.Fatalf("stderr: %q", buf.String())
 		}
 	})
 	t.Run("timeout", func(t *testing.T) {
 		ch := make(chan string) // no sender
 		var buf bytes.Buffer
-		drainNudge(ch, 10*time.Millisecond, nil, &buf)
+		drainNudge(ch, 10*time.Millisecond, nil, "", &buf)
 		if buf.Len() != 0 {
 			t.Fatalf("stderr: %q", buf.String())
 		}
 	})
 }
 
+// TestFirstVerb covers the empty-slice branch and the typical case.
+func TestFirstVerb(t *testing.T) {
+	t.Parallel()
+	if got := firstVerb(nil); got != "" {
+		t.Errorf("nil slice: got %q, want empty", got)
+	}
+	if got := firstVerb([]string{}); got != "" {
+		t.Errorf("empty slice: got %q, want empty", got)
+	}
+	if got := firstVerb([]string{"update", "--json"}); got != "update" {
+		t.Errorf("got %q, want update", got)
+	}
+}
+
 // TestUpdateCmd_Help short-circuits on --help like every other leaf verb.
 func TestUpdateCmd_Help(t *testing.T) {
 	t.Parallel()

internal/update/CLAUDE.md

@@ -7,7 +7,7 @@ Powers the passive "new release available" nudge and the `ana update` self-updat
 - `update.go` — package doc, `HTTPDoer` interface, and the two package-level URL vars (`latestReleaseURL`, `releasesBaseURL`) that tests repoint at `httptest` servers.
 - `semver.go` — `CmpSemver` + `parseSemver`. Three-int semver with optional `v` prefix and `-prerelease` suffix; prerelease sorts below release at the same X.Y.Z so `prerelease: auto` beta→stable flows notify correctly. Malformed input returns 0 (never trigger a nudge on junk).
 - `check.go` — passive nudge surface: `LatestRelease`, `CacheDeps`, `CachePath` (XDG → HOME), `ParseInterval` (`nil`→4h, `"0"`/`"disable"`→off), `CachedCheck` (reads/writes `update-check.json` atomically), plus the unexported `cacheFile` / `readCache` / `writeCache` / `shouldNotify`.
-- `selfupdate.go` — `Deps` + `DefaultDeps` + `resolveDeps`, the `SelfUpdate` orchestration (resolve → compare → download → verify → extract → atomic replace), the `updateStatus` JSON shape + `emitStatus` (routes JSON through `cli.WriteJSON` for byte-compat), and `atomicReplace` (Unix rename-over; Windows rename-aside + rollback).
+- `selfupdate.go` — `Deps` + `DefaultDeps` + `resolveDeps`, the `SelfUpdate` orchestration (resolve → compare → download → verify → extract → atomic replace), the `updateStatus` JSON shape + `emitStatus` (routes JSON through `cli.WriteJSON` for byte-compat), `atomicReplace` (Unix rename-over; Windows rename-aside + rollback), and `formatReplaceErr` (maps `fs.ErrPermission` to platform-aware sudo/Administrator guidance so EACCES isn't a dead-end for users).
 - `download.go` — HTTP helpers (`httpGet`, `downloadFile` — streams body to disk and returns the sha256 via `TeeReader`, avoiding a re-read, `downloadBody` — 1 MiB-capped for `checksums.txt`) and `verifyChecksum` (parses goreleaser's `<hex>  <filename>` or sha256sum's `<hex> *<filename>` format and compares against the caller-supplied hex).
 - `extract.go` — `extractBinary` dispatches on `archiveExt`; `extractFromTarGz` / `extractFromZip` walk the archive for the matching member and hand the reader to `writeBinary` (0755).
 - `<source>_test.go` — one test file per source; shared helpers (`fakeDoer`, `releaseServer`, `stageUpdate`, `wantErr`, `withURLs`, `fakeArchive`) live in `update_test.go` per the repo convention. 100% coverage gate.

internal/update/selfupdate.go

@@ -2,8 +2,10 @@ package update
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -156,21 +158,42 @@ func emitStatus(w io.Writer, jsonOut bool, st updateStatus, plain string) error
 func atomicReplace(deps Deps, exePath, newPath string) error {
 	if deps.GOOS != "windows" {
 		if err := deps.Rename(newPath, exePath); err != nil {
-			return fmt.Errorf("update: replace %s: %w", exePath, err)
+			return formatReplaceErr(deps.GOOS, exePath, err)
 		}
 		return nil
 	}
 	oldPath := exePath + ".old"
 	if err := deps.Rename(exePath, oldPath); err != nil {
-		return fmt.Errorf("update: rename %s -> %s: %w", exePath, oldPath, err)
+		// No .old exists yet, so no rollback wording; route through the
+		// permission-aware formatter for the EACCES-equivalent case.
+		return formatReplaceErr(deps.GOOS, exePath, err)
 	}
 	if err := deps.Rename(newPath, exePath); err != nil {
-		// Roll the old binary back; if that also fails we can't fix it for
-		// the user so tell them where the last-known-good copy lives.
+		// Roll the old binary back; if that also fails and the root cause
+		// is a permission denial, the `.old` we'd name in the recovery
+		// message sits in the same admin-only directory — point the user
+		// at elevation instead of a path they also can't touch.
 		if rbErr := deps.Rename(oldPath, exePath); rbErr != nil {
+			if errors.Is(err, fs.ErrPermission) {
+				return formatReplaceErr(deps.GOOS, exePath, err)
+			}
 			return fmt.Errorf("update: replace %s failed (%w); rollback also failed (%w); recover from %s", exePath, err, rbErr, oldPath)
 		}
-		return fmt.Errorf("update: replace %s: %w", exePath, err)
+		return formatReplaceErr(deps.GOOS, exePath, err)
 	}
 	return nil
 }
+
+// formatReplaceErr turns an os.Rename failure into an actionable message.
+// On fs.ErrPermission we tell the user exactly how to retry (sudo on Unix,
+// Administrator on Windows) instead of leaking the tempdir path the user
+// never picked. Every other error falls through to the plain wrapped form.
+func formatReplaceErr(goos, exePath string, err error) error {
+	if errors.Is(err, fs.ErrPermission) {
+		if goos == "windows" {
+			return fmt.Errorf("update: cannot write to %s: permission denied. Re-run from an elevated (Administrator) shell, or reinstall ana into a user-writable directory (e.g. %%LOCALAPPDATA%%)", exePath)
+		}
+		return fmt.Errorf("update: cannot write to %s: permission denied. Re-run with sudo, or reinstall ana into a user-writable directory", exePath)
+	}
+	return fmt.Errorf("update: replace %s: %w", exePath, err)
+}

internal/update/selfupdate_test.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"os"
 	"strings"
@@ -286,6 +287,130 @@ func TestEmitStatus_WriteError(t *testing.T) {
 	wantErr(t, err, "emit status")
 }
 
+// TestFormatReplaceErr_PermissionGuidance pins the user-facing wording for the
+// EACCES branch on Unix and Windows. The strings are what users will see on
+// perm-denied paths (e.g. /usr/local/bin/ana, C:\Program Files\ana), and the
+// plan commits to exactly this copy — a silent wording change would erase the
+// actionable hint that motivated this fix.
+func TestFormatReplaceErr_PermissionGuidance(t *testing.T) {
+	t.Parallel()
+	t.Run("unix permission denied", func(t *testing.T) {
+		err := formatReplaceErr("linux", "/usr/local/bin/ana", fs.ErrPermission)
+		got := err.Error()
+		if !strings.Contains(got, "cannot write to /usr/local/bin/ana") {
+			t.Errorf("missing path: %q", got)
+		}
+		if !strings.Contains(got, "sudo") {
+			t.Errorf("missing sudo hint: %q", got)
+		}
+		if strings.Contains(got, "/tmp") || strings.Contains(got, "ana-update-") {
+			t.Errorf("leaked tempdir path: %q", got)
+		}
+	})
+	t.Run("windows permission denied", func(t *testing.T) {
+		err := formatReplaceErr("windows", `C:\Program Files\ana\ana.exe`, fs.ErrPermission)
+		got := err.Error()
+		if !strings.Contains(got, "Administrator") {
+			t.Errorf("missing Administrator hint: %q", got)
+		}
+		if !strings.Contains(got, "LOCALAPPDATA") {
+			t.Errorf("missing LOCALAPPDATA hint: %q", got)
+		}
+	})
+	t.Run("non-permission error falls through", func(t *testing.T) {
+		err := formatReplaceErr("linux", "/x/ana", errors.New("disk full"))
+		got := err.Error()
+		if !strings.Contains(got, "disk full") {
+			t.Errorf("should preserve cause: %q", got)
+		}
+		if strings.Contains(got, "sudo") {
+			t.Errorf("should not suggest sudo for non-permission errors: %q", got)
+		}
+	})
+}
+
+// permDeniedErr wraps a *PathError around fs.ErrPermission so errors.Is
+// matches both the stdlib rename error shape (PathError) and the sentinel.
+func permDeniedErr() error {
+	return &fs.PathError{Op: "rename", Path: "x", Err: fs.ErrPermission}
+}
+
+// TestSelfUpdate_UnixEACCES drives the unix rename-permission branch end to end
+// and asserts the user-facing message includes the sudo hint rather than the
+// raw tempdir path.
+func TestSelfUpdate_UnixEACCES(t *testing.T) {
+	exePath, deps := stageUpdate(t, "linux", "amd64", "ana")
+	deps.Rename = func(string, string) error { return permDeniedErr() }
+	r := &releaseServer{tag: "v2.0.0", archiveName: "ana_2.0.0_linux_amd64.tar.gz"}
+	r.archiveBody = fakeArchive(t, "tar.gz", "ana", []byte("NEW"))
+	srv := r.serve(t)
+	defer srv.Close()
+	withURLs(t, srv)
+
+	err := SelfUpdate(context.Background(), deps, "1.0.0", io.Discard, false)
+	wantErr(t, err, "cannot write to "+exePath)
+	if !strings.Contains(err.Error(), "sudo") {
+		t.Errorf("missing sudo hint: %v", err)
+	}
+	if got, _ := os.ReadFile(exePath); string(got) != "old" {
+		t.Errorf("exe must not change on EACCES: %q", got)
+	}
+}
+
+// TestSelfUpdate_WindowsEACCES_AsideFails covers the first-rename permission
+// denial on Windows (no .old yet, no rollback) and pins the Administrator hint.
+func TestSelfUpdate_WindowsEACCES_AsideFails(t *testing.T) {
+	exePath, deps := stageUpdate(t, "windows", "amd64", "ana.exe")
+	deps.Rename = func(string, string) error { return permDeniedErr() }
+	r := &releaseServer{tag: "v2.0.0", archiveName: "ana_2.0.0_windows_amd64.zip"}
+	r.archiveBody = fakeArchive(t, "zip", "ana.exe", []byte("NEW"))
+	srv := r.serve(t)
+	defer srv.Close()
+	withURLs(t, srv)
+
+	err := SelfUpdate(context.Background(), deps, "1.0.0", io.Discard, false)
+	wantErr(t, err, "cannot write to "+exePath)
+	if !strings.Contains(err.Error(), "Administrator") {
+		t.Errorf("missing Administrator hint: %v", err)
+	}
+	// No ".old" recovery wording — the aside rename never succeeded.
+	if strings.Contains(err.Error(), "recover from") {
+		t.Errorf("should not reference .old recovery when aside failed: %v", err)
+	}
+}
+
+// TestSelfUpdate_WindowsEACCES_SecondFailsRollbackFails covers the pathological
+// case: aside succeeded, rename-in failed with EACCES, rollback also failed.
+// The old "recover from <.old>" wording is misleading there — .old sits in the
+// same admin-only directory — so the message must point at elevation.
+func TestSelfUpdate_WindowsEACCES_SecondFailsRollbackFails(t *testing.T) {
+	exePath, deps := stageUpdate(t, "windows", "amd64", "ana.exe")
+	var n int
+	deps.Rename = func(o, np string) error {
+		n++
+		switch n {
+		case 1:
+			return os.Rename(o, np) // aside succeeds
+		default:
+			return permDeniedErr() // rename-in + rollback both fail EACCES
+		}
+	}
+	r := &releaseServer{tag: "v2.0.0", archiveName: "ana_2.0.0_windows_amd64.zip"}
+	r.archiveBody = fakeArchive(t, "zip", "ana.exe", []byte("NEW"))
+	srv := r.serve(t)
+	defer srv.Close()
+	withURLs(t, srv)
+
+	err := SelfUpdate(context.Background(), deps, "1.0.0", io.Discard, false)
+	wantErr(t, err, "cannot write to "+exePath)
+	if !strings.Contains(err.Error(), "Administrator") {
+		t.Errorf("missing Administrator hint: %v", err)
+	}
+	if strings.Contains(err.Error(), "recover from") {
+		t.Errorf("should not point at .old when both sides are admin-only: %v", err)
+	}
+}
+
 func TestDeps(t *testing.T) {
 	t.Parallel()
 	t.Run("DefaultDeps populates everything", func(t *testing.T) {