feat(updater): route in-app updates through R2 with GitHub fallback (#219) - #1003
Conversation
…219) Mainland China users frequently hit slow, intermittent, or blocked GitHub update checks. The prod release feed is already mirrored to Cloudflare R2 (dl.pawwork.ai, PR #1000 / mirror-release-to-r2.yml). This wires that mirror into the desktop updater. electron-updater does not fail over across providers and binds the download source at check time (downloadUpdate reuses the updateInfoAndProvider from the last checkForUpdates). So feed selection is done at runtime via setFeedURL, not by changing the baked app-update.yml: - new update-feed.ts: a provider-agnostic feed selector. check() tries feeds in order (R2 first, GitHub fallback) with a per-feed timeout; the winning feed becomes active and serves the download. download() retries on GitHub if an R2 download fails. A generation guard stops a timed-out check that resolves late from clobbering the active feed. - index.ts: build the ordered feeds (R2 for prod, GitHub fallback; beta is GitHub-only with no mirror) and route the controller's check/download through the selector. setupAutoUpdater sets an initial feed (with its own GitHub fallback) and logs the selection. - app-update.yml is intentionally left provider: github. The runtime always re-selects the feed before each check, so the baked value is only a startup safety net; keeping GitHub there avoids touching CI grep checks, the three app-update tests, and Windows/mac packaging, with no behavior difference. - mirror-release-to-r2.ts now fails the mirror if a latest*.yml pointer references an asset it would not upload, guarding the R2 feed alignment. - dev harness: PAWWORK_DEV_UPDATER=1 enables forceDevUpdateConfig under dev:desktop to exercise the real R2 feed without a signed build. Verification: bun test (423 pass; the lone failure is the pre-existing packaged-app smoke test that needs a built binary), typecheck, lint, and electron-vite build all green. Live check against the current release (v2026.5.29): R2 latest-mac.yml / latest.yml are byte-identical to the GitHub release and every referenced binary HEADs 200 on dl.pawwork.ai. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 49 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an R2-primary/GitHub-fallback updater feed system with per-feed probes and timeout, enforces pointer YAML asset alignment during mirroring, introduces updater configuration constants, rewires updater initialization to use the feed selector, and adds tests for feed selection, download failover, and pointer validation. ChangesMulti-feed Updater with R2 Mirror Fallback
Sequence DiagramsequenceDiagram
participant App as app (setupAutoUpdater)
participant Feed as createUpdateFeed
participant Probe as HEAD probe
participant R2 as R2 feed
participant GH as GitHub feed
participant Updater as electron-updater
App->>Feed: build feeds (R2 first, GitHub fallback)
Feed->>Probe: probe R2 (with timeout)
Probe-->>Feed: reachable / unreachable / timeout
Feed->>Updater: setFeedURL(chosen feed)
App->>Updater: checkForUpdates() (delegated to Feed.check)
Feed->>R2: checkForUpdates (if active)
R2-->>Feed: success / throw
Feed->>GH: fallback check when R2 fails
Feed->>Updater: downloadUpdate() (delegated to Feed.download)
Updater-->>Feed: download failure
Feed->>GH: switch to GitHub, rebind & checkForUpdates, retry download once
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
platform
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/desktop-electron/src/main/constants.ts, packages/desktop-electron/src/main/index.ts, packages/desktop-electron/src/main/update-feed.test.ts, packages/desktop-electron/src/main/update-feed.ts, packages/desktop-electron/src/main/updater.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a runtime update feed selection mechanism for the in-app updater, prioritizing Cloudflare R2 with a fallback to GitHub to improve update reliability and speed. It also adds unit tests and validation to ensure all referenced assets are mirrored before uploading. However, there are several critical code duplications that must be resolved: duplicate constant declarations in constants.ts and index.ts which will cause TypeScript compilation errors, and an identical duplicated test suite in mirror-release-to-r2.test.ts.
A bad edit replay during development inserted the constants.ts feed block and the index.ts UPDATE_FEED_TIMEOUT_MS const twice, which compiled locally in a stale state but failed CI typecheck (TS2451 redeclare) and broke the desktop build/tests. Deduplicate to a single declaration of each. Verified locally: typecheck, biome lint, electron-vite build, and bun test (535 pass, 0 fail) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The source-contract test still grepped the old `!UPDATER_ENABLED` guard, but setupAutoUpdater now early-returns on `!UPDATER_ACTIVE`. This edit was lost in an earlier replay and never made the first commit, so unit-desktop failed. Point the regex at the current guard. Verified: bun test in packages/desktop-electron is 436 pass, 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same edit-replay glitch that duplicated the constants/index declarations also inserted the "pointer reference alignment" describe twice in the mirror test (it passed because the cases just ran twice). Gemini flagged it; remove the second copy. Verified: bun test mirror-release-to-r2.test.ts is 7 pass, 0 fail; biome lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/desktop-electron/src/main/index.ts`:
- Around line 709-714: The renderer-visible updater flag is out of sync with the
main-process enablement (UPDATER_ACTIVE/DEV_UPDATER); make the renderer state
derive from the same source of truth. Update diagnostics() and getWindowConfig()
to compute and return UPDATER_ENABLED from the same constant used to enable the
updater (UPDATER_ACTIVE || DEV_UPDATER or a single exported UPDATER_ACTIVE
constant), and ensure any window config property (e.g., updaterEnabled /
UPDATER_ENABLED) mirrors that value so the renderer and main process always
agree when autoUpdater is enabled.
In `@packages/desktop-electron/src/main/update-feed.ts`:
- Around line 114-120: The fallback path to GitHub must run the same bounded &
validated re-check as the normal flow: when you detect a github feed (deps.feeds
/ github) and before calling deps.downloadUpdate(), call the same
timeout-wrapped check logic (the one used by check()) against
deps.checkForUpdates() so it cannot hang, then validate the returned result is
non-null and result.isUpdateAvailable === true; only then call
deps.downloadUpdate(); otherwise log a clear error via deps.error and
exit/throw/return the failure (do not proceed to download). Also ensure
activeLabel is only set to "github" after the re-check succeeds. Use the
existing symbols deps.checkForUpdates, deps.downloadUpdate, deps.setFeedURL,
activeLabel and the check() timeout wrapper to implement this.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f129c8e-25ab-4dbf-8375-e873872fdf4d
📒 Files selected for processing (8)
packages/desktop-electron/scripts/mirror-release-to-r2.test.tspackages/desktop-electron/scripts/mirror-release-to-r2.tspackages/desktop-electron/src/main/constants.tspackages/desktop-electron/src/main/index-updater-source.test.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/update-feed.test.tspackages/desktop-electron/src/main/update-feed.tspackages/desktop-electron/src/main/updater.ts
There was a problem hiding this comment.
Code Review
This pull request introduces a runtime update feed selection mechanism for the Electron in-app updater, prioritizing a Cloudflare R2 mirror with a fallback to GitHub to improve reachability in mainland China. The feedback highlights two critical issues in the implementation: a race condition in update-feed.ts where a timed-out update check can resolve late and corrupt the shared state of the autoUpdater singleton, and a missing timeout on the fallback update check during the download phase, which could cause the updater to hang indefinitely under poor network conditions.
… check race (#219) Review P1: the R2-timeout fallback raced electron-updater's checkForUpdates() with Promise.race but never cancelled the underlying check. A slow R2 check could resolve late on the shared autoUpdater instance and rebind the provider back to R2 *after* we had fallen back to GitHub — so "fell back to GitHub" was a lie and the download could still go to the slow/unreachable R2 the fallback exists to avoid. The generation guard only protected our return value, not electron-updater's internal updateInfoAndProvider. Fix: choose the feed with a cancellable reachability probe (HEAD on the channel file, aborted on timeout via AbortController), then run exactly one checkForUpdates() against the winning feed. Only one real check ever runs, bound to the chosen feed; downloadUpdate() reuses that provider. No abandoned check can rebind it. Also addresses review P2: the download-fallback now compares the GitHub re-check version against the version the controller validated and fails closed on mismatch, so it can never mark a different release ready than it checked. Verified: typecheck, lint, build, bun test (433 pass, 0 fail). New tests assert single-check-on-fallback (incl. the abort/timeout path) and the version-mismatch fail-closed. Real electron-updater fallback (point R2 at a bad host under PAWWORK_DEV_UPDATER=1 dev:desktop) remains a manual pre-merge check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the review in 96d83bc9c8. P1 (blocking) — fixed. The timeout fallback no longer races the uncancellable P2 — fixed. The download fallback now compares the GitHub re-check version against the version the controller validated and fails closed on mismatch, so it can never mark a release ready that it did not check. New test covers R2 vA → R2 download fail → GitHub re-check vB → throws. Verified locally: typecheck, lint, build, |
… to renderer (#219) - check() now catches R2 checkForUpdates() reject and retries on the next feed (GitHub). Covers "R2 probe 200 but metadata fetch fails" — including the 60 s socket timeout built into builder-util-runtime. - diagnostics() and getWindowConfig() now expose UPDATER_ACTIVE instead of UPDATER_ENABLED so the renderer reflects dev-harness updater state. - 3 new tests: R2 check reject → GitHub fallback, both feeds reject → throw, download fallback re-check reject → fail closed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed in 8510379. P1 (blocking) — fixed. Key electron-updater internals verified in source (
New tests:
P2 — addressed by design. The download fallback's P3 — fixed. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Mainland China users frequently hit slow / intermittent / blocked GitHub update checks. The prod release feed is already mirrored to Cloudflare R2 (
dl.pawwork.ai, #1000 /mirror-release-to-r2.yml). This wires that mirror into the desktop updater so checks and downloads try R2 first and fall back to GitHub.Closes #219.
Approach
electron-updaterdoes not fail over across providers, and it binds the download source at check time (downloadUpdate()reuses theupdateInfoAndProviderfrom the lastcheckForUpdates()). So feed selection is done at runtime viasetFeedURL, not by changing the bakedapp-update.yml.update-feed.ts(new): provider-agnostic feed selector.check()tries feeds in order (R2 first, GitHub fallback) with a per-feed timeout; the winning feed becomes active and serves the download.download()retries on GitHub if an R2 download fails. A generation guard stops a timed-out check that resolves late from clobbering the active feed.index.ts: builds the ordered feeds (R2 + GitHub for prod; GitHub-only for beta, which has no mirror) and routes the controller's check/download through the selector.setupAutoUpdatersets an initial feed (with its own GitHub fallback) and logs the selection.app-update.ymlleft asprovider: githubon purpose. The runtime always re-selects the feed before each check, so the baked value is only a startup safety net. Keeping GitHub there avoids touching the CI grep checks, the three app-update tests, and mac/Windows packaging, with no behavior difference (the baked value is overridden every check). This was the explicit design decision, cross-checked with a second reviewer.mirror-release-to-r2.ts: now fails the mirror if alatest*.ymlpointer references an asset it would not upload — guards the R2 feed alignment (the easy-to-break part).PAWWORK_DEV_UPDATER=1enablesforceDevUpdateConfigunderdev:desktopto exercise the real R2 feed without a signed build.Verification
bun testinpackages/desktop-electron: 423 pass, 1 fail — the lone failure is the pre-existing packaged-app smoke test that needs a built/signed binary (unrelated, fails ondevtoo).typecheck(tsgo),biome linton touched files, andelectron-vite buildall green.latest-mac.yml/latest.ymlare byte-identical to the GitHub release, and every referenced binary (.zip/.dmg/.exe/.blockmap) HEADs200ondl.pawwork.ai— confirms the sha512 chain holds and the feed resolves.Not done here
electron-updaterwalk viadev:desktop(GUI) was not run in this environment. The harness is wired; runPAWWORK_DEV_UPDATER=1 bun run dev:desktopand trigger Check for Updates to watch theupdate feed selected/update metadata fetchedlogs and the GitHub fallback. The live-endpoint checks above cover the network side.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests