feat: make check for updates menu item dynamic#1124
feat: make check for updates menu item dynamic#1124nmggithub wants to merge 7 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 |
e847533 to
7b1d5e1
Compare
|
@nmggithub Merging my PR (#1123) first will establish the core update state machine in Let me know what you think! |
|
@AriajSarkar definitely think there's a lot of overlap. Once your PR merges, I'll try to rebase my PR off yours! |
…update menu items
7b1d5e1 to
cd06a14
Compare
| updateCheckForUpdatesMenuItem(checkForUpdatesMenuItemInHelpMenu, state); | ||
| }); | ||
|
|
||
| let applicationMenu: Menu | null = null; |
There was a problem hiding this comment.
Unused applicationMenu variable is dead code
Low Severity
The applicationMenu variable is declared and assigned in configureApplicationMenu but never read anywhere in the codebase. It's dead code that adds unnecessary state tracking.
Additional Locations (1)
There was a problem hiding this comment.
Actually, no this isn't dead code (I don't think). I think it's meant to keep the reference alive so out dynamic updating of menu items works.
There was a problem hiding this comment.
Did some additional research, it might not be necessary, but I don't think it really needs to be fixed.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| void downloadAvailableUpdate(); | ||
| break; | ||
| case "downloaded": | ||
| void installDownloadedUpdate(); |
There was a problem hiding this comment.
Menu click installs update without user confirmation
High Severity
When the native menu item shows "Update Downloaded" and the user clicks it, handleCheckForUpdatesMenuClick calls installDownloadedUpdate() directly, which destroys all windows and calls autoUpdater.quitAndInstall() without any confirmation dialog. The web UI's equivalent click handler uses window.confirm() before calling bridge.installUpdate(), but the native menu path has no such safeguard. A user clicking the menu item to learn more about the downloaded update would have their app quit and restart immediately with no chance to save work.
Additional Locations (1)
There was a problem hiding this comment.
This works just like the web UI, so I think its fine.


What Changed
EDIT: I was dumb and didn't realize I wasn't actually testing on a newer version of Electron. This PR makes changes and updates electron to the minimal version needed for this feature to work.
This PR cannot be merged in its current state as it depends on features not in upstream Electron yet, but I hope for that to change in the near future. Refs: electron/electron#50266Once Electron truly supports it,this will allow us to change the label of the "Checking for Updates..." menu item on the fly.Why
This provides a much better user experience by giving visual indications of what is happening during the update process.
UI Changes
(had some focus issues near the end, but I believe I covered most of the changes in the video)
Screen.Recording.2026-03-29.at.01.59.40.mov-compressed.mov
Checklist
Note
Make the 'Check for Updates' menu item dynamic based on update state
DesktopUpdateStatusFriendlyLabelMapdefined inpackages/contracts/src/ipc.ts.canCheckForUpdatein the web frontend is tightened to returntrueonly when status is'idle', and the settings panel button label now uses the same canonical map.Macroscope summarized dfa94af.
Note
Medium Risk
Touches the desktop auto-update state machine and menu wiring, plus bumps Electron, so regressions could affect update UX and state transitions. No auth/data-path changes, but behavior is timing- and state-dependent across desktop and web UI.
Overview
Makes the desktop “Check for Updates” menu item dynamic: it is now a persistent
MenuItemwhose label and enabled state update live based onDesktopUpdateState, and clicking it routes to check/download/install depending on current status.Adds a shared
DesktopUpdateStatusFriendlyLabelMapin@t3tools/contractsand reuses it in both the desktop menu and the web Settings “Version” update button; the web UI also tightenscanCheckForUpdateto only allow checks from theidlestate.Introduces a transient-state cleanup path by adding
reduceDesktopUpdateStateToIdleand scheduling an automatic reset back toidleafter “up-to-date” or check-error states, with cleanup of the reset timer on app shutdown/signals. Also bumpselectronto40.7.0to support dynamic menu updates.Written by Cursor Bugbot for commit dfa94af. This will update automatically on new commits. Configure here.