fix(update): actionable EACCES + suppress stale nudge#29
Conversation
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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughUpdates to CLI verb handling and update-related error messages: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Summary
Two papercuts from the first post-release
ana updaterun (issue #28):atomicReplacewrappedos.Renamewith bare%w, so a perm-denied write to/usr/local/bin/anasurfaced as rawpermission denied+ tempdir path. NewformatReplaceErrdetectsfs.ErrPermissionand returns platform-aware guidance (sudo on Unix, Administrator /%LOCALAPPDATA%on Windows). Routed through all three rename error sites inatomicReplace. The Windows rollback-also-fails branch also skips the misleading "recover from<.old>" wording when the root cause is EACCES —.oldsits in the same admin-only directory.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 atdrainNudgevia a newverbparameter: only whenverb == "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 lintcleango build -o /tmp/ana ./cmd/ana && /tmp/ana update→ no trailing nudge on "already at version" pathsudo install -m 0755 /tmp/ana /usr/local/bin/ana && ana update) — to be done by reviewer on release buildCloses #28
Summary by CodeRabbit
Release Notes
Bug Fixes
ana updateexecution