diff --git a/__tests__/components/features/home/workspace-selection-form.test.tsx b/__tests__/components/features/home/workspace-selection-form.test.tsx index 80a36ff3a1..5c96e7a06d 100644 --- a/__tests__/components/features/home/workspace-selection-form.test.tsx +++ b/__tests__/components/features/home/workspace-selection-form.test.tsx @@ -336,6 +336,51 @@ describe("WorkspaceSelectionForm (server-backed workspaces)", () => { ]); }); + it("handles Windows paths when browsing and adding a workspace", async () => { + const homePath = String.raw`C:\Users\me`; + const devPath = String.raw`C:\Users\me\dev`; + const addSpy = vi + .spyOn(WorkspacesService, "addWorkspaces") + .mockResolvedValue({ workspaces: [], workspaceParents: [] }); + mockGetHome.mockResolvedValue({ home: homePath }); + mockSearchSubdirectories.mockImplementation(async (dir: string) => { + if (dir === homePath) { + return { + items: [{ name: "dev", path: devPath }], + next_page_id: null, + }; + } + return { items: [], next_page_id: null }; + }); + renderForm(); + const user = userEvent.setup(); + + await user.click(await screen.findByTestId("workspace-dropdown")); + await user.click(await screen.findByTestId("add-workspaces-button")); + await screen.findByTestId("folder-browser-modal"); + await expect( + screen.getByTestId("folder-browser-current-path"), + ).toHaveTextContent(homePath); + + await user.click(await screen.findByTestId("folder-browser-entry-dev")); + await expect( + screen.getByTestId("folder-browser-current-path"), + ).toHaveTextContent(devPath); + + await user.click(screen.getByTestId("folder-browser-up")); + await expect( + screen.getByTestId("folder-browser-current-path"), + ).toHaveTextContent(homePath); + + await user.click(await screen.findByTestId("folder-browser-entry-dev")); + await user.click(screen.getByTestId("folder-browser-use")); + + await waitFor(() => expect(addSpy).toHaveBeenCalledTimes(1)); + expect(addSpy).toHaveBeenCalledWith([ + { id: devPath, name: "dev", path: devPath }, + ]); + }); + it("auto-selects the newly added workspace after Add Workspace", async () => { // Arrange: start with another workspace already selected, mirroring the // repro in OpenHands/agent-canvas#1212. diff --git a/__tests__/e2e/mock-llm-folder-workspace-paths.test.ts b/__tests__/e2e/mock-llm-folder-workspace-paths.test.ts new file mode 100644 index 0000000000..7582822f9a --- /dev/null +++ b/__tests__/e2e/mock-llm-folder-workspace-paths.test.ts @@ -0,0 +1,43 @@ +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import { + getFolderBrowserPathSegments, + getFolderBrowserRootPath, + resolveFolderWorkspacePaths, + TEST_DIR_NAME, + WORKSPACE_DIR_NAME, +} from "../../tests/e2e/mock-llm/utils/folder-workspace-paths"; + +describe("mock-LLM folder workspace paths", () => { + it("keeps npm-mode Windows host and agent-server paths identical", () => { + const tmpDir = String.raw`C:\Users\me\AppData\Local\Temp`; + + const paths = resolveFolderWorkspacePaths({ + tmpDir, + env: {}, + }); + + const expectedBase = path.win32.join(tmpDir, WORKSPACE_DIR_NAME); + const expectedDir = path.win32.join(expectedBase, TEST_DIR_NAME); + expect(paths.hostDirBase).toBe(expectedBase); + expect(paths.containerDirBase).toBe(expectedBase); + expect(paths.hostDir).toBe(expectedDir); + expect(paths.testDir).toBe(expectedDir); + }); + + it("derives Windows folder-browser roots and path segments", () => { + const target = String.raw`C:\Users\me\AppData\Local\Temp\e2e-folder-workspace-test\my-test-project`; + + expect(getFolderBrowserRootPath(target)).toBe("C:\\"); + expect(getFolderBrowserPathSegments(target)).toEqual([ + "Users", + "me", + "AppData", + "Local", + "Temp", + "e2e-folder-workspace-test", + "my-test-project", + ]); + }); +}); diff --git a/__tests__/scripts/lint-staged-portability.test.ts b/__tests__/scripts/lint-staged-portability.test.ts new file mode 100644 index 0000000000..414bcf1b30 --- /dev/null +++ b/__tests__/scripts/lint-staged-portability.test.ts @@ -0,0 +1,36 @@ +// @vitest-environment node +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../..", +); + +describe("lint-staged portability", () => { + it("runs staged typecheck through Node instead of a POSIX shell", () => { + const packageJson = JSON.parse( + readFileSync(path.join(repoRoot, "package.json"), "utf-8"), + ) as { + "lint-staged": Record; + }; + + const commands = Object.values(packageJson["lint-staged"]).flat(); + + expect(commands).not.toContain("bash -c 'npm run typecheck:staged'"); + expect(commands).toContain("node scripts/run-staged-typecheck.mjs"); + }); + + it("uses npm's JS CLI on Windows instead of spawning npm.cmd directly", () => { + const source = readFileSync( + path.join(repoRoot, "scripts", "run-staged-typecheck.mjs"), + "utf-8", + ); + + expect(source).toContain('"npm-cli.js"'); + expect(source).not.toContain('"npm.cmd"'); + expect(source).toContain("spawnSync"); + }); +}); diff --git a/package.json b/package.json index 89cc53896e..3cce306a16 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ "prettier --write" ], "src/**/*.{ts,tsx}": [ - "bash -c 'npm run typecheck:staged'" + "node scripts/run-staged-typecheck.mjs" ], "src/**/*": [ "npm run check-translation-completeness" diff --git a/scripts/run-staged-typecheck.mjs b/scripts/run-staged-typecheck.mjs new file mode 100644 index 0000000000..96baef01ea --- /dev/null +++ b/scripts/run-staged-typecheck.mjs @@ -0,0 +1,44 @@ +#!/usr/bin/env node +import { spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { dirname, join } from "node:path"; + +function getNpmInvocation() { + if (process.platform !== "win32") { + return { command: "npm", args: ["run", "typecheck:staged"] }; + } + + const npmCliPath = + process.env.npm_execpath ?? + join(dirname(process.execPath), "node_modules", "npm", "bin", "npm-cli.js"); + + if (existsSync(npmCliPath)) { + return { + command: process.execPath, + args: [npmCliPath, "run", "typecheck:staged"], + }; + } + + return { + command: process.env.ComSpec ?? "cmd.exe", + args: ["/d", "/s", "/c", "npm run typecheck:staged"], + }; +} + +const { command, args } = getNpmInvocation(); + +const result = spawnSync(command, args, { + stdio: "inherit", + windowsHide: true, +}); + +if (result.error) { + throw result.error; +} + +if (result.signal) { + console.error(`typecheck:staged terminated by signal ${result.signal}`); + process.exit(1); +} + +process.exit(result.status ?? 1); diff --git a/src/components/features/home/workspace-dropdown/folder-browser-modal.tsx b/src/components/features/home/workspace-dropdown/folder-browser-modal.tsx index 51368fa40e..0c0933c6e9 100644 --- a/src/components/features/home/workspace-dropdown/folder-browser-modal.tsx +++ b/src/components/features/home/workspace-dropdown/folder-browser-modal.tsx @@ -82,11 +82,32 @@ function SidebarSection({ } function getParentPath(path: string): string | null { - const trimmed = path.replace(/\/+$/, ""); - if (!trimmed || trimmed === "/") return null; - const idx = trimmed.lastIndexOf("/"); - if (idx <= 0) return "/"; - return trimmed.slice(0, idx); + const trimmed = trimTrailingSeparators(path); + if (!trimmed || trimmed === "/" || isWindowsDriveRoot(trimmed)) return null; + + const idx = Math.max(trimmed.lastIndexOf("/"), trimmed.lastIndexOf("\\")); + if (idx < 0) return null; + if (idx === 0) return "/"; + + const parent = trimmed.slice(0, idx); + if (/^[A-Za-z]:$/.test(parent)) { + return `${parent}${trimmed[idx]}`; + } + + return parent; +} + +function isWindowsDriveRoot(path: string): boolean { + return /^[A-Za-z]:[\\/]?$/.test(path); +} + +function trimTrailingSeparators(path: string): string { + const trimmed = path.replace(/[\\/]+$/, ""); + if (/^[A-Za-z]:$/.test(trimmed)) { + const separator = path.includes("/") && !path.includes("\\") ? "/" : "\\"; + return `${trimmed}${separator}`; + } + return trimmed; } function shouldDefaultToProjectsPath( @@ -134,7 +155,7 @@ export function FolderBrowserModal({ const favorites: SidebarEntry[] = useMemo(() => { if (!homeData?.home) return []; - const trimmed = homeData.home.replace(/[\\/]+$/, "") || homeData.home; + const trimmed = trimTrailingSeparators(homeData.home) || homeData.home; const backendFavorites = [ { label: "Home", path: trimmed }, ...(homeData.favorites ?? []), @@ -173,10 +194,11 @@ export function FolderBrowserModal({ subdirs.length === 0; const getBasename = (path: string): string => { - const trimmed = path.replace(/\/+$/, ""); + const trimmed = trimTrailingSeparators(path); if (!trimmed) return "/"; - const idx = trimmed.lastIndexOf("/"); - return idx >= 0 ? trimmed.slice(idx + 1) || "/" : trimmed; + if (trimmed === "/" || isWindowsDriveRoot(trimmed)) return trimmed; + const idx = Math.max(trimmed.lastIndexOf("/"), trimmed.lastIndexOf("\\")); + return idx >= 0 ? trimmed.slice(idx + 1) || trimmed : trimmed; }; const handleAddDirectory = () => { diff --git a/tests/e2e/mock-llm/mock-llm-folder-workspace.spec.ts b/tests/e2e/mock-llm/mock-llm-folder-workspace.spec.ts index e7ab7cd5e7..491aec571f 100644 --- a/tests/e2e/mock-llm/mock-llm-folder-workspace.spec.ts +++ b/tests/e2e/mock-llm/mock-llm-folder-workspace.spec.ts @@ -27,8 +27,12 @@ import { deleteConversation, } from "./utils/mock-llm-helpers"; import * as fs from "fs"; -import * as os from "os"; -import * as path from "path"; +import { + getFolderBrowserPathSegments, + getFolderBrowserRootPath, + resolveFolderWorkspacePaths, + TEST_DIR_NAME, +} from "./utils/folder-workspace-paths"; /** * The folder-workspace test creates a directory that the agent-server's folder @@ -43,20 +47,11 @@ import * as path from "path"; * **npm mode**: Host IS the agent-server, so both paths resolve identically * via os.tmpdir(). */ -const WORKSPACE_DIR_NAME = "e2e-folder-workspace-test"; -const HOST_DIR_BASE = - process.env.MOCK_LLM_FOLDER_WORKSPACE_HOST_DIR ?? - path.join(os.tmpdir(), WORKSPACE_DIR_NAME); -/** Container-side path the agent-server sees (always POSIX). In npm mode - * this equals HOST_DIR_BASE; in Docker mode it is set by the config. */ -const CONTAINER_DIR_BASE = - process.env.MOCK_LLM_FOLDER_WORKSPACE_CONTAINER_DIR ?? - path.posix.join(os.tmpdir(), WORKSPACE_DIR_NAME); -const TEST_DIR_NAME = "my-test-project"; -/** Host-side path where we create the directory via fs.mkdirSync. */ -const HOST_DIR = path.join(HOST_DIR_BASE, TEST_DIR_NAME); -/** Container-side path the folder browser UI navigates to (POSIX). */ -const TEST_DIR = path.posix.join(CONTAINER_DIR_BASE, TEST_DIR_NAME); +const { + hostDirBase: HOST_DIR_BASE, + hostDir: HOST_DIR, + testDir: TEST_DIR, +} = resolveFolderWorkspacePaths(); const METADATA_STORAGE_KEY = "openhands-agent-server-conversation-metadata"; @@ -140,9 +135,9 @@ test.describe("mock-LLM folder browser → workspace → conversation", () => { // ── Open the "Open Workspace" dialog ── await test.step("open workspace dialog", async () => { await page.getByTestId("open-workspace-button").click(); - await expect( - page.getByTestId("open-workspace-dialog-body"), - ).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId("open-workspace-dialog-body")).toBeVisible({ + timeout: 10_000, + }); }); // ── Browse to the test directory using the folder browser UI ── @@ -159,6 +154,7 @@ test.describe("mock-LLM folder browser → workspace → conversation", () => { // until we reach "/" (path shows "/" or up button is disabled). const upBtn = page.getByTestId("folder-browser-up"); const currentPathEl = page.getByTestId("folder-browser-current-path"); + const rootPath = getFolderBrowserRootPath(TEST_DIR); // Wait for the modal to finish initializing. `currentPath` starts as // null (rendering an empty path and a disabled up button) until @@ -173,11 +169,11 @@ test.describe("mock-LLM folder browser → workspace → conversation", () => { await upBtn.click(); await page.waitForTimeout(300); } - await expect(currentPathEl).toHaveText("/", { timeout: 5_000 }); + await expect(currentPathEl).toHaveText(rootPath, { timeout: 5_000 }); // Navigate down through each segment of the test directory path. // e.g. /tmp/e2e-folder-workspace-test/my-test-project → ["tmp", "e2e-...", "my-test-project"] - const segments = TEST_DIR.split(path.posix.sep).filter(Boolean); + const segments = getFolderBrowserPathSegments(TEST_DIR); for (const segment of segments) { const entry = page.getByTestId(`folder-browser-entry-${segment}`); await expect(entry).toBeVisible({ timeout: 10_000 }); @@ -202,9 +198,9 @@ test.describe("mock-LLM folder browser → workspace → conversation", () => { // dropdown should now include it. await test.step("select the workspace in the dropdown and confirm", async () => { // The workspace dialog should still be visible - await expect( - page.getByTestId("open-workspace-dialog-body"), - ).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId("open-workspace-dialog-body")).toBeVisible({ + timeout: 10_000, + }); // The workspace dropdown should contain our test directory. const dropdown = page.getByTestId("workspace-dropdown"); @@ -223,17 +219,17 @@ test.describe("mock-LLM folder browser → workspace → conversation", () => { await confirmBtn.click(); // The dialog should close - await expect( - page.getByTestId("open-workspace-dialog-body"), - ).toBeHidden({ timeout: 5_000 }); + await expect(page.getByTestId("open-workspace-dialog-body")).toBeHidden({ + timeout: 5_000, + }); }); // ── Type a message and submit to create a conversation ── await test.step("submit a message to create a conversation", async () => { // Type into the home-page chat input (contentEditable div) - const chatInput = page.getByTestId("home-chat-launcher").locator( - '[contenteditable="true"]', - ); + const chatInput = page + .getByTestId("home-chat-launcher") + .locator('[contenteditable="true"]'); await expect(chatInput).toBeVisible({ timeout: 10_000 }); await chatInput.click(); diff --git a/tests/e2e/mock-llm/utils/folder-workspace-paths.ts b/tests/e2e/mock-llm/utils/folder-workspace-paths.ts new file mode 100644 index 0000000000..6764ba76c2 --- /dev/null +++ b/tests/e2e/mock-llm/utils/folder-workspace-paths.ts @@ -0,0 +1,49 @@ +import * as os from "node:os"; +import * as path from "node:path"; + +export const WORKSPACE_DIR_NAME = "e2e-folder-workspace-test"; +export const TEST_DIR_NAME = "my-test-project"; + +interface ResolveFolderWorkspacePathsOptions { + env?: NodeJS.ProcessEnv; + tmpDir?: string; +} + +export function resolveFolderWorkspacePaths({ + env = process.env, + tmpDir = os.tmpdir(), +}: ResolveFolderWorkspacePathsOptions = {}) { + const hostDirBase = + env.MOCK_LLM_FOLDER_WORKSPACE_HOST_DIR ?? + joinRuntimePath(tmpDir, WORKSPACE_DIR_NAME); + const containerDirBase = + env.MOCK_LLM_FOLDER_WORKSPACE_CONTAINER_DIR ?? hostDirBase; + + return { + hostDirBase, + containerDirBase, + hostDir: joinRuntimePath(hostDirBase, TEST_DIR_NAME), + testDir: joinRuntimePath(containerDirBase, TEST_DIR_NAME), + }; +} + +export function getFolderBrowserRootPath(targetPath: string): string { + const parser = getRuntimePathParser(targetPath); + return parser.parse(targetPath).root || "/"; +} + +export function getFolderBrowserPathSegments(targetPath: string): string[] { + const root = getFolderBrowserRootPath(targetPath); + const relativePath = targetPath.slice(root.length); + return relativePath.split(/[\\/]+/).filter(Boolean); +} + +function joinRuntimePath(parent: string, child: string): string { + return getRuntimePathParser(parent).join(parent, child); +} + +function getRuntimePathParser(targetPath: string) { + return /^[A-Za-z]:[\\/]/.test(targetPath) || targetPath.includes("\\") + ? path.win32 + : path.posix; +}