Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions .github/workflows/build-desktop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,89 @@ jobs:
retention-days: ${{ inputs.artifact_retention_days }}
if-no-files-found: error

build-windows:
name: Build - Windows (x64)
runs-on: windows-latest
environment: production

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Enable git long paths
run: git config --global core.longpaths true

- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: "1.3.2"

- name: Cache dependencies
uses: actions/cache@v4
with:
path: |
~/.bun/install/cache
key: ${{ runner.os }}-bun-${{ github.sha }}
restore-keys: |
${{ runner.os }}-bun-

- name: Install dependencies
run: bun install --frozen

- name: Set version suffix
if: inputs.version_suffix != ''
working-directory: apps/desktop
shell: bash
run: |
CURRENT_VERSION=$(node -p "require('./package.json').version")
NEW_VERSION="${CURRENT_VERSION}${{ inputs.version_suffix }}"
node -e "
const fs = require('fs');
const pkg = require('./package.json');
pkg.version = '$NEW_VERSION';
fs.writeFileSync('./package.json', JSON.stringify(pkg, null, '\t') + '\n');
"

- name: Clean dev folder
working-directory: apps/desktop
run: bun run clean:dev

- name: Generate icons
working-directory: apps/desktop
run: bun run generate:icons

- name: Compile app
working-directory: apps/desktop
run: bun run compile:app
env:
SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
SENTRY_DSN_DESKTOP: ${{ secrets.SENTRY_DSN_DESKTOP }}

Comment on lines +197 to +203
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Pass the same build-time config into the Windows compile step.

compile:app only gets Sentry values here, while the macOS/Linux jobs also inject the web/API URLs and OAuth client IDs. If those values are compiled into the renderer bundle, the Windows artifact will ship with missing auth and endpoint config.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-desktop.yml around lines 197 - 203, The Windows
"Compile app" step that runs "bun run compile:app" only injects
SENTRY_AUTH_TOKEN and SENTRY_DSN_DESKTOP, causing missing auth/endpoints in the
Windows renderer build; update that job step to export the same build-time
environment variables used in the macOS/Linux compile steps (the web/API
endpoint URLs and OAuth client IDs in addition to the Sentry vars) so "bun run
compile:app" receives the full set of config values at compile time.

- name: Copy native modules
working-directory: apps/desktop
run: bun run copy:native-modules

- name: Validate native runtime
working-directory: apps/desktop
run: bun run validate:native-runtime

- name: Build installer
working-directory: apps/desktop
run: bun run build
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The Windows build step ignores the workflow’s electron_builder_config input, so caller-specified configs never apply to Windows artifacts. Pass the config through like the macOS/Linux jobs do.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/build-desktop.yml, line 214:

<comment>The Windows build step ignores the workflow’s `electron_builder_config` input, so caller-specified configs never apply to Windows artifacts. Pass the config through like the macOS/Linux jobs do.</comment>

<file context>
@@ -143,6 +143,89 @@ jobs:
+
+      - name: Build installer
+        working-directory: apps/desktop
+        run: bun run build
+        env:
+          CSC_IDENTITY_AUTO_DISCOVERY: "false"
</file context>
Suggested change
run: bun run build
run: bun run build -- --config ${{ inputs.electron_builder_config }}
Fix with Cubic

env:
CSC_IDENTITY_AUTO_DISCOVERY: "false"

Comment on lines +212 to +217
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The Windows packaging step drops inputs.electron_builder_config.

That workflow input is honored on macOS/Linux but ignored here, so callers can select an alternate builder config for those jobs and silently get a different Windows artifact from the same workflow invocation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-desktop.yml around lines 212 - 217, The Windows
"Build installer" step currently ignores the workflow input
inputs.electron_builder_config; update that step to pass the selected config
into the desktop build command so Windows uses the same builder config as
macOS/Linux — e.g. in the "Build installer" job add either an environment
variable (ELECTRON_BUILDER_CONFIG: ${{ inputs.electron_builder_config }}) or
append a CLI flag to the bun run build invocation (bun run build -- --config ${{
inputs.electron_builder_config }}) so the build script/electron-builder receives
inputs.electron_builder_config and produces the correct Windows artifact.

