Port desktop build and CI for Windows with release artifacts#3261
Open
quueli wants to merge 6 commits into
Open
Port desktop build and CI for Windows with release artifacts#3261quueli wants to merge 6 commits into
quueli wants to merge 6 commits into
Conversation
…acts - Cross-platform postinstall (Bun) + node-pty Spectre patch for Windows - copy-native-modules: unlink symlinks, robocopy, repair @superset workspace dirs - build-desktop: windows-latest NSIS x64 job - release-desktop: stable Superset-x64.exe copy for latest download URLs - electron-builder publish target quueli/superset-windows - README: fork banner, Windows docs, release how-to, contributing
Contributor
📝 WalkthroughWalkthroughThe pull request adds Windows support to the desktop application by introducing a Windows build workflow, updating build tooling with Windows-aware file copying, patching node-pty for Spectre mitigation, and converting shell postinstall logic to cross-platform TypeScript with Windows-specific handling. Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant WinEnv as Windows Runner
participant Git as Git & Bun
participant Build as Build Tools
participant Electron as Electron Builder
participant Artifacts as Artifact Upload
GHA->>WinEnv: Trigger build-windows job
WinEnv->>WinEnv: Set core.longpaths=true
WinEnv->>WinEnv: Enable LongPathsEnabled registry
GHA->>Git: Checkout code
Git->>Bun: Setup Bun + cache
Bun->>Bun: bun install
Bun->>Build: bun run install:deps
Build->>Build: Patch node-pty Spectre
Build->>Build: bun run clean:dev
Build->>Build: bun run generate:icons
Build->>Build: bun run compile:app
Build->>Electron: bun run package
Electron->>Electron: Create NSIS installer (x64)
Electron->>Electron: Generate blockmap & latest.yml
Electron->>Artifacts: Windows x64 artifacts ready
Artifacts->>Artifacts: Verify \\*.exe, blockmap, latest.yml
Artifacts->>GHA: Upload to GitHub Release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
Greptile SummaryThis PR ports the Superset desktop app build pipeline to support Windows, adding a new The overall approach is solid and well-structured, but there are blocking issues that must be resolved before merging:
Additional items worth tracking as follow-ups:
Confidence Score: 2/5Not safe to merge until the fork repo references (especially the auto-update publisher) are corrected to point at the upstream repository. The Windows CI and build infrastructure work is well-executed, but the
|
| Filename | Overview |
|---|---|
| .github/workflows/build-desktop.yml | Adds Windows NSIS build job with long-path support, Bun caching, and artifact uploads; artifact upload glob for .exe is broader than the verification filter (may include uninstaller). |
| .github/workflows/release-desktop.yml | Adds Windows NSIS stable-copy step and latest.yml passthrough to the release job; logic is sound, release job correctly gated to tag pushes only. |
| apps/desktop/electron-builder.ts | Adds Windows NSIS target configuration, but the publish block hard-codes the fork repo (quueli/superset-windows) as the auto-update source — critical issue that must be fixed before merging. |
| apps/desktop/package.json | Repository URL still points to the contributor's fork (quueli/superset-windows); should reference the upstream superset-sh/superset repo. |
| apps/desktop/scripts/copy-native-modules.ts | New script to materialize Bun symlinks into real file copies for electron-builder; adds Windows robocopy path and fetchNpmPackage for cross-compilation with a minor piping concern on Windows shells. |
| scripts/patch-node-pty-spectre-windows.ts | No-op on non-Windows; patches node-pty's gyp files to disable Spectre mitigation, resolving MSB8040 build errors on Windows dev machines lacking mitigated libs. |
| scripts/postinstall.ts | Cross-platform replacement for the shell-based postinstall.sh; correctly handles Windows by using spawnSync with shell: true and skips native dep install in CI. |
| package.json | Root repository.url changed to the contributor's fork URL; postinstall script correctly updated to bun scripts/postinstall.ts for Windows compatibility. |
Sequence Diagram
sequenceDiagram
participant Tag as Git Tag Push
participant Build as build-desktop.yml
participant Win as Windows Runner
participant Mac as macOS Runner (arm64 + x64)
participant Lin as Linux Runner
participant Release as release-desktop.yml
participant GH as GitHub Releases
Tag->>Build: trigger (desktop-v*.*.*)
Build->>Win: build-windows job
Build->>Mac: build-macos job (matrix)
Build->>Lin: build-linux job
Win->>Win: Enable long paths
Win->>Win: bun install --frozen --ignore-scripts
Win->>Win: patch node-pty Spectre (preinstall:deps)
Win->>Win: electron-builder install-app-deps
Win->>Win: compile:app (electron-vite)
Win->>Win: package (NSIS, unsigned)
Win-->>Release: upload .exe, .blockmap, latest.yml
Mac-->>Release: upload .dmg, .zip, *-mac.yml
Lin-->>Release: upload .AppImage, *-linux.yml
Release->>Release: merge mac manifests
Release->>Release: create stable-named copies
Release->>GH: gh release create (draft)
Note over GH: Auto-update polls quueli/superset-windows ⚠️
Comments Outside Diff (1)
-
apps/desktop/scripts/copy-native-modules.ts, line 334-338 (link)curl | tarpipe may fail when run undercmd.exeon WindowsexecSyncon Windows usescmd.exeby default (unless a shell is explicitly specified). While modern Windows 10/11 ships bothcurl.exeandbsdtar.exe, piping a binary stream between them throughcmd.execan silently corrupt data in some edge cases. If this code path is ever hit on a Windows runner, the extracted archive might be malformed.A safer approach is to split into two separate calls:
execSync(`curl -sL -o "${destPath}.tgz" "${url}"`, { stdio: "pipe" }); execSync(`tar xz -C "${destPath}" --strip-components=1 -f "${destPath}.tgz"`, { stdio: "pipe" }); rmSync(`${destPath}.tgz`);
Reviews (1): Last reviewed commit: "ci(windows): GIT_CONFIG_* + registry for..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 86-87: Replace the mixed-language Quick Start bullet lines in
README.md (the "- **Windows x64:** ..." and "- **macOS:** ..." bullets) with
English text; specifically update the "Windows x64" bullet to "Windows x64:
Latest fork release — NSIS installer (`Superset-*-x64.exe` or
`Superset-x64.exe`)" and the "macOS" bullet to "macOS: Official upstream builds"
while preserving the existing release links.
- Around line 139-149: Translate the Russian "Артефакты" section into English:
replace the Russian paragraph with "Artifacts in `apps/desktop/release/` — on
Windows the **NSIS** installer `*-x64.exe`, on macOS `.dmg` / `.zip`, on Linux
`.AppImage` and update the local Windows build instructions to English while
preserving the commands shown (`cd apps/desktop`, `bun run clean:dev && bun run
generate:icons && bun run compile:app`, the environment variable examples `set
CSC_IDENTITY_AUTO_DISCOVERY=false` and PowerShell
`$env:CSC_IDENTITY_AUTO_DISCOVERY='false'`, and `bun run package`) so the
section under README.md referencing apps/desktop/release/ and the build steps is
fully in English.
- Around line 153-192: Translate the entire "## Releases in this fork / Релизы в
форке" section (all Russian text) into clear English while preserving the
original structure and all technical details (tag pattern `desktop-v*.*.*`,
steps referencing `apps/desktop/package.json`, Actions workflow name "Release
Desktop App", job names "build" and "release", and the upstream sync commands).
Replace the Russian paragraphs with their English equivalents and keep the
example commands and filenames unchanged; optionally add the original Russian as
a secondary paragraph below each translated subsection if you want bilingual
docs, but ensure the primary visible text is English for operational clarity.
- Line 76: Replace the mixed-language table row that starts with the "| **OS** |
**Windows 10+ x64**" entry so it is written entirely in English and clarifies
the long-path setting instruction; specifically update the OS cell text (the
table row with the "**OS**" header) to say that Windows builds come from this
fork, macOS/Linux follow upstream, and that Windows users should enable long
paths by running git config --global core.longpaths true before cloning to avoid
path-length errors.
- Around line 291-302: The Contributing section in README.md is written in
Russian and should be replaced with an English version to make contribution
instructions accessible to international contributors; locate the paragraph
starting with "Contributions to **this fork** (Windows, CI, документация):" and
replace that Russian bullet list with the suggested English text (keeping links
and markdown formatting intact), ensuring the items about cloning
quueli/superset-windows, creating a feature branch, committing, pushing, opening
a PR to quueli/superset-windows, and guidance for upstream vs fork issues and
the CONTRIBUTING.md reference are included exactly as in the suggested version.
- Around line 97-104: Replace the Russian Windows build instructions block with
an English version: keep the git clone snippet and add a short sentence
referencing the detailed "Requirements" section for long-path setup (e.g., "On
Windows, enable long paths before cloning if you see 'Filename too long' — see
Requirements"), and a concise sentence that native desktop modules require
Visual Studio Build Tools with the "Desktop development with C++" workload (MSVC
+ Windows SDK); remove the duplicated long-path detail here and rely on the
Requirements section for full instructions.
- Line 7: Replace or augment the Russian-only fork description line (the "**Fork
[quueli/superset-windows](https://github.com/quueli/superset-windows)** —
Windows 10+ сборка десктопа, кроссплатформенный `postinstall`, job CI для
Windows. Upstream:
[superset-sh/superset](https://github.com/superset-sh/superset).") with an
English version or add an English translation inline; e.g., update the README
line to: "**Fork
[quueli/superset-windows](https://github.com/quueli/superset-windows)** —
Windows 10+ desktop builds, cross-platform `postinstall`, Windows CI workflows.
Upstream: [superset-sh/superset](https://github.com/superset-sh/superset)."
Ensure both languages are present if you want to keep the Russian text.
- Around line 313-315: Update the second bullet in the "Community" links list so
its wording matches the others by using consistent language — change the
description for the "**[Issues (this
fork)](https://github.com/quueli/superset-windows/issues)**" list item to a
phrase like "Windows builds and fork-specific issues" to match the style used
for the upstream Issues and Discussions entries.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f598c3f2-5f1a-4ed2-816d-15f8a8fcc423
📒 Files selected for processing (9)
.github/workflows/build-desktop.yml.github/workflows/release-desktop.ymlREADME.mdapps/desktop/electron-builder.tsapps/desktop/package.jsonapps/desktop/scripts/copy-native-modules.tspackage.jsonscripts/patch-node-pty-spectre-windows.tsscripts/postinstall.ts
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/scripts/copy-native-modules.ts">
<violation number="1" location="apps/desktop/scripts/copy-native-modules.ts:61">
P1: `spawnSync` launch failures (`status === null` / `error` set) are coerced to success by `st.status ?? 0`, causing silent robocopy failures.</violation>
</file>
<file name=".github/workflows/build-desktop.yml">
<violation number="1" location=".github/workflows/build-desktop.yml:379">
P2: The upload glob `*-x64.exe` will also capture the NSIS uninstaller (`*__uninstaller*.exe`) that electron-builder places in the release directory. The verification step above correctly filters it out with `grep -v __uninstaller`, but the upload step doesn't apply the same exclusion. This means the artifact will contain both the installer and the uninstaller, which can confuse users or downstream tooling expecting a single installer.</violation>
</file>
<file name="apps/desktop/electron-builder.ts">
<violation number="1" location="apps/desktop/electron-builder.ts:35">
P1: The `publish` block hard-codes the fork repository (`quueli/superset-windows`) as the GitHub Releases source for auto-updates. When shipped, `electron-updater` will poll this fork for new releases instead of the canonical `superset-sh/superset` repository. If the fork diverges or becomes unavailable, installed clients will silently stop receiving updates — or could receive updates from an unreviewed source.</violation>
</file>
<file name="apps/desktop/package.json">
<violation number="1" location="apps/desktop/package.json:10">
P2: The root monorepo `repository.url` was changed to point to the fork (`quueli/superset-windows`). This affects any tooling that reads the root manifest for the canonical project URL (e.g., `npm publish`, GitHub integrations, release scripts).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/build-desktop.yml">
<violation number="1" location=".github/workflows/build-desktop.yml:375">
P2: Misplaced `path: |` block is inside the bash `run` script, so the Windows job will error out trying to execute `path:` as a command and stop before artifact uploads.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-desktop.yml:
- Around line 361-377: The "Verify Windows NSIS + update manifest exist" step
currently has an invalid top-level "path" key inside its run block which makes
the workflow malformed and prevents the installer artifact from being uploaded;
remove the stray "path:" entry from that run step and add a new separate step
after it in the "build-windows" job that uses actions/upload-artifact@v4 (name
it e.g. "Upload Windows installer") with with.path set to
"apps/desktop/release/*-x64.exe" (and exclude uninstaller patterns as needed,
e.g. using a glob that omits "*__uninstaller*.exe" or add a subsequent
cleanup/upload of only the intended file), ensuring the new step is a sibling
step (not inside run) so the built installer is actually uploaded.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8a3e926-1bca-494c-a8d3-ba2c7ce40756
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
…indows CI artifact upload - electron-builder publish + package.json repository URLs use canonical upstream - Restore NSIS upload step; exclude __uninstaller from artifact glob - README: English fork blurb and OS requirements (CodeRabbit)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-desktop.yml:
- Around line 385-391: The upload step "Upload Windows block map" currently uses
"if-no-files-found: warn" which allows CI to succeed when no .blockmap is
produced; change that setting to "if-no-files-found: error" (or remove the key
so missing files fail) in the actions/upload-artifact@v4 step to make the job
fail if apps/desktop/release/*.blockmap is absent, keeping the step name, with,
and path fields unchanged.
- Around line 370-373: The workflow currently hard-codes "latest.yml" for
Windows which breaks non-stable channels; update the checks and upload steps
that reference "latest.yml" to be channel-aware by using the workflow input
"channel" or glob pattern matching (e.g., match "*-${{ inputs.channel }}.yml" or
a Windows-specific glob like "*-win*.yml") instead of the literal "latest.yml",
and ensure the pre-upload existence test and subsequent artifact upload step
both use the same dynamic pattern so canary builds (canary.yml) and other
channels are detected and uploaded correctly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a79ce646-347f-4f16-a479-abd3535d462f
📒 Files selected for processing (4)
.github/workflows/build-desktop.ymlREADME.mdapps/desktop/package.jsonpackage.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/package.json
- package.json
- README.md
This was referenced Apr 21, 2026
Kitenite
added a commit
that referenced
this pull request
Apr 21, 2026
…aces Two match sites keyed on branch name alone, so any cross-fork PR whose `headRefName` collided with a local branch got attached — e.g. PR #3261 (`quueli/superset-windows:main` → `superset-sh/superset:main`) latched onto every local `main` workspace in the sidebar. - `pr-resolution.ts` (v1 `usePRStatus` via `workspaces.getGitHubStatus`): reject `isCrossRepository` when the local branch has no fork-owner prefix. Added a v1-sunset marker — don't evolve this module. - `pull-requests.ts` (`_dashboard` sidebar + v2 review tab via `pullRequests.getByWorkspaces` / `git.getPullRequest`): key the match on a `(upstreamOwner, upstreamRepo, upstreamBranch)` tuple. Adds those three columns to the host-service `workspaces` table, populated in `syncWorkspaceBranches` by resolving `@{push}` first and falling back to the same config chain `gh` uses. Tuple key lowercases owner/repo (GitHub is case-insensitive there) but preserves branch casing. `upstreamBranch` is separate from local `branch` so `gh pr checkout`-style renames (`main` → `quueli-main`) still match the PR's `headRefName`. Query extended with `isCrossRepository`, `headRepositoryOwner`, `headRepository`. Local SQLite migration `0003_workspace_upstream_ref` auto-applies on next host-service start; workspaces self-heal within one 30s sync cycle.
Kitenite
added a commit
that referenced
this pull request
Apr 22, 2026
…aces (#3625) * fix(host-service): stop misattributing cross-fork PRs to local workspaces Two match sites keyed on branch name alone, so any cross-fork PR whose `headRefName` collided with a local branch got attached — e.g. PR #3261 (`quueli/superset-windows:main` → `superset-sh/superset:main`) latched onto every local `main` workspace in the sidebar. - `pr-resolution.ts` (v1 `usePRStatus` via `workspaces.getGitHubStatus`): reject `isCrossRepository` when the local branch has no fork-owner prefix. Added a v1-sunset marker — don't evolve this module. - `pull-requests.ts` (`_dashboard` sidebar + v2 review tab via `pullRequests.getByWorkspaces` / `git.getPullRequest`): key the match on a `(upstreamOwner, upstreamRepo, upstreamBranch)` tuple. Adds those three columns to the host-service `workspaces` table, populated in `syncWorkspaceBranches` by resolving `@{push}` first and falling back to the same config chain `gh` uses. Tuple key lowercases owner/repo (GitHub is case-insensitive there) but preserves branch casing. `upstreamBranch` is separate from local `branch` so `gh pr checkout`-style renames (`main` → `quueli-main`) still match the PR's `headRefName`. Query extended with `isCrossRepository`, `headRepositoryOwner`, `headRepository`. Local SQLite migration `0003_workspace_upstream_ref` auto-applies on next host-service start; workspaces self-heal within one 30s sync cycle. * fix(host-service): require branch.<n>.merge in upstream fallback Without an explicit merge ref, a repo-wide `remote.pushDefault` would let an untracked local branch attach to any PR on that remote whose headRefName happens to match. That's the same branch-name-collision bug this PR was introduced to prevent — just via a different route. Reported by coderabbitai on #3625. * Update plan
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds a Windows desktop build and CI that produce an x64 NSIS installer, blockmap, and auto‑update manifest. Also adds a cross‑platform postinstall and hardens Windows native module handling; release flow creates a stable
Superset-x64.exeand aligns update metadata with the upstream repo.New Features
build-desktop.yml: builds x64 NSIS, uploads*.blockmapandlatest.yml, fixes artifact upload and excludes__uninstallerbinaries; handles long paths and Bun cache.Superset-x64.execopy for predictable “latest” downloads.superset-sh/superset; repository URLs set to canonical upstream.postinstall(scripts/postinstall.ts) plusnode-ptySpectre‑mitigation patch to avoid MSVC MSB8040 on Windows.robocopyon Windows, replaces symlinks, and repairs@superset/*workspace packages when needed.@superset/desktopto 1.4.8 and expand README with Windows install/build/release docs.Migration
desktop-v*.*.*to trigger full build and GitHub Release; manual runs build artifacts only.productionsecrets forcompile:appare set in CI.git config --global core.longpaths trueand install Visual Studio Build Tools (Desktop C++).Superset-x64.exe.Written for commit df692d6. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores