add auto update#203
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds macOS auto-update support (electron-updater) invoked at app startup with hourly checks; updates desktop release automation to upload an auto-update manifest (latest-mac.yml), tighten artifact-not-found behavior in CI, and add a --publish option to the release script and docs. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/main/index.ts (1)
5-48: Good integration point; consider guarding auto-updater setupImporting and calling
setupAutoUpdater()afterapp.whenReady()and the main window setup looks solid and lines up with the main-process responsibilities.To match the robustness of the
setupAgentHooks()block, you might optionally wrapsetupAutoUpdater()in a try/catch so a misconfigured feed or updater issue can’t crash the main process on startup, e.g.:try { setupAgentHooks(); } catch (error) { console.error("[main] Failed to set up agent hooks:", error); // App can continue without agent hooks, but log the failure } - await makeAppSetup(() => MainWindow()); - setupAutoUpdater(); + await makeAppSetup(() => MainWindow()); + try { + setupAutoUpdater(); + } catch (error) { + console.error("[main] Failed to set up auto-updater:", error); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/release-desktop.yml(1 hunks)apps/desktop/RELEASE.md(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/main/index.ts(2 hunks)apps/desktop/src/main/lib/auto-updater.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/index.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/index.ts
apps/desktop/src/main/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Node.js modules (fs, path, os, net, etc.) can be used in main process code only
Files:
apps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/index.ts
apps/desktop/src/main/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Load
.envfile withoverride: trueat the start of the main process before any imports
Files:
apps/desktop/src/main/index.ts
🧬 Code graph analysis (2)
apps/desktop/src/main/lib/auto-updater.ts (1)
apps/desktop/src/shared/constants.ts (2)
ENVIRONMENT(1-3)PLATFORM(5-9)
apps/desktop/src/main/index.ts (1)
apps/desktop/src/main/lib/auto-updater.ts (1)
setupAutoUpdater(9-61)
🪛 LanguageTool
apps/desktop/RELEASE.md
[uncategorized] ~79-~79: The official name of this software platform is spelled with a capital “H”.
Context: ... or delete the stable copies created in .github/workflows/release-desktop.yml. 4. If y...
(GITHUB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
apps/desktop/package.json (1)
59-59: No changes needed;electron-updaterv6 is compatible with Electron 39.1.2.The version
"6"is compatible with Electron 39.1.2 and is actively maintained. The shorthand notation allows patch and minor version updates (equivalent to^6.0.0), which is appropriate for production auto-update functionality. For consistency with other dependencies in this file, consider using"^6.0.0"instead, but this is optional.
| ## Auto-update | ||
|
|
||
| The desktop app checks for updates on macOS at launch (and every 6 hours) using the GitHub release tagged with the `desktop-v*.*.*` pattern. The workflow creates stable artifact names and uploads the auto-update manifest so the updater and direct download links always point to the latest build. | ||
|
|
||
| - Update manifest (macOS): `https://github.com/superset-sh/superset/releases/latest/download/latest-mac.yml` | ||
| - Update payload (macOS arm64 zip): `https://github.com/superset-sh/superset/releases/latest/download/Superset-arm64-mac.zip` | ||
| - Installer fallback: `https://github.com/superset-sh/superset/releases/latest/download/Superset-arm64.dmg` | ||
|
|
||
| To keep auto-update working: | ||
| 1. Ship releases via the tag-based workflow so `releases/latest` advances. | ||
| 2. Keep `latest-mac.yml` attached in the release (the workflow uploads it by default). | ||
| 3. Do not rename or delete the stable copies created in `.github/workflows/release-desktop.yml`. | ||
| 4. If you add another architecture, upload its auto-update manifest and create a stable copy for the zip so it has a `releases/latest/download/` URL. | ||
| 5. macOS auto-update requires signed builds; ensure signing is configured when publishing. | ||
|
|
There was a problem hiding this comment.
Align documented check interval with implementation
The docs say the app checks for updates “every 6 hours”, but UPDATE_CHECK_INTERVAL_MS in apps/desktop/src/main/lib/auto-updater.ts is currently 1 hour. Please update either the constant or this text so they match and avoid confusion.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: The official name of this software platform is spelled with a capital “H”.
Context: ... or delete the stable copies created in .github/workflows/release-desktop.yml. 4. If y...
(GITHUB)
🤖 Prompt for AI Agents
In apps/desktop/RELEASE.md around lines 68 to 82, the documented auto-update
check interval ("every 6 hours") is inconsistent with the implementation;
UPDATE_CHECK_INTERVAL_MS in apps/desktop/src/main/lib/auto-updater.ts is set to
1 hour. Update one of them so they match: either change the prose in RELEASE.md
to "every 1 hour" (or "hourly") to reflect the current constant, or change
UPDATE_CHECK_INTERVAL_MS to 6 hours in apps/desktop/src/main/lib/auto-updater.ts
if the intended behavior is 6-hour checks; ensure the wording and the constant
use the same time unit and consider adding a short comment in the code linking
to RELEASE.md for future clarity.
| import { app } from "electron"; | ||
| import { autoUpdater } from "electron-updater"; | ||
| import { ENVIRONMENT, PLATFORM } from "shared/constants"; | ||
|
|
||
| const UPDATE_CHECK_INTERVAL_MS = 1000 * 60 * 60 * 1; // 1 hour | ||
| const UPDATE_FEED_URL = | ||
| "https://github.com/superset-sh/superset/releases/latest/download"; | ||
|
|
||
| export function setupAutoUpdater(): void { | ||
| if (ENVIRONMENT.IS_DEV || !PLATFORM.IS_MAC) { | ||
| return; | ||
| } | ||
|
|
||
| autoUpdater.autoDownload = true; | ||
| autoUpdater.autoInstallOnAppQuit = true; | ||
| autoUpdater.allowDowngrade = false; | ||
|
|
||
| autoUpdater.setFeedURL({ | ||
| provider: "generic", | ||
| url: UPDATE_FEED_URL, | ||
| }); | ||
|
|
||
| autoUpdater.on("error", (error) => { | ||
| console.error("[auto-updater] Error during update check:", error); | ||
| }); | ||
|
|
||
| autoUpdater.on("update-available", (info) => { | ||
| console.info( | ||
| `[auto-updater] Update available: ${info.version}. Downloading...`, | ||
| ); | ||
| }); | ||
|
|
||
| autoUpdater.on("update-not-available", () => { | ||
| console.info("[auto-updater] No updates available"); | ||
| }); | ||
|
|
||
| autoUpdater.on("update-downloaded", (info) => { | ||
| console.info( | ||
| `[auto-updater] Update downloaded (${info.version}). Will install on quit.`, | ||
| ); | ||
| }); | ||
|
|
||
| const checkForUpdates = () => | ||
| autoUpdater.checkForUpdatesAndNotify().catch((error) => { | ||
| console.error("[auto-updater] Failed to check for updates:", error); | ||
| }); | ||
|
|
||
| const interval = setInterval(checkForUpdates, UPDATE_CHECK_INTERVAL_MS); | ||
| interval.unref(); | ||
|
|
||
| if (app.isReady()) { | ||
| void checkForUpdates(); | ||
| } else { | ||
| app | ||
| .whenReady() | ||
| .then(() => checkForUpdates()) | ||
| .catch((error) => { | ||
| console.error("[auto-updater] Failed to start update checks:", error); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "RELEASE.md" -o -name "tsconfig.json" | head -20Repository: superset-sh/superset
Length of output: 309
🏁 Script executed:
if [ -f "apps/desktop/RELEASE.md" ]; then cat "apps/desktop/RELEASE.md"; fiRepository: superset-sh/superset
Length of output: 5279
🏁 Script executed:
if [ -f "tsconfig.json" ]; then cat "tsconfig.json"; fiRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
if [ -f "apps/desktop/tsconfig.json" ]; then cat "apps/desktop/tsconfig.json"; fiRepository: superset-sh/superset
Length of output: 752
Update interval and documentation should be consistent; verify TypeScript configuration for setInterval typing
The structure and error handling are solid. Two items need attention:
-
Interval mismatch: The code implements a 1-hour check interval (
UPDATE_CHECK_INTERVAL_MS = 1000 * 60 * 60 * 1), butapps/desktop/RELEASE.mdstates "every 6 hours". Align these—either update the constant to 6 hours or update the documentation. -
TypeScript typing for setInterval: The
apps/desktop/tsconfig.jsondoesn't include"node"in thelibarray, which may causesetInterval().unref()to fail type checking. Either add"node"to thelibarray in tsconfig or cast toNodeJS.Timeoutas shown in the snippet above.
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/auto-updater.ts around lines 1 to 61, the update
interval constant currently sets a 1-hour check but RELEASE.md documents 6
hours, and the call to setInterval(...).unref() can fail TypeScript checks
because tsconfig.json does not include the "node" lib; either change
UPDATE_CHECK_INTERVAL_MS to 1000 * 60 * 60 * 6 to match the docs (or update
RELEASE.md to 1 hour) and fix typing by adding "node" to
apps/desktop/tsconfig.json lib array so setInterval returns NodeJS.Timeout, or
keep the current tsconfig and cast the setInterval result to NodeJS.Timeout
(store it in a variable) before calling unref().
- Release script now creates draft releases by default for review - Added --publish flag to auto-publish when build completes - Trimmed RELEASE.md to be concise and accurate - Fixed code signing secret names in docs - Removed .zip references (no longer used) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/RELEASE.md(1 hunks)apps/desktop/create-release.sh(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build - macOS (arm64)
🔇 Additional comments (5)
apps/desktop/create-release.sh (3)
56-80: ✓ Argument parsing handles both positional and flag arguments correctly.The logic properly parses VERSION as the first positional arg and --publish as an optional flag, with appropriate error handling for unknown options. The implementation is resilient to flag position (users can pass flags before or after the version).
279-309: ✓ Conditional publish logic is sound.The branching correctly implements two release modes: auto-publish (with
gh release edit --draft=false) when the--publishflag is used, and draft-by-default otherwise. Both branches provide clear, helpful messaging to guide users through their chosen workflow.
7-9: ✓ Documentation accurately reflects the new release modes.The usage examples and feature descriptions clearly communicate the draft-by-default behavior and the optional
--publishflag for auto-publishing, helping users understand their options upfront.Also applies to: 17-22
apps/desktop/RELEASE.md (2)
3-33: ✓ Quick Start provides clear, streamlined release process.The condensed workflow with helpful examples and explicit mention of draft-by-default behavior sets proper expectations. The inclusion of both script-based and manual publish paths gives users flexibility.
55-76: ✓ Code Signing, Local Testing, and Troubleshooting sections are clear and helpful.The reference information for secrets, build commands, and common issues is well-organized and should guide developers through local setup and troubleshooting effectively.
| ## Auto-update | ||
|
|
||
| ## Build Outputs | ||
| The app checks for updates at launch and every 6 hours using: | ||
|
|
||
| ### macOS (arm64) | ||
| - `Superset-<version>-arm64.dmg` - DMG installer | ||
| - `Superset-<version>-arm64-mac.zip` - Zipped app bundle | ||
|
|
||
| ## Troubleshooting | ||
| - **Manifest**: `https://github.com/superset-sh/superset/releases/latest/download/latest-mac.yml` | ||
| - **Installer**: `https://github.com/superset-sh/superset/releases/latest/download/Superset-arm64.dmg` | ||
|
|
||
| ### Build fails on macOS | ||
| The workflow creates stable-named copies (without version) so these URLs always point to the latest build. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'UPDATE_CHECK_INTERVAL_MS' apps/desktop/src/main/lib/auto-updater.ts -A 2 -B 2Repository: superset-sh/superset
Length of output: 407
Correct auto-update interval documentation from 6 hours to 1 hour.
Line 48 states the app checks for updates "every 6 hours," but the implementation in apps/desktop/src/main/lib/auto-updater.ts uses UPDATE_CHECK_INTERVAL_MS = 1000 * 60 * 60 * 1 which equals 1 hour. Update the documentation to reflect the actual interval.
🤖 Prompt for AI Agents
In apps/desktop/RELEASE.md around lines 46 to 53, the documentation wrongly
states the app checks for updates "every 6 hours" while the actual
implementation uses a 1-hour interval; update the sentence to read "every 1
hour" (or "every hour") and ensure any adjacent wording or bullet points reflect
the 1-hour interval to match UPDATE_CHECK_INTERVAL_MS in
apps/desktop/src/main/lib/auto-updater.ts.
Setting publish: null prevented latest-mac.yml from being generated. The github provider generates the manifest for auto-update while the workflow handles the actual release upload. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-desktop.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build - macOS (arm64)
- GitHub Check: Build
🔇 Additional comments (2)
.github/workflows/release-desktop.yml (2)
89-95: Verify that the auto-update manifest is generated and is not per-architecture.The new manifest upload expects a single
apps/desktop/release/latest-mac.ymlfile. Unlike the DMG and ZIP uploads (lines 76, 84) which include${{ matrix.arch }}in the artifact name, the manifest upload uses a static namedesktop-mac-update-manifest(line 92).If electron-builder generates per-architecture manifests (e.g.,
latest-mac-arm64.yml,latest-mac-x64.yml), only the last one will be retained in GitHub Artifacts. Verify:
- Whether
latest-mac.ymlis a single unified manifest or per-architecture- If per-architecture, the artifact name should include
${{ matrix.arch }}to avoid conflicts- Whether the release job (line 140) properly consumes this manifest artifact
Also confirm that the manifest is actually generated during the
bun run packagestep (line 70).
73-95: Artifact upload strategy addresses past critical review.The previous review flagged that ZIP artifacts were missing from the workflow. This PR now adds:
- ZIP artifact upload with arch-specific naming and strict validation (lines 81–87)
- Auto-update manifest upload with strict validation (lines 89–95)
- Consistent
if-no-files-found: erroracross all artifact uploads for early failure detectionThis approach ensures the build fails fast if required release artifacts are not generated, preventing incomplete releases.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.