History

fix: harden open in folder behavior

t3-code[bot] committed 3 months ago

Commit f5965c21 parent 39788e87 files · +246 −25Browse files

Unified · Split

7 files
  1. apps/server/src/process/externalLauncher.test.ts +110 −8
  2. apps/server/src/process/externalLauncher.ts +38 −7
  3. apps/web/src/components/ChatMarkdown.tsx +13 −6
  4. apps/web/src/components/chat/fileLinkContextMenu.test.ts +42 −1
  5. apps/web/src/components/chat/fileLinkContextMenu.ts +20 −1
  6. apps/web/src/markdown-links.test.ts +21 −0
  7. apps/web/src/markdown-links.ts +2 −2
apps/server/src/process/externalLauncher.test.ts+110−8
@@ -106,12 +106,12 @@ it.effect("reveals files with the linux file manager", () => · 105 unmodified lines
106106 let spawned: ChildProcess.StandardCommand | undefined;
107107 yield* Effect.gen(function* () {
108108 const launcher = yield* ExternalLauncher.ExternalLauncher;
109− yield* launcher.revealInFileManager({ path: "/tmp/project/src/index.ts" });
109+ yield* launcher.revealInFileManager({ path: "/tmp/project with spaces/src/index.ts" });
110110 }).pipe(
111111 Effect.provide(
112112 testLayer({
113113 platform: "linux",
114− env: { PATH: binDir },
114+ env: { PATH: binDir, DISPLAY: ":0" },
115115 onSpawn: (command) => {
116116 spawned = command;
117117 },
@@ -121,11 +121,11 @@ it.effect("reveals files with the linux file manager", () => · 3 unmodified lines
121121
122122 assert.ok(spawned);
123123 assert.equal(spawned.command, "xdg-open");
124− assert.deepEqual(spawned.args, ["/tmp/project/src"]);
124+ assert.deepEqual(spawned.args, ["/tmp/project with spaces/src"]);
125125 }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
126126 );
127127
128−it.effect("reveals files with Finder", () =>
128+it.effect("opens the containing folder with Finder", () =>
129129 Effect.gen(function* () {
130130 const fileSystem = yield* FileSystem.FileSystem;
131131 const path = yield* Path.Path;
@@ -137,7 +137,7 @@ it.effect("reveals files with Finder", () => · 5 unmodified lines
137137 let spawned: ChildProcess.StandardCommand | undefined;
138138 yield* Effect.gen(function* () {
139139 const launcher = yield* ExternalLauncher.ExternalLauncher;
140− yield* launcher.revealInFileManager({ path: "/tmp/project/src/index.ts" });
140+ yield* launcher.revealInFileManager({ path: "/tmp/project with spaces/src/index.ts" });
141141 }).pipe(
142142 Effect.provide(
143143 testLayer({
@@ -152,11 +152,11 @@ it.effect("reveals files with Finder", () => · 8 unmodified lines
152152
153153 assert.ok(spawned);
154154 assert.equal(spawned.command, "open");
155− assert.deepEqual(spawned.args, ["-R", "/tmp/project/src/index.ts"]);
155+ assert.deepEqual(spawned.args, ["/tmp/project with spaces/src"]);
156156 }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
157157 );
158158
159−it.effect("reveals files with Windows Explorer", () =>
159+it.effect("opens the containing folder with Windows Explorer", () =>
160160 Effect.gen(function* () {
161161 const fileSystem = yield* FileSystem.FileSystem;
162162 const path = yield* Path.Path;
@@ -181,7 +181,8 @@ it.effect("reveals files with Windows Explorer", () => · 18 unmodified lines
181181
182182 assert.ok(spawned);
183183 assert.equal(spawned.command, "explorer");
184− assert.deepEqual(spawned.args, [`/select,"C:\\project files\\src\\index.ts"`]);
184+ assert.deepEqual(spawned.args, [`C:\\project files\\src`]);
185+ assert.equal(spawned.options.shell, false);
185186 }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
186187 );
187188
@@ -371,6 +372,107 @@ it.effect("rescans after an interrupted discovery instead of caching the interru · 183 unmodified lines
371372 ),
372373 );
373374 });
375+it.effect("does not advertise a file manager on headless Linux", () =>
376+ Effect.gen(function* () {
377+ const fileSystem = yield* FileSystem.FileSystem;
378+ const path = yield* Path.Path;
379+ const binDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-headless-linux-" });
380+ const commandPath = path.join(binDir, "xdg-open");
381+ yield* fileSystem.writeFileString(commandPath, "#!/bin/sh\n");
382+ yield* fileSystem.chmod(commandPath, 0o755);
383+
384+ const editors = yield* Effect.gen(function* () {
385+ const launcher = yield* ExternalLauncher.ExternalLauncher;
386+ return yield* launcher.resolveAvailableEditors();
387+ }).pipe(Effect.provide(testLayer({ platform: "linux", env: { PATH: binDir } })));
388+
389+ assert.equal(editors.includes("file-manager"), false);
390+ }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
391+);
392+
393+it.effect("advertises a file manager in a Linux graphical session", () =>
394+ Effect.gen(function* () {
395+ const fileSystem = yield* FileSystem.FileSystem;
396+ const path = yield* Path.Path;
397+ const binDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-desktop-linux-" });
398+ const commandPath = path.join(binDir, "xdg-open");
399+ yield* fileSystem.writeFileString(commandPath, "#!/bin/sh\n");
400+ yield* fileSystem.chmod(commandPath, 0o755);
401+
402+ const editors = yield* Effect.gen(function* () {
403+ const launcher = yield* ExternalLauncher.ExternalLauncher;
404+ return yield* launcher.resolveAvailableEditors();
405+ }).pipe(
406+ Effect.provide(
407+ testLayer({ platform: "linux", env: { PATH: binDir, WAYLAND_DISPLAY: "wayland-0" } }),
408+ ),
409+ );
410+
411+ assert.equal(editors.includes("file-manager"), true);
412+ }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
413+);
414+
415+it.effect("rejects a direct file manager reveal on headless Linux", () =>
416+ Effect.gen(function* () {
417+ const launcher = yield* ExternalLauncher.ExternalLauncher;
418+ const error = yield* launcher
419+ .revealInFileManager({ path: "/tmp/project/src/index.ts" })
420+ .pipe(Effect.flip);
421+ assert.instanceOf(error, ExternalLauncher.ExternalLauncherUnsupportedEditorError);
422+ }).pipe(Effect.provide(testLayer({ platform: "linux", env: { PATH: "" } }))),
423+);
424+
425+it.effect("does not advertise a file manager over SSH", () =>
426+ Effect.gen(function* () {
427+ const fileSystem = yield* FileSystem.FileSystem;
428+ const path = yield* Path.Path;
429+ const binDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-ssh-macos-" });
430+ const commandPath = path.join(binDir, "open");
431+ yield* fileSystem.writeFileString(commandPath, "#!/bin/sh\n");
432+ yield* fileSystem.chmod(commandPath, 0o755);
433+
434+ const editors = yield* Effect.gen(function* () {
435+ const launcher = yield* ExternalLauncher.ExternalLauncher;
436+ return yield* launcher.resolveAvailableEditors();
437+ }).pipe(
438+ Effect.provide(
439+ testLayer({
440+ platform: "darwin",
441+ env: { PATH: binDir, SSH_CONNECTION: "client server" },
442+ }),
443+ ),
444+ );
445+
446+ assert.equal(editors.includes("file-manager"), false);
447+ }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
448+);
449+
450+it.effect("does not advertise Explorer from a Windows service", () =>
451+ Effect.gen(function* () {
452+ const fileSystem = yield* FileSystem.FileSystem;
453+ const path = yield* Path.Path;
454+ const binDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-windows-service-" });
455+ yield* fileSystem.writeFileString(path.join(binDir, "explorer.CMD"), "@echo off\r\n");
456+
457+ const editors = yield* Effect.gen(function* () {
458+ const launcher = yield* ExternalLauncher.ExternalLauncher;
459+ return yield* launcher.resolveAvailableEditors();
460+ }).pipe(
461+ Effect.provide(
462+ testLayer({
463+ platform: "win32",
464+ env: {
465+ PATH: binDir,
466+ PATHEXT: ".COM;.EXE;.BAT;.CMD",
467+ SESSIONNAME: "Services",
468+ },
469+ }),
470+ ),
471+ );
472+
473+ assert.equal(editors.includes("file-manager"), false);
474+ }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
475+);
374476
375477 it.effect("rejects unknown editors through the service API", () =>
376478 Effect.gen(function* () {
apps/server/src/process/externalLauncher.ts+38−7
@@ -107,6 +107,11 @@ const CommandLookupEnvConfig = Config.all({ · 106 unmodified lines
107107 Path: Config.string("Path").pipe(Config.option),
108108 path: Config.string("path").pipe(Config.option),
109109 PATHEXT: Config.string("PATHEXT").pipe(Config.option),
110+ DISPLAY: Config.string("DISPLAY").pipe(Config.option),
111+ WAYLAND_DISPLAY: Config.string("WAYLAND_DISPLAY").pipe(Config.option),
112+ SSH_CONNECTION: Config.string("SSH_CONNECTION").pipe(Config.option),
113+ SSH_TTY: Config.string("SSH_TTY").pipe(Config.option),
114+ SESSIONNAME: Config.string("SESSIONNAME").pipe(Config.option),
110115 }).pipe(Config.map(compactEnv));
111116
112117 const readBrowserLaunchEnv = BrowserLaunchEnvConfig.pipe(Effect.orElseSucceed(() => ({})));
@@ -224,6 +229,30 @@ function resolveWindowsBrowserLaunch(target: string, command: string): ProcessLa · 111 unmodified lines
224229 };
225230 }
226231
232+function hasGraphicalFileManagerSession(
233+ platform: NodeJS.Platform,
234+ env: NodeJS.ProcessEnv,
235+): boolean {
236+ if (env.SSH_CONNECTION?.trim() || env.SSH_TTY?.trim()) return false;
237+ if (platform === "linux") {
238+ return Boolean(env.DISPLAY?.trim() || env.WAYLAND_DISPLAY?.trim());
239+ }
240+ if (platform === "win32") {
241+ return env.SESSIONNAME?.trim().toLowerCase() !== "services";
242+ }
243+ return true;
244+}
245+
246+function fileManagerFolderPath(platform: NodeJS.Platform, target: string, path: Path.Path): string {
247+ if (platform !== "win32") return path.dirname(target);
248+
249+ const normalized = target.replaceAll("/", "\\");
250+ const separatorIndex = normalized.lastIndexOf("\\");
251+ if (separatorIndex < 0) return ".";
252+ if (separatorIndex === 2 && normalized[1] === ":") return normalized.slice(0, 3);
253+ return normalized.slice(0, separatorIndex) || "\\";
254+}
255+
227256 function fileManagerCommandForPlatform(platform: NodeJS.Platform): string {
228257 switch (platform) {
229258 case "darwin":
@@ -271,6 +300,7 @@ const buildAvailableEditors = Effect.fn("externalLauncher.buildAvailableEditors" · 41 unmodified lines
271300
272301 for (const editor of EDITORS) {
273302 if (editor.commands === null) {
303+ if (!hasGraphicalFileManagerSession(platform, env)) continue;
274304 const command = fileManagerCommandForPlatform(platform);
275305 if (yield* isCommandAvailable(command, { env })) {
276306 available.push(editor.id);
@@ -390,15 +420,16 @@ const resolveEditorLaunch = Effect.fn("resolveEditorLaunch")(function* ( · 113 unmodified lines
390420 });
391421
392422 const resolveFileManagerRevealLaunch = Effect.fn("externalLauncher.resolveFileManagerRevealLaunch")(
393− function* (input: RevealInFileManagerInput): Effect.fn.Return<EditorLaunch, never, Path.Path> {
423+ function* (
424+ input: RevealInFileManagerInput,
425+ ): Effect.fn.Return<EditorLaunch, ExternalLauncherError, Path.Path> {
394426 const platform = yield* HostProcessPlatform;
427+ const env = yield* readCommandLookupEnv;
428+ if (!hasGraphicalFileManagerSession(platform, env)) {
429+ return yield* new ExternalLauncherUnsupportedEditorError({ editor: "file-manager" });
430+ }
395431 const path = yield* Path.Path;
396− const args =
397− platform === "darwin"
398− ? ["-R", input.path]
399− : platform === "win32"
400− ? [`/select,"${input.path}"`]
401− : [path.dirname(input.path)];
432+ const args = [fileManagerFolderPath(platform, input.path, path)];
402433
403434 yield* Effect.annotateCurrentSpan({
404435 "externalLauncher.target": input.path,
apps/web/src/components/ChatMarkdown.tsx+13−6
@@ -51,7 +51,11 @@ import { · 50 unmodified lines
5151 resolveExternalWebLinkHost,
5252 showExternalLinkContextMenu,
5353 } from "./chat/externalLinkContextMenu";
54−import { buildFileLinkContextMenuItems } from "./chat/fileLinkContextMenu";
54+import {
55+ buildFileLinkContextMenuItems,
56+ canRevealFileLinkInManager,
57+ resolveFileLinkEnvironmentId,
58+} from "./chat/fileLinkContextMenu";
5559 import { hasSpecificPierreIconForFileName, syntheticFileNameForLanguageId } from "../pierre-icons";
5660 import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip";
5761 import { Button } from "./ui/button";
@@ -85,6 +89,7 @@ import { readLocalApi } from "../localApi"; · 27 unmodified lines
8589 import { cn } from "../lib/utils";
8690 import { useRightPanelStore } from "../rightPanelStore";
8791 import { useActiveEnvironmentId } from "../state/entities";
92+import { useEnvironment } from "../state/environments";
8893 import { serverEnvironment } from "../state/server";
8994 import { assetEnvironment } from "../state/assets";
9095 import { usePreparedConnection } from "../state/session";
@@ -1394,15 +1399,17 @@ function ChatMarkdown({ · 1303 unmodified lines
13941399 reportFailure: false,
13951400 });
13961401 const activeEnvironmentId = useActiveEnvironmentId();
1397− const environmentId = threadRef?.environmentId ?? activeEnvironmentId;
1402+ const environmentId = resolveFileLinkEnvironmentId(threadRef?.environmentId, activeEnvironmentId);
1403+ const environment = useEnvironment(environmentId);
13981404 const preparedConnection = usePreparedConnection(environmentId);
13991405 const serverConfig = useAtomValue(serverEnvironment.configValueAtom(environmentId));
14001406 const availableEditors = serverConfig?.availableEditors ?? [];
14011407 const openInPreferredEditor = useOpenInPreferredEditor(environmentId, availableEditors);
1402− const canRevealInFileManager =
1403− preparedConnection._tag === "Some" &&
1404− serverConfig?.environment.capabilities.fileManagerReveal === true &&
1405− availableEditors.includes("file-manager");
1408+ const canRevealInFileManager = canRevealFileLinkInManager({
1409+ connectionPhase: environment?.connection.phase,
1410+ supportsRevealRpc: serverConfig?.environment.capabilities.fileManagerReveal === true,
1411+ availableEditors,
1412+ });
14061413 const diffThemeName = resolveDiffThemeName(resolvedTheme);
14071414 const markdownFileLinkMetaByHref = useMemo(() => {
14081415 const metaByHref = new Map<
apps/web/src/components/chat/fileLinkContextMenu.test.ts+42−1
@@ -1,6 +1,47 @@
1+import { EnvironmentId } from "@t3tools/contracts";
12 import { describe, expect, it } from "vite-plus/test";
23
3−import { buildFileLinkContextMenuItems } from "./fileLinkContextMenu";
4+import {
5+ buildFileLinkContextMenuItems,
6+ canRevealFileLinkInManager,
7+ resolveFileLinkEnvironmentId,
8+} from "./fileLinkContextMenu";
9+
10+describe("file link environment", () => {
11+ it("routes through the thread environment before the active environment", () => {
12+ const threadEnvironmentId = EnvironmentId.make("thread-environment");
13+ expect(
14+ resolveFileLinkEnvironmentId(threadEnvironmentId, EnvironmentId.make("active-environment")),
15+ ).toBe(threadEnvironmentId);
16+ });
17+
18+ it("falls back to the active environment without thread context", () => {
19+ const activeEnvironmentId = EnvironmentId.make("active-environment");
20+ expect(resolveFileLinkEnvironmentId(undefined, activeEnvironmentId)).toBe(activeEnvironmentId);
21+ });
22+
23+ it.each([
24+ {
25+ connectionPhase: "reconnecting",
26+ supportsRevealRpc: true,
27+ availableEditors: ["file-manager"],
28+ },
29+ { connectionPhase: "connected", supportsRevealRpc: false, availableEditors: ["file-manager"] },
30+ { connectionPhase: "connected", supportsRevealRpc: true, availableEditors: [] },
31+ ] as const)("hides reveal when support is incomplete", (input) => {
32+ expect(canRevealFileLinkInManager(input)).toBe(false);
33+ });
34+
35+ it("allows reveal only when connected with rpc and file manager support", () => {
36+ expect(
37+ canRevealFileLinkInManager({
38+ connectionPhase: "connected",
39+ supportsRevealRpc: true,
40+ availableEditors: ["file-manager"],
41+ }),
42+ ).toBe(true);
43+ });
44+});
445
546 describe("chat file link context menu", () => {
647 it("puts Open in folder first when the environment supports it", () => {
apps/web/src/components/chat/fileLinkContextMenu.ts+20−1
@@ -1,4 +1,4 @@
1−import type { ContextMenuItem } from "@t3tools/contracts";
1+import type { ContextMenuItem, EditorId, EnvironmentId } from "@t3tools/contracts";
22
33 export type FileLinkContextMenuAction =
44 | "open-in-folder"
@@ -7,6 +7,25 @@ export type FileLinkContextMenuAction = · 2 unmodified lines
77 | "copy-relative"
88 | "copy-full";
99
10+export function resolveFileLinkEnvironmentId(
11+ threadEnvironmentId: EnvironmentId | undefined,
12+ activeEnvironmentId: EnvironmentId | null,
13+): EnvironmentId | null {
14+ return threadEnvironmentId ?? activeEnvironmentId;
15+}
16+
17+export function canRevealFileLinkInManager(input: {
18+ readonly connectionPhase: string | undefined;
19+ readonly supportsRevealRpc: boolean;
20+ readonly availableEditors: ReadonlyArray<EditorId>;
21+}): boolean {
22+ return (
23+ input.connectionPhase === "connected" &&
24+ input.supportsRevealRpc &&
25+ input.availableEditors.includes("file-manager")
26+ );
27+}
28+
1029 export function buildFileLinkContextMenuItems(input: {
1130 readonly canRevealInFileManager: boolean;
1231 readonly canOpenInBrowser: boolean;
apps/web/src/markdown-links.test.ts+21−0
@@ -108,6 +108,27 @@ describe("resolveMarkdownFileLinkTarget", () => { · 107 unmodified lines
108108 });
109109 });
110110
111+ it("resolves the exact file path used by open in folder", () => {
112+ expect(resolveMarkdownFileLinkMeta("src/file%20name.ts:12:4", "/repo/project")).toMatchObject({
113+ filePath: "/repo/project/src/file name.ts",
114+ targetPath: "/repo/project/src/file name.ts:12:4",
115+ line: 12,
116+ column: 4,
117+ });
118+ expect(resolveMarkdownFileLinkMeta("/tmp/report.ts:9", "/repo/project")).toMatchObject({
119+ filePath: "/tmp/report.ts",
120+ targetPath: "/tmp/report.ts:9",
121+ line: 9,
122+ });
123+ });
124+
125+ it("resolves missing paths without requiring browser-side file access", () => {
126+ expect(resolveMarkdownFileLinkMeta("src/not-created-yet.ts", "/repo/project")).toMatchObject({
127+ filePath: "/repo/project/src/not-created-yet.ts",
128+ workspaceRelativePath: "src/not-created-yet.ts",
129+ });
130+ });
131+
111132 it("normalizes slash-prefixed windows drive paths before resolving", () => {
112133 expect(
113134 resolveMarkdownFileLinkTarget(
apps/web/src/markdown-links.ts+2−2
@@ -5,8 +5,8 @@ const WINDOWS_DRIVE_PATH_PATTERN = /^[A-Za-z]:[\\/]/; · 4 unmodified lines
55 const WINDOWS_UNC_PATH_PATTERN = /^\\\\/;
66 const EXTERNAL_SCHEME_PATTERN = /^([A-Za-z][A-Za-z0-9+.-]*):(.*)$/;
77 const RELATIVE_PATH_PREFIX_PATTERN = /^(~\/|\.{1,2}\/)/;
8−const RELATIVE_FILE_PATH_PATTERN = /^[A-Za-z0-9._-]+(?:\/[A-Za-z0-9._-]+)+(?::\d+){0,2}$/;
9−const RELATIVE_FILE_NAME_PATTERN = /^[A-Za-z0-9._-]+\.[A-Za-z0-9_-]+(?::\d+){0,2}$/;
8+const RELATIVE_FILE_PATH_PATTERN = /^[A-Za-z0-9._ -]+(?:\/[A-Za-z0-9._ -]+)+(?::\d+){0,2}$/;
9+const RELATIVE_FILE_NAME_PATTERN = /^[A-Za-z0-9._ -]+\.[A-Za-z0-9_-]+(?::\d+){0,2}$/;
1010 const POSITION_SUFFIX_PATTERN = /:\d+(?::\d+)?$/;
1111 const POSITION_ONLY_PATTERN = /^\d+(?::\d+)?$/;
1212 // Standard OS and dev-container roots; deliberately excludes app-route-ish