Add release packaging and Pear install site#3
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds CI and release workflows, production and preview SST deploy pipelines with AWS account assertion, an installer landing site with platform installers, electron-builder packaging and release config, and an Electron auto-update module wired into the app. ChangesPear Distribution & Deployment System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| updater.on('update-downloaded', async (info) => { | ||
| console.log(`[updater] Update downloaded: ${info.version}`) | ||
| if (mainWindow.isDestroyed()) return | ||
|
|
||
| const result = await dialog.showMessageBox(mainWindow, { | ||
| type: 'info', | ||
| buttons: ['Restart', 'Later'], | ||
| defaultId: 0, | ||
| cancelId: 1, | ||
| message: 'A Pear update is ready to install.', | ||
| detail: 'Restart Pear to apply the downloaded update.' | ||
| }) | ||
|
|
||
| if (result.response === 0) { | ||
| updater.quitAndInstall() | ||
| } | ||
| }) |
There was a problem hiding this comment.
🟡 Stale BrowserWindow reference in updater causes silent update prompt failure on macOS re-activate
configureAutoUpdates at src/main/index.ts:138 captures the initial mainWindow BrowserWindow instance by value into the update-downloaded event handler closure (src/main/updater.ts:78-93). On macOS, when the user closes all windows (the app stays alive per src/main/index.ts:147) and later clicks the dock icon, createWindow() is called at src/main/index.ts:141 creating a new BrowserWindow—but the updater still holds a reference to the old, destroyed window. When an update is downloaded, the mainWindow.isDestroyed() check at line 80 returns true and the handler silently returns, so the user is never prompted to restart and install the update.
Prompt for agents
The configureAutoUpdates function captures the BrowserWindow instance passed at setup time. On macOS, when the user closes the window (app stays alive) and re-opens via dock click, a new BrowserWindow is created but the updater still references the old destroyed one.
Approach 1: Instead of capturing a specific BrowserWindow, resolve the focused/active window at the time the event fires using BrowserWindow.getFocusedWindow() or BrowserWindow.getAllWindows()[0].
Approach 2: Have configureAutoUpdates accept a getter function () => BrowserWindow | null instead of a direct reference, and have the caller pass a function that returns the current mainWindow module variable.
The fix should be applied in src/main/updater.ts in the update-downloaded handler (line 78-94), and potentially the function signature of configureAutoUpdates (line 49). The caller in src/main/index.ts:138 would need to be updated accordingly.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6761d67557
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (mainWindow.isDestroyed()) return | ||
|
|
There was a problem hiding this comment.
configureAutoUpdates captures the first BrowserWindow instance and then never rebinds it (updaterConfigured short-circuits future calls). On macOS, window-all-closed does not quit the app (src/main/index.ts), so if the user closes and reopens the window later, update-downloaded can fire while the captured window is destroyed and return early, which suppresses the restart prompt even though an update is ready.
Useful? React with 👍 / 👎.
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>
Summary
Verification