- name: Upload artifacts
uses: actions/upload-artifact@v4
with:
name: ${{ inputs.artifact_prefix }}-windows-x64
path: |
apps/desktop/release/*.exe
apps/desktop/release/*.yml
apps/desktop/release/*.blockmap
retention-days: ${{ inputs.artifact_retention_days }}
if-no-files-found: error

build-linux:
name: Build - Linux (x64)
runs-on: ubuntu-latest
Expand Down
22 changes: 22 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,25 @@ jobs:
env:
NEXT_PUBLIC_OUTLIT_KEY: ${{ secrets.NEXT_PUBLIC_OUTLIT_KEY || 'ci-outlit-placeholder-key' }}
run: bun turbo run build --filter=@superset/desktop

test-windows:
name: Test (Windows)
runs-on: windows-latest
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Enable git long paths
run: git config --global core.longpaths true

- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: "1.3.2"
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The Windows CI job pins Bun 1.3.2 while the rest of the workflow uses 1.3.6, which can cause version-specific test behavior and inconsistent CI results across platforms.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 147:

<comment>The Windows CI job pins Bun 1.3.2 while the rest of the workflow uses 1.3.6, which can cause version-specific test behavior and inconsistent CI results across platforms.</comment>

<file context>
@@ -130,3 +130,25 @@ jobs:
+      - name: Setup Bun
+        uses: oven-sh/setup-bun@v1
+        with:
+          bun-version: "1.3.2"
+
+      - name: Install dependencies
</file context>
Suggested change
bun-version: "1.3.2"
bun-version: "1.3.6"
Fix with Cubic


- name: Install dependencies
run: bun install --frozen

- name: Run desktop tests
working-directory: apps/desktop
run: bun test
8 changes: 8 additions & 0 deletions .github/workflows/release-desktop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ jobs:
echo "Created stable copy: Superset-${arch}.AppImage"
fi
done
# Windows exe
for file in *.exe; do
if [[ -f "$file" ]]; then
arch=$(echo "$file" | sed -E 's/.*-([^-]+)\.exe$/\1/')
cp "$file" "Superset-${arch}.exe"
echo "Created stable copy: Superset-${arch}.exe"
fi
done
# Keep Linux updater manifest at a stable filename for generic provider lookups.
for file in *-linux.yml; do
if [[ -f "$file" && "$file" != "latest-linux.yml" ]]; then
Expand Down
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,21 @@ If it runs in a terminal, it runs on Superset

| Requirement | Details |
|:------------|:--------|
| **OS** | macOS (Windows/Linux untested) |
| **OS** | macOS, Windows 10+ (Linux untested) |
| **Runtime** | [Bun](https://bun.sh/) v1.0+ |
| **Version Control** | Git 2.20+ |
| **GitHub CLI** | [gh](https://cli.github.com/) |
| **Caddy** | [caddy](https://caddyserver.com/docs/install) (for dev server) |

### Windows Support

| Requirement | Details |
|---|---|
| OS | Windows 10 1809+ (ConPTY support required) |
| Developer Mode | Enable in Settings > Privacy & Security > For developers |
| Git long paths | `git config --global core.longpaths true` |
| Build Tools | Visual Studio Build Tools with "Desktop development with C++" (for native modules) |
Comment on lines +66 to +79
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a Windows download path alongside this support claim.

Line 66 now advertises Windows support, but the prebuilt CTAs at Line 15 and Line 85 still point to macOS only. Windows users will end up on a README that says they're supported without showing which .exe to download.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 66 - 79, The README advertises Windows support but
the prebuilt download CTAs only point to macOS; update the prebuilt download
links/CTAs (the top "Prebuilt" CTA and the bottom download CTA referenced near
the macOS link) to include a Windows download path (e.g., add the .exe or
Windows installer alongside the macOS binary), update the download
filenames/labels to show both macOS and Windows targets, and ensure the "Windows
Support" section's guidance matches the actual download entries so Windows users
can find the correct .exe installer.


## Getting Started

### Quick Start (Pre-built)
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/electron-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ const config: Configuration = {
},
],
artifactName: `${productName}-${pkg.version}-\${arch}.\${ext}`,
signAndEditExecutable: false,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: signAndEditExecutable: false disables Windows executable resource editing, so the configured app icon/metadata won’t be applied to the final .exe.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/electron-builder.ts, line 201:

<comment>`signAndEditExecutable: false` disables Windows executable resource editing, so the configured app icon/metadata won’t be applied to the final `.exe`.</comment>

<file context>
@@ -198,6 +198,8 @@ const config: Configuration = {
 			},
 		],
 		artifactName: `${productName}-${pkg.version}-\${arch}.\${ext}`,
+		signAndEditExecutable: false,
+		forceCodeSigning: false,
 	},
</file context>
Suggested change
signAndEditExecutable: false,
signAndEditExecutable: true,
Fix with Cubic

forceCodeSigning: false,
Comment on lines +201 to +202
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C2 'electron_builder_config|signAndEditExecutable|forceCodeSigning|CSC_IDENTITY_AUTO_DISCOVERY|CSC_LINK|WIN_CSC' .github/workflows apps/desktop

Repository: superset-sh/superset

Length of output: 3847


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if electron-builder.canary.ts exists and compare Windows configs
echo "=== Checking electron-builder.canary.ts ==="
if [ -f "apps/desktop/electron-builder.canary.ts" ]; then
  sed -n '/windows:/,/^[[:space:]]*},$/p' apps/desktop/electron-builder.canary.ts | head -30
else
  echo "electron-builder.canary.ts not found"
fi

echo -e "\n=== Checking for any Windows code signing config in electron-builder.ts ==="
sed -n '/windows:/,/^[[:space:]]*},$/p' apps/desktop/electron-builder.ts | grep -E 'sign|csc|CSC|certificate'

echo -e "\n=== Checking context around lines 201-202 in electron-builder.ts ==="
sed -n '195,210p' apps/desktop/electron-builder.ts

Repository: superset-sh/superset

Length of output: 184


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# List electron-builder files
echo "=== Files in apps/desktop matching electron-builder ==="
ls -la apps/desktop/electron-builder* 2>/dev/null || echo "No electron-builder files found"

# Read the full Windows section from electron-builder.ts with more context
echo -e "\n=== Full Windows config in electron-builder.ts ==="
awk '/windows:/{flag=1} flag{print NR": "$0} /^[[:space:]]*},/{if(flag && prev_close) flag=0} {prev_close=/^[[:space:]]*},/}' apps/desktop/electron-builder.ts | head -50

Repository: superset-sh/superset

Length of output: 342


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read electron-builder.ts to see the full context
echo "=== Reading electron-builder.ts ==="
cat -n apps/desktop/electron-builder.ts

echo -e "\n=== Reading electron-builder.canary.ts ==="
cat -n apps/desktop/electron-builder.canary.ts

Repository: superset-sh/superset

Length of output: 9765


This disables signing for all release builds, not just development.

Both stable (release-desktop.yml) and canary (release-desktop-canary.yml) release workflows use unsigned Windows configurations:

  • Stable release uses electron-builder.ts directly, which has lines 201–202 disabling code signing
  • Canary release uses electron-builder.canary.ts, which inherits the unsigned Windows config via ...baseConfig.win

No alternate signed Windows builder config exists for the release paths. If code signing should be enabled for public releases, the Windows config must be split into signed and unsigned variants or gated by environment/channel.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/electron-builder.ts` around lines 201 - 202, The Windows config
in electron-builder.ts currently sets signAndEditExecutable: false and
forceCodeSigning: false which disables signing for all release builds; update
the build configuration used by electron-builder.ts and
electron-builder.canary.ts (and the shared baseConfig.win) to provide two
variants — a signedWindowsConfig for public/stable releases and an
unsignedWindowsConfig for dev/canary — or gate the
signAndEditExecutable/forceCodeSigning flags by a release channel/environment
variable (e.g., process.env.RELEASE_CHANNEL) so that signAndEditExecutable and
forceCodeSigning are true for production releases and remain false for
dev/canary; ensure electron-builder.canary.ts does not inherit an unsigned-only
baseConfig.win for public release paths.

},

// NSIS installer (Windows)
Expand Down
6 changes: 4 additions & 2 deletions apps/desktop/scripts/copy-native-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ function copyModuleIfSymlink(
console.log(` ${moduleName}: symlink -> replacing with real files`);
console.log(` Real path: ${realPath}`);

// Remove the symlink
rmSync(modulePath);
// Remove the symlink/junction
// On Windows, Bun creates junctions for symlinked node_modules.
// rmSync without options fails on junctions, so use recursive+force.
rmSync(modulePath, { recursive: true, force: true });

// Copy the actual files
cpSync(realPath, modulePath, { recursive: true });
Expand Down
22 changes: 4 additions & 18 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { execFile, spawn } from "node:child_process";
import { execFile } from "node:child_process";
import { randomUUID } from "node:crypto";
import { mkdir, rename } from "node:fs/promises";
import { mkdir, rename, rm } from "node:fs/promises";
import { dirname, join, resolve } from "node:path";
import { promisify } from "node:util";

Expand Down Expand Up @@ -680,26 +680,12 @@ export async function removeWorktree(
});

// Delete the moved directory in the background — don't block the caller.
// Use spawned `rm -rf` instead of Node's fs.rm which can hang on macOS
// when encountering .app bundles with extended attributes.
const child = spawn("/bin/rm", ["-rf", tempPath], {
detached: true,
stdio: "ignore",
});
child.unref();
child.on("error", (err) => {
rm(tempPath, { recursive: true, force: true }).catch((err) => {
console.error(
`[removeWorktree] Failed to spawn rm for ${tempPath}:`,
`[removeWorktree] Background cleanup of ${tempPath} failed:`,
err.message,
);
});
child.on("exit", (code: number | null) => {
if (code !== 0) {
console.error(
`[removeWorktree] Background cleanup of ${tempPath} failed (exit ${code})`,
);
}
});
} catch (error) {
const code = (error as NodeJS.ErrnoException).code;
// If the worktree directory is already gone, just prune metadata
Expand Down
14 changes: 14 additions & 0 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,20 @@ async function getShellEnvWithTimeout(): Promise<Record<string, string>> {
* Results are cached for 1 minute to avoid spawning shells repeatedly.
*/
export async function getShellEnvironment(): Promise<Record<string, string>> {
// Windows: process.env already contains the full user environment.
// Unlike macOS GUI apps launched from Finder/Dock, Windows apps inherit
// the complete user environment including PATH modifications.
if (process.platform === "win32") {
const env: Record<string, string> = {};
for (const [key, value] of Object.entries(process.env)) {
if (typeof value === "string") {
env[key] = value;
}
}
return env;
}
Comment on lines +57 to +68
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't short-circuit Windows to raw process.env.

This function is the source of truth for getProcessEnvWithShellPath() and execWithShellEnv(). Returning the inherited Electron env here means Windows never sees shell/profile-derived PATH changes, so commands can resolve differently from the integrated terminal. That directly affects callers like gh in apps/desktop/src/lib/trpc/routers/projects/utils/github.ts:1-20, rg in apps/desktop/src/lib/trpc/routers/filesystem/index.ts:1-50, and git operations that depend on getProcessEnvWithShellPath(). Please derive the Windows env from the selected shell/init path instead of bypassing shell resolution entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts` around lines
57 - 68, The Windows branch currently short-circuits and returns raw process.env
(inside the process.platform === "win32" block), which prevents shell/profile
PATH resolution for getProcessEnvWithShellPath() and execWithShellEnv();
instead, invoke the selected shell/init script to produce the environment (or at
minimum the PATH) and merge those values into process.env before returning;
update the logic in the function that contains the platform switch (the block
around process.platform === "win32") to run the same shell-derived env
resolution used for macOS/Linux, merge the resulting PATH and any other
shell-provided variables with process.env, and return that combined env so
callers like getProcessEnvWithShellPath(), execWithShellEnv(), and downstream
users (e.g. gh/rg/git callers) see the shell-derived PATH on Windows.


// Existing macOS/Linux code follows...
const now = Date.now();
const ttl = isFallbackCache ? fallbackCacheTtlMs : CACHE_TTL_MS;
if (cachedEnv && now - cacheTime < ttl) {
Expand Down
19 changes: 13 additions & 6 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {
getCommandShellArgs,
getShellEnv,
} from "main/lib/agent-setup/shell-wrappers";
import { buildSafeEnv, sanitizeEnv } from "main/lib/terminal/env";
import { buildSafeEnv, getDefaultShell, sanitizeEnv } from "main/lib/terminal/env";
import { SUPERSET_DIR_NAME } from "shared/constants";
import treeKill from "tree-kill";
import { removeWorktree } from "./git";
import { loadSetupConfig } from "./setup";

Expand Down Expand Up @@ -42,9 +43,7 @@ export async function runTeardown({
console.log(`[teardown] Running for "${workspaceName}": ${command}`);

try {
const shell =
process.env.SHELL ||
(process.platform === "darwin" ? "/bin/zsh" : "/bin/bash");
const shell = process.env.SHELL || getDefaultShell();
Comment on lines 45 to +46
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't let SHELL override Windows shell selection here.

On Windows, an inherited SHELL value from Git Bash/MSYS will bypass the new getDefaultShell() logic and run teardown under the wrong shell. Gate the env override to non-Windows, or just use getDefaultShell() on Windows.

Suggested fix
-		const shell = process.env.SHELL || getDefaultShell();
+		const shell =
+			process.platform === "win32"
+				? getDefaultShell()
+				: process.env.SHELL || getDefaultShell();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts` around lines
45 - 46, The current teardown logic lets process.env.SHELL override the shell
selection even on Windows, which can force Git Bash/MSYS shells; change the
assignment in teardown.ts so that on Windows (process.platform === 'win32') you
always use getDefaultShell(), and only allow process.env.SHELL to override on
non-Windows platforms—i.e., replace the current const shell = process.env.SHELL
|| getDefaultShell() with a platform-gated expression so Shell selection is
correct for getDefaultShell() and the teardown routine.

const supersetHomeDir =
process.env.SUPERSET_HOME_DIR || join(homedir(), SUPERSET_DIR_NAME);
const shellWrapperPaths = {
Expand All @@ -60,7 +59,8 @@ export async function runTeardown({
const output = await new Promise<string>((resolve, reject) => {
const child = spawn(shell, args, {
cwd: worktreePath,
detached: true,
detached: process.platform !== "win32",
...(process.platform === "win32" ? { windowsHide: true } : {}),
stdio: ["ignore", "pipe", "pipe"],
env: {
...baseEnv,
Expand Down Expand Up @@ -114,7 +114,14 @@ export async function runTeardown({
`[teardown] Timed out after ${TEARDOWN_TIMEOUT_MS}ms, killing process group`,
);
try {
if (child.pid) process.kill(-child.pid, "SIGKILL");
if (child.pid) {
if (process.platform === "win32") {
// Windows: can't use negative PID for process group kill
treeKill(child.pid, "SIGKILL");
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Handle treeKill with a callback so kill failures are surfaced. The current fire-and-forget call can fail silently in the timeout path.

(Based on your team's feedback about handling async failures instead of silently swallowing them.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts, line 120:

<comment>Handle `treeKill` with a callback so kill failures are surfaced. The current fire-and-forget call can fail silently in the timeout path.

(Based on your team's feedback about handling async failures instead of silently swallowing them.) </comment>

<file context>
@@ -114,7 +114,14 @@ export async function runTeardown({
+						if (child.pid) {
+						if (process.platform === "win32") {
+							// Windows: can't use negative PID for process group kill
+							treeKill(child.pid, "SIGKILL");
+						} else {
+							process.kill(-child.pid, "SIGKILL");
</file context>
Suggested change
treeKill(child.pid, "SIGKILL");
treeKill(child.pid, "SIGKILL", (err) => {
if (err) {
console.warn("[teardown] Failed to SIGKILL process tree:", err.message);
}
});
Fix with Cubic

} else {
process.kill(-child.pid, "SIGKILL");
}
Comment on lines +117 to +123
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find and read the teardown.ts file
fd -p "teardown.ts" apps/desktop/src/lib/trpc

Repository: superset-sh/superset

Length of output: 126


🏁 Script executed:

# Read the teardown.ts file to understand the full context
cat -n apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts

Repository: superset-sh/superset

Length of output: 6152


🏁 Script executed:

# Check the tree-kill package to understand its API
fd -p "node_modules/tree-kill" --max-depth 1 && cat node_modules/tree-kill/package.json 2>/dev/null | head -20

Repository: superset-sh/superset

Length of output: 46


🌐 Web query:

tree-kill npm package API callback signature

💡 Result:

tree-kill exports a single function with this API:

  • kill(pid[, signal][, callback]) (npmjs.com)
  • callback is Node-style and is invoked as callback(err) where err is an Error (or undefined/null if no error). (npmjs.com)

Citations:


Wait for tree-kill to finish before rejecting the timeout.

On Windows, treeKill() is called without a callback at line 120, making it asynchronous. The code then immediately rejects at lines 126-128 without waiting for the tree-kill operation to complete. This causes the teardown promise to resolve while child processes are still alive and holding file locks, leading to intermittent cleanup failures.

Use the callback-based API and only reject after the kill completes:

Suggested fix
 					if (child.pid) {
 						if (process.platform === "win32") {
 							// Windows: can't use negative PID for process group kill
-							treeKill(child.pid, "SIGKILL");
+							treeKill(child.pid, "SIGKILL", () => {
+								reject(
+									new Error(`Teardown timed out after ${TEARDOWN_TIMEOUT_MS}ms`),
+								);
+							});
+							return;
 						} else {
 							process.kill(-child.pid, "SIGKILL");
 						}
 					}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts` around lines
117 - 123, The code currently calls treeKill(child.pid, "SIGKILL") on Windows
without waiting for completion, causing the teardown timeout to reject before
child processes are actually killed; update the logic in the teardown routine
that handles child termination (the block referencing child.pid and treeKill) to
use the callback-based API: call treeKill(child.pid, "SIGKILL", (err) => { if
(err) reject(errOrTimeout); else resolve()); } ) and only reject the outer
timeout after the callback reports failure; for the non-Windows branch that uses
process.kill(-child.pid, "SIGKILL") wrap it in try/catch and resolve/reject the
same promise path so both branches wait for termination before the teardown
promise settles.

}
} catch {}
reject(
new Error(`Teardown timed out after ${TEARDOWN_TIMEOUT_MS}ms`),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
} from "./agent-wrappers-common";
import { HOOKS_DIR } from "./paths";

export const COPILOT_HOOK_SCRIPT_NAME = "copilot-hook.sh";
export const COPILOT_HOOK_SCRIPT_NAME =
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Windows now selects a .ps1 hook script, but hooks JSON still defines only bash commands. Copilot CLI expects powershell commands on Windows, so Copilot hooks can fail to run there.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/agent-setup/agent-wrappers-copilot.ts, line 11:

<comment>Windows now selects a `.ps1` hook script, but hooks JSON still defines only `bash` commands. Copilot CLI expects `powershell` commands on Windows, so Copilot hooks can fail to run there.</comment>

<file context>
@@ -8,7 +8,8 @@ import {
 import { HOOKS_DIR } from "./paths";
 
-export const COPILOT_HOOK_SCRIPT_NAME = "copilot-hook.sh";
+export const COPILOT_HOOK_SCRIPT_NAME =
+	process.platform === "win32" ? "copilot-hook.ps1" : "copilot-hook.sh";
 
</file context>
Fix with Cubic

process.platform === "win32" ? "copilot-hook.ps1" : "copilot-hook.sh";

const COPILOT_HOOK_SIGNATURE = "# Superset copilot hook";
const COPILOT_HOOK_VERSION = "v1";
Expand All @@ -17,7 +18,9 @@ export const COPILOT_HOOK_MARKER = `${COPILOT_HOOK_SIGNATURE} ${COPILOT_HOOK_VER
const COPILOT_HOOK_TEMPLATE_PATH = path.join(
__dirname,
"templates",
"copilot-hook.template.sh",
process.platform === "win32"
? "copilot-hook.template.ps1"
: "copilot-hook.template.sh",
);

export function getCopilotHookScriptPath(): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
} from "./agent-wrappers-common";
import { HOOKS_DIR } from "./paths";

export const GEMINI_HOOK_SCRIPT_NAME = "gemini-hook.sh";
export const GEMINI_HOOK_SCRIPT_NAME =
process.platform === "win32" ? "gemini-hook.ps1" : "gemini-hook.sh";

const GEMINI_HOOK_SIGNATURE = "# Superset gemini hook";
const GEMINI_HOOK_VERSION = "v1";
Expand All @@ -20,7 +21,9 @@ export const GEMINI_HOOK_MARKER = `${GEMINI_HOOK_SIGNATURE} ${GEMINI_HOOK_VERSIO
const GEMINI_HOOK_TEMPLATE_PATH = path.join(
__dirname,
"templates",
"gemini-hook.template.sh",
process.platform === "win32"
? "gemini-hook.template.ps1"
: "gemini-hook.template.sh",
);

interface GeminiHookConfig {
Expand Down
7 changes: 5 additions & 2 deletions apps/desktop/src/main/lib/agent-setup/notify-hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import path from "node:path";
import { env } from "shared/env.shared";
import { HOOKS_DIR } from "./paths";

export const NOTIFY_SCRIPT_NAME = "notify.sh";
export const NOTIFY_SCRIPT_NAME =
process.platform === "win32" ? "notify.ps1" : "notify.sh";
Comment on lines +6 to +7
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how notify script is invoked across agent wrappers
rg -n "notifyScriptPath|NOTIFY_SCRIPT" apps/desktop/src/main/lib/agent-setup/*.ts | head -50

Repository: superset-sh/superset

Length of output: 3531


🏁 Script executed:

cat -n apps/desktop/src/main/lib/agent-setup/notify-hook.ts

Repository: superset-sh/superset

Length of output: 1941


🏁 Script executed:

sed -n '60,85p' apps/desktop/src/main/lib/agent-setup/agent-wrappers-mastra.ts

Repository: superset-sh/superset

Length of output: 980


🏁 Script executed:

# Check if there's any platform detection or Windows-specific handling for Mastra
rg -n "win32|platform|Windows" apps/desktop/src/main/lib/agent-setup/agent-wrappers-mastra.ts

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check where Mastra wrapper is integrated
rg -n "getMastraHooksJsonContent|setupMastraHooks" apps/desktop/src/main/lib/agent-setup/*.ts

Repository: superset-sh/superset

Length of output: 769


🏁 Script executed:

# Check if there's Windows-specific agent setup logic
rg -n "platform.*win32|win32.*platform" apps/desktop/src/main/lib/agent-setup/*.ts

Repository: superset-sh/superset

Length of output: 1220


🏁 Script executed:

# Check agent setup initialization to see if Mastra is conditionally enabled
head -50 apps/desktop/src/main/lib/agent-setup/index.ts

Repository: superset-sh/superset

Length of output: 1254


🏁 Script executed:

# Check the full setupAgentHooks to see if Mastra creation is conditional
sed -n '36,100p' apps/desktop/src/main/lib/agent-setup/index.ts

Repository: superset-sh/superset

Length of output: 1105


🏁 Script executed:

# Check how agent-wrappers.ts uses getMastraHooksJsonContent
sed -n '55,100p' apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts

Repository: superset-sh/superset

Length of output: 359


🏁 Script executed:

# Check if Droid and other wrappers have similar bash hardcoding
sed -n '50,80p' apps/desktop/src/main/lib/agent-setup/agent-wrappers-droid.ts

Repository: superset-sh/superset

Length of output: 828


🏁 Script executed:

# Check the full getMastraHooksJsonContent function to understand the scope
sed -n '55,100p' apps/desktop/src/main/lib/agent-setup/agent-wrappers-mastra.ts

Repository: superset-sh/superset

Length of output: 1649


🏁 Script executed:

# Check if Gemini and Copilot wrappers have similar bash hardcoding issues
rg -n "bash.*quoteShellPath|notifyCommand.*bash" apps/desktop/src/main/lib/agent-setup/agent-wrappers-*.ts

Repository: superset-sh/superset

Length of output: 196


🏁 Script executed:

# Check the Droid wrapper to see how it handles the notify script
sed -n '120,145p' apps/desktop/src/main/lib/agent-setup/agent-wrappers-droid.ts

Repository: superset-sh/superset

Length of output: 613


Mastra wrapper hardcodes bash invocation, incompatible with Windows PowerShell scripts.

While notify-hook.ts correctly implements platform-aware script selection (notify.ps1 on Windows, notify.sh otherwise), the Mastra wrapper at agent-wrappers-mastra.ts:69 hardcodes bash invocation:

const notifyCommand = `bash ${quoteShellPath(notifyScriptPath)}`;

On Windows, this produces bash /path/to/notify.ps1, which fails because bash cannot execute PowerShell scripts. The Droid wrapper correctly passes the script path directly without a shell prefix, which is the proper approach for platform-aware scripts. Update getMastraHooksJsonContent to conditionally invoke powershell on Windows or pass the script path directly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/agent-setup/notify-hook.ts` around lines 6 - 7, The
Mastra wrapper currently builds the notify invocation as `bash <path>` which
breaks Windows PowerShell scripts; update getMastraHooksJsonContent to choose
the correct invocation based on NOTIFY_SCRIPT_NAME/platform: if the selected
script is a PowerShell script (process.platform === "win32" or
NOTIFY_SCRIPT_NAME endsWith ".ps1") build the notifyCommand to invoke PowerShell
(e.g. use powershell/pwsh with -ExecutionPolicy Bypass -File and the quoted
notifyScriptPath), otherwise keep the current bash <quotedPath> (or simply the
script path for non-shell scripts); locate getMastraHooksJsonContent and the
notifyCommand construction in agent-wrappers-mastra.ts and replace the hardcoded
`bash ${quoteShellPath(notifyScriptPath)}` with this conditional logic so
Windows runs PowerShell scripts correctly.

export const NOTIFY_SCRIPT_MARKER = "# Superset agent notification hook";

const NOTIFY_SCRIPT_TEMPLATE_PATH = path.join(
__dirname,
"templates",
"notify-hook.template.sh",
process.platform === "win32"
? "notify-hook.template.ps1"
: "notify-hook.template.sh",
);

function writeFileIfChanged(
Expand Down
Loading