Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
"@tiptap/extension-task-list": "^3.17.1",
"@tiptap/extension-text": "^3.17.1",
"@tiptap/extension-underline": "^3.17.1",
"@tiptap/pm": "^3.17.1",
"@tiptap/react": "^3.17.1",
"@tiptap/starter-kit": "^3.17.1",
"@tiptap/suggestion": "^3.17.1",
Expand Down
253 changes: 253 additions & 0 deletions apps/desktop/plans/20260422-2100-v1-to-v2-port.md

Large diffs are not rendered by default.

44 changes: 40 additions & 4 deletions apps/desktop/src/lib/trpc/routers/external/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import os from "node:os";
import path from "node:path";
import { getAppCommand, resolvePath, stripPathWrappers } from "./helpers";
import {
getAppCommand,
RelativePathWithoutCwdError,
resolvePath,
stripPathWrappers,
} from "./helpers";

describe("getAppCommand", () => {
const originalPlatform = process.platform;
Expand Down Expand Up @@ -205,9 +210,8 @@ describe("resolvePath", () => {
expect(result).toBe("/project/sibling/file.ts");
});

test("resolves relative path against process.cwd() when no cwd provided", () => {
const result = resolvePath("file.ts");
expect(result).toBe(path.resolve("file.ts"));
test("throws RelativePathWithoutCwdError when no cwd provided", () => {
expect(() => resolvePath("file.ts")).toThrow(RelativePathWithoutCwdError);
});
});

Expand Down Expand Up @@ -581,3 +585,35 @@ describe("stripPathWrappers", () => {
});
});
});

describe("resolvePath guards against process.cwd() fallback", () => {
test("throws RelativePathWithoutCwdError for a relative path with no cwd", () => {
expect(() => resolvePath("apps/desktop/src/index.ts")).toThrow(
RelativePathWithoutCwdError,
);
});

test("throws for a wrapped/quoted relative path with no cwd", () => {
expect(() => resolvePath('"apps/desktop/src/index.ts"')).toThrow(
RelativePathWithoutCwdError,
);
});

test("absolute paths do not need a cwd", () => {
expect(() => resolvePath("/Users/me/file.ts")).not.toThrow();
});

test("~-prefixed paths do not need a cwd", () => {
expect(() => resolvePath("~/file.ts")).not.toThrow();
});

test("file:// URLs do not need a cwd", () => {
expect(() => resolvePath("file:///Users/me/file.ts")).not.toThrow();
});

test("a relative path with a cwd resolves correctly", () => {
expect(resolvePath("src/index.ts", "/workspace")).toBe(
"/workspace/src/index.ts",
);
});
});
23 changes: 20 additions & 3 deletions apps/desktop/src/lib/trpc/routers/external/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,28 @@ export function stripPathWrappers(filePath: string): string {
return result;
}

export class RelativePathWithoutCwdError extends Error {
readonly originalPath: string;
constructor(originalPath: string) {
super(
`resolvePath received a relative path (${JSON.stringify(originalPath)}) without a cwd. ` +
"Pass an absolute path, or supply cwd (e.g. the workspace worktreePath). " +
"Falling back to process.cwd() would resolve against Electron's working directory and silently produce wrong paths.",
);
this.name = "RelativePathWithoutCwdError";
this.originalPath = originalPath;
}
}

