Skip to content

fix(desktop): repair macOS auto-updater (ESM autoUpdater + app_ready handler)#462

Merged
skevetter merged 3 commits into
mainfrom
debug/desktop-update-check
May 30, 2026
Merged

fix(desktop): repair macOS auto-updater (ESM autoUpdater + app_ready handler)#462
skevetter merged 3 commits into
mainfrom
debug/desktop-update-check

Conversation

@skevetter

@skevetter skevetter commented May 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Two independent bugs were preventing the desktop auto-updater from working on macOS in v1.10.0-beta.14 (and earlier):

  1. autoUpdater was undefined under ESM. desktop/package.json has "type": "module", so the main bundle runs as ESM. await import("electron-updater") against electron-updater's CJS module does not surface autoUpdater as a named export — it's defined via Object.defineProperty(exports, "autoUpdater", { get }), which Node's cjs-module-lexer doesn't detect. Result: const { autoUpdater } = await import("electron-updater") was always undefined, the !autoUpdater guard in updater.ts always fired, and the UI showed "Update check failed: Updates require a packaged build" even on properly signed/notarized builds.

    Verified directly:

    $ node --input-type=module -e "const m=await import('electron-updater'); console.log(typeof m.autoUpdater)"
    undefined
    

    Fixed by introducing a small loadAutoUpdater() helper that reads mod.default?.autoUpdater ?? mod.autoUpdater (the default path invokes the CJS getter). Used at all three call sites in updater.ts.

    Prior PRs (fix(desktop): handle missing autoUpdater on unsigned macOS builds #391, feat(desktop): redesign update UX #459) misread the symptom as a code-signing limitation; the real cause is the ESM/CJS interop.

  2. app_ready IPC handler was never registered. The renderer invokes appReady() on mount (App.svelte:111), but no ipcMain.handle("app_ready", ...) existed in main. The promise rejection was silently swallowed until feat(desktop): redesign update UX #459 added a console.warn surfacing it. Handler added next to the other update IPCs; it replays the current updater status so a freshly-mounted UI immediately sees state main already knows.

Also widened normalizeReleaseNotes to accept note: string | null to match electron-updater's real ReleaseNoteInfo type (the helper's stricter return type surfaced this).

Not fixed here

  • The https://dl.devsy.sh/desktop/latest-mac.yml 404 turned out to be expected — beta releases publish beta-mac.yml only (release.yml gates latest-* on non-prerelease), and beta-mac.yml returns 200 with a healthy manifest. No publishing-pipeline change needed.

Verification

  • npx tsc --noEmit net-improves type errors (2 updater.ts errors removed; 3 pre-existing errors in cli.test.ts / analytics.ts unchanged).
  • Needs a fresh signed + notarized build to verify end-to-end in the installed app on macOS.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced release notes handling to gracefully manage missing or invalid content.
    • Improved auto-updater module initialization for better reliability.
  • New Features

    • App now communicates update status information immediately upon becoming ready, enabling faster update notifications to users.

Review Change Stack

skevetter added 2 commits May 29, 2026 19:54
Main runs as ESM (`"type": "module"` in desktop/package.json). Node's
cjs-module-lexer doesn't surface electron-updater's `autoUpdater` getter
(defined via Object.defineProperty on exports) as a named export, so
`const { autoUpdater } = await import("electron-updater")` was always
undefined and the unsupported-build error fired on every packaged build.

Reach the getter through `mod.default.autoUpdater` (CJS module.exports)
via a small `loadAutoUpdater()` helper, used by all three call sites.
The renderer invokes `app_ready` on mount (App.svelte) but no main-side
handler existed, so the promise always rejected with "No handler
registered for 'app_ready'". The error was silently swallowed until
PR #459 added a console.warn surfacing it.

Register the handler alongside the other update IPCs. It replays the
current updater status to the renderer so a freshly-mounted (or
reloaded) UI immediately reflects state main already knows.
@netlify

netlify Bot commented May 30, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 1857671
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a1a360db0460300081849d2

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b36b7127-e8ff-4e86-9744-307e7811ac21

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce80d5 and 1857671.

📒 Files selected for processing (3)
  • desktop/src/main/__tests__/updater.test.ts
  • desktop/src/main/ipc.ts
  • desktop/src/main/updater.ts

📝 Walkthrough

Walkthrough

The PR refactors Electron updater module loading to correctly handle ESM/CJS interop, adds an app-ready IPC handler to broadcast update status, and hardens release notes parsing for null values. The core change introduces loadAutoUpdater() to centralize accessing the CommonJS autoUpdater getter under ESM, replacing scattered destructuring imports throughout the updater initialization code.

Changes

Updater system and IPC integration

Layer / File(s) Summary
Electron updater ESM/CJS compatibility
desktop/src/main/updater.ts, desktop/src/main/__tests__/updater.test.ts
New loadAutoUpdater() helper centralizes electron-updater module loading and resolves the CommonJS autoUpdater getter via mod.default?.autoUpdater ?? mod.autoUpdater ?? null. All call sites (initAutoUpdater(), getUpdater(), setAutoDownloadEnabled()) now use the helper instead of direct destructuring. Test mock updated to expose both named and default exports to match the loader's fallback chain.
Release notes array robustness
desktop/src/main/updater.ts
normalizeReleaseNotes() now maps array note objects to their note field and filters out null values before joining into a single string.
App-ready status broadcast
desktop/src/main/ipc.ts
New "app_ready" IPC handler sends the current update status to the renderer on app startup via setImmediate, guarded by !event.sender.isDestroyed() to avoid errors after sender teardown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • devsy-org/devsy#391: Modifies desktop/src/main/updater.ts to change how electron-updater's autoUpdater is accessed and initialized, addressing similar module loading and availability patterns in the updater setup code path.

Suggested labels

size/m

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

The updater now reads autoUpdater via mod.default?.autoUpdater (the
CJS/ESM interop path required under "type": "module"). The test
mock only defined the top-level autoUpdater export, so accessing
mod.default threw a vitest strict-mock error. Make the mock mirror
the real module shape.
@skevetter skevetter marked this pull request as ready for review May 30, 2026 01:13
@skevetter skevetter merged commit 30e2060 into main May 30, 2026
19 of 20 checks passed
@skevetter skevetter deleted the debug/desktop-update-check branch May 30, 2026 01:14
@coderabbitai coderabbitai Bot added the size/m label May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant