Skip to content

fix(desktop): use full PATH for open in app in production builds #185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
Nov 29, 2025
Merged

fix(desktop): use full PATH for open in app in production builds #185

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 27 additions & 20 deletions apps/desktop/src/lib/trpc/routers/external/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { publicProcedure, router } from "../..";
const ExternalAppSchema = z.enum(EXTERNAL_APPS);

/**
* Get the command and args to open a path in the specified app
* Get the command and args to open a path in the specified app.
* Uses `open -a` for macOS apps to avoid PATH issues in production builds.
Comment on lines +12 to +13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Nov 29, 2025

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
# First, check the structure of the external router file and look for platform detection
head -50 apps/desktop/src/lib/trpc/routers/external/index.ts

Repository: superset-sh/superset

Length of output: 1687

🏁 Script executed:

#!/bin/bash
# Search for platform-specific checks in the trpc routers directory
rg -n "process\.platform|darwin|win32|linux" apps/desktop/src/lib/trpc/ -A 2 -B 2

Repository: superset-sh/superset

Length of output: 383

🏁 Script executed:

#!/bin/bash
# Check if there are other platform-specific router files or variants
fd -t f "router" apps/desktop/src/lib/trpc/ | head -20

Repository: superset-sh/superset

Length of output: 46

🏁 Script executed:

#!/bin/bash
# Look for how the external router is used/exported
rg -n "external" apps/desktop/src/lib/trpc/ -A 2 -B 2 | head -50

Repository: superset-sh/superset

Length of output: 1913

🏁 Script executed:

#!/bin/bash
# Check electron-builder config to understand target platforms
cat electron-builder.json 2>/dev/null || cat electron-builder.js 2>/dev/null || find . -maxdepth 2 -name "*electron-builder*" -type f

Repository: superset-sh/superset

Length of output: 46

🏁 Script executed:

#!/bin/bash
# Get more context from the external router, especially the createExternalRouter function
sed -n '70,200p' apps/desktop/src/lib/trpc/routers/external/index.ts

Repository: superset-sh/superset

Length of output: 2742

🏁 Script executed:

#!/bin/bash
# Search for calls to openFileInEditor to see if platform detection happens at call site
rg -n "openFileInEditor|\.external\." apps/desktop/src/ -A 2 -B 2

Repository: superset-sh/superset

Length of output: 5474

🏁 Script executed:

#!/antml/bash
# Check how the routers are conditionally mounted or used in main process
rg -n "createExternalRouter|process\.platform" apps/desktop/src/lib/trpc/ -A 3 -B 3

Repository: superset-sh/superset

Length of output: 2537

🏁 Script executed:

#!/bin/bash
# Look for any main process entry points to understand platform handling
find apps/desktop/src -name "*main*" -o -name "*index.ts" | grep -E "(main|index)" | head -10

Repository: superset-sh/superset

Length of output: 521

🏁 Script executed:

#!/bin/bash
# Check the electron-builder config more broadly
find . -maxdepth 3 -type f \( -name "*.json" -o -name "*.js" \) | xargs grep -l "electron-builder\|build.*electron" 2>/dev/null | head -5

Repository: superset-sh/superset

Length of output: 120

🏁 Script executed:

#!/bin/bash
# Check if there's any platform detection in the main process setup or app initialization
cat apps/desktop/src/main/index.ts | head -100

Repository: superset-sh/superset

Length of output: 1479

🏁 Script executed:

#!/bin/bash
# Look for any build-time or runtime configuration that might disable certain features on non-macOS
rg -n "darwin|platform.*darwin" apps/desktop/src/ -B 2 -A 2

Repository: superset-sh/superset

Length of output: 1333

🏁 Script executed:

#!/bin/bash
# Check package.json to understand target platforms for the electron build
cat apps/desktop/package.json | grep -A 20 '"build"'

Repository: superset-sh/superset

Length of output: 840

Add platform detection to openInApp and getAppCommand procedures to prevent runtime failures on Windows/Linux builds.

The concern is valid: open -a commands (for VSCode, Cursor, Xcode, iTerm, Warp, Terminal) are macOS-only and will fail on Windows/Linux. The external router is unconditionally mounted in the main router with no platform guards, and callers (e.g., WorkspaceView, Terminal helpers) invoke openInApp without checking the platform despite getPlatform being available in the renderer.

The codebase has platform detection utilities available (e.g., process.platform === "darwin" checks in menu.ts, PLATFORM constants in shared/constants.ts, and renderer-side platform queries), but they are not applied to the external router procedures.

Recommendation: Add platform detection in getAppCommand to return appropriate fallbacks (e.g., null or error) on non-macOS platforms, or add guards in the openInApp procedure to handle platform-specific behavior.

🤖 Prompt for AI Agents 
In apps/desktop/src/lib/trpc/routers/external/index.ts around lines 12-13, the
getAppCommand/openInApp logic uses macOS-only "open -a" commands unguarded,
which will fail on Windows/Linux; update getAppCommand to detect platform (use
PLATFORM constant or process.platform === "darwin") and return null or a clear
error for non-macOS, and update openInApp to check the returned command and
either throw a descriptive RPC error or perform a platform-specific fallback
(e.g., use shell commands for Windows/Linux) so callers don't attempt to execute
macOS-only commands on other platforms.

*/
const getAppCommand = (
app: ExternalApp,
Expand All @@ -19,9 +20,12 @@ const getAppCommand = (
case "finder":
return null; // Handled specially with shell.showItemInFolder
case "vscode":
return { command: "code", args: [targetPath] };
return {
command: "open",
args: ["-a", "Visual Studio Code", targetPath],
};
case "cursor":
return { command: "cursor", args: [targetPath] };
return { command: "open", args: ["-a", "Cursor", targetPath] };
case "xcode":
return { command: "open", args: ["-a", "Xcode", targetPath] };
case "iterm":
Expand All @@ -47,14 +51,22 @@ const spawnAsync = (command: string, args: string[]): Promise<void> => {
});

child.on("error", (error) => {
reject(error);
reject(
new Error(
`Failed to spawn '${command}': ${error.message}. Ensure the application is installed.`,
),
);
});

child.on("exit", (code) => {
if (code === 0) {
resolve();
} else {
reject(new Error(`Process exited with code ${code}`));
reject(
new Error(
`'${command}' exited with code ${code}. The application may not be installed.`,
),
);
}
});
});
Expand Down Expand Up @@ -116,7 +128,6 @@ export const createExternalRouter = () => {
}),
)
.mutation(async ({ input }) => {
console.log("[external] openFileInEditor called with:", input);
let filePath = input.path;

// Expand home directory - needed because editors expect absolute paths
Expand All @@ -134,11 +145,7 @@ export const createExternalRouter = () => {
: path.resolve(filePath);
}

console.log("[external] Resolved file path:", filePath);

const editors = ["cursor", "code"];

// Build the file location string (file:line:column format expected by --goto flag)
// Build the file location string (file:line:column format for URL schemes)
let location = filePath;
if (input.line) {
location += `:${input.line}`;
Expand All @@ -147,21 +154,21 @@ export const createExternalRouter = () => {
}
}

console.log("[external] Opening location:", location);
// Try editor URL schemes - these work reliably without PATH issues
// Format: cursor://file/path:line:column or vscode://file/path:line:column
const editorSchemes = ["cursor", "vscode"];

for (const editor of editors) {
for (const scheme of editorSchemes) {
try {
console.log(`[external] Trying editor: ${editor}`);
await spawnAsync(editor, ["--goto", location]);
console.log(`[external] Successfully opened with ${editor}`);
const url = `${scheme}://file${location}`;
await shell.openExternal(url);
return;
} catch (error) {
console.log(`[external] ${editor} failed:`, error);
} catch {
// Editor not installed or URL scheme not registered, try next
}
}

console.log("[external] Falling back to system default");

// Fall back to system default
await shell.openPath(filePath);
}),
});
Expand Down