fix: don't let un-updateable builds check for an update#1808
fix: don't let un-updateable builds check for an update#1808nmggithub wants to merge 4 commits intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Some people are annoying because they build their own version and then complain that they can’t update it. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6763cb6. Configure here.
| }): string | null { | ||
| if (!args.hasFeedURL) { | ||
| return "Automatic updates are not available because no update feed URL is configured."; | ||
| } |
There was a problem hiding this comment.
Feed URL check precedes more fundamental dev/packaged checks
Medium Severity
The hasFeedURL check is ordered before the isDevelopment and isPackaged checks. This causes development and unpackaged builds, which typically lack a feed URL, to display the less accurate "no update feed URL is configured" message instead of "only available in packaged production builds." Tests currently mask this behavior.
Reviewed by Cursor Bugbot for commit 6763cb6. Configure here.
ApprovabilityVerdict: Needs human review This is a targeted bug fix preventing update checks on builds without update feeds. There's an unresolved review comment about the order of condition checks causing less accurate error messages for dev builds - worth addressing before merging. You can customize Macroscope's approvability policy. Learn more. |


What Changed
This stops our updater from trying to update when we don't have a usable feed.
Why
When local builds would try and update, users would get errors like the below:

This is because they don't have an update feed built into them. This PR disables our auto-updater when we don't have a feed. We should maybe upstream (to Electron Updater) better checks so errors like the above don't leak into consumer (our) code.
UI Changes
N/A
Checklist
I included before/after screenshots for any UI changesI included a video for animation/interaction changesNote
Medium Risk
Changes updater enablement logic and initialization order, which could unintentionally disable updates or alter update behavior across packaged builds. Scope is limited to desktop auto-updater gating and messaging.
Overview
Prevents desktop builds without a configured update feed from attempting update checks.
getAutoUpdateDisabledReasonnow requireshasUpdateFeedConfigand returns a clear disabled reason when no feed exists, and both the menu-driven “Check for Updates” flow and auto-update enablement consult this flag.configureAutoUpdateris reordered so feed URL overrides (GitHub token/private feed and mock update server) are applied before computing whether updates are enabled, ensuring the updater is only configured and scheduled when a valid feed is present. Tests are updated/expanded to cover the new feed-required behavior.Reviewed by Cursor Bugbot for commit 802d85f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix update checks to require a configured update feed before offering updates
getAutoUpdateDisabledReasonin updateState.ts now accepts ahasUpdateFeedConfigboolean and returns a "no update feed is configured" reason when neitherapp-update.ymlnor the mock env var is present.handleCheckForUpdatesMenuClickandshouldEnableAutoUpdatesin main.ts computehasUpdateFeedConfigand pass it through, so builds without a feed show the "Updates unavailable" dialog instead of attempting a check.configureAutoUpdaterreorders feed URL setup (GitHub token / mock URL) to run before evaluatingshouldEnableAutoUpdates, ensuring feed config is in place when the check runs.app-update.ymland no mock env now report auto-updates as disabled rather than silently attempting an update check.Macroscope summarized 802d85f.