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
3 changes: 2 additions & 1 deletion apps/desktop/src/shell/DesktopShellEnvironment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ describe("DesktopShellEnvironment", () => {
FNM_DIR: "C:\\Users\\testuser\\AppData\\Roaming\\fnm",
FNM_MULTISHELL_PATH: "C:\\Users\\testuser\\AppData\\Local\\fnm_multishells\\123",
})
: envOutput({ PATH: "C:\\Custom\\Bin;C:\\Windows\\System32" });
: envOutput({ PATH: 'C:\\Custom\\Bin;C:";C:\\Windows\\System32' });
},
});

Expand All @@ -337,6 +337,7 @@ describe("DesktopShellEnvironment", () => {
"C:\\Users\\testuser\\.bun\\bin",
"C:\\Users\\testuser\\scoop\\shims",
"C:\\Custom\\Bin",
"C:",
].join(";"),
);
assert.equal(env.FNM_DIR, "C:\\Users\\testuser\\AppData\\Roaming\\fnm");
Expand Down
11 changes: 7 additions & 4 deletions apps/desktop/src/shell/DesktopShellEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ const pathComparisonKey = (entry: string, platform: NodeJS.Platform) => {
return platform === "win32" ? normalized.toLowerCase() : normalized;
};

const sanitizePathEntry = (entry: string, platform: NodeJS.Platform) =>
platform === "win32" ? entry.replaceAll('"', "") : entry;

