feat(desktop): multi-worktree development support#891
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds multi-worktree development support: allocates per-workspace port bases, persists them to workspace records, propagates derived ports and cross-app URLs into .env and terminal sessions, makes desktop app workspace-aware (dir name and single-instance lock), and wires per-app dev scripts to read dynamic ports via dotenv. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Setup as setup.sh
participant Env as .env
participant WS as Workspace Service
participant DB as Database
participant Desktop as Desktop App
participant Terminal as Terminal Session
Dev->>Setup: set SUPERSET_PORT_BASE
Setup->>Setup: compute BASE + derived ports/URLs
Setup->>Env: write SUPERSET_PORT_BASE, derived ports, URL overrides
Dev->>WS: create workspace
WS->>DB: allocatePortBase()
DB-->>WS: portBase
WS->>DB: insert workspace with portBase
Desktop->>DB: read workspace (portBase)
Desktop->>Terminal: create session (portBase)
Terminal->>Terminal: build env (include SUPERSET_PORT_BASE)
Terminal->>Terminal: launch shell with workspace env
sequenceDiagram
participant App as Desktop Init
participant GetWS as getWorkspaceName()
participant Lock as Single-Instance Lock
participant Main as Main Window
App->>GetWS: read SUPERSET_WORKSPACE_NAME
GetWS-->>App: workspaceName or undefined
alt workspaceName present
App->>App: shouldEnforceLock = false
App->>App: set app name to "Superset (workspace)"
App->>Main: create window (no single-instance enforcement)
else no workspaceName
App->>Lock: requestSingleInstanceLock()
Lock-->>App: gotTheLock
alt gotTheLock
App->>Main: create window
else
App->>App: handle second instance (focus existing)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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)
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
force-pushed
the
parallelize-desktop
branch
from
06abeb9 to
b09b42b
Compare
saddlepaddle
force-pushed
the
parallelize-desktop
branch
from
b09b42b to
127d2f9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
339-352: PR worktree path still creates workspaces withoutportBase.
createWorkspaceFromWorktree()(used by PR flows) inserts withoutportBase, so those workspaces won’t get isolated ports. Add allocation there to keep behavior consistent.🐛 Suggested fix
function createWorkspaceFromWorktree({ projectId, worktreeId, branch, name, }: CreateWorkspaceFromWorktreeParams) { const maxTabOrder = getMaxWorkspaceTabOrder(projectId); + const portBase = allocatePortBase(); const workspace = localDb .insert(workspaces) .values({ projectId, worktreeId, type: "worktree", branch, name, tabOrder: maxTabOrder + 1, + portBase, }) .returning() .get();apps/desktop/src/main/index.ts (1)
1-10: Add dotenv env loading before imports in src/main/index.ts.The
getWorkspaceName()function (line 10) directly readsprocess.env.SUPERSET_WORKSPACE_NAMEto determine workspace isolation. However, there is no env preloading in this file before any imports. You must load the monorepo root.envwithoverride: truebefore the import chain, as required by the coding guidelines. Add at the very start of the file:import { config } from "dotenv"; import { resolve } from "node:path"; config({ path: resolve(__dirname, "../../.env"), override: true });Then place all other imports after this. (Note:
electron.vite.config.tscorrectly handles env loading at line 22, so no changes needed there.)
🤖 Fix all issues with AI agents
In `@apps/desktop/docs/MULTI_WORKTREE_PLAN.md`:
- Around line 388-390: The table row for "feature-c" currently has an extra cell
and is missing the trailing pipe; remove the stray extra column so the row has
exactly the same number of pipe-delimited columns as the other rows (match
"feature-d"), add the missing trailing "|" at the end of that row, and move the
"(reused)" annotation out of the table into a following comment or separate
line; locate the row by the unique text "feature-c" and the "(reused)" token to
make the edit.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts`:
- Around line 12-15: allocatePortBase currently picks a free port base by
scanning workspaces but has no uniqueness guard, causing races on concurrent
creates; add a uniqueness guarantee by creating a unique DB index on
workspaces.port_base and update allocatePortBase (and the create path that
inserts the workspace) to retry-on-conflict: attempt allocation+insert in a
transaction or optimistic loop that catches unique-constraint violations (or
lock/select-for-update) and retries a few times before failing. Reference the
allocatePortBase function and the workspaces.port_base column/index when
implementing the unique index and retry logic.
In `@apps/desktop/src/main/index.ts`:
- Around line 203-222: The single-instance lock is being skipped when
workspaceName is set, which allows duplicate instances of the same workspace;
always request the single-instance lock instead and always register the
second-instance handler so deep links and focusMainWindow work reliably; update
the logic around shouldEnforceLock/gotTheLock to always call
app.requestSingleInstanceLock() (remove or ignore the workspaceName check) and
keep the app.on("second-instance", ...) block unchanged (using
findDeepLinkInArgv, processDeepLink, and focusMainWindow) so the lock is
enforced per workspace-scoped app name.
In `@apps/desktop/src/main/lib/agent-setup/utils.ts`:
- Around line 51-58: The filter that builds paths uses a hardcoded Unix
separator in the check !(p.startsWith(supersetPrefix) && p.includes("/bin/")),
which fails on Windows; update the logic in the paths computation (referencing
supersetPrefix, p, allPaths, and supersetBinDirs) to perform a platform-agnostic
check by normalizing p (or using path.sep or path.join to construct the bin
segment) and then checking for the bin directory (e.g., compare against
path.join(supersetPrefix, "bin") or split/join using path.sep) so workspace
".superset-*/bin" directories are excluded on all platforms.
In `@packages/local-db/src/schema/schema.ts`:
- Around line 106-111: The schema now defines a new column portBase
(integer("port_base")) on the workspaces entity but there is no DB migration to
add that column and no backfill strategy for existing rows; create a migration
that alters the workspaces table to add a nullable integer column port_base
(matching the portBase field), update the migration rollback to remove it, and
add a one-time backfill script or migration step that assigns non-conflicting
port bases for existing workspaces (e.g., compute/allocate ranges of 10 ports
per workspace or set a default and record allocation metadata) to avoid
collisions; ensure the code that reads/writes portBase (references to portBase
and integer("port_base")) can handle null until backfill completes.
🧹 Nitpick comments (7)
apps/api/package.json (1)
9-10: Consider aligning thestartscript with the env-driven port pattern.The
devscript now supports${API_PORT:-3001}, but thestartscript still uses a hardcoded--port 3001. If the intent is to support multi-worktree in production-like local runs as well, consider updatingstartsimilarly. Ifstartis only for deployed environments where ports are fixed, the current approach is fine.♻️ Optional: align start script
- "start": "next start --port 3001", + "start": "dotenv -- next start --port ${API_PORT:-3001}",apps/api/src/proxy.ts (1)
5-10: Normalize env URLs to origins to avoid trailing‑slash mismatches.
IfNEXT_PUBLIC_DESKTOP_URLincludes a trailing slash or path, the origin check will fail. Normalizing toURL.originkeeps CORS robust.♻️ Suggested refactor
-const allowedOrigins = [ - env.NEXT_PUBLIC_WEB_URL, - env.NEXT_PUBLIC_ADMIN_URL, - // Desktop dev server - use env var to support multi-worktree dev instances - env.NEXT_PUBLIC_DESKTOP_URL, -].filter(Boolean); +const allowedOrigins = [ + env.NEXT_PUBLIC_WEB_URL, + env.NEXT_PUBLIC_ADMIN_URL, + // Desktop dev server - use env var to support multi-worktree dev instances + env.NEXT_PUBLIC_DESKTOP_URL, +] + .filter((value): value is string => Boolean(value)) + .map((value) => new URL(value).origin);apps/desktop/src/shared/worktree-id.ts (2)
20-20: Extract magic numbers to named constants.Per coding guidelines, magic numbers should be extracted to named constants at module top for clarity and maintainability.
Proposed refactor
+const DEFAULT_PORT_BASE = 3000; +const MAX_WORKSPACE_NAME_LENGTH = 32; + /** * Get workspace name for instance isolation. * Returns a sanitized name suitable for use in directory names. */ export function getWorkspaceName(): string | undefined { const name = process.env.SUPERSET_WORKSPACE_NAME; if (!name) return undefined; // Sanitize for use in directory names return name .toLowerCase() .replace(/[^a-z0-9-]/g, "-") - .slice(0, 32); + .slice(0, MAX_WORKSPACE_NAME_LENGTH); } /** * Get allocated port base for this workspace. * Returns the base port from which all workspace ports are calculated. */ export function getPortBase(): number { const base = process.env.SUPERSET_PORT_BASE; if (base) { const parsed = Number(base); if (!Number.isNaN(parsed) && parsed > 0) { return parsed; } } - return 3000; // default + return DEFAULT_PORT_BASE; }Also applies to: 35-35
13-21: Consider potential name collision after truncation.When workspace names are truncated to 32 characters, different names could result in the same sanitized output (e.g., two 40-character names sharing the first 32 chars). This could cause home directory conflicts. Consider appending a short hash suffix or documenting this limitation.
apps/desktop/docs/MULTI_WORKTREE_PLAN.md (1)
21-25: Add language specifier to code block.The ASCII diagram code block should have a language specified (e.g.,
textorplaintext) for proper rendering and to satisfy markdown linting.Proposed fix
-``` +```text SUPERSET_WORKSPACE_NAME set? ├─ Yes → ~/.superset-{name}/ + allocated ports + no instance lock └─ No → ~/.superset/ + default ports + instance lock</details> </blockquote></details> <details> <summary>apps/desktop/vite/helpers.ts (1)</summary><blockquote> `7-8`: **Validate DESKTOP_VITE_PORT and extract default constant.** The `Number(...) || 5927` pattern treats `0`, `""`, and `NaN` the same, which can mask misconfiguration. A small parse guard and a named default improves clarity and debuggability. <details> <summary>♻️ Suggested refactor</summary> ```diff +const DEFAULT_DEV_SERVER_PORT = 5927; +const parsedDevServerPort = Number(process.env.DESKTOP_VITE_PORT); -export const DEV_SERVER_PORT = Number(process.env.DESKTOP_VITE_PORT) || 5927; +export const DEV_SERVER_PORT = + Number.isInteger(parsedDevServerPort) && parsedDevServerPort > 0 + ? parsedDevServerPort + : DEFAULT_DEV_SERVER_PORT;As per coding guidelines, extract hardcoded values to named constants.
apps/desktop/src/shared/constants.ts (1)
11-18: Extract default port numbers into named constants.Line 11-18: This file is already a constants module, so pulling the numeric defaults into named constants will improve readability and align with the no-magic-numbers guideline. As per coding guidelines, extract magic numbers to named constants at module top.
♻️ Suggested refactor
+const DEFAULT_VITE_PORT = 5927; +const DEFAULT_NOTIFICATIONS_PORT = 31416; +const ELECTRIC_PORT = 31418; + export const PORTS = { - VITE_DEV_SERVER: Number(env.DESKTOP_VITE_PORT) || 5927, - NOTIFICATIONS: Number(env.DESKTOP_NOTIFICATIONS_PORT) || 31416, - // Electric SQL proxy port (local-first sync) - not yet workspace-isolated - ELECTRIC: 31418, + VITE_DEV_SERVER: Number(env.DESKTOP_VITE_PORT) || DEFAULT_VITE_PORT, + NOTIFICATIONS: Number(env.DESKTOP_NOTIFICATIONS_PORT) || DEFAULT_NOTIFICATIONS_PORT, + // Electric SQL proxy port (local-first sync) - not yet workspace-isolated + ELECTRIC: ELECTRIC_PORT, };
| | (delete a) | - | - | - | - | - | - | - | - | | ||
| | feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 | (reused) | ||
| | feature-d | 3020 | 3020 | 3021 | 3022 | 3023 | 3024 | 3025 | 3026 | |
There was a problem hiding this comment.
Fix table formatting on the "reused" row.
The table row on line 389 has an extra column and missing trailing pipe, which causes rendering issues. The "(reused)" note should be in a comment or separate line.
Proposed fix
| feature-a | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 |
| feature-b | 3010 | 3010 | 3011 | 3012 | 3013 | 3014 | 3015 | 3016 |
| (delete a) | - | - | - | - | - | - | - | - |
-| feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 | (reused)
+| feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 |
| feature-d | 3020 | 3020 | 3021 | 3022 | 3023 | 3024 | 3025 | 3026 |
+
+> Note: feature-c reuses the port base freed by deleting feature-a.📝 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.
| | (delete a) | - | - | - | - | - | - | - | - | | |
| | feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 | (reused) | |
| | feature-d | 3020 | 3020 | 3021 | 3022 | 3023 | 3024 | 3025 | 3026 | | |
| | (delete a) | - | - | - | - | - | - | - | - | | |
| | feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 | | |
| | feature-d | 3020 | 3020 | 3021 | 3022 | 3023 | 3024 | 3025 | 3026 | | |
| > Note: feature-c reuses the port base freed by deleting feature-a. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
389-389: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
389-389: Table column count
Expected: 9; Actual: 10; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
In `@apps/desktop/docs/MULTI_WORKTREE_PLAN.md` around lines 388 - 390, The table
row for "feature-c" currently has an extra cell and is missing the trailing
pipe; remove the stray extra column so the row has exactly the same number of
pipe-delimited columns as the other rows (match "feature-d"), add the missing
trailing "|" at the end of that row, and move the "(reused)" annotation out of
the table into a following comment or separate line; locate the row by the
unique text "feature-c" and the "(reused)" token to make the edit.
| // Port allocation constants for multi-worktree dev instances | ||
| const START_PORT = 3000; | ||
| const PORT_RANGE = 10; | ||
|
|
There was a problem hiding this comment.
Potential duplicate portBase under concurrent creates.
allocatePortBase() scans a snapshot without a uniqueness guard, so parallel creates could select the same base and collide. Consider adding a unique index on workspaces.port_base with a retry-on-conflict loop, or wrap allocation + insert in a transaction/lock.
Also applies to: 322-338
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts` around
lines 12 - 15, allocatePortBase currently picks a free port base by scanning
workspaces but has no uniqueness guard, causing races on concurrent creates; add
a uniqueness guarantee by creating a unique DB index on workspaces.port_base and
update allocatePortBase (and the create path that inserts the workspace) to
retry-on-conflict: attempt allocation+insert in a transaction or optimistic loop
that catches unique-constraint violations (or lock/select-for-update) and
retries a few times before failing. Reference the allocatePortBase function and
the workspaces.port_base column/index when implementing the unique index and
retry logic.
| const supersetPrefix = path.join(homedir, ".superset-"); | ||
| const paths = allPaths.filter( | ||
| (p) => p && !supersetBinDirs.some((dir) => p.startsWith(dir)), | ||
| (p) => | ||
| p && | ||
| !supersetBinDirs.some((dir) => p.startsWith(dir)) && | ||
| // Also filter any .superset-*/bin directories (workspace instances) | ||
| !(p.startsWith(supersetPrefix) && p.includes("/bin/")), | ||
| ); |
There was a problem hiding this comment.
Potential cross-platform issue with hardcoded /bin/ path separator.
The check p.includes("/bin/") uses a Unix-style path separator, which won't match on Windows where path.join produces backslashes. Since the function already handles Windows (lines 37-40), this filter may silently fail to exclude workspace bin directories on Windows.
🔧 Suggested fix for cross-platform compatibility
const supersetPrefix = path.join(homedir, ".superset-");
+ const binSegment = `${path.sep}bin${path.sep}`;
const paths = allPaths.filter(
(p) =>
p &&
!supersetBinDirs.some((dir) => p.startsWith(dir)) &&
// Also filter any .superset-*/bin directories (workspace instances)
- !(p.startsWith(supersetPrefix) && p.includes("/bin/")),
+ !(p.startsWith(supersetPrefix) && p.includes(binSegment)),
);📝 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.
| const supersetPrefix = path.join(homedir, ".superset-"); | |
| const paths = allPaths.filter( | |
| (p) => p && !supersetBinDirs.some((dir) => p.startsWith(dir)), | |
| (p) => | |
| p && | |
| !supersetBinDirs.some((dir) => p.startsWith(dir)) && | |
| // Also filter any .superset-*/bin directories (workspace instances) | |
| !(p.startsWith(supersetPrefix) && p.includes("/bin/")), | |
| ); | |
| const supersetPrefix = path.join(homedir, ".superset-"); | |
| const binSegment = `${path.sep}bin${path.sep}`; | |
| const paths = allPaths.filter( | |
| (p) => | |
| p && | |
| !supersetBinDirs.some((dir) => p.startsWith(dir)) && | |
| // Also filter any .superset-*/bin directories (workspace instances) | |
| !(p.startsWith(supersetPrefix) && p.includes(binSegment)), | |
| ); |
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/agent-setup/utils.ts` around lines 51 - 58, The
filter that builds paths uses a hardcoded Unix separator in the check
!(p.startsWith(supersetPrefix) && p.includes("/bin/")), which fails on Windows;
update the logic in the paths computation (referencing supersetPrefix, p,
allPaths, and supersetBinDirs) to perform a platform-agnostic check by
normalizing p (or using path.sep or path.join to construct the bin segment) and
then checking for the bin directory (e.g., compare against
path.join(supersetPrefix, "bin") or split/join using path.sep) so workspace
".superset-*/bin" directories are excluded on all platforms.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
45-69: MissingportBaseallocation increateWorkspaceFromWorktreehelper.This helper function is used by
handleNewWorktree(line 215) when creating workspaces from PRs, but it doesn't allocate aportBase. This means PR-created workspaces won't have port isolation, unlike workspaces created throughcreate,createBranchWorkspace, oropenWorktree.Proposed fix
function createWorkspaceFromWorktree({ projectId, worktreeId, branch, name, }: CreateWorkspaceFromWorktreeParams) { const maxTabOrder = getMaxWorkspaceTabOrder(projectId); + const portBase = allocatePortBase(); const workspace = localDb .insert(workspaces) .values({ projectId, worktreeId, type: "worktree", branch, name, tabOrder: maxTabOrder + 1, + portBase, }) .returning() .get(); setLastActiveWorkspace(workspace.id); return workspace; }apps/desktop/src/main/index.ts (1)
1-11: Load monorepo.envbefore other imports in the main process entry point.The
electron.vite.config.tsbuild config correctly preloads the.envfile, butapps/desktop/src/main/index.tslacks runtime env loading. This means when the app runs, env variables likeSUPERSET_WORKSPACE_NAMEand configured ports won't be available during module initialization in development. The entry point must preload the.envfile before any other imports.🔧 Suggested fix (index.ts)
+import "./env"; import path from "node:path"; import { settings } from "@superset/local-db";New file:
apps/desktop/src/main/env.tsimport path from "node:path"; import { config } from "dotenv"; config({ path: path.resolve(process.cwd(), ".env"), override: true, });
🤖 Fix all issues with AI agents
In `@apps/api/package.json`:
- Line 9: The dev script uses Unix-only expansion (${API_PORT:-3001}) which
fails on Windows; fix by moving the default into your .env and stop using shell
expansion: add API_PORT=3001 to your .env and change the "dev" script (the "dev"
entry in package.json that currently invokes dotenv and next dev with
${API_PORT:-3001}) to simply run dotenv -- next dev (or handle the port in
Next.js config), so dotenv loads API_PORT cross-platform instead of relying on
shell expansion.
In `@apps/desktop/docs/MULTI_WORKTREE_PLAN.md`:
- Around line 21-25: The markdown code fences for the diagram starting with
"SUPERSET_WORKSPACE_NAME set?" and the .env sample beginning with
"SUPERSET_PORT_BASE=3000" lack language identifiers, triggering MD040; update
those fenced blocks to add appropriate languages (e.g., ```text for the ASCII
diagram and ```env for the environment variable block) wherever they appear
(including the other occurrence around lines 400–407) so markdownlint no longer
flags them.
- Around line 328-370: The summary tables in the "Files Summary" section violate
markdownlint MD058 by not having blank lines before and after each table; edit
MULTI_WORKTREE_PLAN.md (the Files Summary / Desktop App / Setup Script / API
CORS / Desktop Vite Config / App Dev Scripts tables) and insert a single blank
line immediately before and immediately after each table so every table is
separated from surrounding paragraph text.
In `@apps/desktop/src/shared/constants.ts`:
- Around line 37-42: Update the comment above SUPERSET_DIR_NAMES to clarify
intent: state that SUPERSET_DIR_NAMES lists known production and legacy
development static directories (e.g., ".superset" and ".superset-dev") rather
than all possible Superset directories, and note that workspace-specific
variants (like ".superset-<suffix>") are handled separately by the
prefix-matching logic in agent-setup's utilities; keep the constant values
unchanged and only change the comment text to reflect this distinction.
♻️ Duplicate comments (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (1)
322-338: Race condition concern already addressed in prior review.The potential for duplicate
portBaseallocation under concurrent creates was flagged in a previous review. The recommendation to add a unique index onworkspaces.port_basewith retry-on-conflict remains valid.apps/desktop/src/main/index.ts (1)
203-222: Keep the single-instance lock per workspace.Skipping the lock for named workspaces allows multiple instances of the same workspace to run concurrently, which can collide on ports and the workspace-specific home directory. Since the app name is already workspace-scoped, you can enforce the lock and still run different worktrees simultaneously.
🔧 Suggested fix
-const shouldEnforceLock = !workspaceName; -const gotTheLock = shouldEnforceLock ? app.requestSingleInstanceLock() : true; +const gotTheLock = app.requestSingleInstanceLock(); if (!gotTheLock) { app.exit(0); } else { - // Only relevant when instance lock is enforced - if (shouldEnforceLock) { - app.on("second-instance", async (_event, argv) => { - focusMainWindow(); - const url = findDeepLinkInArgv(argv); - if (url) { - await processDeepLink(url); - } - }); - } + app.on("second-instance", async (_event, argv) => { + focusMainWindow(); + const url = findDeepLinkInArgv(argv); + if (url) { + await processDeepLink(url); + } + }); }apps/desktop/docs/MULTI_WORKTREE_PLAN.md (1)
388-390: Fix the “feature-c” row column count.The row has an extra cell and missing trailing pipe, which breaks rendering (MD055/MD056).
🔧 Suggested fix
-| feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 | (reused) +| feature-c | 3000 | 3000 | 3001 | 3002 | 3003 | 3004 | 3005 | 3006 |Consider moving the “(reused)” note to a line below the table.
| "build": "next build", | ||
| "clean": "git clean -xdf .cache .next .turbo node_modules", | ||
| "dev": "next dev --port 3001", | ||
| "dev": "dotenv -- next dev --port ${API_PORT:-3001}", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the package.json to see the exact dotenv version
cat -n apps/api/package.json | grep -A 2 -B 2 "dotenv"Repository: superset-sh/superset
Length of output: 473
🌐 Web query:
dotenv-cli Windows cmd.exe PowerShell variable expansion ${VAR:-default} support
💡 Result:
-
Inside your
.envfile: Yes.dotenv-cli(the Node.js packagedotenv-cli) uses dotenv-expand for variable expansion, and dotenv-expand supports bash-style default syntax like${KEY:-default}(and${KEY}/$KEY). [1], [2]Example:
DB_HOST=${DB_HOST:-localhost}
-
In Windows
cmd.exe/ PowerShell command text: No.${VAR:-default}is not Windows shell syntax. Also,dotenv-clican’t “fix” shell expansion in the command you type because the shell expands/rewrites the command beforedotenv-cliruns it. [1]cmd.exeuses%VAR%- PowerShell uses
$env:VAR(or${env:VAR})
If you need expansion/defaulting while running a command, do it in the .env file (supported), or run a subshell/script where you control the expansion behavior. [1]
Sources: dotenv-cli README (variable expansion + command expansion caveat) [1]; dotenv-expand expansion rules (includes ${KEY:-default}) [2].
🌐 Web query:
dotenv-cli documentation cross-platform environment variable syntax
💡 Result:
In dotenv-cli (entropitor/dotenv-cli), the cross-platform way to set env vars inline (works on Windows, macOS, Linux) is to use the -v flag, because the usual POSIX syntax VAR=value <command> doesn’t work on Windows. (github.com)
# cross-platform (recommended)
dotenv -v VARIABLE=somevalue -- <command>
# multiple vars
dotenv -v VARIABLE1=somevalue1 -v VARIABLE2=somevalue2 -- <command>Use -- to separate dotenv flags from the underlying command’s args. (github.com)
Citations:
- 1: https://github.com/entropitor/dotenv-cli?utm_source=openai
- 2: https://github.com/entropitor/dotenv-cli?utm_source=openai
Cross-platform shell variable expansion may fail on Windows.
The ${API_PORT:-3001} syntax relies on Unix shell expansion and will not work in Windows cmd.exe or PowerShell. The shell expands variables before dotenv executes, so the syntax never reaches dotenv-cli.
Recommended fix: Define API_PORT in the .env file with a default value. The dotenv-cli and dotenv-expand libraries will properly expand it across all platforms:
.env configuration approach
In .env:
API_PORT=3001
Then the script becomes:
-"dev": "dotenv -- next dev --port ${API_PORT:-3001}",
+"dev": "dotenv -- next dev --port $API_PORT",Or handle the port entirely in Next.js configuration and omit the flag from the dev script.
🤖 Prompt for AI Agents
In `@apps/api/package.json` at line 9, The dev script uses Unix-only expansion
(${API_PORT:-3001}) which fails on Windows; fix by moving the default into your
.env and stop using shell expansion: add API_PORT=3001 to your .env and change
the "dev" script (the "dev" entry in package.json that currently invokes dotenv
and next dev with ${API_PORT:-3001}) to simply run dotenv -- next dev (or handle
the port in Next.js config), so dotenv loads API_PORT cross-platform instead of
relying on shell expansion.
127d2f9 to
f5b1e2d
Compare
saddlepaddle
force-pushed
the
parallelize-desktop
branch
from
f5b1e2d to
6986923
Compare
saddlepaddle
force-pushed
the
parallelize-desktop
branch
from
8e4d2d4 to
49873d4
Compare
saddlepaddle
force-pushed
the
parallelize-desktop
branch
from
49873d4 to
12d0cb7
Compare
saddlepaddle
force-pushed
the
parallelize-desktop
branch
2 times, most recently
from
2c6f8bd to
793785c
Compare
- Replace hardcoded .superset-dev directory references with dynamic
workspace-based isolation (.superset-{workspace})
- Use STREAMS_URL env var instead of hardcoded localhost:8080 fallback
in ChatInterface and renderer define block
- Update agent-wrappers shell case to glob-match .superset-*/bin
- Update test files to use .superset-test with SUPERSET_WORKSPACE_NAME
- Update EXTERNAL_FILES.md to reflect workspace-based isolation
Add -e ../../.env to dotenv in all Next.js app dev scripts so workspace-allocated ports from the root .env are picked up.
Named workspaces use "superset-{name}" protocol (e.g. superset-testo)
so auth callbacks route to the correct Electron instance.
…hemes
Each dev instance now uses superset-{workspace}:// instead of the shared
superset-dev:// scheme, preventing deep link collisions between worktrees.
Threads desktop_protocol param through the full OAuth flow so the web
callback redirects to the correct Electron instance. Setup script now
writes SUPERSET_WORKSPACE_NAME and missing streams/electric URL overrides.
- Remove unnecessary comments and dead code paths - Simplify auth deep link validation to single protocol check - Move getWorkspaceName() into env.shared.ts, keep worktree-id.ts for predev script only - Make SUPERSET_WORKSPACE_NAME required in Zod schema - Add SUPERSET_WORKSPACE_NAME to CI build workflow - Add defaults for port env vars so CI builds pass - Remove hardcoded superset-dev fallback from web auth callback - Remove redundant STREAMS_URL fallback in ChatInterface
Zod validation fails in CI tests because SUPERSET_WORKSPACE_NAME is not set. Adding a default of "superset" (production value) so tests pass without requiring the env var.
1 participant
Summary
apps/desktop/docs/MULTI_WORKTREE_PLAN.mdKey Changes
Port Allocation
portBasecolumn to workspace schema for port allocationEnvironment Variables
SUPERSET_WORKSPACE_NAME- Workspace identifier (sanitized)SUPERSET_PORT_BASE- Base port for the workspace (e.g., 3000, 3010, 3020)Instance Isolation
~/.superset-{workspace}App Dev Scripts
dotenv-clifor reading .env in npm scriptsTest Plan
Note
The outer dev instance needs manual env var setup (or just use default ports). The inner instances will have ports auto-configured through workspace creation.
Summary by CodeRabbit
New Features
Developer Experience
Database
Documentation