Notify users about new gh-stack releases - #545
Conversation
7e100f2 to
0b05b27
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is non-blocking, failure-isolated, and comprehensively tested across its key behaviors.
Review effort: Balanced
Findings: None
What changed in this PR
Adds a background release notifier that preserves command output and exit behavior.
Changes:
- Checks official releases and caches daily check/reminder state.
- Integrates non-blocking notifications into successful commands.
- Adds comprehensive tests and user documentation.
| File | Description |
|---|---|
README.md |
Documents updates and notifier behavior. |
cmd/root.go |
Integrates background checks into command lifecycle. |
cmd/root_test.go |
Tests notifier integration and exclusions. |
internal/update/update.go |
Implements eligibility, release checks, caching, and notices. |
internal/update/update_test.go |
Covers update-check behavior and failure modes. |
go.mod |
Adds direct notifier dependencies. |
go.sum |
Records module checksums. |
docs/src/content/docs/reference/cli.md |
Adds detailed notifier reference documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
francisfuzz
left a comment
There was a problem hiding this comment.
✨ This is thoughtful work. The install validation, pre-request reservation, state recovery, and test coverage are all much stronger than the spike I put together.
❓ Asking to learn: are we intentionally choosing successful commands as the delivery point for the gh-stack notice?
I spiked the inverse on spike/update-notifier: let core gh own successful invocations, then have gh-stack cover failures because core gh skips its extension PostRun notice when the extension returns an error.
With the current approach, a modern gh installation can potentially show both notices after a successful command:
A new release of gh-stack is available: 0.1.0 -> 0.1.1
To upgrade, run: gh extension upgrade stack
A new release of stack is available: 0.1.0 → 0.1.1
To upgrade, run: gh extension upgrade stack
Meanwhile, a failed command, which is when someone is most likely to file a bug, does not show either notice.
I am not suggesting we replace this implementation with the spike. The implementation here is stronger. I mainly want to confirm that accepting possible duplication on success, while leaving failures untouched, is the intended product tradeoff. If so, could we capture that rationale in the Boundary section?
Not blocking my approval. I want to make sure this behavior is deliberate rather than incidental to PersistentPostRun. ✅
|
@francisfuzz Thanks for the review and feedback 🙏
That's a good point. I did this originally thinking it would be less annoying, but you are absolutely right that those failures (especially when a fix may be available in an update) are probably where the upgrade notice would be most valuable. I've made this update in c8e7b43 to also show the notice on failures, while also preserving the standard error outputs.
In my experience (and those reported by users), the upgrade prompt that should be coming from In the meantime, I think a potential duplication is acceptable, and if we hear lots of reports of that from users, we can adjust our approach in the next release. |
Adds a lightweight upgrade nudge for users running an older gh-stack release, pointing them to
gh extension upgrade stackwithout upgrading automatically.Functionality and user impact
/releases/latestendpoint—the same release source used by extension upgrades—and require a newer stable version with a matching platform binary.StateDir()/gh-stack/state.yml, shared across repositories for the user.GH_STACK_NO_UPDATE_NOTIFIER=1disables the notifier; update-check failures are diagnostic-only underGH_DEBUG.Boundary: commands never wait for the network check. Short commands may miss a notice; completed results can be shown later. Local/development, prerelease, and pinned installations are excluded, as are help, version, and completion commands. Usage errors and explicit user cancellations do not display a notice.
Delivery stays independent of core
ghto cover failures, non-interactive use, and pending notices that core may not deliver. A modern interactiveghcan therefore show its own notice as well after a successful command; that overlap is an intentional tradeoff. Concurrent processes may also occasionally duplicate a check or reminder.Key areas to review
internal/update/update.gocheck,noticeDue,readState, andwriteStateseparate attempts from successful checks and reminder delivery, enforce the cooldowns, recover invalid state, and replace the YAML file atomically.eligibleandfetchLatestrestrict notices to official, unpinned installs and avoid downgrade or unsupported-platform suggestions.cmd/root.go: pre-run startup, buffered result delivery, and execution finalization deliver notices after the original command diagnostics without waiting or changing exit codes. Cancellation markers in command handlers distinguish user cancellations from already-reported operational failures.Related issues