Skip to content

fix(update): actionable EACCES + suppress stale nudge#29

Merged
bradfair merged 1 commit into
mainfrom
fix/update-eacces-stale-nudge
Apr 23, 2026
Merged

fix(update): actionable EACCES + suppress stale nudge#29
bradfair merged 1 commit into
mainfrom
fix/update-eacces-stale-nudge

Conversation

@bradfair
Copy link
Copy Markdown
Contributor

@bradfair bradfair commented Apr 23, 2026

Summary

Two papercuts from the first post-release ana update run (issue #28):

  • Hostile EACCES. atomicReplace wrapped os.Rename with bare %w, so a perm-denied write to /usr/local/bin/ana surfaced as raw permission denied + tempdir path. New formatReplaceErr detects fs.ErrPermission and returns platform-aware guidance (sudo on Unix, Administrator / %LOCALAPPDATA% on Windows). Routed through all three rename error sites in atomicReplace. The Windows rollback-also-fails branch also skips the misleading "recover from <.old>" wording when the root cause is EACCES — .old sits in the same admin-only directory.
  • Stale nudge after ana update. The passive update-check goroutine captures the pre-swap build-time version, so it still emitted "new version available" after a successful self-update. Suppressed at drainNudge via a new verb parameter: only when verb == "update" && verbErr == nil. Failed updates (EACCES etc.) still nudge so the user gets the retry hint.

Test plan

  • make test (race)
  • make cover./internal/... still 100%
  • make lint clean
  • Smoke: go build -o /tmp/ana ./cmd/ana && /tmp/ana update → no trailing nudge on "already at version" path
  • Manual smoke on a perm-denied path (sudo install -m 0755 /tmp/ana /usr/local/bin/ana && ana update) — to be done by reviewer on release build

Closes #28

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Enhanced error messages for failed updates due to insufficient permissions, with clear OS-specific remediation steps (elevated privileges guidance)
  • Suppressed unnecessary update-check output following successful ana update execution

atomicReplace wrapped os.Rename with %w, so a permission-denied write
to /usr/local/bin/ana surfaced as raw "permission denied" plus the
tempdir path — users had to guess that sudo was the fix.
formatReplaceErr now detects fs.ErrPermission and returns
platform-aware guidance: "Re-run with sudo" on Unix, "Re-run from an
elevated (Administrator) shell" on Windows. The Windows rollback path
also skips the misleading "recover from <.old>" wording when the
root cause is EACCES, since .old sits in the same admin-only dir.

The passive update-check goroutine captures the build-time version at
process start and emitted its "new version available" nudge even
after `ana update` had just swapped the binary. Suppress at the
drainNudge layer when verb=="update" and verbErr==nil. Failed updates
still nudge so the user gets the retry hint on the next run.

Closes #28
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 49845847-9765-43ed-9816-88b3f0022d31

📥 Commits

Reviewing files that changed from the base of the PR and between f9e094d and 24537c5.

⛔ Files ignored due to path filters (1)
  • internal/update/CLAUDE.md is excluded by !**/*.md
📒 Files selected for processing (4)
  • cmd/ana/main.go
  • cmd/ana/main_test.go
  • internal/update/selfupdate.go
  • internal/update/selfupdate_test.go

📝 Walkthrough

Walkthrough

Updates to CLI verb handling and update-related error messages: run now captures the verb argument and passes it to drainNudge, which suppresses the update nudge after a successful ana update. Permission-denied errors in the update process are detected and routed through a new helper that provides OS-specific elevation guidance.

Changes

Cohort / File(s) Summary
CLI Verb Handling
cmd/ana/main.go, cmd/ana/main_test.go
Adds firstVerb helper to extract the leading verb from positional arguments and threads it through drainNudge to conditionally suppress nudge output on successful ana update invocations. New test cases verify both branches (suppressed on success, printed on error).
Update Error Handling
internal/update/selfupdate.go, internal/update/selfupdate_test.go
Introduces formatReplaceErr to detect fs.ErrPermission during the rename phase and emit user-actionable messages: sudo on Unix, elevated Administrator shell guidance on Windows. All other rename errors retain wrapped error text. Comprehensive test suite covers both OS-specific permission scenarios and success paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A hop and a bound through error and nudge,
No pesky reminders when updates do budge.
Permission denied? We'll tell you to sudo,
Each rabbit's delight—a message that's true. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(update): actionable EACCES + suppress stale nudge' directly summarizes the two main changes: fixing EACCES error handling and suppressing the stale nudge post-update.
Description check ✅ Passed The PR description comprehensively covers both issues, implementation details, and test validation. It follows the template structure with summary and explicit test completion status.
Linked Issues check ✅ Passed The PR implementation fully satisfies issue #28 objectives: actionable EACCES messages via formatReplaceErr on Unix/Windows, suppression of stale nudge via verb parameter in drainNudge, and comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes (refactored atomicReplace error handling, added drainNudge verb parameter, new firstVerb helper, and comprehensive tests) are directly scoped to fixing issue #28 with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/update-eacces-stale-nudge

Comment @coderabbitai help to get the list of available commands and usage tips.

@bradfair bradfair added this pull request to the merge queue Apr 23, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 23, 2026
@bradfair bradfair added this pull request to the merge queue Apr 23, 2026
Merged via the queue into main with commit 9ab198c Apr 23, 2026
10 checks passed
@bradfair bradfair deleted the fix/update-eacces-stale-nudge branch April 23, 2026 18:13
@hpt-bot hpt-bot Bot mentioned this pull request Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ana update: hostile EACCES error + stale nudge after successful update

1 participant