fix(server): prefer stable Zed paths when opening in Zed#2066

closed1
main
GitHub fork: fix/zed-stable-open
openedApr 16, 2026bySsrothgan
View on GitHubTwo-way sync enabledPR last imported 21d ago

What Changed

  • resolve Zed launch commands to executable paths before choosing a command
  • prefer clearly stable Zed paths over preview/nightly paths when both are present
  • only override launch PATH when needed to force the selected Zed binary
  • add focused test coverage for Windows plus path-based macOS/Linux cases

Why

Closes: #1978

On Windows, Open -> Zed can launch Zed Nightly instead of stable Zed when command-name ordering on PATH resolves the wrong install first.

This keeps the change scoped to apps/server editor launch resolution and avoids hardcoding a command alias as always meaning stable Zed.

UI Changes

None.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Note

Low Risk Scoped to Zed editor launch and PATH enumeration helpers; other editors unchanged aside from optional env on spawn when Zed overrides PATH.

Overview Fixes Open → Zed picking Zed Nightly when stable Zed is also on PATH (e.g. on Windows).

Shared shell: Adds resolveCommandPaths to return every executable match for a command in PATH order; resolveCommandPath keeps returning the first match (or not-found). Dedupes paths case-insensitively on Windows.

External launcher (Zed only): resolvePreferredZedLaunch walks resolved Zed binaries and chooses the first path that does not look like nightly/preview (path contains those substrings). If that binary is not already first in resolution order, it prepends its directory to PATH/Path on a per-spawn env so spawn resolves the right binary. Other editors still use the first available command name.

Spawn: Editor launches can carry optional env; detached editor spawns pass that env through to resolveSpawnCommand and the child process.

Tests cover Windows multi-install Zed preference and resolveCommandPaths ordering.

Reviewed by Cursor Bugbot for commit 66cad925a73365e855bf333e03d1717154f0c98d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Prefer stable Zed installation over nightly/preview when opening files in Zed

  • Adds resolvePreferredZedLaunch in externalLauncher.ts to find all Zed executables on PATH and select the first non-prerelease one (paths not containing 'nightly' or 'preview').
  • When the stable binary is not first on PATH, prepends its directory to PATH in the spawned process environment so the correct binary is resolved at launch time.
  • Extends resolveCommandPaths in shell.ts to return all matching executables for a command in PATH order, rather than just the first match.
  • Behavioral Change: launchEditorProcess for zed now spawns with a potentially modified PATH; all other editors are unaffected.
📊 Macroscope summarized 66cad92. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

Synced from GitHub

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6aa35acd-b3f8-4a17-910d-1c95107ecc81

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

apps/server/src/open.test.tsline 283Outdated · e12f8c4Open thread
……@@ -229,6 +229,147 @@ it.layer(NodeServices.layer)("resolveEditorLaunch", (it) => {
280+ assert.equal(result.command, "zed");
281+ assert.deepEqual(result.args, ["/tmp/workspace"]);
282+ assert.equal(typeof result.env?.PATH, "string");
283+ assert.ok(result.env?.PATH?.split(":")[0]?.includes("Zed.app\\Contents\\MacOS"));

Test assertion uses wrong path separator for darwin

Medium Severity

The assertion checks for "Zed.app\\Contents\\MacOS" (literal backslashes), but on macOS/Linux where this test runs, dirname() from node:path produces paths with forward slashes. The actual PATH entry will contain Zed.app/Contents/MacOS, so includes("Zed.app\Contents\MacOS") returns false, causing the test to fail on Unix CI.

Reviewed by Cursor Bugbot for commit e12f8c41402e68234cd6405b56f5712a1e137da3. Configure here.

Synced from GitHub

Approvability

Verdict: Needs human review

1 blocking correctness issue found. Two high-severity unresolved review comments identify that the new env parameter passed to spawn options replaces rather than extends the parent environment, potentially breaking editor launches by stripping critical environment variables like HOME, USER, and DISPLAY.

You can customize Macroscope's approvability policy. Learn more.

apps/server/src/open.tsline 282Outdated · 819a8fbOpen thread
……@@ -101,6 +174,112 @@ function fileManagerCommandForPlatform(platform: NodeJS.Platform): string {
279+ }
280+ }
281+ return resolvedPaths;
282+}

