fix(editor): detect Zed.app on macOS when CLI is not in PATH

[gy1016]

Summary

Fixes #3287

On macOS, Zed editor is typically installed via Homebrew or direct download, but the CLI command zed is not automatically added to PATH. This causes the /editor command to show "Zed (Not installed)" even when Zed is already installed.

Changes

• Added detection for Zed.app bundle at /Applications/Zed.app and ~/Applications/Zed.app
• Falls back to using the CLI inside the app bundle when CLI is not in PATH
• Added comprehensive tests for the new detection logic

How to verify

1. On macOS, install Zed via Homebrew: brew install --cask zed
2. Run qwen /editor
3. Zed should now be detected and selectable

[wenshao]

[Critical] useLaunchEditor is unaware of the Zed app bundle fallback

packages/cli/src/ui/hooks/useLaunchEditor.ts uses editorCommands + commandExists directly to resolve the editor executable (line 33-48). It has no knowledge of the new Zed macOS app bundle fallback added in this PR.

When a macOS user has Zed installed only as Zed.app (no CLI in PATH):

1. checkHasEditorType('zed') returns true (via zedAppExists) → Zed appears available in settings
2. User selects Zed as preferred editor
3. useLaunchEditor → getExecutableCommand → throws: "No available editor command found for zed. Tried: zed, zeditor."

The diff path (getDiffCommand) was updated, but the file-opening path was missed.

Suggested fix: Export a getEditorExecutable(editorType) function from editor.ts that encapsulates both CLI and app bundle resolution, and use it in both getDiffCommand and useLaunchEditor.

— glm-5.1 via Qwen Code /review

[wenshao]

[Suggestion] Detection predicate mismatch: zedAppExists() checks for .app directory, getZedCommand() checks for the binary inside

checkHasEditorType('zed') uses zedAppExists() which checks if /Applications/Zed.app directory exists (line ~66). But getDiffCommand uses getZedCommand() which checks for the actual CLI binary at Contents/MacOS/zed inside the bundle.

If the .app directory exists but Contents/MacOS/zed does not (corrupted install, different bundle structure), checkHasEditorType returns true while getDiffCommand returns null — Zed appears installed but diff silently fails.

This also causes redundant execSync calls: checkHasEditorType runs commandExists('zed') + commandExists('zeditor'), then getDiffCommand → getZedCommand() runs the same checks again.

Suggested change

	return cmd;
	// Use the same resolution logic as getDiffCommand to ensure consistency
	if (editor === 'zed') {
	return getZedCommand() !== null;
	}

— glm-5.1 via Qwen Code /review

[tanzhenxin]

Review

Good approach — detecting Zed.app in /Applications and ~/Applications when the CLI isn't in PATH addresses a real pain point for macOS users.

Issues

1. Wrong binary path inside the app bundle

   The PR uses Contents/MacOS/zed — this is the main GUI binary, which does not support --wait (verified locally: --wait produces error: unexpected argument '--wait' found). The correct path is Contents/MacOS/cli, which is Zed's actual CLI tool and supports both --wait and --diff. This means the feature breaks for exactly the users it's designed to help.

   Suggestion: change Contents/MacOS/zed to Contents/MacOS/cli in both getZedCommand() and getEditorExecutable().

2. Tests don't verify the exact binary name

   Tests assert the path contains Zed.app but never check whether it ends with /cli vs /zed, which is why the wrong path wasn't caught. Tightening the assertions would prevent this.

Verdict

REQUEST_CHANGES — the wrong binary path breaks the core functionality. Easy fix.

[gy1016]

@wenshao Thanks for the review! This has been addressed in commit e88d1b7. I exported \getEditorExecutable(editorType)\ from \editor.ts\ and updated \useLaunchEditor.ts\ to use it. Now both \getDiffCommand\ and \useLaunchEditor\ share the same logic for detecting Zed CLI and macOS app bundle fallback.

[gy1016]

@tanzhenxin Great catch! Fixed in commit c865a71. Changed \Contents/MacOS/zed\ to \Contents/MacOS/cli\ in both \getZedCommand()\ and \getEditorExecutable(). Also updated tests to verify the correct CLI path using regex matching.

[tanzhenxin]

Review (re-review)

The critical issue from the first review is fixed — binary path is now correctly Contents/MacOS/cli. Nice work on that. However the refactoring introduced two new issues.

Issues

1. getEditorExecutable silently swallows "editor not found"

getEditorExecutable always returns a string — when no command is found, it falls through to return commands[commands.length - 1] (a command name that doesn't exist on the system). The caller in useLaunchEditor.ts checks if (!execCmd), but this can never be falsy, so the helpful error message ("No available editor found for X...") is dead code. Users whose preferred editor isn't installed will get an unhelpful spawnSync error instead of clear guidance.

Suggestion: return string | null (returning null when nothing is found), or throw like the original getExecutableCommand did.

2. Typecheck failure: unused getAppBundleCliPath

editor.test.ts declares getAppBundleCliPath but never uses it. This will fail npm run typecheck under noUnusedLocals: true — CI blocker.

Verdict

REQUEST_CHANGES — The typecheck failure is a hard CI blocker, and the error handling regression silently degrades UX. Both should be straightforward to fix. The minor issues (platform-specific commands in getZedCommand, duplicated resolution logic) can be addressed in a follow-up.

[tanzhenxin]

Thanks for the fix — the app-bundle fallback is the right approach for Homebrew-cask Zed installs, and spotting that Contents/MacOS/cli (not Zed) is the correct CLI binary was a good catch. A few blockers before this can merge:

1. The new test at packages/core/src/utils/editor.test.ts:737-739 has a stray toEqual(...) call with no expect(...) wrapper:

   it('should prefer CLI in PATH over app bundle', () => {
     toEqual(['--wait', '--diff', 'old.txt', 'new.txt']);
   });
   

   This makes npx vitest run src/utils/editor.test.ts fail with ReferenceError: toEqual is not defined, and npm run typecheck fail with TS2304: Cannot find name 'toEqual'. The it() title is also duplicated at line 741. Looks like a merge-resolve artifact — delete the stray block.

2. check_braces.js and pr-body.md at the repo root look like scratch files that were accidentally committed (a brace-counting script and the PR description draft). Please remove them.

3. zedAppExists() only checks whether the .app directory exists, but getZedCommand / getEditorExecutable require Contents/MacOS/cli inside it. That means a partial/broken install (bundle present, inner CLI binary missing) will report Zed as "available" in checkHasEditorType and then silently fail in getDiffCommand. Routing availability through the same executable-resolution helper (e.g. getEditorExecutable('zed') !== null) keeps the two checks in sync.

A few smaller things for a follow-up if you'd like: the test mocks 'fs' but the source imports 'node:fs' (works, but worth aligning); the macOS tests use path.includes('Zed.app') without mocking os.homedir(), so they don't actually distinguish the /Applications vs ~/Applications branches; and the useLaunchEditor.ts refactor drops the commandExistsCache that was added in 48bc0f3 specifically to fix a CI timeout — worth verifying that scenario still holds or pushing the cache into core's commandExists.

Verdict: request_changes on the three items above.