Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new GitHub Actions release workflow for the desktop app, enables productName-driven Electron Builder metadata and macOS notarization, replaces displayName with productName in package.json and main window title, adjusts tab naming scope and a rename input class, and adds a detailed RELEASE.md for desktop release procedures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant GH as GitHub
participant CI as Actions (Release Desktop)
participant Mac as macOS Runner (build)
participant Art as Artifact Store
participant Rel as Release Job (Ubuntu)
participant GHRel as GitHub Release
Note over Dev,GH: Two triggers: Tag push (desktop-v*) or Manual dispatch
Dev->>GH: push tag / manual dispatch
GH->>CI: start "Release Desktop App"
CI->>Mac: build job (checkout → setup Bun → install → prebuild → compile → package → notarize)
Mac->>Art: upload artifacts (dmg/zip)
CI->>Rel: release job downloads artifacts from Art
Rel->>GHRel: create draft "Superset Desktop <tag>" and attach artifacts
GHRel-->>Dev: draft release created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 (2)
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: 1
🧹 Nitpick comments (2)
.github/workflows/release-desktop.yml (2)
9-12: Remove unused workflow_dispatch version input.The
versioninput is defined for manual dispatch but never referenced in the workflow. The release creation job (line 133) only runs for tag-based releases anyway. If manual dispatch is strictly for testing (per RELEASE.md line 39), this parameter should be removed to avoid confusing users.workflow_dispatch: - inputs: - version: - description: "Version to release (e.g., 1.0.0)" - required: true - type: stringAlternatively, if you intend to support manual releases in the future, use the version input in the create-release step for the release name.
44-44: Consider pinning Bun version for reproducible release builds.Using
bun-version: latestcan cause inconsistent builds across runs if newer versions introduce changes. For release builds, consider pinning to a specific stable version to ensure reproducibility and prevent unexpected failures.- bun-version: latest + bun-version: "1.x" # or specify a specific version like "1.0.24"This is especially important for production releases where consistency matters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release-desktop.yml(1 hunks)apps/desktop/RELEASE.md(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/release-desktop.yml
145-145: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 LanguageTool
apps/desktop/RELEASE.md
[uncategorized] ~43-~43: The official name of this software platform is spelled with a capital “H”.
Context: ...orkflow Overview The release workflow (.github/workflows/release-desktop.yml) perform...
(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 (3)
apps/desktop/RELEASE.md (1)
1-155: Comprehensive and well-structured release documentation.The release guide clearly explains both the tag-based and manual workflow dispatch methods, provides troubleshooting guidance, and aligns well with the workflow implementation. The prerequisites and local testing sections will help developers successfully perform releases.
.github/workflows/release-desktop.yml (2)
68-95: Platform-specific build steps are well-structured.Each platform correctly sets the
GH_TOKENfor artifact uploads, and code signing environment variables are properly commented out as optional. The matrix-based approach correctly targets the right architecture/platform combinations.
98-126: Artifact uploads properly configured for all platforms.Each platform uploads the expected release binaries (.dmg/.zip for macOS, .exe/.msi for Windows, .AppImage/.deb for Linux) with appropriate 30-day retention. The
nameattribute uses matrix variables for clarity.
| path: release-artifacts | ||
|
|
||
| - name: Create Release | ||
| uses: softprops/action-gh-release@v1 |
There was a problem hiding this comment.
🧩 Analysis chain
Update outdated GitHub release action.
The softprops/action-gh-release@v1 action is flagged by actionlint as too old and may have compatibility issues with current GitHub Actions. Update to a newer version to ensure reliability for production releases.
Run the following to check the latest available version and update:
Once you identify the latest stable version, update line 145:
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2(Replace v2 with the actual latest stable version)
🏁 Script executed:
#!/bin/bash
# Check the latest release of softprops/action-gh-release
gh api repos/softprops/action-gh-release/releases/latest --jq '.tag_name'Length of output: 6
Update outdated GitHub release action to the latest stable version.
The softprops/action-gh-release@v1 action is outdated. The latest stable version is v2.4.2. Update line 145 to use the current release:
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2.4.2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: softprops/action-gh-release@v1 | |
| uses: softprops/action-gh-release@v2.4.2 |
🧰 Tools
🪛 actionlint (1.7.8)
145-145: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/release-desktop.yml around line 145, update the GitHub
release action reference from the outdated softprops/action-gh-release@v1 to the
current stable version softprops/action-gh-release@v2.4.2; edit line 145 to
replace the version tag so the workflow uses v2.4.2 and commit the change to
ensure the action runs the latest stable release.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-desktop.yml (1)
88-88: Update outdated GitHub release action.The
softprops/action-gh-release@v1action is flagged by actionlint as too old and may have compatibility issues. Update to the latest stable versionv2.4.2for production reliability.- uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2.4.2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release-desktop.yml(1 hunks)apps/desktop/RELEASE.md(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/release-desktop.yml
88-88: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 LanguageTool
apps/desktop/RELEASE.md
[uncategorized] ~43-~43: The official name of this software platform is spelled with a capital “H”.
Context: ...orkflow Overview The release workflow (.github/workflows/release-desktop.yml) perform...
(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
| This will automatically: | ||
| 1. Build the app for macOS (arm64), Windows (x64), and Linux (x64) | ||
| 2. Create artifacts for each platform | ||
| 3. Create a draft GitHub release with all binaries attached |
There was a problem hiding this comment.
Documentation claims multi-platform builds but workflow only supports macOS.
Lines 25-28 state the workflow will "Build the app for macOS (arm64), Windows (x64), and Linux (x64)", but the actual workflow (.github/workflows/release-desktop.yml) only configures macOS builds (line 17: runs-on: macos-latest). This creates incorrect expectations for users following the release process.
Either update the documentation to reflect current macOS-only support:
This will automatically:
-1. Build the app for macOS (arm64), Windows (x64), and Linux (x64)
+1. Build the app for macOS (arm64)
2. Create artifacts for each platform
3. Create a draft GitHub release with all binaries attachedOr extend the workflow to support Windows and Linux builds as described. See lines 148–154 for the intended multi-platform approach.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This will automatically: | |
| 1. Build the app for macOS (arm64), Windows (x64), and Linux (x64) | |
| 2. Create artifacts for each platform | |
| 3. Create a draft GitHub release with all binaries attached | |
| This will automatically: | |
| 1. Build the app for macOS (arm64) | |
| 2. Create artifacts for each platform | |
| 3. Create a draft GitHub release with all binaries attached |
Updated cache key to use 'bun.lock' instead of '**/bun.lockb' to fix hashFiles error on macOS runners. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
electron-builder requires repository field for publishing artifacts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Set CI to manual dispatch only while fixing release workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
hashFiles fails with bun.lock binary file, use commit SHA instead. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Switch from push to tag-based releases (desktop-v*.*.*) - Add matrix strategy with fail-fast: false for future multi-arch - Rename job from 'release' to 'build' for clarity - Update artifact naming to include arch - Simplify file pattern in release step Based on best practices from onlook-dev/desktop workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
| pull_request: | ||
| push: | ||
| branches: [main] | ||
| workflow_dispatch: |
There was a problem hiding this comment.
just for testing sake
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-desktop.yml (1)
95-95: Update action version as previously noted.This has already been flagged in previous review comments. The
softprops/action-gh-release@v1action should be updated tov2.4.2for compatibility with current GitHub Actions runners.
🧹 Nitpick comments (1)
.github/workflows/release-desktop.yml (1)
35-42: Optimize cache key for better reuse across commits.The cache key currently uses
${{ github.sha }}, which creates a new cache entry for every commit. This prevents cache reuse even when dependencies haven't changed.Apply this diff to use the lockfile hash for better cache hits:
- name: Cache dependencies uses: actions/cache@v4 with: path: | ~/.bun/install/cache - key: ${{ runner.os }}-bun-${{ github.sha }} + key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }} restore-keys: | ${{ runner.os }}-bun-This matches the pattern used in the CI workflow and will significantly improve build performance by reusing cached dependencies when the lockfile hasn't changed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks).github/workflows/release-desktop.yml(1 hunks)apps/desktop/electron-builder.ts(1 hunks)apps/desktop/package.json(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/release-desktop.yml
95-95: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
apps/desktop/package.json (1)
8-11: LGTM! Repository metadata properly added.The repository field is correctly configured and will help package managers and Electron Builder identify the source repository. This aligns well with the new release workflow.
apps/desktop/electron-builder.ts (1)
61-63: LGTM! Notarization properly configured for macOS distribution.The notarization configuration with the Team ID is correctly set up and aligns with the Apple Developer requirements. The release workflow properly provides the necessary credentials via secrets (
APPLEIDandAPPLEIDPASS), andhardenedRuntimeis already enabled as required.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/electron-builder.ts (1)
50-62: Consider target architecture for production releases.The macOS configuration correctly sets
hardenedRuntime: true, which is required for notarization. The build target is currently set toarm64only with a comment indicating this is "for faster testing."For production releases, you may want to consider building universal binaries (both
arm64andx64) to support Intel-based Macs, or document this decision ifarm64-only is intentional.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/release-desktop.yml (1)
96-96: Update outdated GitHub release action to the latest stable version.The
softprops/action-gh-release@v1action is outdated and flagged by static analysis. The latest stable version isv2.4.2. Please update to ensure compatibility and reliability for production releases.Apply this diff to update the action version:
- name: Create Release - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2.4.2 with: files: release-artifacts/**/* draft: true
🧹 Nitpick comments (1)
.github/workflows/release-desktop.yml (1)
35-42: Consider using lockfile hash for better cache efficiency.The current cache key uses
github.sha, which creates a unique cache entry for every commit. This prevents cache reuse across commits with identical dependencies. Consider using a hash of the lockfile instead for better cache hit rates and faster builds.Apply this diff to improve cache efficiency:
- name: Cache dependencies uses: actions/cache@v4 with: path: | ~/.bun/install/cache - key: ${{ runner.os }}-bun-${{ github.sha }} + key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }} restore-keys: | ${{ runner.os }}-bun-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-desktop.yml(1 hunks)apps/desktop/electron-builder.ts(1 hunks)apps/desktop/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/package.json
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/release-desktop.yml
96-96: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
apps/desktop/electron-builder.ts (1)
61-64: LGTM! Notarization properly configured.The notarization enablement and bundle metadata extension are correctly implemented. The workflow environment variables for
APPLE_ID,APPLE_APP_SPECIFIC_PASSWORD, andAPPLE_TEAM_IDare properly configured to support the notarization process..github/workflows/release-desktop.yml (1)
57-67: LGTM! Code signing and notarization environment variables are correctly configured.The environment variables for macOS code signing (
CSC_LINK,CSC_KEY_PASSWORD) and notarization (APPLE_ID,APPLE_APP_SPECIFIC_PASSWORD,APPLE_TEAM_ID) are properly set with the correct names expected by electron-builder. UsingAPPLE_APP_SPECIFIC_PASSWORDis the recommended secure approach.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/electron-builder.ts (1)
14-14: Add a fallback or assertion forproductName.If
pkg.productNameis undefined inpackage.json, this will silently propagateundefinedthrough the configuration, potentially causing build failures or incorrect artifact naming.-const productName = pkg.productName; +const productName = pkg.productName ?? pkg.name;Alternatively, add a runtime assertion to fail fast if the required field is missing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/electron-builder.ts(3 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/main/windows/main.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/package.json
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/electron-builder.tsapps/desktop/src/main/windows/main.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/electron-builder.tsapps/desktop/src/main/windows/main.ts
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/electron-builder.tsapps/desktop/src/main/windows/main.ts
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/electron-builder.tsapps/desktop/src/main/windows/main.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/electron-builder.tsapps/desktop/src/main/windows/main.ts
🧠 Learnings (3)
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/lib/*-ipcs.ts : Implement Electron IPC handlers in dedicated files under apps/desktop/src/main/lib/ (e.g., workspace-ipcs.ts, terminal-ipcs.ts)
Applied to files:
apps/desktop/src/main/windows/main.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/main/windows/main.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules in apps/desktop/src/lib/electron-router-dom.ts as it is shared between main and renderer processes
Applied to files:
apps/desktop/src/main/windows/main.ts
⏰ 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 (4)
apps/desktop/src/main/windows/main.ts (1)
6-6: LGTM — consistent productName usage.The import and window title now correctly reference
productNamefrom package.json, aligning with the standardized naming convention across the codebase.Also applies to: 20-20
apps/desktop/electron-builder.ts (3)
54-59: macOS build targets arm64 only — Intel Macs excluded.This configuration excludes x64 builds, meaning Intel Mac users won't be able to run the application. If this is intentional to reduce build complexity or artifact size, consider documenting this limitation in
RELEASE.md.To support both architectures, you can add x64:
target: [ { target: "default", - arch: ["arm64"], + arch: ["arm64", "x64"], }, ],
63-66: Good addition for macOS bundle consistency.Setting
CFBundleNameandCFBundleDisplayNameensures the app name displays correctly in Finder, Dock, and system dialogs, aligning with the centralizedproductName.
71-72: LGTM — consistent naming in protocol and Windows artifacts.The protocol name and Windows
artifactNamecorrectly use the centralizedproductName, ensuring consistent branding across deep-link registration and release artifacts.Also applies to: 92-92
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.