const mergePaths = (
platform: NodeJS.Platform,
values: ReadonlyArray<Option.Option<string>>,
Expand All @@ -163,14 +166,14 @@ const mergePaths = (
if (Option.isNone(value)) continue;

for (const entry of value.value.split(delimiter)) {
const trimmed = entry.trim();
if (trimmed.length === 0) continue;
const sanitized = sanitizePathEntry(entry.trim(), platform);
if (sanitized.length === 0) continue;

const key = pathComparisonKey(trimmed, platform);
const key = pathComparisonKey(sanitized, platform);
if (key.length === 0 || seen.has(key)) continue;

seen.add(key);
entries.push(trimmed);
entries.push(sanitized);
}
}

Expand Down
14 changes: 10 additions & 4 deletions apps/server/src/process/externalLauncher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ it.effect("reveals a file in File Explorer through PowerShell on Windows", () =>
const launcher = yield* ExternalLauncher.ExternalLauncher;
yield* launcher.launchEditor({
editor: "file-manager",
cwd: "C:\\workspace with spaces\\media\\author's clip.mp4",
// Web file links normalize separators even when the server runs on
// Windows. Explorer's `/select` switch requires Windows separators.
cwd: "C:/workspace with spaces/media/author's clip.mp4",
reveal: true,
});
return yield* launcher.resolveFileManagerRevealKind();
Expand Down Expand Up @@ -261,8 +263,12 @@ it.skipIf(process.platform !== "win32")(
const outputPath = NodePath.join(tempDir, "argv.txt");
NodeFS.writeFileSync(recorderPath, `@echo off\r\n>"${outputPath}" echo(%*\r\n`);

const target = "C:\\workspace with spaces\\media\\author's clip.mp4";
const source = ExternalLauncher.buildFileExplorerRevealPowerShellSource(recorderPath, target);
const target = "C:/workspace with spaces/media/author's clip.mp4";
const explorerTarget = target.replaceAll("/", "\\");
const source = ExternalLauncher.buildFileExplorerRevealPowerShellSource(
recorderPath,
explorerTarget,
);
const powerShellPath = `${process.env.SYSTEMROOT ?? "C:\\Windows"}\\System32\\WindowsPowerShell\\v1.0\\powershell.exe`;
NodeChildProcess.execFileSync(
powerShellPath,
Expand Down Expand Up @@ -290,7 +296,7 @@ it.skipIf(process.platform !== "win32")(
}
await sleep(200);
const recorded = NodeFS.readFileSync(outputPath, "utf8").trim();
assert.equal(recorded, `/select,"${target}"`);
assert.equal(recorded, `/select,"${explorerTarget}"`);
} finally {
NodeFS.rmSync(tempDir, { recursive: true, force: true });
}
Expand Down
10 changes: 9 additions & 1 deletion apps/server/src/process/externalLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,10 @@ function fileExplorerRevealLaunch(
};
}

function normalizeWindowsFileManagerPath(target: string): string {
return target.replaceAll("/", "\\");
}

const resolveFileManagerRevealLaunch = Effect.fn("resolveFileManagerRevealLaunch")(function* (
target: string,
platform: NodeJS.Platform,
Expand All @@ -626,7 +630,11 @@ const resolveFileManagerRevealLaunch = Effect.fn("resolveFileManagerRevealLaunch
}

if (platform === "win32") {
return fileExplorerRevealLaunch(target, target, resolvePowerShellPath(env));
return fileExplorerRevealLaunch(
target,
normalizeWindowsFileManagerPath(target),
resolvePowerShellPath(env),
);
}

if (
Expand Down
24 changes: 17 additions & 7 deletions packages/shared/src/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,18 +304,28 @@ describe("readEnvironmentFromWindowsShell", () => {
});

describe("mergePathValues", () => {
it("dedupes case-insensitively on Windows while preserving preferred order", () => {
it("sanitizes and dedupes Windows entries while preserving preferred order", () => {
expect(
mergePathValues(
'C:\\Users\\testuser\\AppData\\Roaming\\npm;"C:\\Program Files\\nodejs"',
"c:\\users\\testuser\\appdata\\roaming\\npm;C:\\Windows\\System32",
"win32",
),
).toBe(
'C:\\Users\\testuser\\AppData\\Roaming\\npm;"C:\\Program Files\\nodejs";C:\\Windows\\System32',
"C:\\Users\\testuser\\AppData\\Roaming\\npm;C:\\Program Files\\nodejs;C:\\Windows\\System32",
);
});

it("removes stray quotes from Windows entries", () => {
expect(
mergePathValues(
'C:\\Windows\\System32;C:\\cloudflared.exe;C:";C:\\Program Files\\nodejs',
undefined,
"win32",
),
).toBe("C:\\Windows\\System32;C:\\cloudflared.exe;C:;C:\\Program Files\\nodejs");
});

it("dedupes case-sensitively on POSIX", () => {
expect(mergePathValues("/usr/local/bin:/usr/bin", "/usr/bin:/USR/BIN", "linux")).toBe(
"/usr/local/bin:/usr/bin:/USR/BIN",
Expand Down Expand Up @@ -450,7 +460,7 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
});

effectIt.layer(NodeServices.layer)("resolveWindowsEnvironment", (it) => {
it.effect("returns the baseline no-profile PATH patch when node is already available", () =>
it.effect("uses known CLI directories as a fallback without changing shell PATH priority", () =>
Effect.gen(function* () {
const readEnvironment = vi.fn(
(_names: ReadonlyArray<string>, options?: { loadProfile?: boolean }) =>
Expand All @@ -473,15 +483,15 @@ effectIt.layer(NodeServices.layer)("resolveWindowsEnvironment", (it) => {
),
).toEqual({
PATH: [
"C:\\Shell\\Bin",
"C:\\Windows\\System32",
"C:\\Users\\testuser\\AppData\\Roaming\\npm",
"C:\\Users\\testuser\\AppData\\Local\\Programs\\nodejs",
"C:\\Users\\testuser\\AppData\\Local\\Volta\\bin",
"C:\\Users\\testuser\\AppData\\Local\\pnpm",
"C:\\Users\\testuser\\.local\\bin",
"C:\\Users\\testuser\\.bun\\bin",
"C:\\Users\\testuser\\scoop\\shims",
"C:\\Shell\\Bin",
"C:\\Windows\\System32",
].join(";"),
});
expect(readEnvironment).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -522,14 +532,14 @@ effectIt.layer(NodeServices.layer)("resolveWindowsEnvironment", (it) => {
PATH: [
"C:\\Profile\\Node",
"C:\\Windows\\System32",
"C:\\Shell\\Bin",
"C:\\Users\\testuser\\AppData\\Roaming\\npm",
"C:\\Users\\testuser\\AppData\\Local\\Programs\\nodejs",
"C:\\Users\\testuser\\AppData\\Local\\Volta\\bin",
"C:\\Users\\testuser\\AppData\\Local\\pnpm",
"C:\\Users\\testuser\\.local\\bin",
"C:\\Users\\testuser\\.bun\\bin",
"C:\\Users\\testuser\\scoop\\shims",
"C:\\Shell\\Bin",
].join(";"),
FNM_DIR: "C:\\Users\\testuser\\AppData\\Roaming\\fnm",
FNM_MULTISHELL_PATH: "C:\\Users\\testuser\\AppData\\Local\\fnm_multishells\\123",
Expand Down Expand Up @@ -566,11 +576,11 @@ effectIt.layer(NodeServices.layer)("resolveWindowsEnvironment", (it) => {
),
).toEqual({
PATH: [
"C:\\Windows\\System32",
"C:\\Users\\testuser\\AppData\\Roaming\\npm",
"C:\\Users\\testuser\\.local\\bin",
"C:\\Users\\testuser\\.bun\\bin",
"C:\\Users\\testuser\\scoop\\shims",
"C:\\Windows\\System32",
].join(";"),
FNM_DIR: "C:\\Users\\testuser\\AppData\\Roaming\\fnm",
});
Expand Down
16 changes: 11 additions & 5 deletions packages/shared/src/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,10 @@ function normalizePathEntryForComparison(entry: string, platform: NodeJS.Platfor
return platform === "win32" ? normalized.toLowerCase() : normalized;
}

function sanitizePathEntry(entry: string, platform: NodeJS.Platform): string {
return platform === "win32" ? entry.replaceAll('"', "") : entry;
}

export function mergePathValues(
preferredPath: string | undefined,
inheritedPath: string | undefined,
Expand All @@ -427,14 +431,14 @@ export function mergePathValues(
if (!rawValue) continue;

for (const entry of rawValue.split(delimiter)) {
const trimmed = entry.trim();
if (trimmed.length === 0) continue;
const sanitized = sanitizePathEntry(entry.trim(), platform);
if (sanitized.length === 0) continue;

const normalized = normalizePathEntryForComparison(trimmed, platform);
const normalized = normalizePathEntryForComparison(sanitized, platform);
if (normalized.length === 0 || seen.has(normalized)) continue;

seen.add(normalized);
merged.push(trimmed);
merged.push(sanitized);
}
}

Expand Down Expand Up @@ -724,7 +728,9 @@ export const resolveWindowsEnvironment = Effect.fn("shell.resolveWindowsEnvironm
}).PATH;
const mergedPath = mergePathValues(shellPath, inheritedPath, "win32");
const knownCliPath = resolveKnownWindowsCliDirs(env).join(WINDOWS_PATH_DELIMITER);
const baselinePath = mergePathValues(knownCliPath, mergedPath, "win32");
// Preserve the order a user's shell uses. These directories fill gaps when
// desktop apps launch without the full interactive-shell PATH.
const baselinePath = mergePathValues(mergedPath, knownCliPath, "win32");
const baselinePatch: Partial<NodeJS.ProcessEnv> = baselinePath ? { PATH: baselinePath } : {};
const baselineEnv = mergeWindowsEnv(env, baselinePatch);

Expand Down
Loading