/**
* Resolve a path by expanding ~ and converting relative paths to absolute.
* Also handles file:// URLs by converting them to regular file paths.
* Strips wrapping characters like quotes, parentheses, brackets, etc.
*
* Throws `RelativePathWithoutCwdError` if the input resolves to a relative
* path and no `cwd` was supplied — callers must be explicit about what
* relative paths are relative to. (A silent `process.cwd()` fallback would
* point at Electron's working directory, not the workspace.)
*/
export function resolvePath(filePath: string, cwd?: string): string {
let resolved = stripPathWrappers(filePath);
Expand All @@ -293,9 +311,8 @@ export function resolvePath(filePath: string, cwd?: string): string {
}

if (!nodePath.isAbsolute(resolved)) {
resolved = cwd
? nodePath.resolve(cwd, resolved)
: nodePath.resolve(resolved);
if (!cwd) throw new RelativePathWithoutCwdError(filePath);
resolved = nodePath.resolve(cwd, resolved);
}

return resolved;
Expand Down
143 changes: 92 additions & 51 deletions apps/desktop/src/lib/trpc/routers/external/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from "node:fs";
import { access, readFile, writeFile } from "node:fs/promises";
import nodePath from "node:path";
import {
EXTERNAL_APPS,
NON_EDITOR_APPS,
Expand All @@ -24,10 +25,28 @@ import { getWorkspacePath } from "../workspaces/utils/worktree";
import {
type ExternalApp,
getAppCommand,
RelativePathWithoutCwdError,
resolvePath,
spawnAsync,
} from "./helpers";

/**
* Wraps a tRPC handler so a `RelativePathWithoutCwdError` (thrown by
* `resolvePath` when a relative path arrives without a `worktreePath`)
* surfaces as a clear BAD_REQUEST with the root-cause message instead
* of a generic 500.
*/
async function withResolveGuard<T>(fn: () => Promise<T> | T): Promise<T> {
try {
return await fn();
} catch (err) {
if (err instanceof RelativePathWithoutCwdError) {
throw new TRPCError({ code: "BAD_REQUEST", message: err.message });
}
throw err;
}
}

const ExternalAppSchema = z.enum(EXTERNAL_APPS);
const FileFilterSchema = z.object({
name: z.string(),
Expand Down Expand Up @@ -63,7 +82,7 @@ async function assertPathExists(filePath: string): Promise<void> {
}
}

function normalizeOpenInAppError(error: unknown): never {
function _normalizeOpenInAppError(error: unknown): never {
if (error instanceof TRPCError) {
throw error;
}
Expand Down Expand Up @@ -210,13 +229,17 @@ export const createExternalRouter = () => {
}),
)
.mutation(async ({ input }) => {
// Avoid turning deleted/moved files into INTERNAL_SERVER_ERROR during app launch.
await assertPathExists(input.path);
try {
await openPathInApp(input.path, input.app);
} catch (error) {
normalizeOpenInAppError(error);
// openInApp hands `path` directly to the editor CLI / shell; with no
// cwd input there's no safe way to interpret a relative path, so we
// reject them loudly instead of silently resolving against Electron's
// working directory.
if (!nodePath.isAbsolute(input.path)) {
throw new TRPCError({
code: "BAD_REQUEST",
message: `openInApp requires an absolute path (got ${JSON.stringify(input.path)}).`,
});
}
await openPathInApp(input.path, input.app);

// Persist defaults only after successful launch
if (input.projectId) {
Expand Down Expand Up @@ -335,10 +358,13 @@ export const createExternalRouter = () => {
.input(
z.object({
path: z.string(),
cwd: z.string().optional(),
/** Absolute workspace worktree path — relative `path`s are resolved against this. */
worktreePath: z.string().optional(),
}),
)
.query(({ input }) => resolvePath(input.path, input.cwd)),
.query(({ input }) =>
withResolveGuard(() => resolvePath(input.path, input.worktreePath)),
),

statPath: publicProcedure
.input(
Expand All @@ -347,61 +373,76 @@ export const createExternalRouter = () => {
workspaceId: z.string().optional(),
}),
)
.mutation(async ({ input }) => {
const workspace = input.workspaceId
? getWorkspace(input.workspaceId)
: null;
// If a workspaceId was provided but we couldn't find the workspace,
// return null rather than resolving relative to process.cwd().
if (input.workspaceId && !workspace) return null;
const cwd = workspace
? (getWorkspacePath(workspace) ?? undefined)
: undefined;
const resolved = resolvePath(input.path, cwd);
try {
const stats = await fs.promises.stat(resolved);
return {
isDirectory: stats.isDirectory(),
resolvedPath: resolved,
};
} catch {
return null;
}
}),
.mutation(({ input }) =>
withResolveGuard(async () => {
const workspace = input.workspaceId
? getWorkspace(input.workspaceId)
: null;
const cwd = workspace
? (getWorkspacePath(workspace) ?? undefined)
: undefined;
const resolved = resolvePath(input.path, cwd);
try {
const stats = await fs.promises.stat(resolved);
return {
isDirectory: stats.isDirectory(),
resolvedPath: resolved,
};
} catch {
return null;
}
}),
),

openFileInEditor: publicProcedure
.input(
z.object({
path: z.string(),
line: z.number().optional(),
column: z.number().optional(),
cwd: z.string().optional(),
/**
* Absolute workspace worktree path. Required when `path` is
* relative; ignored when `path` is already absolute. Using the
* workspace's worktreePath (rather than an arbitrary cwd) means
* relative diff/tree paths always resolve against the workspace
* the user is in, never Electron's process cwd.
*/
worktreePath: z.string().optional(),
projectId: z.string().optional(),
/**
* Explicit app override from the caller (e.g. the v2 CMD+O
* choice stored client-side in tanstack-db). When provided,
* bypasses the server-side `resolveDefaultEditor` lookup —
* which only knows about v1 localDb tables and would
* otherwise return a stale global default for v2 projects.
*/
app: ExternalAppSchema.optional(),
}),
)
.mutation(async ({ input }) => {
const filePath = resolvePath(input.path, input.cwd);
// Editor open is also triggered from stale paths in the UI, so normalize ENOENT here too.
await assertPathExists(filePath);
const app = resolveDefaultEditor(input.projectId);

if (!app) {
// No preferred editor configured yet.
// Fall back to OS default file handler so Cmd/Ctrl+click still works
// even when Cursor (or any specific editor) isn't installed.
const openError = await shell.openPath(filePath);
if (openError) {
throw new Error(openError);
.mutation(({ input }) =>
withResolveGuard(async () => {
const filePath = resolvePath(input.path, input.worktreePath);
const app = input.app ?? resolveDefaultEditor(input.projectId);

if (!app) {
// No preferred editor configured yet.
// Fall back to OS default file handler so Cmd/Ctrl+click still works
// even when Cursor (or any specific editor) isn't installed.
// `shell.openPath` returns a non-empty string on failure instead of
// throwing — surface that so callers see a meaningful error.
const openError = await shell.openPath(filePath);
if (openError) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: `Failed to open file: ${openError}`,
});
}
return;
}
return;
}

try {
await openPathInApp(filePath, app);
} catch (error) {
normalizeOpenInAppError(error);
}
}),
}),
),
});
};

Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/lib/trpc/routers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { createGitHubMetricsRouter } from "./github-metrics";
import { createHostServiceCoordinatorRouter } from "./host-service-coordinator";
import { createLanguageServicesRouter } from "./language-services";
import { createMenuRouter } from "./menu";
import { createMigrationRouter } from "./migration";
import { createNotificationsRouter } from "./notifications";
import { createPermissionsRouter } from "./permissions";
import { createPortsRouter } from "./ports";
Expand Down Expand Up @@ -90,6 +91,7 @@ export const createAppRouter = (
vibrancy: createVibrancyRouter(wm),
vscodeExtensions: createVscodeExtensionsRouter(),
todoAgent: createTodoAgentRouter(),
migration: createMigrationRouter(),
});
};

Expand Down
Loading
Loading