diff --git a/src/browser/utils/chatCommands.test.ts b/src/browser/utils/chatCommands.test.ts index f75b5fd478..bec5552bf3 100644 --- a/src/browser/utils/chatCommands.test.ts +++ b/src/browser/utils/chatCommands.test.ts @@ -594,31 +594,15 @@ describe("handlePlanOpenCommand", () => { expect(context.api.workspace.getInfo).toHaveBeenCalledWith({ workspaceId: "test-workspace-id", }); - expect(context.api.general.openInEditor).toHaveBeenCalledWith({ - workspaceId: "test-workspace-id", - targetPath: "/path/to/plan.md", - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - editorConfig: expect.any(Object), - }); + // Note: Built-in editors (VS Code/Cursor/Zed) now use deep links directly + // via window.open(), not the backend API. The backend API is only used + // for custom editors. }); - test("shows error toast when editor fails to open", async () => { - const context = createMockContext( - { success: true, data: { content: "# My Plan", path: "/path/to/plan.md" } }, - { success: false, error: "No editor configured" } - ); - - const result = await handlePlanOpenCommand(context); - - expect(result.clearInput).toBe(true); - expect(result.toastShown).toBe(true); - expect(context.setToast).toHaveBeenCalledWith( - expect.objectContaining({ - type: "error", - message: "No editor configured", - }) - ); - }); + // Note: The "editor fails to open" test was removed because built-in editors + // (VS Code/Cursor/Zed) now use deep links that open via window.open() and + // always succeed from the app's perspective. Failures happen in the external + // editor, not in our code path. }); describe("handleCompactCommand", () => { diff --git a/src/browser/utils/editorDeepLinks.test.ts b/src/browser/utils/editorDeepLinks.test.ts index 6bc66c6a52..0d66001fb2 100644 --- a/src/browser/utils/editorDeepLinks.test.ts +++ b/src/browser/utils/editorDeepLinks.test.ts @@ -19,6 +19,31 @@ describe("getEditorDeepLink", () => { expect(url).toBe("cursor://file/home/user/project/file.ts"); }); + test("normalizes Windows drive paths for local deep links", () => { + const url = getEditorDeepLink({ + editor: "vscode", + path: "C:\\Users\\Me\\proj\\file.ts", + }); + expect(url).toBe("vscode://file/C:/Users/Me/proj/file.ts"); + }); + + test("normalizes Windows drive paths with forward slashes", () => { + const url = getEditorDeepLink({ + editor: "cursor", + path: "C:/Users/Me/proj/file.ts", + line: 42, + column: 10, + }); + expect(url).toBe("cursor://file/C:/Users/Me/proj/file.ts:42:10"); + }); + + test("strips surrounding quotes from local deep link paths", () => { + const url = getEditorDeepLink({ + editor: "vscode", + path: "'C:\\Users\\Me\\proj\\file.ts'", + }); + expect(url).toBe("vscode://file/C:/Users/Me/proj/file.ts"); + }); test("generates zed:// URL for local path", () => { const url = getEditorDeepLink({ editor: "zed", diff --git a/src/browser/utils/editorDeepLinks.ts b/src/browser/utils/editorDeepLinks.ts index 23e088b898..daff2f7d25 100644 --- a/src/browser/utils/editorDeepLinks.ts +++ b/src/browser/utils/editorDeepLinks.ts @@ -52,7 +52,13 @@ export function getEditorDeepLink(options: DeepLinkOptions): string | null { } // Local format: vscode://file/path - let url = `${scheme}://file${path}`; + // + // Note: On Windows, callers may provide native paths like `C:\\Users\\...`. + // VS Code/Cursor/Zed expect forward slashes and a leading `/` after `file`: + // vscode://file/C:/Users/... + const normalizedPath = normalizeLocalPathForEditorDeepLink(path); + + let url = `${scheme}://file${normalizedPath}`; if (line != null) { url += `:${line}`; if (column != null) { @@ -62,6 +68,24 @@ export function getEditorDeepLink(options: DeepLinkOptions): string | null { return url; } +function normalizeLocalPathForEditorDeepLink(path: string): string { + const trimmed = path.trim(); + const unquoted = + (trimmed.startsWith('"') && trimmed.endsWith('"')) || + (trimmed.startsWith("'") && trimmed.endsWith("'")) + ? trimmed.slice(1, -1) + : trimmed; + + const pathWithSlashes = unquoted.replace(/\\/g, "/"); + + // Ensure the URL parses as `scheme://file/`. + if (pathWithSlashes.startsWith("/")) { + return pathWithSlashes; + } + + return `/${pathWithSlashes}`; +} + /** * Check if a hostname represents localhost. */ diff --git a/src/browser/utils/openInEditor.ts b/src/browser/utils/openInEditor.ts index 07b7630cf5..bdda21f288 100644 --- a/src/browser/utils/openInEditor.ts +++ b/src/browser/utils/openInEditor.ts @@ -13,7 +13,6 @@ import { import type { RuntimeConfig } from "@/common/types/runtime"; import { isSSHRuntime, isDockerRuntime } from "@/common/types/runtime"; import type { APIClient } from "@/browser/contexts/API"; -import { getEditorDeepLinkFallbackUrl } from "@/browser/utils/openInEditorDeepLinkFallback"; export interface OpenInEditorResult { success: boolean; @@ -23,6 +22,13 @@ export interface OpenInEditorResult { // Browser mode: window.api is not set (only exists in Electron via preload) const isBrowserMode = typeof window !== "undefined" && !window.api; +// Helper for opening URLs - allows testing in Node environment +function openUrl(url: string): void { + if (typeof window !== "undefined" && window.open) { + window.open(url, "_blank"); + } +} + export async function openInEditor(args: { api: APIClient | null | undefined; openSettings?: (section?: string) => void; @@ -88,20 +94,12 @@ export async function openInEditor(args: { return { success: false, error: `${editorConfig.editor} does not support Docker containers` }; } - window.open(deepLink, "_blank"); + openUrl(deepLink); return { success: true }; } - // Browser mode: use deep links instead of backend spawn - if (isBrowserMode) { - // Custom editor can't work via deep links - if (editorConfig.editor === "custom") { - return { - success: false, - error: "Custom editors are not supported in browser mode. Use VS Code, Cursor, or Zed.", - }; - } - + // VS Code / Cursor / Zed: always use deep links (works in browser + Electron) + if (editorConfig.editor !== "custom") { // Determine SSH host for deep link let sshHost: string | undefined; if (isSSH && args.runtimeConfig?.type === "ssh") { @@ -110,7 +108,7 @@ export async function openInEditor(args: { if (editorConfig.editor === "zed" && args.runtimeConfig.port != null) { sshHost = sshHost + ":" + args.runtimeConfig.port; } - } else if (!isLocalhost(window.location.hostname)) { + } else if (isBrowserMode && !isLocalhost(window.location.hostname)) { // Remote server + local workspace: need SSH to reach server's files const serverSshHost = await args.api?.server.getSshHost(); sshHost = serverSshHost ?? window.location.hostname; @@ -130,11 +128,20 @@ export async function openInEditor(args: { }; } - window.open(deepLink, "_blank"); + openUrl(deepLink); return { success: true }; } - // Electron mode: call the backend API + // Custom editor: + // - Browser mode: can't spawn processes on the server + // - Electron mode: spawn via backend API + if (isBrowserMode) { + return { + success: false, + error: "Custom editors are not supported in browser mode. Use VS Code, Cursor, or Zed.", + }; + } + const result = await args.api?.general.openInEditor({ workspaceId: args.workspaceId, targetPath: args.targetPath, @@ -146,21 +153,6 @@ export async function openInEditor(args: { } if (!result.success) { - const deepLink = - typeof window === "undefined" - ? null - : getEditorDeepLinkFallbackUrl({ - editor: editorConfig.editor, - targetPath: args.targetPath, - runtimeConfig: args.runtimeConfig, - error: result.error, - }); - - if (deepLink) { - window.open(deepLink, "_blank"); - return { success: true }; - } - return { success: false, error: result.error }; } diff --git a/src/browser/utils/openInEditorDeepLinkFallback.test.ts b/src/browser/utils/openInEditorDeepLinkFallback.test.ts deleted file mode 100644 index 610a61922e..0000000000 --- a/src/browser/utils/openInEditorDeepLinkFallback.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { - getEditorDeepLinkFallbackUrl, - shouldAttemptEditorDeepLinkFallback, -} from "./openInEditorDeepLinkFallback"; - -import type { RuntimeConfig } from "@/common/types/runtime"; - -describe("shouldAttemptEditorDeepLinkFallback", () => { - test("returns true for EditorService command-not-found error", () => { - expect(shouldAttemptEditorDeepLinkFallback("Editor command not found: code")).toBe(true); - }); - - test("returns false for unrelated errors", () => { - expect(shouldAttemptEditorDeepLinkFallback("Some other error")).toBe(false); - expect(shouldAttemptEditorDeepLinkFallback(undefined)).toBe(false); - }); -}); - -describe("getEditorDeepLinkFallbackUrl", () => { - test("returns vscode://file URL for local path", () => { - const url = getEditorDeepLinkFallbackUrl({ - editor: "vscode", - targetPath: "/home/user/project/file.ts", - error: "Editor command not found: code", - }); - expect(url).toBe("vscode://file/home/user/project/file.ts"); - }); - - test("returns cursor://vscode-remote URL for SSH runtime", () => { - const runtimeConfig: RuntimeConfig = { - type: "ssh", - host: "devbox", - srcBaseDir: "~/mux", - }; - - const url = getEditorDeepLinkFallbackUrl({ - editor: "cursor", - targetPath: "/home/user/project/file.ts", - runtimeConfig, - error: "Editor command not found: cursor", - }); - - expect(url).toBe("cursor://vscode-remote/ssh-remote+devbox/home/user/project/file.ts"); - }); - - test("returns zed://ssh URL for SSH runtime", () => { - const runtimeConfig: RuntimeConfig = { - type: "ssh", - host: "devbox", - srcBaseDir: "~/mux", - }; - - const url = getEditorDeepLinkFallbackUrl({ - editor: "zed", - targetPath: "/home/user/project/file.ts", - runtimeConfig, - error: "Editor command not found: zed", - }); - - expect(url).toBe("zed://ssh/devbox/home/user/project/file.ts"); - }); - - test("includes port in zed://ssh URL for SSH runtime when provided", () => { - const runtimeConfig: RuntimeConfig = { - type: "ssh", - host: "devbox", - port: 2222, - srcBaseDir: "~/mux", - }; - - const url = getEditorDeepLinkFallbackUrl({ - editor: "zed", - targetPath: "/home/user/project/file.ts", - runtimeConfig, - error: "Editor command not found: zed", - }); - - expect(url).toBe("zed://ssh/devbox:2222/home/user/project/file.ts"); - }); - - test("returns null when error is not command-not-found", () => { - const url = getEditorDeepLinkFallbackUrl({ - editor: "vscode", - targetPath: "/home/user/project/file.ts", - error: "Permission denied", - }); - - expect(url).toBeNull(); - }); -}); diff --git a/src/browser/utils/openInEditorDeepLinkFallback.ts b/src/browser/utils/openInEditorDeepLinkFallback.ts deleted file mode 100644 index 52b70bb153..0000000000 --- a/src/browser/utils/openInEditorDeepLinkFallback.ts +++ /dev/null @@ -1,45 +0,0 @@ -import type { EditorType } from "@/common/constants/storage"; -import type { RuntimeConfig } from "@/common/types/runtime"; -import { isSSHRuntime } from "@/common/types/runtime"; -import { getEditorDeepLink, type DeepLinkEditor } from "@/browser/utils/editorDeepLinks"; - -export function shouldAttemptEditorDeepLinkFallback(error: string | undefined): boolean { - if (!error) return false; - - // Primary signal from our backend EditorService. - if (error.startsWith("Editor command not found:")) return true; - - return false; -} - -export function getEditorDeepLinkFallbackUrl(args: { - editor: EditorType; - targetPath: string; - runtimeConfig?: RuntimeConfig; - error?: string; -}): string | null { - if (!shouldAttemptEditorDeepLinkFallback(args.error)) return null; - - if (args.editor === "custom") return null; - - const deepLinkEditor: DeepLinkEditor | null = - args.editor === "vscode" || args.editor === "cursor" || args.editor === "zed" - ? args.editor - : null; - - if (!deepLinkEditor) return null; - - let sshHost: string | undefined; - if (isSSHRuntime(args.runtimeConfig)) { - sshHost = args.runtimeConfig.host; - if (deepLinkEditor === "zed" && args.runtimeConfig.port != null) { - sshHost = sshHost + ":" + args.runtimeConfig.port; - } - } - - return getEditorDeepLink({ - editor: deepLinkEditor, - path: args.targetPath, - sshHost, - }); -} diff --git a/src/node/services/editorService.test.ts b/src/node/services/editorService.test.ts new file mode 100644 index 0000000000..cbf92488ab --- /dev/null +++ b/src/node/services/editorService.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from "bun:test"; +import type { Config } from "@/node/config"; +import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; +import { EditorService } from "./editorService"; + +describe("EditorService", () => { + test("rejects non-custom editors (renderer must use deep links)", async () => { + const editorService = new EditorService({} as Config); + + const result = await editorService.openInEditor("ws1", "/tmp", { + editor: "vscode", + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("deep links"); + } + }); + + test("validates custom editor executable exists before spawning", async () => { + const workspace: FrontendWorkspaceMetadata = { + id: "ws1", + name: "ws1", + projectName: "proj", + projectPath: "/tmp/proj", + createdAt: "2025-01-01T00:00:00.000Z", + runtimeConfig: { type: "worktree", srcBaseDir: "/tmp/src" }, + namedWorkspacePath: "/tmp/src/proj/ws1", + }; + + const mockConfig: Pick = { + getAllWorkspaceMetadata: () => Promise.resolve([workspace]), + } as unknown as Pick; + + const editorService = new EditorService(mockConfig as Config); + + const result = await editorService.openInEditor("ws1", "/tmp", { + editor: "custom", + customCommand: "definitely-not-a-command", + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Editor command not found"); + } + }); + + test("errors on invalid custom editor command quoting", async () => { + const workspace: FrontendWorkspaceMetadata = { + id: "ws1", + name: "ws1", + projectName: "proj", + projectPath: "/tmp/proj", + createdAt: "2025-01-01T00:00:00.000Z", + runtimeConfig: { type: "worktree", srcBaseDir: "/tmp/src" }, + namedWorkspacePath: "/tmp/src/proj/ws1", + }; + + const mockConfig: Pick = { + getAllWorkspaceMetadata: () => Promise.resolve([workspace]), + } as unknown as Pick; + + const editorService = new EditorService(mockConfig as Config); + + const result = await editorService.openInEditor("ws1", "/tmp", { + editor: "custom", + customCommand: '"unterminated', + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Invalid custom editor command"); + } + }); +}); diff --git a/src/node/services/editorService.ts b/src/node/services/editorService.ts index 828565ebd7..8de14b48cd 100644 --- a/src/node/services/editorService.ts +++ b/src/node/services/editorService.ts @@ -1,18 +1,53 @@ import { spawn, spawnSync } from "child_process"; +import * as fsPromises from "fs/promises"; import type { Config } from "@/node/config"; -import { isSSHRuntime } from "@/common/types/runtime"; +import { isDockerRuntime, isSSHRuntime } from "@/common/types/runtime"; import { log } from "@/node/services/log"; -import { createRuntime } from "@/node/runtime/runtimeFactory"; /** * Quote a string for safe use in shell commands. - * Uses single quotes with proper escaping for embedded single quotes. + * + * IMPORTANT: Prefer spawning commands with an args array instead of building a + * single shell string. This helper exists only for custom editor commands. */ function shellQuote(value: string): string { - if (value.length === 0) return "''"; + if (value.length === 0) return process.platform === "win32" ? '""' : "''"; + + // cmd.exe: use double quotes (single quotes are treated as literal characters) + if (process.platform === "win32") { + return `"${value.replace(/"/g, '""')}"`; + } + + // POSIX shells: single quotes with proper escaping for embedded single quotes. return "'" + value.replace(/'/g, "'\"'\"'") + "'"; } +function getExecutableFromShellCommand(command: string): string | null { + const trimmed = command.trim(); + if (!trimmed) return null; + + const quote = trimmed[0]; + if (quote === '"' || quote === "'") { + const endQuoteIndex = trimmed.indexOf(quote, 1); + if (endQuoteIndex === -1) { + return null; + } + return trimmed.slice(1, endQuoteIndex); + } + + return trimmed.split(/\s+/)[0] ?? null; +} + +function looksLikePath(command: string): boolean { + return ( + command.startsWith("./") || + command.startsWith("../") || + command.includes("/") || + command.includes("\\") || + /^[A-Za-z]:/.test(command) + ); +} + export interface EditorConfig { editor: string; customCommand?: string; @@ -20,27 +55,21 @@ export interface EditorConfig { /** * Service for opening workspaces in code editors. - * Supports VS Code, Cursor, Zed, and custom editors. - * For SSH workspaces: VS Code/Cursor use Remote-SSH; Zed opens an ssh:// URL. + * + * NOTE: VS Code/Cursor/Zed are opened via deep links in the renderer. + * This service is only responsible for spawning the user's custom editor command. */ export class EditorService { private readonly config: Config; - private static readonly EDITOR_COMMANDS: Record = { - vscode: "code", - cursor: "cursor", - zed: "zed", - }; - constructor(config: Config) { this.config = config; } /** * Open a path in the user's configured code editor. - * For SSH workspaces with Remote-SSH extension enabled, opens directly in the editor. * - * @param workspaceId - The workspace (used to determine if SSH and get remote host) + * @param workspaceId - The workspace (used to determine runtime + validate constraints) * @param targetPath - The path to open (workspace directory or specific file) * @param editorConfig - Editor configuration from user settings */ @@ -50,6 +79,19 @@ export class EditorService { editorConfig: EditorConfig ): Promise<{ success: true; data: void } | { success: false; error: string }> { try { + if (editorConfig.editor !== "custom") { + return { + success: false, + error: + "Built-in editors are opened via deep links. Select Custom editor to use a command.", + }; + } + + const customCommand = editorConfig.customCommand?.trim(); + if (!customCommand) { + return { success: false, error: "No editor command configured" }; + } + const allMetadata = await this.config.getAllWorkspaceMetadata(); const workspace = allMetadata.find((w) => w.id === workspaceId); @@ -57,82 +99,42 @@ export class EditorService { return { success: false, error: `Workspace not found: ${workspaceId}` }; } - const runtimeConfig = workspace.runtimeConfig; - const isSSH = isSSHRuntime(runtimeConfig); - - // Determine the editor command - const editorCommand = - editorConfig.editor === "custom" - ? editorConfig.customCommand - : EditorService.EDITOR_COMMANDS[editorConfig.editor]; + // Remote runtimes: custom commands run on the local machine and can't access remote paths. + if (isSSHRuntime(workspace.runtimeConfig)) { + return { + success: false, + error: "Custom editors do not support SSH connections for SSH workspaces", + }; + } - if (!editorCommand) { - return { success: false, error: "No editor command configured" }; + if (isDockerRuntime(workspace.runtimeConfig)) { + return { success: false, error: "Custom editors do not support Docker containers" }; } - // Check if editor is available - const isAvailable = this.isCommandAvailable(editorCommand); - if (!isAvailable) { - return { success: false, error: `Editor command not found: ${editorCommand}` }; + const executable = getExecutableFromShellCommand(customCommand); + if (!executable) { + return { success: false, error: `Invalid custom editor command: ${customCommand}` }; } - if (isSSH) { - // Resolve tilde paths to absolute paths for SSH. - // (VS Code's Remote-SSH won't expand ~, and Zed expects a concrete ssh:// URL.) - const runtime = createRuntime(runtimeConfig, { projectPath: workspace.projectPath }); - const resolvedPath = await runtime.resolvePath(targetPath); - - if (editorConfig.editor === "zed") { - const hostWithPort = - runtimeConfig.port != null - ? runtimeConfig.host + ":" + runtimeConfig.port - : runtimeConfig.host; - const sshUrl = `ssh://${hostWithPort}${resolvedPath}`; - const shellCmd = `${editorCommand} ${shellQuote(sshUrl)}`; - - log.info(`Opening SSH path in editor: ${shellCmd}`); - const child = spawn(shellCmd, [], { - detached: true, - stdio: "ignore", - shell: true, - }); - child.unref(); - } else { - // VS Code/Cursor SSH workspace handling via Remote-SSH - if (editorConfig.editor !== "vscode" && editorConfig.editor !== "cursor") { - return { - success: false, - error: `${editorConfig.editor} does not support SSH connections for SSH workspaces`, - }; - } - - // Build the remote command: code --remote ssh-remote+host /remote/path - // Quote the path to handle spaces; the remote host arg doesn't need quoting - const shellCmd = `${editorCommand} --remote ${shellQuote(`ssh-remote+${runtimeConfig.host}`)} ${shellQuote(resolvedPath)}`; - - log.info(`Opening SSH path in editor: ${shellCmd}`); - const child = spawn(shellCmd, [], { - detached: true, - stdio: "ignore", - shell: true, - }); - child.unref(); - } - } else { - // Local - expand tilde and open the path (quote to handle spaces) - const resolvedPath = targetPath.startsWith("~/") - ? targetPath.replace("~", process.env.HOME ?? "~") - : targetPath; - const shellCmd = `${editorCommand} ${shellQuote(resolvedPath)}`; - log.info(`Opening local path in editor: ${shellCmd}`); - const child = spawn(shellCmd, [], { - detached: true, - stdio: "ignore", - shell: true, - }); - child.unref(); + if (!(await this.isCommandAvailable(executable))) { + return { success: false, error: `Editor command not found: ${executable}` }; } + // Local - expand tilde (shellQuote prevents shell expansion) + const resolvedPath = targetPath.startsWith("~/") + ? targetPath.replace("~", process.env.HOME ?? "~") + : targetPath; + + const shellCmd = `${customCommand} ${shellQuote(resolvedPath)}`; + log.info(`Opening local path in custom editor: ${shellCmd}`); + const child = spawn(shellCmd, [], { + detached: true, + stdio: "ignore", + shell: true, + windowsHide: true, + }); + child.unref(); + return { success: true, data: undefined }; } catch (err) { const message = err instanceof Error ? err.message : String(err); @@ -145,9 +147,15 @@ export class EditorService { * Check if a command is available in the system PATH. * Inherits enriched PATH from process.env (set by initShellEnv at startup). */ - private isCommandAvailable(command: string): boolean { + private async isCommandAvailable(command: string): Promise { try { - const result = spawnSync("which", [command], { encoding: "utf8" }); + if (looksLikePath(command)) { + await fsPromises.access(command); + return true; + } + + const lookupCommand = process.platform === "win32" ? "where" : "which"; + const result = spawnSync(lookupCommand, [command], { encoding: "utf8" }); return result.status === 0; } catch { return false; diff --git a/tests/ipc/planCommands.test.ts b/tests/ipc/planCommands.test.ts index 612f803b17..9a6989e382 100644 --- a/tests/ipc/planCommands.test.ts +++ b/tests/ipc/planCommands.test.ts @@ -288,11 +288,14 @@ describeIntegration("Plan Commands Integration", () => { }, 30000); it("should return error when workspace not found", async () => { + // Note: Built-in editors (vscode/cursor/zed) are now opened via deep links + // on the frontend, so the backend API only handles custom editors. const result = await env.orpc.general.openInEditor({ workspaceId: "nonexistent-workspace-id", targetPath: "/some/path", editorConfig: { - editor: "vscode", + editor: "custom", + customCommand: "code", }, });