From 302323a64b0e4342af013784f4422020b7c1fa53 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 2 Mar 2026 18:39:58 -0800 Subject: [PATCH] Fix Windows command detection and branch checkout error flow - tighten win32 command availability checks for PATHEXT and PATH delimiter handling - add server tests for Windows executable resolution edge cases - stop branch/create flows after checkout/create failures to avoid stale optimistic UI - remove OpenInPicker side effect that persisted last editor on render --- apps/server/src/open.test.ts | 35 ++++++++++++++++ apps/server/src/open.ts | 49 ++++++++++++++++++----- apps/web/src/components/BranchToolbar.tsx | 3 ++ apps/web/src/components/ChatView.tsx | 6 --- 4 files changed, 76 insertions(+), 17 deletions(-) diff --git a/apps/server/src/open.test.ts b/apps/server/src/open.test.ts index 3a55bde8569e8ff2abd82a360c9882a99be0cc25..0f864554e91e1808b20ab33d840134fe1fa0e224 100644 --- a/apps/server/src/open.test.ts +++ b/apps/server/src/open.test.ts @@ -167,6 +167,41 @@ describe("isCommandAvailable", () => { } satisfies NodeJS.ProcessEnv; assert.equal(isCommandAvailable("definitely-not-installed", { platform: "win32", env }), false); }); + + it("does not treat bare files without executable extension as available on win32", () => { + withTempDir((dir) => { + fs.writeFileSync(path.join(dir, "npm"), "echo nope\r\n", "utf8"); + const env = { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + } satisfies NodeJS.ProcessEnv; + assert.equal(isCommandAvailable("npm", { platform: "win32", env }), false); + }); + }); + + it("appends PATHEXT for commands with non-executable extensions on win32", () => { + withTempDir((dir) => { + fs.writeFileSync(path.join(dir, "my.tool.CMD"), "@echo off\r\n", "utf8"); + const env = { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + } satisfies NodeJS.ProcessEnv; + assert.equal(isCommandAvailable("my.tool", { platform: "win32", env }), true); + }); + }); + + it("uses platform-specific PATH delimiter for platform overrides", () => { + withTempDir((firstDir) => { + withTempDir((secondDir) => { + fs.writeFileSync(path.join(secondDir, "code.CMD"), "@echo off\r\n", "utf8"); + const env = { + PATH: `${firstDir};${secondDir}`, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + } satisfies NodeJS.ProcessEnv; + assert.equal(isCommandAvailable("code", { platform: "win32", env }), true); + }); + }); + }); }); describe("resolveAvailableEditors", () => { diff --git a/apps/server/src/open.ts b/apps/server/src/open.ts index b15ec72de36d72176dde8c577ab295caebca9ccc..eff87b39b4de2deb3200f8508af763d5b445e41d 100644 --- a/apps/server/src/open.ts +++ b/apps/server/src/open.ts @@ -8,7 +8,7 @@ */ import { spawn } from "node:child_process"; import { accessSync, constants, statSync } from "node:fs"; -import { delimiter, extname, join } from "node:path"; +import { extname, join } from "node:path"; import { EDITORS, type EditorId } from "@t3tools/contracts"; import { ServiceMap, Schema, Effect, Layer } from "effect"; @@ -78,24 +78,44 @@ function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray, ): ReadonlyArray { if (platform !== "win32") return [command]; - if (extname(command).length > 0) return [command]; + const extension = extname(command); + const normalizedExtension = extension.toUpperCase(); - const candidates = [command]; - for (const extension of resolveWindowsPathExtensions(env)) { + if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) { + const commandWithoutExtension = command.slice(0, -extension.length); + return Array.from( + new Set([ + command, + `${commandWithoutExtension}${normalizedExtension}`, + `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`, + ]), + ); + } + + const candidates: string[] = []; + for (const extension of windowsPathExtensions) { candidates.push(`${command}${extension}`); candidates.push(`${command}${extension.toLowerCase()}`); } return Array.from(new Set(candidates)); } -function isExecutableFile(filePath: string, platform: NodeJS.Platform): boolean { +function isExecutableFile( + filePath: string, + platform: NodeJS.Platform, + windowsPathExtensions: ReadonlyArray, +): boolean { try { const stat = statSync(filePath); if (!stat.isFile()) return false; - if (platform === "win32") return true; + if (platform === "win32") { + const extension = extname(filePath); + if (extension.length === 0) return false; + return windowsPathExtensions.includes(extension.toUpperCase()); + } accessSync(filePath, constants.X_OK); return true; } catch { @@ -103,28 +123,35 @@ function isExecutableFile(filePath: string, platform: NodeJS.Platform): boolean } } +function resolvePathDelimiter(platform: NodeJS.Platform): string { + return platform === "win32" ? ";" : ":"; +} + export function isCommandAvailable( command: string, options: CommandAvailabilityOptions = {}, ): boolean { const platform = options.platform ?? process.platform; const env = options.env ?? process.env; - const commandCandidates = resolveCommandCandidates(command, platform, env); + const windowsPathExtensions = platform === "win32" ? resolveWindowsPathExtensions(env) : []; + const commandCandidates = resolveCommandCandidates(command, platform, windowsPathExtensions); if (command.includes("/") || command.includes("\\")) { - return commandCandidates.some((candidate) => isExecutableFile(candidate, platform)); + return commandCandidates.some((candidate) => + isExecutableFile(candidate, platform, windowsPathExtensions), + ); } const pathValue = resolvePathEnvironmentVariable(env); if (pathValue.length === 0) return false; const pathEntries = pathValue - .split(delimiter) + .split(resolvePathDelimiter(platform)) .map((entry) => stripWrappingQuotes(entry.trim())) .filter((entry) => entry.length > 0); for (const pathEntry of pathEntries) { for (const candidate of commandCandidates) { - if (isExecutableFile(join(pathEntry, candidate), platform)) { + if (isExecutableFile(join(pathEntry, candidate), platform, windowsPathExtensions)) { return true; } } diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index c4560b110812d8d631aa8a7808c831d28412ebd6..11112e39476a1b6f1def4e6e0f3502dd252175d2 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -183,6 +183,7 @@ export default function BranchToolbar({ title: "Failed to checkout branch.", description: error instanceof Error ? error.message : "An error occurred.", }); + return; } let nextBranchName = selectedBranchName; @@ -219,6 +220,7 @@ export default function BranchToolbar({ title: "Failed to checkout branch.", description: error instanceof Error ? error.message : "An error occurred.", }); + return; } } catch (error) { toastManager.add({ @@ -226,6 +228,7 @@ export default function BranchToolbar({ title: "Failed to create branch.", description: error instanceof Error ? error.message : "An error occurred.", }); + return; } setOptimisticBranch(name); diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index f6a6142f5c559d6cd574eda93a2e6a722d0ab619..abd8bbc202fd6dfbc28551776abfa3c5840ce138 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -3720,12 +3720,6 @@ const OpenInPicker = memo(function OpenInPicker({ : (options[0]?.value ?? null); const primaryOption = options.find(({ value }) => value === effectiveEditor) ?? null; - useEffect(() => { - if (!effectiveEditor || effectiveEditor === lastEditor) return; - localStorage.setItem(LAST_EDITOR_KEY, effectiveEditor); - setLastEditor(effectiveEditor); - }, [effectiveEditor, lastEditor]); - const activeThread = useStore((store) => activeThreadId ? store.threads.find((thread) => thread.id === activeThreadId) : undefined, ); -- 2.54.0