Fix auto update#3278
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe auto-updater module in the desktop app was updated to replace the direct app termination mechanism with a new quit flow. The change introduces a call to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Greptile SummaryThis PR refactors Key changes:
Confidence Score: 1/5Not safe to merge —
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/lib/auto-updater.ts | Replaces forced exitImmediately() with setSkipQuitConfirmation() before quitAndInstall, but setSkipQuitConfirmation is never defined or imported — causes a ReferenceError at runtime; also adds an unused BrowserWindow import. |
Sequence Diagram
sequenceDiagram
participant UI as Renderer
participant AU as auto-updater.ts
participant Index as main/index.ts
participant EU as electron-updater
UI->>AU: installUpdate()
AU->>AU: setSkipQuitConfirmation() ❌ ReferenceError
Note over AU: Function not defined — throws here
AU--xEU: quitAndInstall() never reached
Note over Index: skipQuitConfirmation flag never set
Note over Index: before-quit confirmation dialog would block quit anyway
Reviews (1): Last reviewed commit: "Save" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| emitStatus(AUTO_UPDATE_STATUS.IDLE); | ||
| return; | ||
| } | ||
| setSkipQuitConfirmation(); |
There was a problem hiding this comment.
setSkipQuitConfirmation is undefined — will throw ReferenceError at runtime
setSkipQuitConfirmation is called here but is never defined in this file and is not imported from anywhere. The only reference to it in the entire codebase is this call site (apps/desktop/src/main/lib/auto-updater.ts:94), meaning installUpdate() will throw a ReferenceError every time a user tries to install an update.
In apps/desktop/src/main/index.ts (line 154) there is a module-level skipQuitConfirmation variable that quitApp() sets to true before calling app.quit(). The intent here appears to be skipping the quit-confirmation dialog so autoUpdater.quitAndInstall() can proceed unblocked. The missing piece is a dedicated exported setter in main/index.ts:
// In apps/desktop/src/main/index.ts — add and export this function
export function setSkipQuitConfirmation(): void {
skipQuitConfirmation = true;
}Then import it in auto-updater.ts:
import { quitApp, setSkipQuitConfirmation } from "main/index";Without this fix the update install path is completely broken.
| @@ -1,8 +1,8 @@ | |||
| import { EventEmitter } from "node:events"; | |||
| import { app, dialog } from "electron"; | |||
| import { BrowserWindow, app, dialog } from "electron"; | |||
There was a problem hiding this comment.
BrowserWindow import
BrowserWindow was added to the import on this line but is never referenced anywhere in the file. This will likely trigger a lint/compiler warning and should be removed.
| import { BrowserWindow, app, dialog } from "electron"; | |
| import { app, dialog } from "electron"; |
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by cubic
Fixes a crash during auto-update by removing the forced immediate exit and skipping quit confirmation so
autoUpdater.quitAndInstallcan restart the app cleanly.setSkipQuitConfirmation()beforequitAndInstallto avoid quit prompts.quitAndInstall, preventing macOS crashes/hangs.auto-updater.ts.Written for commit 427076c. Summary will update on new commits.
Summary by CodeRabbit