Six helper functions duplicated from shared shell module

Medium Severity

The new resolveCommandPaths function and its six supporting helpers (stripWrappingQuotes, resolvePathEnvironmentVariable, resolveWindowsPathExtensions, resolveCommandCandidates, isExecutableFile, resolvePathDelimiter) are near-verbatim copies of private functions already in packages/shared/src/shell.ts. The only functional difference is that resolveCommandPaths collects all matching paths instead of returning a boolean like isCommandAvailable. These ~80 lines of duplicated logic could diverge silently if bugs are fixed in only one copy. Exporting the shared helpers or adding resolveCommandPaths to the shared module would eliminate this risk.

Additional Locations (2)

Reviewed by Cursor Bugbot for commit 819a8fb39bebb9170bf4ad9f626f5bc14b428ce5. Configure here.

Added 1 commit

  • fix(server): prefer stable Zed launch paths over preview builds66cad92
Ccursor[bot]reviewed · 1 inline threadJun 16, 2026
Synced from GitHub

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 66cad925a73365e855bf333e03d1717154f0c98d. Configure here.

……@@ -387,6 +435,7 @@ const launchEditorProcess = Effect.fn("externalLauncher.launchEditorProcess")(fu
387435 args: spawnCommand.args,
388436 options: {
389437 detached: true,
438+ env,

Editor spawn gets partial environment

High Severity

launchEditorProcess now always sets options.env on the detached spawn to launch.env ?? readCommandLookupEnv. That lookup env only carries PATH/Path/path and PATHEXT, not the full process environment. With Effect’s default (no extendEnv), the child inherits just those keys instead of process.env, which can break every external editor launch and especially Zed when a modified PATH is applied.

Reviewed by Cursor Bugbot for commit 66cad925a73365e855bf333e03d1717154f0c98d. Configure here.

……@@ -387,6 +435,7 @@ const launchEditorProcess = Effect.fn("externalLauncher.launchEditorProcess")(fu
387435 args: spawnCommand.args,
388436 options: {
389437 detached: true,
438+ env,
MmacroscopeappBoton lines 437–438Jun 16, 2026

🟠 High process/externalLauncher.ts:437

Line 438 passes env to the spawn options, which replaces (not merges with) the parent environment. This strips all variables except PATH/Path/path/PATHEXT, breaking editors that need HOME, USER, DISPLAY, XDG_*, etc. Before this change, the child inherited the full parent environment. Consider merging env with process.env so PATH adjustments don't discard other critical variables.

diff
1- env,
2+ env: { ...process.env, ...env },
🤖 Copy this AI Prompt to have your agent fix this:
text
1In file @apps/server/src/process/externalLauncher.ts around lines 437-438:
2
3Line 438 passes `env` to the spawn options, which replaces (not merges with) the parent environment. This strips all variables except `PATH`/`Path`/`path`/`PATHEXT`, breaking editors that need `HOME`, `USER`, `DISPLAY`, `XDG_*`, etc. Before this change, the child inherited the full parent environment. Consider merging `env` with `process.env` so PATH adjustments don't discard other critical variables.
4
5Evidence trail:
61. PR diff: `git_diff base=MERGE_BASE head=REVIEWED_COMMIT path=apps/server/src/process/externalLauncher.ts` shows `env` added to spawn options at line 438 (new in this PR).
72. `readCommandLookupEnv` definition at lines 91-99 of externalLauncher.ts only extracts PATH/Path/path/PATHEXT.
83. Prior to this PR (commit d15909af), `launchEditorProcess` (then named the same) did not pass `env` to spawn options - child inherited full parent env.
94. `resolvePreferredZedLaunch` (new in this PR, lines ~158-199) builds env from the same limited readCommandLookupEnv base.
105. Node.js spawn behavior: when `env` option is specified, child only gets those variables (https://nodejs.org/api/child_process.html#child_processspawncommand-args-options).

Closing as part of backlog cleanup: this PR is stale, currently conflicting with main, has no approval, and has not had active remediation for 30+ days. If the change is still relevant, please rebase onto current main and request reopening or submit a focused current diff. Thank you for the contribution.