From a9f36b8f28bde5abe8435f4978445905a747e67e Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 10:03:49 +0800 Subject: [PATCH 1/4] Add folder creation and multi-file drag moves --- FEATURES.md | 10 + docs/ARCHITECTURE.md | 14 + docs/DEPLOYMENT.md | 4 + .../src/connections/profile-process.test.ts | 41 ++ server/src/index.ts | 13 + server/src/workspace/atomic-file-move.ts | 110 ++++ server/src/workspace/file-mutations.test.ts | 480 +++++++++++++++ server/src/workspace/file-paths.ts | 12 + server/src/workspace/files.ts | 41 ++ server/src/workspace/local-files.ts | 36 ++ server/src/workspace/remote-files.ts | 53 ++ web/src/components/FileExplorerDialog.css | 29 +- .../components/FileExplorerDialog.dom.test.ts | 551 +++++++++++++++++ web/src/components/FileExplorerDialog.tsx | 581 ++++++++++++++---- .../components/fileExplorerSelection.test.ts | 32 + web/src/components/fileExplorerSelection.ts | 22 + 16 files changed, 1906 insertions(+), 123 deletions(-) create mode 100644 server/src/workspace/atomic-file-move.ts create mode 100644 server/src/workspace/file-mutations.test.ts create mode 100644 web/src/components/FileExplorerDialog.dom.test.ts create mode 100644 web/src/components/fileExplorerSelection.test.ts create mode 100644 web/src/components/fileExplorerSelection.ts diff --git a/FEATURES.md b/FEATURES.md index a4288e27..35b4d16f 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -411,6 +411,16 @@ after fetch, and abort conflicts. They never push. or long-press. Upload/delete stay checkout-scoped. Operations work over SSH. Desktop layout downloads directly, including installed web apps. Mobile layout keeps native iOS sharing and its browser fallback behavior. +- Create folders in the checkout root with **New folder**, or inside a folder + through its actions menu. Right-click the file tree background to create a + folder in the checkout root or refresh files. Select files with Cmd/Ctrl-click + or Shift-click, then drag them onto a folder (or empty tree space for the + checkout root) to move them. Cmd/Ctrl+A selects visible files; Space selects + the focused file. + Moves never overwrite existing names; failed files remain selected. Moves + require source and destination to be on the same filesystem; SSH moves also + require Python 3 on the remote host. Read-only host browsing does not allow + creating folders or moving files. - With [host file reveal enabled](docs/DEPLOYMENT.md#host-file-reveal), local profiles over loopback offer **Reveal on host** and **Open folder on host** in explorer and Changes menus. These open the Roamgate host's desktop, not diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index bd349972..cca45e01 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -347,6 +347,20 @@ view. Search filters loaded entries only. Absolute previews use `scope=filesystem` download URLs; relative Markdown links and images resolve beside their source. Upload/delete remain checkout-scoped. +`file.mkdir` takes `workspace_id`, a relative `directory` (empty for the root), +and a single-component `name`. `file.move` takes `workspace_id`, a relative +source `path`, and a relative destination `directory`. Both reject filesystem +scope and absolute paths, and confine resolved parent directories to the +checkout. Moves retain the source filename, accept files and symlinks, and +never overwrite existing entries. Moves use the host's atomic exclusive rename +primitive (`renamex_np`, `renameat2`, or `MoveFileExW` without replacement or +copy flags), so concurrent saves are never removed by a later source deletion. +Cross-filesystem moves and unsupported primitives fail without a copying +fallback. SSH moves require Python 3 to call the primitive through its standard +library. The client submits one move request at a time and reports partial +failures, then refreshes source and destination listings sequentially and +retires moved source previews. + `file.reveal` opens the host file manager with a fixed per-platform argv (`explorer.exe`, `open`, `xdg-open`), never a shell. It is disabled unless the host opts in with `ROAMGATE_ALLOW_FILE_REVEAL=1`, and still refuses SSH profiles diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 49819924..4e4e1d38 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -747,6 +747,10 @@ passwords/keys/passphrases/options. Verify host keys and noninteractive service- authentication first. The Roamgate bridge must run on Linux/macOS; a Windows bridge supports native local profiles only. +Moving files on Linux/macOS SSH hosts requires `python3` on the remote user's +PATH and a filesystem supporting atomic exclusive renames. Unsupported hosts +or cross-filesystem destinations fail without copying or deleting the source. + For a **Windows SSH host**, select **Windows** as the remote host OS. Install Herdr 0.9.1 or a later compatible release on that host, including the `conpty/` directory from its Windows archive, and start the desired Herdr session first. diff --git a/server/src/connections/profile-process.test.ts b/server/src/connections/profile-process.test.ts index 94688078..7d5e0395 100644 --- a/server/src/connections/profile-process.test.ts +++ b/server/src/connections/profile-process.test.ts @@ -3,6 +3,8 @@ import { chmodSync, mkdirSync, realpathSync, + readFileSync, + existsSync, rmSync, writeFileSync, } from "node:fs"; @@ -473,6 +475,45 @@ test("production dispatcher isolates two local profiles and profile CRUD", async ); expect(initialAlpha.connection_generation).toBe(oldAlphaGeneration); expect(initialAlpha.result.workspaces[0].name).toBe("from-alpha"); + expect( + await rpc( + "file.mkdir", + { workspace_id: "shared-workspace", name: "files-target" }, + "alpha", + oldAlphaGeneration, + ), + ).toMatchObject({ path: "files-target" }); + writeFileSync(join(root, "move-me.txt"), "file contents"); + expect( + await rpc( + "file.move", + { + workspace_id: "shared-workspace", + path: "move-me.txt", + directory: "files-target", + }, + "alpha", + oldAlphaGeneration, + ), + ).toMatchObject({ path: "files-target/move-me.txt" }); + expect(existsSync(join(root, "move-me.txt"))).toBe(false); + expect(readFileSync(join(root, "files-target/move-me.txt"), "utf8")).toBe( + "file contents", + ); + writeFileSync(join(root, "move-me.txt"), "keep source"); + await expect( + rpc( + "file.move", + { + workspace_id: "shared-workspace", + path: "move-me.txt", + directory: "files-target", + }, + "alpha", + oldAlphaGeneration, + ), + ).rejects.toThrow(); + expect(readFileSync(join(root, "move-me.txt"), "utf8")).toBe("keep source"); // A native teardown failure must stop before dispatching removal to Herdr. const hookPath = join(root, "roamgate.json"); writeFileSync(join(root, "paseo.json"), '{"worktree":{"teardown":"true"}}'); diff --git a/server/src/index.ts b/server/src/index.ts index eb66803b..46cfa4e2 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -942,6 +942,8 @@ async function handleRpc(ws: ServerWebSocket, raw: string) { readWorkspaceFile, searchWorkspaceFiles, revealWorkspaceFile, + createWorkspaceDirectory, + moveWorkspaceFile, readGitDiffSummary, readWorkspacePullRequest, readGitDiffFile, @@ -1082,6 +1084,17 @@ async function handleRpc(ws: ServerWebSocket, raw: string) { } return; } + if (method === "file.mkdir" || method === "file.move") { + try { + const result = await (method === "file.mkdir" + ? createWorkspaceDirectory + : moveWorkspaceFile)(params ?? {}); + sendReply({ id, result }, method); + } catch (e) { + sendError(`${method}-error`, e); + } + return; + } if (method === "file.read") { try { const result = await readWorkspaceFile(params ?? {}); diff --git a/server/src/workspace/atomic-file-move.ts b/server/src/workspace/atomic-file-move.ts new file mode 100644 index 00000000..98588800 --- /dev/null +++ b/server/src/workspace/atomic-file-move.ts @@ -0,0 +1,110 @@ +import { dlopen, FFIType, ptr, read } from "bun:ffi"; +import { getSystemErrorName } from "node:util"; + +function moveError(errno: number, source: string, target: string) { + const code = getSystemErrorName(-errno); + return Object.assign( + new Error(`${code}: cannot move ${source} to ${target}`), + { code, errno, path: source, dest: target }, + ); +} + +function openExclusiveRename() { + if (process.platform === "win32") { + const { symbols } = dlopen("kernel32.dll", { + MoveFileExW: { + args: [FFIType.ptr, FFIType.ptr, FFIType.u32], + returns: FFIType.i32, + }, + GetLastError: { args: [], returns: FFIType.u32 }, + }); + return (source: string, target: string) => { + const from = Buffer.from(`${source}\0`, "utf16le"); + const to = Buffer.from(`${target}\0`, "utf16le"); + // Zero flags prohibit both replacement and cross-filesystem copying. + if (symbols.MoveFileExW(ptr(from), ptr(to), 0) === 0) { + throw new Error( + `Cannot move file: Windows error ${symbols.GetLastError()}`, + ); + } + }; + } + if (process.platform === "darwin") { + const { symbols } = dlopen("/usr/lib/libSystem.B.dylib", { + renamex_np: { + args: [FFIType.ptr, FFIType.ptr, FFIType.u32], + returns: FFIType.i32, + }, + __error: { args: [], returns: FFIType.ptr }, + }); + return (source: string, target: string) => { + const from = Buffer.from(`${source}\0`); + const to = Buffer.from(`${target}\0`); + if (symbols.renamex_np(ptr(from), ptr(to), 4) !== 0) { + throw moveError(read.i32(symbols.__error()!), source, target); + } + }; + } + if (process.platform === "linux") { + const definitions = { + renameat2: { + args: [FFIType.i32, FFIType.ptr, FFIType.i32, FFIType.ptr, FFIType.u32], + returns: FFIType.i32, + }, + __errno_location: { args: [], returns: FFIType.ptr }, + } as const; + let library; + try { + library = dlopen("libc.so.6", definitions); + } catch { + library = dlopen( + `/lib/ld-musl-${process.arch === "arm64" ? "aarch64" : "x86_64"}.so.1`, + definitions, + ); + } + const { symbols } = library; + return (source: string, target: string) => { + const from = Buffer.from(`${source}\0`); + const to = Buffer.from(`${target}\0`); + if (symbols.renameat2(-100, ptr(from), -100, ptr(to), 1) !== 0) { + throw moveError(read.i32(symbols.__errno_location()!), source, target); + } + }; + } + throw new Error("Atomic file moves are unsupported on this host"); +} + +let exclusiveRename: ReturnType | undefined; + +export function renameFileExclusive(source: string, target: string) { + if (source === target) + throw new Error("Source and destination are identical"); + exclusiveRename ??= openExclusiveRename(); + exclusiveRename(source, target); +} + +// Python's standard library exposes the host's atomic primitive without installing a helper. +export const REMOTE_EXCLUSIVE_RENAME = ` +import ctypes, os, sys +source, target = sys.argv[1:] +if source == target: + sys.exit("Source and destination are identical") +libc = ctypes.CDLL(None, use_errno=True) +try: + if sys.platform == "darwin": + rename = libc.renamex_np + rename.argtypes = [ctypes.c_char_p, ctypes.c_char_p, ctypes.c_uint] + args = [os.fsencode(source), os.fsencode(target), 4] + elif sys.platform.startswith("linux"): + rename = libc.renameat2 + rename.argtypes = [ctypes.c_int, ctypes.c_char_p, ctypes.c_int, ctypes.c_char_p, ctypes.c_uint] + args = [-100, os.fsencode(source), -100, os.fsencode(target), 1] + else: + raise AttributeError() +except AttributeError: + sys.exit("Atomic file moves are unsupported on this host") +rename.restype = ctypes.c_int +result = rename(*args) +if result != 0: + sys.exit(os.strerror(ctypes.get_errno())) +`; diff --git a/server/src/workspace/file-mutations.test.ts b/server/src/workspace/file-mutations.test.ts new file mode 100644 index 00000000..d25afec6 --- /dev/null +++ b/server/src/workspace/file-mutations.test.ts @@ -0,0 +1,480 @@ +import { describe, expect, spyOn, test } from "bun:test"; +import { renameSync, writeFileSync } from "node:fs"; +import { + lstat, + chmod, + mkdir, + mkdtemp, + readFile, + readlink, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { shQuote } from "../utils/process-utils"; +import { createFileHandlers } from "./files"; +import { runBinaryProcessWithTimeout } from "./process"; +import * as atomicMove from "./atomic-file-move"; + +for (const remote of [false, true]) { + describe(`${remote ? "SSH shell" : "local"} file mutations`, () => { + async function fixture( + run: ( + files: ReturnType, + root: string, + outside: string, + ) => Promise, + transformCommand: (command: string) => string = (command) => command, + ) { + const temp = await mkdtemp(join(tmpdir(), "roamgate-mutations-")); + const root = join(temp, "checkout"); + const outside = join(temp, "outside"); + await mkdir(root); + await mkdir(outside); + const files = createFileHandlers({ + herdr: { + call: async () => ({ + workspace: { + workspace_id: "w1", + worktree: { checkout_path: root }, + }, + }), + } as any, + sshHost: () => (remote ? "example.invalid" : undefined), + shQuote, + runProcessWithCodeTimeout: async (argv, timeout) => { + const result = await runBinaryProcessWithTimeout( + ["bash", "-c", transformCommand(argv.at(-1)!)], + timeout, + ); + return { ...result, stdout: result.stdout.toString("utf8") }; + }, + }); + try { + await run(files, root, outside); + } finally { + await rm(temp, { recursive: true, force: true }); + } + } + + const params = { workspace_id: "w1" }; + + test("creates root/nested folders and preserves existing entries", async () => { + await fixture(async (files, root) => { + const name = "folder 'quoted $name"; + expect( + await files.createWorkspaceDirectory({ ...params, name }), + ).toMatchObject({ path: name }); + expect( + await files.createWorkspaceDirectory({ + ...params, + directory: name, + name: "nested", + }), + ).toMatchObject({ path: `${name}/nested` }); + await expect( + files.createWorkspaceDirectory({ ...params, name }), + ).rejects.toThrow(); + await writeFile(join(root, "existing"), "keep"); + await expect( + files.createWorkspaceDirectory({ ...params, name: "existing" }), + ).rejects.toThrow(); + expect(await readFile(join(root, "existing"), "utf8")).toBe("keep"); + }); + }); + + test("moves files into folders and back to root without changing their contents", async () => { + await fixture(async (files, root) => { + await mkdir(join(root, "target")); + const name = "a 'quoted $name.txt"; + await writeFile(join(root, name), "contents"); + expect( + await files.moveWorkspaceFile({ + ...params, + path: name, + directory: "target", + }), + ).toMatchObject({ path: `target/${name}` }); + await expect(lstat(join(root, name))).rejects.toThrow(); + expect(await readFile(join(root, "target", name), "utf8")).toBe( + "contents", + ); + expect( + await files.moveWorkspaceFile({ + ...params, + path: `target/${name}`, + directory: "", + }), + ).toMatchObject({ path: name }); + }); + }); + + test.skipIf(process.platform === "win32")( + "literal backslashes in POSIX names do not move a different file", + async () => { + await fixture(async (files, root) => { + await mkdir(join(root, "from")); + await mkdir(join(root, "target")); + await writeFile(join(root, "from/name"), "other file"); + await writeFile(join(root, "from\\name"), "selected file"); + await files.moveWorkspaceFile({ + ...params, + path: "from\\name", + directory: "target", + }); + expect(await readFile(join(root, "target/from\\name"), "utf8")).toBe( + "selected file", + ); + expect(await readFile(join(root, "from/name"), "utf8")).toBe( + "other file", + ); + await expect(lstat(join(root, "from\\name"))).rejects.toThrow(); + }); + }, + ); + + test("never overwrites a destination file, directory or symlink", async () => { + await fixture(async (files, root, outside) => { + await mkdir(join(root, "target")); + for (const name of ["file", "folder", "link"]) + await writeFile(join(root, name), "source"); + await writeFile(join(root, "target/file"), "destination"); + await mkdir(join(root, "target/folder")); + await symlink(join(outside, "missing"), join(root, "target/link")); + for (const path of ["file", "folder", "link"]) { + await expect( + files.moveWorkspaceFile({ ...params, path, directory: "target" }), + ).rejects.toThrow(); + expect(await readFile(join(root, path), "utf8")).toBe("source"); + } + expect(await readFile(join(root, "target/file"), "utf8")).toBe( + "destination", + ); + expect((await lstat(join(root, "target/folder"))).isDirectory()).toBe( + true, + ); + expect(await readlink(join(root, "target/link"))).toBe( + join(outside, "missing"), + ); + }); + }); + + test.skipIf(process.platform === "win32")( + "trailing newlines in directory names do not redirect a move", + async () => { + await fixture(async (files, root) => { + for (const directory of ["source", "source\n", "target", "target\n"]) + await mkdir(join(root, directory)); + await writeFile(join(root, "source/a"), "other file"); + await writeFile(join(root, "source\n/a"), "selected file"); + await files.moveWorkspaceFile({ + ...params, + path: "source\n/a", + directory: "target\n", + }); + expect(await readFile(join(root, "target\n/a"), "utf8")).toBe( + "selected file", + ); + expect(await readFile(join(root, "source/a"), "utf8")).toBe( + "other file", + ); + await expect(lstat(join(root, "source\n/a"))).rejects.toThrow(); + await expect(lstat(join(root, "target/a"))).rejects.toThrow(); + }); + }, + ); + + test("keeps batch failures intact while independent files move", async () => { + await fixture(async (files, root) => { + await mkdir(join(root, "target")); + await writeFile(join(root, "a"), "a"); + await writeFile(join(root, "b"), "b"); + await writeFile(join(root, "target/b"), "keep"); + const results = await Promise.allSettled( + ["a", "b"].map((path) => + files.moveWorkspaceFile({ ...params, path, directory: "target" }), + ), + ); + expect(results.map((result) => result.status)).toEqual([ + "fulfilled", + "rejected", + ]); + expect(await readFile(join(root, "target/a"), "utf8")).toBe("a"); + expect(await readFile(join(root, "b"), "utf8")).toBe("b"); + expect(await readFile(join(root, "target/b"), "utf8")).toBe("keep"); + }); + }); + + test("preserves saves immediately before and after the atomic move", async () => { + const renameExclusive = atomicMove.renameFileExclusive; + const spy = remote + ? undefined + : spyOn(atomicMove, "renameFileExclusive").mockImplementation( + (source, target) => { + renameSync(`${source}.save`, source); + renameExclusive(source, target); + writeFileSync(source, "later save"); + }, + ); + const script = atomicMove.REMOTE_EXCLUSIVE_RENAME.replace( + "result = rename(*args)", + `os.replace(source + ".save", source) +result = rename(*args) +if result == 0: + with open(source, "w") as saved: + saved.write("later save")`, + ); + try { + await fixture( + async (files, root) => { + await mkdir(join(root, "target")); + await writeFile(join(root, "a"), "original"); + await writeFile(join(root, "a.save"), "concurrent save"); + await files.moveWorkspaceFile({ + ...params, + path: "a", + directory: "target", + }); + expect(await readFile(join(root, "target/a"), "utf8")).toBe( + "concurrent save", + ); + expect(await readFile(join(root, "a"), "utf8")).toBe("later save"); + if (spy) expect(spy).toHaveBeenCalledTimes(1); + }, + (command) => + command.replace( + shQuote(atomicMove.REMOTE_EXCLUSIVE_RENAME), + shQuote(script), + ), + ); + } finally { + spy?.mockRestore(); + } + }); + + test.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "a read-only source directory leaves no destination and can be retried", + async () => { + await fixture(async (files, root) => { + const source = join(root, "source"); + await mkdir(source); + await mkdir(join(root, "target")); + await writeFile(join(source, "a"), "keep"); + await chmod(source, 0o500); + try { + await expect( + files.moveWorkspaceFile({ + ...params, + path: "source/a", + directory: "target", + }), + ).rejects.toThrow(); + expect(await readFile(join(source, "a"), "utf8")).toBe("keep"); + await expect(lstat(join(root, "target/a"))).rejects.toThrow(); + } finally { + await chmod(source, 0o700); + } + await files.moveWorkspaceFile({ + ...params, + path: "source/a", + directory: "target", + }); + expect(await readFile(join(root, "target/a"), "utf8")).toBe("keep"); + }); + }, + ); + + test("concurrent moves to the same name preserve the losing source", async () => { + await fixture(async (files, root) => { + for (const directory of ["one", "two", "target"]) + await mkdir(join(root, directory)); + await writeFile(join(root, "one/a"), "one"); + await writeFile(join(root, "two/a"), "two"); + const results = await Promise.allSettled( + ["one", "two"].map((directory) => + files.moveWorkspaceFile({ + ...params, + path: `${directory}/a`, + directory: "target", + }), + ), + ); + expect( + results.filter((result) => result.status === "fulfilled"), + ).toHaveLength(1); + const loser = results[0].status === "rejected" ? "one" : "two"; + expect(await readFile(join(root, loser, "a"), "utf8")).toBe(loser); + expect(await readFile(join(root, "target/a"), "utf8")).toBe( + loser === "one" ? "two" : "one", + ); + }); + }); + + if (remote) { + test("unsupported atomic primitives fail without changing either path", async () => { + const script = atomicMove.REMOTE_EXCLUSIVE_RENAME.replace( + "import ctypes, os, sys", + 'import ctypes, os, sys\nsys.platform = "unsupported"', + ); + await fixture( + async (files, root) => { + await mkdir(join(root, "target")); + await writeFile(join(root, "a"), "keep"); + await expect( + files.moveWorkspaceFile({ + ...params, + path: "a", + directory: "target", + }), + ).rejects.toThrow("unsupported"); + expect(await readFile(join(root, "a"), "utf8")).toBe("keep"); + await expect(lstat(join(root, "target/a"))).rejects.toThrow(); + }, + (command) => + command.replace( + shQuote(atomicMove.REMOTE_EXCLUSIVE_RENAME), + shQuote(script), + ), + ); + }); + + test("missing Python fails before changing either path", async () => { + await fixture( + async (files, root) => { + await mkdir(join(root, "target")); + await writeFile(join(root, "a"), "keep"); + await expect( + files.moveWorkspaceFile({ + ...params, + path: "a", + directory: "target", + }), + ).rejects.toThrow("requires Python 3"); + expect(await readFile(join(root, "a"), "utf8")).toBe("keep"); + await expect(lstat(join(root, "target/a"))).rejects.toThrow(); + }, + (command) => command.replace("command -v python3", "false"), + ); + }); + } + + test("moves symlinks themselves and rejects directory or same-parent moves", async () => { + await fixture(async (files, root, outside) => { + await mkdir(join(root, "target")); + await symlink(join(outside, "missing"), join(root, "link")); + await files.moveWorkspaceFile({ + ...params, + path: "link", + directory: "target", + }); + expect(await readlink(join(root, "target/link"))).toBe( + join(outside, "missing"), + ); + await expect( + files.moveWorkspaceFile({ ...params, path: "target", directory: "" }), + ).rejects.toThrow(); + await expect( + files.moveWorkspaceFile({ + ...params, + path: "target/link", + directory: "target", + }), + ).rejects.toThrow(); + expect((await lstat(join(root, "target/link"))).isSymbolicLink()).toBe( + true, + ); + }); + }); + + test("blocks symlink parent escapes for both source and destination", async () => { + await fixture(async (files, root, outside) => { + await symlink(outside, join(root, "escape")); + await writeFile(join(root, "a"), "inside"); + await writeFile(join(outside, "a"), "outside"); + await expect( + files.createWorkspaceDirectory({ + ...params, + directory: "escape", + name: "new", + }), + ).rejects.toThrow(); + await expect( + files.moveWorkspaceFile({ + ...params, + path: "a", + directory: "escape", + }), + ).rejects.toThrow(); + await expect( + files.moveWorkspaceFile({ + ...params, + path: "escape/a", + directory: "", + }), + ).rejects.toThrow(); + expect(await readFile(join(root, "a"), "utf8")).toBe("inside"); + expect(await readFile(join(outside, "a"), "utf8")).toBe("outside"); + await expect(lstat(join(outside, "new"))).rejects.toThrow(); + }); + }); + + test("rejects invalid mutation paths, names, scopes and missing workspace identity", async () => { + await fixture(async (files) => { + for (const directory of [ + "../outside", + "/absolute", + "C:\\outside", + "bad\0path", + ]) { + await expect( + files.createWorkspaceDirectory({ + ...params, + directory, + name: "new", + }), + ).rejects.toThrow(); + await expect( + files.moveWorkspaceFile({ ...params, path: "a", directory }), + ).rejects.toThrow(); + } + for (const path of [ + "", + ".", + "../outside", + "/absolute", + "C:\\outside", + "bad\0path", + ]) + await expect( + files.moveWorkspaceFile({ ...params, path }), + ).rejects.toThrow(); + for (const name of ["", ".", "..", "a/b", "a\\b", "bad\0name"]) + await expect( + files.createWorkspaceDirectory({ ...params, name }), + ).rejects.toThrow(); + await expect( + files.createWorkspaceDirectory({ + ...params, + name: "new", + scope: "filesystem", + }), + ).rejects.toThrow("workspace scope"); + await expect( + files.moveWorkspaceFile({ + ...params, + path: "a", + scope: "filesystem", + }), + ).rejects.toThrow("workspace scope"); + await expect( + files.createWorkspaceDirectory({ name: "new" }), + ).rejects.toThrow("workspace_id"); + await expect(files.moveWorkspaceFile({ path: "a" })).rejects.toThrow( + "workspace_id", + ); + }); + }); + }); +} diff --git a/server/src/workspace/file-paths.ts b/server/src/workspace/file-paths.ts index b156a447..bea333ae 100644 --- a/server/src/workspace/file-paths.ts +++ b/server/src/workspace/file-paths.ts @@ -11,6 +11,18 @@ export function sanitizeExplorerPath(value: unknown): string { return parts.join("/"); } +export function sanitizeMutationPath(value: unknown): string { + if (typeof value !== "string" || /^(?:[\\/]|[a-z]:)/i.test(value)) { + throw new Error("file operations require a workspace-relative path"); + } + if (process.platform === "win32") return sanitizeExplorerPath(value); + const parts = value.split("/").filter(Boolean); + if (parts.some((part) => part === ".." || part.includes("\0"))) { + throw new Error("invalid file explorer path"); + } + return parts.join("/"); +} + /** Filesystem browsing requires an explicit absolute directory on the host. */ export function sanitizeFilesystemPath(value: unknown): string { const path = diff --git a/server/src/workspace/files.ts b/server/src/workspace/files.ts index b11fda5a..614764b0 100644 --- a/server/src/workspace/files.ts +++ b/server/src/workspace/files.ts @@ -5,6 +5,7 @@ import { downloadContentDisposition, inlineContentDisposition, sanitizeExplorerPath, + sanitizeMutationPath, sanitizeFilesystemPath, sanitizePreviewPath, sanitizeUploadFilename, @@ -22,6 +23,8 @@ import { readLocalFile, resolveLocalFilePaths, uploadLocalFile, + createLocalDirectory, + moveLocalFile, } from "./local-files"; import { deleteRemoteFile, @@ -30,6 +33,7 @@ import { readRemoteFile, resolveRemoteFilePaths, uploadRemoteFile, + mutateRemoteFile, } from "./remote-files"; import { pullGit, @@ -487,6 +491,39 @@ export function createFileHandlers({ }; } + async function mutateFile(params: Record, move: boolean) { + const method = move ? "file.move" : "file.mkdir"; + const workspaceId = String(params.workspace_id ?? ""); + if (!workspaceId) throw new Error(`${method} requires workspace_id`); + if (params.scope !== undefined && params.scope !== "workspace") + throw new Error(`${method} only supports workspace scope`); + const directory = sanitizeMutationPath(params.directory ?? ""); + const path = move ? sanitizeMutationPath(params.path) : undefined; + if (move && !path) throw new Error("file.move requires path"); + const name = + path === undefined + ? sanitizeUploadFilename(params.name) + : path.split("/").pop()!; + const workspace = await getWorkspace(workspaceId); + const rootPath = await explorerRoot(workspaceId, workspace); + if (!rootPath) throw new Error("workspace has no directory path"); + const host = sshHost(); + const result = host + ? await mutateRemoteFile({ + host, + rootPath, + directory, + name, + path, + runProcessWithCodeTimeout, + shQuote, + }) + : path === undefined + ? await createLocalDirectory(rootPath, directory, name) + : await moveLocalFile(rootPath, path, directory); + return { workspace_id: workspaceId, ...result }; + } + async function gitRoot(workspaceId: string, workspace: any) { const host = sshHost(); const tried = new Set(); @@ -667,6 +704,10 @@ export function createFileHandlers({ downloadWorkspaceFile: downloadFile, uploadWorkspaceFile: uploadFile, deleteWorkspaceFile: deleteFile, + createWorkspaceDirectory: (params: Record) => + mutateFile(params, false), + moveWorkspaceFile: (params: Record) => + mutateFile(params, true), readGitDiffSummary, readWorkspacePullRequest, readGitDiffFile, diff --git a/server/src/workspace/local-files.ts b/server/src/workspace/local-files.ts index 283b88d9..d1a5e839 100644 --- a/server/src/workspace/local-files.ts +++ b/server/src/workspace/local-files.ts @@ -1,5 +1,6 @@ import { lstat, + mkdir, readdir, realpath, rm, @@ -25,6 +26,7 @@ import type { import { runBinaryProcessWithTimeout } from "./process"; import { decodePreviewBuffer, previewLimitForPath } from "./preview"; import { fileDownloadRange, type FileDownloadOptions } from "./file-download"; +import { renameFileExclusive } from "./atomic-file-move"; export async function listLocalFiles( rootPath: string, @@ -250,3 +252,37 @@ export async function deleteLocalFile( : "file", }; } + +export async function createLocalDirectory( + rootPath: string, + directory: string, + name: string, +) { + const rootReal = await realpath(rootPath); + const parentReal = await realpath(resolve(rootReal, directory)); + assertInsideRoot(rootReal, parentReal); + const target = join(parentReal, name); + await mkdir(target); + return { path: relativePreviewPath(rootReal, target) }; +} + +export async function moveLocalFile( + rootPath: string, + path: string, + directory: string, +) { + const rootReal = await realpath(rootPath); + const sourceParent = await realpath(dirname(resolve(rootReal, path))); + const destination = await realpath(resolve(rootReal, directory)); + assertInsideRoot(rootReal, sourceParent); + assertInsideRoot(rootReal, destination); + const source = join(sourceParent, basename(path)); + const info = await lstat(source); + if (!info.isFile() && !info.isSymbolicLink()) + throw new Error("Only files can be moved"); + if (!(await stat(destination)).isDirectory()) + throw new Error("Move target is not a directory"); + const target = join(destination, basename(path)); + renameFileExclusive(source, target); + return { path: relativePreviewPath(rootReal, target) }; +} diff --git a/server/src/workspace/remote-files.ts b/server/src/workspace/remote-files.ts index 69cf481e..59390af0 100644 --- a/server/src/workspace/remote-files.ts +++ b/server/src/workspace/remote-files.ts @@ -31,6 +31,7 @@ import type { } from "./file-types"; import { runProcessWithInputTimeout } from "./process"; import { decodePreviewBuffer, previewLimitForPath } from "./preview"; +import { REMOTE_EXCLUSIVE_RENAME } from "./atomic-file-move"; export function parseRemoteFileList( stdout: string, @@ -590,3 +591,55 @@ printf 'META\\t%s\\t%s\\n' "$(printf '%s' "$rel" | base64 | tr -d '\\n')" "$type } return parseRemoteFileDelete(result.stdout); } + +export async function mutateRemoteFile({ + host, + rootPath, + directory, + name, + path, + runProcessWithCodeTimeout, + shQuote, +}: { + host: string; + rootPath: string; + directory: string; + name: string; + path?: string; + runProcessWithCodeTimeout: RunProcessWithCodeTimeout; + shQuote: (value: string) => string; +}) { + const command = ` +set -euo pipefail +# Keep trailing newlines in directory names through command substitution. +root_real="$(cd -P -- ${shQuote(rootPath)} && printf '%s.' "$PWD")" +root_real="\${root_real%.}" +directory_real="$(cd -P -- "$root_real"/${shQuote(directory || ".")} && printf '%s.' "$PWD")" +directory_real="\${directory_real%.}" +case "$directory_real/" in "\${root_real%/}/"*) ;; *) echo "path escaped the workspace checkout" >&2; exit 13 ;; esac +${ + path === undefined + ? `mkdir -- "$directory_real"/${shQuote(name)}` + : ` +source_parent="$(cd -P -- "$root_real"/${shQuote(path.includes("/") ? path.slice(0, path.lastIndexOf("/")) : ".")} && printf '%s.' "$PWD")" +source_parent="\${source_parent%.}" +case "$source_parent/" in "\${root_real%/}/"*) ;; *) echo "path escaped the workspace checkout" >&2; exit 13 ;; esac +source="$source_parent"/${shQuote(name)} +if [ ! -f "$source" ] && [ ! -L "$source" ]; then echo "Only files can be moved" >&2; exit 14; fi +command -v python3 >/dev/null || { echo "Moving files over SSH requires Python 3" >&2; exit 14; } +python3 -c ${shQuote(REMOTE_EXCLUSIVE_RENAME)} "$source" "$directory_real"/${shQuote(name)} +` +} +`; + const result = await runProcessWithCodeTimeout( + sshCommandArgv(host, `bash -lc ${shQuote(command)}`), + DELETE_TIMEOUT_MS, + ); + if (result.code !== 0) + throw new Error( + (result.stderr || result.stdout || `file operation exited ${result.code}`) + .trim() + .slice(0, 1000), + ); + return { path: relativeExplorerPath(directory, name) }; +} diff --git a/web/src/components/FileExplorerDialog.css b/web/src/components/FileExplorerDialog.css index 93b6f03b..d2322767 100644 --- a/web/src/components/FileExplorerDialog.css +++ b/web/src/components/FileExplorerDialog.css @@ -81,10 +81,33 @@ flex-basis: 100%; } +.file-explorer-controls { + display: flex; + align-items: center; + gap: 6px; + min-width: 0; +} + +.file-explorer-side .file-explorer-controls { + flex: 1 1 100%; +} + +.file-explorer-controls .file-toolbar-end { + flex-shrink: 0; +} + +.file-explorer-controls .file-toolbar-icon { + width: 28px; + height: 28px; +} + .file-search-mode.themed-select-trigger { - min-width: 90px; - height: 34px; - padding: 0 9px; + flex: 1; + min-width: 0; + max-width: 110px; + gap: 4px; + height: 28px; + padding: 0 6px; border: 1px solid var(--viewer-border); border-radius: 8px; background: var(--viewer-content-bg); diff --git a/web/src/components/FileExplorerDialog.dom.test.ts b/web/src/components/FileExplorerDialog.dom.test.ts new file mode 100644 index 00000000..e2c3469c --- /dev/null +++ b/web/src/components/FileExplorerDialog.dom.test.ts @@ -0,0 +1,551 @@ +import { expect, spyOn, test } from "bun:test"; +import { + mkdtemp, + readFile, + readdir, + realpath, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Window } from "happy-dom"; +import { act, createElement } from "react"; +import { + createLocalDirectory, + listLocalFiles, + moveLocalFile, + readLocalFile, +} from "../../../server/src/workspace/local-files"; +import { bridge, type ConnectionClient } from "../api"; +import { __storeTesting, store } from "../store"; +import { WORKSPACE_PATH_DRAG_TYPE } from "../workspacePathDrag"; +import { requestFilePreview } from "./fileExplorerResources"; + +if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { + test("file explorer interactions with real file handlers in an isolated DOM runtime", async () => { + const child = Bun.spawn([process.execPath, "test", import.meta.path], { + env: { ...process.env, ROAMGATE_FILE_EXPLORER_DOM_TEST: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + if (code !== 0) throw new Error(`${stdout}\n${stderr}`); + expect(code).toBe(0); + }, 15_000); +} else { + test("creates folders, selects files, moves batches and preserves failed selections", async () => { + const browser = new Window({ url: "http://localhost" }); + const originals = new Map(); + for (const [key, value] of Object.entries({ + window: browser, + document: browser.document, + navigator: browser.navigator, + HTMLElement: browser.HTMLElement, + HTMLButtonElement: browser.HTMLButtonElement, + HTMLInputElement: browser.HTMLInputElement, + MutationObserver: browser.MutationObserver, + Element: browser.Element, + Node: browser.Node, + localStorage: browser.localStorage, + sessionStorage: browser.sessionStorage, + IS_REACT_ACT_ENVIRONMENT: true, + })) { + originals.set(key, Object.getOwnPropertyDescriptor(globalThis, key)); + Object.defineProperty(globalThis, key, { + value, + configurable: true, + writable: true, + }); + } + const { createRoot } = await import("react-dom/client"); + const { FileExplorerPanel, FileExplorerDialog } = await import( + "./FileExplorerDialog" + ); + const directory = await mkdtemp(join(tmpdir(), "roamgate-files-dom-")); + for (const name of ["a.txt", "b.txt", "c.txt"]) + await writeFile(join(directory, name), name); + const previous = store.get(); + const pending = new Set>(); + let activeMoves = 0; + let peakMoves = 0; + const calls: Array<{ method: string; params: Record }> = + []; + const client: ConnectionClient = { + connectionId: "local", + generation: 1, + serverRuntimeGeneration: 1, + isCurrent: () => true, + acceptsServerGeneration: () => true, + call(method, params = {}) { + calls.push({ method, params }); + const handlers: Record< + string, + (params: Record) => Promise + > = { + "file.list": async (params) => ({ + ...(await listLocalFiles( + directory, + String(params.path ?? ""), + false, + )), + workspace_id: "w1", + checkout_path: directory, + }), + "file.read": (params) => + readLocalFile(directory, String(params.path)), + "file.mkdir": (params) => + createLocalDirectory( + directory, + String(params.directory), + String(params.name), + ), + "file.move": (params) => + moveLocalFile( + directory, + String(params.path), + String(params.directory), + ), + "git.diff_summary": async () => ({ entries: [], counts: {} }), + }; + const task = handlers[method]!(params); + if (method === "file.move") + peakMoves = Math.max(peakMoves, ++activeMoves); + pending.add(task); + void task.then( + () => { + pending.delete(task); + if (method === "file.move") activeMoves -= 1; + }, + () => { + pending.delete(task); + if (method === "file.move") activeMoves -= 1; + }, + ); + return task; + }, + }; + const connection = spyOn(bridge, "connection").mockReturnValue(client); + const notify = spyOn(store, "notify"); + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + async function update(action: () => void) { + await act(async () => { + action(); + }); + await act(async () => { + while (pending.size) await Promise.allSettled([...pending]); + }); + } + async function submitFolderName(name: string) { + await update(() => { + const input = container.querySelector( + "[role='dialog'] input", + )!; + Object.getOwnPropertyDescriptor( + browser.HTMLInputElement.prototype, + "value", + )!.set!.call(input, name); + input.dispatchEvent( + new browser.Event("input", { bubbles: true }) as unknown as Event, + ); + }); + await update(() => + container.querySelector("form")!.dispatchEvent( + new browser.Event("submit", { + bubbles: true, + cancelable: true, + }) as unknown as Event, + ), + ); + } + function row(path: string) { + const element = [ + ...container.querySelectorAll(".file-row[data-file-path]"), + ].find((element) => element.dataset.filePath === path); + if (!element) throw new Error(`Missing row ${path}`); + return element; + } + const click = ( + element: Element, + options: { + ctrlKey?: boolean; + shiftKey?: boolean; + metaKey?: boolean; + } = {}, + ) => + element.dispatchEvent( + new browser.MouseEvent("click", { + bubbles: true, + ...options, + }) as unknown as Event, + ); + function selected() { + return [ + ...container.querySelectorAll("[aria-selected='true']"), + ].map((row) => row.dataset.filePath); + } + function contextMenu(element: Element) { + const event = new browser.MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + }); + element.dispatchEvent(event as unknown as Event); + return event.defaultPrevented; + } + const menuLabels = () => + [...container.querySelectorAll("[role='menuitem']")].map( + (item) => item.textContent, + ); + function dragStart(path: string) { + const values = new Map(); + const transfer = { + effectAllowed: "none", + dropEffect: "none", + types: [] as string[], + getData: (type: string) => values.get(type) ?? "", + setData(type: string, value: string) { + values.set(type, value); + this.types = [...values.keys()]; + }, + }; + row(path).dispatchEvent( + new browser.PointerEvent("pointerdown", { + bubbles: true, + pointerType: "mouse", + }) as unknown as Event, + ); + drag(row(path), "dragstart", transfer); + return transfer; + } + function drag(element: Element, type: string, transfer: unknown) { + const event = new browser.Event(type, { + bubbles: true, + cancelable: true, + }); + Object.defineProperty(event, "dataTransfer", { value: transfer }); + element.dispatchEvent(event as unknown as Event); + return event; + } + try { + __storeTesting.replaceState({ + ...previous, + activeConnectionId: "local", + workspaces: [ + { + workspace_id: "w1", + number: 1, + label: "Files test", + focused: true, + pane_count: 0, + tab_count: 0, + agent_status: "idle", + cwd: directory, + }, + ], + }); + await update(() => + root.render( + createElement(FileExplorerPanel, { + open: true, + workspaceId: "w1", + onClose: () => {}, + }), + ), + ); + expect( + container + .querySelector("[role='tree']") + ?.getAttribute("aria-multiselectable"), + ).toBe("true"); + await update(() => + click(container.querySelector("[aria-label='New folder']")!), + ); + await submitFolderName("target"); + expect(await readdir(directory)).toContain("target"); + expect(row("target")).toBeDefined(); + await update(() => + row("target").dispatchEvent( + new browser.MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + }) as unknown as Event, + ), + ); + await update(() => + click( + [...container.querySelectorAll("[role='menuitem']")].find( + (item) => item.textContent === "New folder", + )!, + ), + ); + await submitFolderName("nested"); + expect(await readdir(join(directory, "target"))).toContain("nested"); + await update(() => { + expect( + contextMenu(container.querySelector(".file-explorer-browser")!), + ).toBe(true); + }); + expect(menuLabels()).toEqual(["New folder", "Refresh files"]); + await update(() => click(container.querySelector("[role='menuitem']")!)); + await submitFolderName("background-folder"); + expect(await readdir(directory)).toContain("background-folder"); + expect( + calls.filter((call) => call.method === "file.mkdir").pop()?.params, + ).toMatchObject({ directory: "", name: "background-folder" }); + await update(() => { + contextMenu(row("a.txt")); + }); + expect(menuLabels()).toContain("Download file"); + expect(menuLabels()).not.toContain("New folder"); + expect(menuLabels()).not.toContain("Refresh files"); + await update(() => { + contextMenu(container.querySelector(".file-tree")!); + }); + expect(menuLabels()).toEqual(["New folder", "Refresh files"]); + const listingsBeforeRefresh = calls.filter( + (call) => call.method === "file.list", + ).length; + await update(() => + click([...container.querySelectorAll("[role='menuitem']")][1]), + ); + expect( + calls.filter((call) => call.method === "file.list").length, + ).toBeGreaterThan(listingsBeforeRefresh); + expect(menuLabels()).toEqual([]); + await update(() => { + expect( + contextMenu( + container.querySelector("[aria-label='Search checkout']")!, + ), + ).toBe(false); + }); + expect(menuLabels()).toEqual([]); + await update(() => click(row("a.txt"))); + await update(() => click(row("c.txt"), { shiftKey: true })); + expect(selected()).toEqual(["a.txt", "b.txt", "c.txt"]); + await update(() => click(row("b.txt"), { metaKey: true })); + expect(selected()).toEqual(["a.txt", "c.txt"]); + await writeFile(join(directory, "target/c.txt"), "keep"); + await writeFile(join(directory, "target/a.txt"), "old target preview"); + await requestFilePreview("w1", "target/a.txt", { client }); + await rm(join(directory, "target/a.txt")); + let transfer!: ReturnType; + await update(() => { + transfer = dragStart("a.txt"); + }); + expect(transfer.getData(WORKSPACE_PATH_DRAG_TYPE)).toBe( + `${await realpath(directory)}/a.txt`, + ); + expect(transfer.effectAllowed).toBe("copyMove"); + await update(() => { + expect(drag(row("b.txt"), "dragover", transfer).defaultPrevented).toBe( + false, + ); + }); + await update(() => { + expect(drag(row("target"), "dragover", transfer).defaultPrevented).toBe( + true, + ); + }); + expect(transfer.dropEffect).toBe("move"); + await update(() => { + drag(row("target"), "drop", transfer); + }); + expect( + calls + .filter((call) => call.method === "file.move") + .map((call) => call.params.path), + ).toEqual(["a.txt", "c.txt"]); + expect(peakMoves).toBe(1); + expect(await readFile(join(directory, "target/a.txt"), "utf8")).toBe( + "a.txt", + ); + expect(await readFile(join(directory, "target/c.txt"), "utf8")).toBe( + "keep", + ); + expect(selected()).toEqual(["c.txt"]); + expect( + await requestFilePreview("w1", "target/a.txt", { client }), + ).toMatchObject({ text: "a.txt" }); + expect( + notify.mock.calls[notify.mock.calls.length - 1]?.[0], + ).toMatchObject({ + kind: "error", + detail: expect.stringContaining("1 of 2 moved"), + }); + await update(() => { + transfer = dragStart("b.txt"); + drag(row("target"), "drop", transfer); + }); + expect(await readFile(join(directory, "target/b.txt"), "utf8")).toBe( + "b.txt", + ); + expect(row("target/a.txt")).toBeDefined(); + await update(() => { + transfer = dragStart("target/a.txt"); + drag(container.querySelector(".file-tree")!, "drop", transfer); + }); + expect(await readFile(join(directory, "a.txt"), "utf8")).toBe("a.txt"); + await update(() => + row("a.txt").dispatchEvent( + new browser.KeyboardEvent("keydown", { + key: "a", + ctrlKey: true, + bubbles: true, + }) as unknown as Event, + ), + ); + expect(selected()).toEqual([ + "target/b.txt", + "target/c.txt", + "a.txt", + "c.txt", + ]); + const batch = Array.from( + { length: 30 }, + (_, index) => `batch-${String(index).padStart(2, "0")}.txt`, + ); + for (const path of batch) await writeFile(join(directory, path), path); + await update(() => + click(container.querySelector("[aria-label='Refresh files']")!), + ); + await update(() => click(row(batch[0]))); + await update(() => + click(row(batch[batch.length - 1]), { shiftKey: true }), + ); + expect(selected()).toEqual(batch); + await update(() => { + transfer = dragStart(batch[0]); + drag(row("target"), "drop", transfer); + }); + expect(peakMoves).toBe(1); + for (const path of batch) + expect(await readFile(join(directory, "target", path), "utf8")).toBe( + path, + ); + await update(() => click(row("a.txt"), { shiftKey: true })); + expect(selected()).toEqual(["a.txt"]); + await update(() => click(row("c.txt"), { shiftKey: true })); + expect(selected()).toEqual(["a.txt", "c.txt"]); + await update(() => { + transfer = dragStart("a.txt"); + }); + const movesBeforeSwitch = calls.filter( + (call) => call.method === "file.move", + ).length; + await update(() => { + const state = store.get(); + __storeTesting.replaceState({ + ...state, + workspaces: state.workspaces.map((workspace) => ({ + ...workspace, + workspace_id: "w2", + })), + }); + root.render( + createElement(FileExplorerPanel, { + open: true, + workspaceId: "w2", + onClose: () => {}, + }), + ); + }); + await update(() => { + drag(row("target"), "drop", transfer); + }); + expect(calls.filter((call) => call.method === "file.move")).toHaveLength( + movesBeforeSwitch, + ); + await update(() => + click(container.querySelector("[aria-label='Browse filesystem']")!), + ); + expect(container.querySelector("[aria-label='New folder']")).toBeNull(); + await update(() => { + expect( + contextMenu(container.querySelector(".file-explorer-browser")!), + ).toBe(false); + }); + expect(menuLabels()).toEqual([]); + let closes = 0; + await update(() => + root.render( + createElement(FileExplorerDialog, { + open: true, + workspaceId: "w2", + onClose: () => { + closes += 1; + }, + }), + ), + ); + await update(() => { + contextMenu(container.querySelector(".file-explorer-browser")!); + }); + // Allow the menu to attach its deferred keyboard listener and focus its first action. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + const actions = [ + ...container.querySelectorAll("[role='menuitem']"), + ]; + expect(document.activeElement).toBe(actions[0]); + await update(() => + actions[0].dispatchEvent( + new browser.KeyboardEvent("keydown", { + key: "ArrowDown", + bubbles: true, + }) as unknown as Event, + ), + ); + expect(document.activeElement).toBe(actions[1]); + await update(() => + actions[1].dispatchEvent( + new browser.KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + }) as unknown as Event, + ), + ); + expect(closes).toBe(0); + expect(menuLabels()).toEqual([]); + await update(() => + click(container.querySelector("button[aria-label='New folder']")!), + ); + await update(() => + container + .querySelector("[aria-label='New folder'] input")! + .dispatchEvent( + new browser.KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + }) as unknown as Event, + ), + ); + expect(closes).toBe(0); + expect( + container.querySelector("[role='dialog'][aria-label='New folder']"), + ).toBeNull(); + expect( + container.querySelector("[role='dialog'][aria-label='File Explorer']"), + ).not.toBeNull(); + } finally { + await act(async () => root.unmount()); + connection.mockRestore(); + notify.mockRestore(); + __storeTesting.replaceState(previous); + container.remove(); + await browser.happyDOM.close(); + for (const [key, descriptor] of originals) { + if (descriptor) Object.defineProperty(globalThis, key, descriptor); + else delete (globalThis as Record)[key]; + } + await rm(directory, { recursive: true, force: true }); + } + }); +} diff --git a/web/src/components/FileExplorerDialog.tsx b/web/src/components/FileExplorerDialog.tsx index 3bf4a20e..4576608d 100644 --- a/web/src/components/FileExplorerDialog.tsx +++ b/web/src/components/FileExplorerDialog.tsx @@ -19,6 +19,7 @@ import { File, Folder, FolderOpen, + FolderPlus, RefreshCw, Search, Upload, @@ -55,7 +56,8 @@ import type { GitDiffEntry, } from "../types"; import { CloseButton } from "./CloseButton"; -import { ConfirmDialog } from "./ModalDialogs"; +import { ConfirmDialog, TextInputDialog } from "./ModalDialogs"; +import { selectFilePaths } from "./fileExplorerSelection"; import { ThemedSelect } from "./ThemedSelect"; import { focusTreeItem, @@ -104,6 +106,7 @@ const LONG_PRESS_MOVE_PX = 10; const FILE_TREE_INDENT = 10; const FILE_TREE_BASE_INDENT = 6; +const FILE_MOVE_DRAG_TYPE = "application/x-roamgate-files"; export function FileExplorerDialog({ open, @@ -190,12 +193,11 @@ export function FileExplorerPanel({ type FileExplorerEntryMenuState = { x: number; y: number; - entry: FileExplorerEntry; + entry: FileExplorerEntry | null; }; -// Keep ENTRY_MENU_ITEM_COUNT in sync with the items rendered in -// FileExplorerEntryMenu; the height estimate drives clamping and flip placement. -const ENTRY_MENU_ITEM_COUNT = 4; +// Maximum number of directory actions, used to position the actions popup. +const ENTRY_MENU_ITEM_COUNT = 5; const ENTRY_MENU_WIDTH = 220; const ENTRY_MENU_HEIGHT = ENTRY_MENU_ITEM_COUNT * 34 + 8; @@ -206,6 +208,8 @@ function FileExplorerEntryMenu({ onCopy, onReveal, onDelete, + onCreateDirectory, + onRefresh, }: { state: FileExplorerEntryMenuState | null; onClose: () => void; @@ -213,6 +217,8 @@ function FileExplorerEntryMenu({ onCopy: (entry: FileExplorerEntry) => void; onReveal?: (entry: FileExplorerEntry) => void; onDelete?: (entry: FileExplorerEntry) => void; + onCreateDirectory?: (directory: string) => void; + onRefresh: () => void; }) { const ref = useRef(null); const onCloseRef = useRef(onClose); @@ -270,35 +276,49 @@ function FileExplorerEntryMenu({ if (!state) return null; const { entry } = state; - const isDirectory = entry.type === "directory"; - // Keep ENTRY_MENU_ITEM_COUNT in sync with this array. - const items = [ - { - label: isDirectory ? "Download directory" : "Download file", - action: () => onDownload(entry), - }, - { - label: "Copy absolute path", - action: () => onCopy(entry), - }, - ...(onReveal - ? [ - { - label: revealMenuLabel(isDirectory), - action: () => onReveal(entry), - }, - ] - : []), - ...(onDelete - ? [ - { - label: isDirectory ? "Delete directory" : "Delete file", - danger: true, - action: () => onDelete(entry), - }, - ] - : []), - ]; + const isDirectory = entry?.type === "directory"; + const items: { label: string; action: () => void; danger?: boolean }[] = entry + ? [ + ...(isDirectory && onCreateDirectory + ? [ + { + label: "New folder", + action: () => onCreateDirectory(entry.path), + }, + ] + : []), + { + label: isDirectory ? "Download directory" : "Download file", + action: () => onDownload(entry), + }, + { + label: "Copy absolute path", + action: () => onCopy(entry), + }, + ...(onReveal + ? [ + { + label: revealMenuLabel(isDirectory), + action: () => onReveal(entry), + }, + ] + : []), + ...(onDelete + ? [ + { + label: isDirectory ? "Delete directory" : "Delete file", + danger: true, + action: () => onDelete(entry), + }, + ] + : []), + ] + : [ + ...(onCreateDirectory + ? [{ label: "New folder", action: () => onCreateDirectory("") }] + : []), + { label: "Refresh files", action: onRefresh }, + ]; const menuMargin = 8; const menuWidth = ENTRY_MENU_WIDTH; const style: React.CSSProperties = { @@ -310,7 +330,10 @@ function FileExplorerEntryMenu({ ), top: Math.max( menuMargin, - Math.min(state.y, window.innerHeight - ENTRY_MENU_HEIGHT - menuMargin), + Math.min( + state.y, + window.innerHeight - (items.length * 34 + 8) - menuMargin, + ), ), zIndex: 1000, }; @@ -405,6 +428,19 @@ function FileExplorerContent({ ); const [pendingDeleteEntry, setPendingDeleteEntry] = useState(null); + const [newFolderDirectory, setNewFolderDirectory] = useState( + null, + ); + const [selectedPaths, setSelectedPaths] = useState>( + () => new Set(), + ); + const selectionAnchor = useRef(null); + const draggedEntries = useRef<{ + context: string; + entries: FileExplorerEntry[]; + } | null>(null); + const movingRef = useRef(false); + const [moving, setMoving] = useState(false); const [previewEntry, setPreviewEntry] = useState( null, ); @@ -451,6 +487,9 @@ function FileExplorerContent({ const filesystem = filesystemContext === runtimeContext; useEffect(() => { setFilesystemContext(null); + draggedEntries.current = null; + movingRef.current = false; + setMoving(false); }, [runtimeContext]); const runtimeContextRef = useRef(runtimeContext); runtimeContextRef.current = runtimeContext; @@ -666,6 +705,10 @@ function FileExplorerContent({ setDropTargetPath(null); setEntryMenu(null); setPendingDeleteEntry(null); + setNewFolderDirectory(null); + setSelectedPaths(new Set(activePath ? [activePath] : [])); + selectionAnchor.current = null; + draggedEntries.current = null; if (resourceChanged && !activePath) { setPreviewEntry(null); setPreview(null); @@ -688,7 +731,14 @@ function FileExplorerContent({ let removeKeyHandler = () => {}; if (showCloseButton) { const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape") onClose(); + if ( + e.key === "Escape" && + !( + e.target instanceof Element && + e.target.closest(".compact-modal, .context-menu") + ) + ) + onClose(); }; window.addEventListener("keydown", onKey); removeKeyHandler = () => window.removeEventListener("keydown", onKey); @@ -709,6 +759,14 @@ function FileExplorerContent({ showCloseButton, ]); + useEffect(() => { + if (activePath && !movingRef.current) { + setSelectedPaths((current) => + current.size > 1 ? current : new Set([activePath]), + ); + } + }, [activePath]); + useEffect(() => { if ( !open || @@ -1244,6 +1302,15 @@ function FileExplorerContent({ ); if (!runtimeContextIsCurrent(requestContext)) return; removeEntryFromCache(entry); + setSelectedPaths( + (current) => + new Set( + [...current].filter( + (path) => + path !== entry.path && !path.startsWith(`${entry.path}/`), + ), + ), + ); clearDeletedPreview(entry); await loadDirectory(parentDirectoryPath(entry.path), true); if (!runtimeContextIsCurrent(requestContext)) return; @@ -1269,7 +1336,7 @@ function FileExplorerContent({ } }; - const invalidateUploadedPreviews = (paths: string[]) => { + const invalidateWrittenPreviews = (paths: string[]) => { if (!workspace?.workspace_id || !paths.length) return; for (const path of paths) { invalidateFilePreviewCache( @@ -1286,6 +1353,129 @@ function FileExplorerContent({ const isFileDrag = (event: DragEvent) => Array.from(event.dataTransfer.types).includes("Files"); + const isMoveDrag = (event: DragEvent) => + !movingRef.current && + draggedEntries.current?.context === runtimeContext && + Array.from(event.dataTransfer.types).includes(FILE_MOVE_DRAG_TYPE); + + const createDirectory = async (name: string) => { + if (!workspace?.workspace_id || newFolderDirectory === null) return; + const directory = newFolderDirectory; + const requestContext = runtimeContext; + try { + await connectionClient.call("file.mkdir", { + workspace_id: workspace.workspace_id, + directory, + name, + }); + if (!runtimeContextIsCurrent(requestContext)) return; + setNewFolderDirectory(null); + if (directory) + updateCache({ expanded: new Set(expanded).add(directory) }); + await loadDirectory(directory, true); + if (!runtimeContextIsCurrent(requestContext)) return; + setSearchRevision((value) => value + 1); + store.notify({ + kind: "success", + message: "Folder created", + detail: name, + autoDismissMs: 5000, + }); + } catch (error) { + if (runtimeContextIsCurrent(requestContext)) + store.notify({ + kind: "error", + message: "Create folder failed", + detail: (error as Error).message, + }); + } + }; + + const moveDroppedFiles = async (directory: string) => { + const drag = draggedEntries.current; + draggedEntries.current = null; + setDropTargetPath(null); + if ( + !workspace?.workspace_id || + movingRef.current || + drag?.context !== runtimeContext || + !connectionClient.isCurrent() + ) + return; + const entries = drag.entries.filter( + (entry) => parentDirectoryPath(entry.path) !== directory, + ); + if (!entries.length) return; + const requestContext = runtimeContext; + movingRef.current = true; + setMoving(true); + try { + const results: PromiseSettledResult[] = []; + for (const entry of entries) { + if (!runtimeContextIsCurrent(requestContext)) return; + try { + const value = await connectionClient.call("file.move", { + workspace_id: workspace.workspace_id, + path: entry.path, + directory, + }); + results.push({ status: "fulfilled", value }); + } catch (reason) { + results.push({ status: "rejected", reason }); + } + } + if (!runtimeContextIsCurrent(requestContext)) return; + const moved = entries.filter( + (_, index) => results[index]?.status === "fulfilled", + ); + for (const entry of moved) { + removeEntryFromCache(entry); + clearDeletedPreview(entry); + } + invalidateWrittenPreviews( + moved.map((entry) => + directory ? `${directory}/${entry.name}` : entry.name, + ), + ); + setSelectedPaths( + (current) => + new Set( + [...current].filter( + (path) => !moved.some((entry) => entry.path === path), + ), + ), + ); + if (directory) + updateCache({ expanded: new Set(expanded).add(directory) }); + const directories = new Set([ + directory, + ...entries.map((entry) => parentDirectoryPath(entry.path)), + ]); + for (const path of directories) { + if (!runtimeContextIsCurrent(requestContext)) return; + await loadDirectory(path, true); + } + if (!runtimeContextIsCurrent(requestContext)) return; + setSearchRevision((value) => value + 1); + void loadGitStatus(true); + const failed = results.find( + (result): result is PromiseRejectedResult => + result.status === "rejected", + ); + store.notify({ + kind: failed ? "error" : "success", + message: failed ? "Some files could not be moved" : "Files moved", + detail: `${moved.length} of ${entries.length} moved${failed ? `: ${String(failed.reason?.message ?? failed.reason)}` : ""}`, + autoDismissMs: failed ? undefined : 5000, + }); + } finally { + if (runtimeContextIsCurrent(requestContext)) { + movingRef.current = false; + setMoving(false); + } + } + }; + const markUploadPath = (path: string, uploading: boolean) => { setUploadingPaths((current) => { const next = new Set(current); @@ -1324,7 +1514,7 @@ function FileExplorerContent({ } await loadDirectory(directory, true); if (!runtimeContextIsCurrent(requestContext)) return; - invalidateUploadedPreviews( + invalidateWrittenPreviews( results.flatMap((result) => result.status === "fulfilled" ? [result.value.path] : [], ), @@ -1358,43 +1548,64 @@ function FileExplorerContent({ const handleDirectoryDragOver = ( event: DragEvent, directory: string, + isDirectory = true, ) => { - if (!isFileDrag(event)) return; + const move = isMoveDrag(event); + if (!move && !isFileDrag(event)) return; + if (move && !isDirectory) { + event.stopPropagation(); + return; + } event.preventDefault(); event.stopPropagation(); - event.dataTransfer.dropEffect = "copy"; + event.dataTransfer.dropEffect = move ? "move" : "copy"; setDropTargetPath(directory); }; const handleDirectoryDrop = ( event: DragEvent, directory: string, + isDirectory = true, ) => { - if (!isFileDrag(event)) return; + const move = isMoveDrag(event); + if (!move && !isFileDrag(event)) return; + if (move && !isDirectory) { + event.stopPropagation(); + return; + } event.preventDefault(); event.stopPropagation(); - void uploadDroppedFiles(directory, event.dataTransfer.files); + if (move) void moveDroppedFiles(directory); + else void uploadDroppedFiles(directory, event.dataTransfer.files); }; const handleRootDragOver = (event: DragEvent) => { - if (!isFileDrag(event)) return; + if (!isFileDrag(event) && !isMoveDrag(event)) return; if ((event.target as HTMLElement | null)?.closest(".file-row")) return; event.preventDefault(); - event.dataTransfer.dropEffect = "copy"; + event.dataTransfer.dropEffect = isMoveDrag(event) ? "move" : "copy"; setDropTargetPath(""); }; const handleRootDrop = (event: DragEvent) => { - if (!isFileDrag(event)) return; + if (!isFileDrag(event) && !isMoveDrag(event)) return; if ((event.target as HTMLElement | null)?.closest(".file-row")) return; event.preventDefault(); - void uploadDroppedFiles("", event.dataTransfer.files); + if (isMoveDrag(event)) void moveDroppedFiles(""); + else void uploadDroppedFiles("", event.dataTransfer.files); }; const openEntryMenu = (entry: FileExplorerEntry, x: number, y: number) => { setEntryMenu({ entry, x, y }); }; + const refreshFiles = () => { + setSearchRevision((value) => value + 1); + const paths = new Set(["", ...expanded]); + for (const path of paths) void loadDirectory(path, true); + void loadGitStatus(true); + }; + const handleEntryPointerDown = ( event: ReactPointerEvent, entry: FileExplorerEntry, @@ -1436,6 +1647,49 @@ function FileExplorerContent({ return; } setWorkspacePathDragData(event.dataTransfer, entryAbsolutePath(entry)); + if (entry.type === "directory" || movingRef.current) return; + const entries = selectedPaths.has(entry.path) + ? Object.values(children) + .flat() + .filter( + (candidate) => + candidate.type !== "directory" && + selectedPaths.has(candidate.path), + ) + : [entry]; + if (!selectedPaths.has(entry.path)) { + setSelectedPaths(new Set([entry.path])); + selectionAnchor.current = entry.path; + } + draggedEntries.current = { context: runtimeContext, entries }; + event.dataTransfer.effectAllowed = "copyMove"; + event.dataTransfer.setData(FILE_MOVE_DRAG_TYPE, runtimeContext); + }; + + const visibleFilePaths = () => + Array.from( + fileTreeRef.current?.querySelectorAll( + ".file-row[data-file-type]:not([data-file-type='directory'])", + ) ?? [], + ).map((row) => row.dataset.filePath!); + + const selectEntry = ( + entry: FileExplorerEntry, + range = false, + toggle = false, + ) => { + if (entry.type === "directory") { + setSelectedPaths(new Set()); + selectionAnchor.current = null; + return; + } + const anchor = selectionAnchor.current; + const paths = visibleFilePaths(); + setSelectedPaths((current) => + selectFilePaths(current, entry.path, paths, anchor, range, toggle), + ); + if (!range || anchor === null || !paths.includes(anchor)) + selectionAnchor.current = entry.path; }; const activateEntry = (entry: FileExplorerEntry) => { @@ -1471,6 +1725,22 @@ function FileExplorerContent({ isExpanded: boolean, ) => { if (event.target !== event.currentTarget) return; + if ((event.metaKey || event.ctrlKey) && event.key.toLowerCase() === "a") { + event.preventDefault(); + event.stopPropagation(); + setSelectedPaths(new Set(visibleFilePaths())); + return; + } + if (event.key === "Escape") { + setSelectedPaths(new Set()); + return; + } + if (event.key === " " && !isDirectory) { + event.preventDefault(); + event.stopPropagation(); + selectEntry(entry, event.shiftKey, event.metaKey || event.ctrlKey); + return; + } const action = treeKeyboardAction(event.key, event.shiftKey); if (!action) return; event.preventDefault(); @@ -1482,6 +1752,20 @@ function FileExplorerContent({ action === "last" ) { focusTreeItem(event.currentTarget, action); + if (event.shiftKey) { + const path = (document.activeElement as HTMLElement | null)?.dataset + .filePath; + const target = Object.values(children) + .flat() + .find( + (candidate) => + candidate.path === path && candidate.type !== "directory", + ); + if (target) { + selectionAnchor.current ??= entry.path; + selectEntry(target, true); + } + } return; } if (action === "expand") { @@ -1505,8 +1789,10 @@ function FileExplorerContent({ return; } event.stopPropagation(); - if (action === "activate") activateEntry(entry); - else { + if (action === "activate") { + selectEntry(entry); + activateEntry(entry); + } else { const point = keyboardContextMenuPoint(event.currentTarget); openEntryMenu(entry, point.x, point.y); } @@ -1536,7 +1822,7 @@ function FileExplorerContent({
handleEntryDragStart(e, entry)} - onDragOver={(e) => handleDirectoryDragOver(e, uploadDirectory)} + onDragEnd={() => { + draggedEntries.current = null; + setDropTargetPath(null); + }} + onDragOver={(e) => + handleDirectoryDragOver(e, uploadDirectory, isDirectory) + } onDragLeave={(e) => { if (!e.currentTarget.contains(e.relatedTarget as Node | null)) { setDropTargetPath(null); } }} - onDrop={(e) => handleDirectoryDrop(e, uploadDirectory)} + onDrop={(e) => handleDirectoryDrop(e, uploadDirectory, isDirectory)} onContextMenu={(e) => { e.preventDefault(); e.stopPropagation(); @@ -1588,7 +1881,13 @@ function FileExplorerContent({ longPressTriggered.current = false; return; } - activateEntry(entry); + selectEntry( + entry, + !isDirectory && event.shiftKey, + !isDirectory && (event.metaKey || event.ctrlKey), + ); + if (!event.shiftKey && !event.metaKey && !event.ctrlKey) + activateEntry(entry); }} onDoubleClick={(event) => { if ( @@ -1769,7 +2068,22 @@ function FileExplorerContent({ ) : null}
-
+
{ + if ( + filesystem || + !workspace || + (event.target as Element).closest( + "[data-file-path], .file-explorer-toolbar, .file-search-result", + ) + ) + return; + event.preventDefault(); + event.stopPropagation(); + setEntryMenu({ entry: null, x: event.clientX, y: event.clientY }); + }} + > {filesystem && workspace ? ( - - setSearchMode(value as "files" | "content" | "loaded") - } - /> - - - +
+ + setSearchMode(value as "files" | "content" | "loaded") + } + /> - + + + + + +
{error ?

{error}

: null} + {selectedPaths.size > 1 || moving ? ( +
+ {moving + ? "Moving files..." + : `${selectedPaths.size} files selected`} +
+ ) : null} {rootInfo?.truncated ? (

This directory is truncated at 1000 entries. @@ -1888,6 +2212,10 @@ function FileExplorerContent({ ref={fileTreeRef} className={`file-tree ${dropTargetPath === "" ? "is-drop-target" : ""}`} role={query && searchMode !== "loaded" ? undefined : "tree"} + aria-label="Workspace files" + aria-multiselectable={ + query && searchMode !== "loaded" ? undefined : true + } onFocusCapture={() => setTreeHasFocus(true)} onBlurCapture={(event) => { if ( @@ -1914,7 +2242,9 @@ function FileExplorerContent({ ) : dropTargetPath === "" ? (

- Drop files to upload to workspace root + {draggedEntries.current + ? "Drop files to move to workspace root" + : "Drop files to upload to workspace root"}
) : null} {query && searchMode === "loaded" ? ( @@ -2049,6 +2379,17 @@ function FileExplorerContent({ : undefined } onDelete={filesystem ? undefined : setPendingDeleteEntry} + onCreateDirectory={filesystem ? undefined : setNewFolderDirectory} + onRefresh={refreshFiles} + /> + setNewFolderDirectory(null)} + onSubmit={(name) => void createDirectory(name)} /> { + const selected = new Set(["a", "b"]); + expect([ + ...selectFilePaths(selected, "c", ["a", "b", "c"], "a", false, false), + ]).toEqual(["c"]); + expect([ + ...selectFilePaths(selected, "b", ["a", "b", "c"], "a", false, true), + ]).toEqual(["a"]); + expect([ + ...selectFilePaths(selected, "c", ["a", "b", "c"], "a", false, true), + ]).toEqual(["a", "b", "c"]); + expect([...selected]).toEqual(["a", "b"]); +}); + +test("Shift selects the visible range in either direction and can extend a selection", () => { + const paths = ["a", "b", "c", "d"]; + expect([ + ...selectFilePaths(new Set(["d"]), "c", paths, "a", true, false), + ]).toEqual(["a", "b", "c"]); + expect([...selectFilePaths(new Set(), "a", paths, "c", true, false)]).toEqual( + ["a", "b", "c"], + ); + expect([ + ...selectFilePaths(new Set(["d"]), "b", paths, "a", true, true), + ]).toEqual(["d", "a", "b"]); + expect([ + ...selectFilePaths(new Set(["d"]), "c", paths, "missing", true, false), + ]).toEqual(["c"]); +}); diff --git a/web/src/components/fileExplorerSelection.ts b/web/src/components/fileExplorerSelection.ts new file mode 100644 index 00000000..c69207ed --- /dev/null +++ b/web/src/components/fileExplorerSelection.ts @@ -0,0 +1,22 @@ +export function selectFilePaths( + selected: Set, + path: string, + visiblePaths: string[], + anchor: string | null, + range: boolean, + toggle: boolean, +) { + const start = anchor === null ? -1 : visiblePaths.indexOf(anchor); + const end = visiblePaths.indexOf(path); + if (range && start >= 0 && end >= 0) { + return new Set([ + ...(toggle ? selected : []), + ...visiblePaths.slice(Math.min(start, end), Math.max(start, end) + 1), + ]); + } + if (!toggle) return new Set([path]); + const next = new Set(selected); + if (next.has(path)) next.delete(path); + else next.add(path); + return next; +} From 6a2e1a48b3c1809a04cb60139a8a1164219ae385 Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 10:43:16 +0800 Subject: [PATCH 2/4] Prevent duplicate folder creation requests --- .../components/FileExplorerDialog.dom.test.ts | 86 ++++++++++++++++--- web/src/components/FileExplorerDialog.tsx | 19 +++- web/src/components/ModalDialogs.tsx | 10 ++- 3 files changed, 100 insertions(+), 15 deletions(-) diff --git a/web/src/components/FileExplorerDialog.dom.test.ts b/web/src/components/FileExplorerDialog.dom.test.ts index e2c3469c..4031b6d7 100644 --- a/web/src/components/FileExplorerDialog.dom.test.ts +++ b/web/src/components/FileExplorerDialog.dom.test.ts @@ -73,6 +73,7 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { const pending = new Set>(); let activeMoves = 0; let peakMoves = 0; + let mkdirDelay: Promise | undefined; const calls: Array<{ method: string; params: Record }> = []; const client: ConnectionClient = { @@ -98,12 +99,14 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { }), "file.read": (params) => readLocalFile(directory, String(params.path)), - "file.mkdir": (params) => - createLocalDirectory( + "file.mkdir": async (params) => { + await mkdirDelay; + return createLocalDirectory( directory, String(params.directory), String(params.name), - ), + ); + }, "file.move": (params) => moveLocalFile( directory, @@ -142,7 +145,7 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { while (pending.size) await Promise.allSettled([...pending]); }); } - async function submitFolderName(name: string) { + async function setFolderName(name: string) { await update(() => { const input = container.querySelector( "[role='dialog'] input", @@ -155,15 +158,19 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { new browser.Event("input", { bubbles: true }) as unknown as Event, ); }); - await update(() => - container.querySelector("form")!.dispatchEvent( - new browser.Event("submit", { - bubbles: true, - cancelable: true, - }) as unknown as Event, - ), + } + function submitFolder() { + container.querySelector("form")!.dispatchEvent( + new browser.Event("submit", { + bubbles: true, + cancelable: true, + }) as unknown as Event, ); } + async function submitFolderName(name: string) { + await setFolderName(name); + await update(submitFolder); + } function row(path: string) { const element = [ ...container.querySelectorAll(".file-row[data-file-path]"), @@ -266,9 +273,64 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { await update(() => click(container.querySelector("[aria-label='New folder']")!), ); - await submitFolderName("target"); + await setFolderName("target"); + const creation = Promise.withResolvers(); + mkdirDelay = creation.promise; + try { + await act(async () => { + submitFolder(); + submitFolder(); + }); + expect( + calls.filter((call) => call.method === "file.mkdir"), + ).toHaveLength(1); + expect( + container.querySelector("button[type='submit']")! + .disabled, + ).toBe(true); + expect( + container.querySelector("[role='dialog'] input")! + .disabled, + ).toBe(true); + await act(async () => submitFolder()); + expect( + calls.filter((call) => call.method === "file.mkdir"), + ).toHaveLength(1); + } finally { + await update(() => creation.resolve()); + mkdirDelay = undefined; + } expect(await readdir(directory)).toContain("target"); expect(row("target")).toBeDefined(); + expect( + notify.mock.calls.map(([notification]) => notification.kind), + ).toEqual(["success"]); + expect(container.querySelector("form")).toBeNull(); + await update(() => + click(container.querySelector("[aria-label='New folder']")!), + ); + await submitFolderName("target"); + expect( + notify.mock.calls[notify.mock.calls.length - 1]?.[0], + ).toMatchObject({ + kind: "error", + message: "Create folder failed", + }); + expect( + container.querySelector("[role='dialog'] input")! + .value, + ).toBe("target"); + expect( + container.querySelector("[role='dialog'] input")! + .disabled, + ).toBe(false); + expect( + container.querySelector("button[type='submit']")! + .disabled, + ).toBe(false); + await submitFolderName("retry-folder"); + expect(await readdir(directory)).toContain("retry-folder"); + expect(container.querySelector("form")).toBeNull(); await update(() => row("target").dispatchEvent( new browser.MouseEvent("contextmenu", { diff --git a/web/src/components/FileExplorerDialog.tsx b/web/src/components/FileExplorerDialog.tsx index 4576608d..c65bc9b1 100644 --- a/web/src/components/FileExplorerDialog.tsx +++ b/web/src/components/FileExplorerDialog.tsx @@ -431,6 +431,8 @@ function FileExplorerContent({ const [newFolderDirectory, setNewFolderDirectory] = useState( null, ); + const creatingDirectoryRef = useRef(false); + const [creatingDirectory, setCreatingDirectory] = useState(false); const [selectedPaths, setSelectedPaths] = useState>( () => new Set(), ); @@ -490,6 +492,8 @@ function FileExplorerContent({ draggedEntries.current = null; movingRef.current = false; setMoving(false); + creatingDirectoryRef.current = false; + setCreatingDirectory(false); }, [runtimeContext]); const runtimeContextRef = useRef(runtimeContext); runtimeContextRef.current = runtimeContext; @@ -1359,9 +1363,16 @@ function FileExplorerContent({ Array.from(event.dataTransfer.types).includes(FILE_MOVE_DRAG_TYPE); const createDirectory = async (name: string) => { - if (!workspace?.workspace_id || newFolderDirectory === null) return; + if ( + !workspace?.workspace_id || + newFolderDirectory === null || + creatingDirectoryRef.current + ) + return; const directory = newFolderDirectory; const requestContext = runtimeContext; + creatingDirectoryRef.current = true; + setCreatingDirectory(true); try { await connectionClient.call("file.mkdir", { workspace_id: workspace.workspace_id, @@ -1388,6 +1399,11 @@ function FileExplorerContent({ message: "Create folder failed", detail: (error as Error).message, }); + } finally { + if (runtimeContextIsCurrent(requestContext)) { + creatingDirectoryRef.current = false; + setCreatingDirectory(false); + } } }; @@ -2388,6 +2404,7 @@ function FileExplorerContent({ label="Folder name" placeholder="New folder" submitLabel="Create" + submitting={creatingDirectory} onClose={() => setNewFolderDirectory(null)} onSubmit={(name) => void createDirectory(name)} /> diff --git a/web/src/components/ModalDialogs.tsx b/web/src/components/ModalDialogs.tsx index 3ccc4c12..1a17c8e9 100644 --- a/web/src/components/ModalDialogs.tsx +++ b/web/src/components/ModalDialogs.tsx @@ -10,6 +10,7 @@ export function TextInputDialog({ initialValue = "", placeholder, submitLabel = "Save", + submitting = false, onSubmit, onClose, }: { @@ -19,6 +20,7 @@ export function TextInputDialog({ initialValue?: string; placeholder?: string; submitLabel?: string; + submitting?: boolean; onSubmit: (value: string) => void; onClose: () => void; }) { @@ -44,7 +46,7 @@ export function TextInputDialog({ const submit = (e: React.FormEvent) => { e.preventDefault(); - onSubmit(value); + if (!submitting) onSubmit(value); }; return ( @@ -54,6 +56,7 @@ export function TextInputDialog({ role="dialog" aria-modal="true" aria-label={title} + aria-busy={submitting} onSubmit={submit} onMouseDown={(e) => e.stopPropagation()} > @@ -66,6 +69,7 @@ export function TextInputDialog({ {label} setValue(e.currentTarget.value)} placeholder={placeholder} @@ -76,7 +80,9 @@ export function TextInputDialog({ - +
From 6814719cf004371a98bf11a89ce18d4a1519f43f Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 11:09:43 +0800 Subject: [PATCH 3/4] Keep nested Escape from closing the file explorer --- .../components/FileExplorerDialog.dom.test.ts | 48 ++++++++++++++++++- web/src/components/FileExplorerDialog.tsx | 26 ++++++---- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/web/src/components/FileExplorerDialog.dom.test.ts b/web/src/components/FileExplorerDialog.dom.test.ts index 4031b6d7..008e8a1a 100644 --- a/web/src/components/FileExplorerDialog.dom.test.ts +++ b/web/src/components/FileExplorerDialog.dom.test.ts @@ -148,7 +148,7 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { async function setFolderName(name: string) { await update(() => { const input = container.querySelector( - "[role='dialog'] input", + "[role='dialog'][aria-label='New folder'] input", )!; Object.getOwnPropertyDescriptor( browser.HTMLInputElement.prototype, @@ -596,6 +596,52 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { expect( container.querySelector("[role='dialog'][aria-label='File Explorer']"), ).not.toBeNull(); + await update(() => + click(container.querySelector("button[aria-label='New folder']")!), + ); + await setFolderName("pending-folder"); + const pendingCreation = Promise.withResolvers(); + mkdirDelay = pendingCreation.promise; + try { + await act(async () => submitFolder()); + const input = container.querySelector( + "[aria-label='New folder'] input", + )!; + expect(input.disabled).toBe(true); + // Model browser focus loss when the focused input becomes disabled. + document.body.focus(); + expect(document.activeElement === document.body).toBe(true); + await act(async () => + document.body.dispatchEvent( + new browser.KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + }) as unknown as Event, + ), + ); + expect(closes).toBe(0); + expect( + container.querySelector("[role='dialog'][aria-label='New folder']"), + ).toBeNull(); + expect( + container.querySelector( + "[role='dialog'][aria-label='File Explorer']", + ), + ).not.toBeNull(); + } finally { + await update(() => pendingCreation.resolve()); + mkdirDelay = undefined; + } + expect(await readdir(directory)).toContain("pending-folder"); + await update(() => + document.body.dispatchEvent( + new browser.KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + }) as unknown as Event, + ), + ); + expect(closes).toBe(1); } finally { await act(async () => root.unmount()); connection.mockRestore(); diff --git a/web/src/components/FileExplorerDialog.tsx b/web/src/components/FileExplorerDialog.tsx index c65bc9b1..84375bed 100644 --- a/web/src/components/FileExplorerDialog.tsx +++ b/web/src/components/FileExplorerDialog.tsx @@ -5,6 +5,7 @@ import { type PointerEvent as ReactPointerEvent, Suspense, useEffect, + useEffectEvent, useMemo, useRef, useState, @@ -669,6 +670,20 @@ function FileExplorerContent({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [explorerRefreshKey, explorerRefreshVersion]); + const handleExplorerKeyDown = useEffectEvent((event: KeyboardEvent) => { + if ( + event.key === "Escape" && + newFolderDirectory === null && + pendingDeleteEntry === null && + entryMenu === null && + !( + event.target instanceof Element && + event.target.closest(".compact-modal, .context-menu") + ) + ) + onClose(); + }); + useEffect(() => { if (!open) return; const resourceChanged = @@ -734,16 +749,7 @@ function FileExplorerContent({ void loadGitStatus(); let removeKeyHandler = () => {}; if (showCloseButton) { - const onKey = (e: KeyboardEvent) => { - if ( - e.key === "Escape" && - !( - e.target instanceof Element && - e.target.closest(".compact-modal, .context-menu") - ) - ) - onClose(); - }; + const onKey = (event: KeyboardEvent) => handleExplorerKeyDown(event); window.addEventListener("keydown", onKey); removeKeyHandler = () => window.removeEventListener("keydown", onKey); } From 9be426c1d2c78b312275f9d5a6fd96bd269a8f6a Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 11:17:31 +0800 Subject: [PATCH 4/4] Fix file selection and POSIX path edge cases --- server/src/workspace/file-mutations.test.ts | 46 ++++++++++++++----- server/src/workspace/file-paths.ts | 6 ++- .../components/FileExplorerDialog.dom.test.ts | 14 ++++++ web/src/components/FileExplorerDialog.tsx | 2 + 4 files changed, 56 insertions(+), 12 deletions(-) diff --git a/server/src/workspace/file-mutations.test.ts b/server/src/workspace/file-mutations.test.ts index d25afec6..f13d3fd1 100644 --- a/server/src/workspace/file-mutations.test.ts +++ b/server/src/workspace/file-mutations.test.ts @@ -135,6 +135,34 @@ for (const remote of [false, true]) { }, ); + test.skipIf(process.platform === "win32")( + "moves POSIX names that resemble Windows paths", + async () => { + await fixture(async (files, root) => { + await files.createWorkspaceDirectory({ ...params, name: "a:target" }); + await files.createWorkspaceDirectory({ + ...params, + directory: "a:target", + name: "nested", + }); + for (const name of ["a:notes", "C:\\notes", "\\notes"]) { + await writeFile(join(root, name), name); + expect( + await files.moveWorkspaceFile({ + ...params, + path: name, + directory: "a:target/nested", + }), + ).toMatchObject({ path: `a:target/nested/${name}` }); + expect( + await readFile(join(root, "a:target/nested", name), "utf8"), + ).toBe(name); + await expect(lstat(join(root, name))).rejects.toThrow(); + } + }); + }, + ); + test("never overwrites a destination file, directory or symlink", async () => { await fixture(async (files, root, outside) => { await mkdir(join(root, "target")); @@ -422,12 +450,15 @@ if result == 0: test("rejects invalid mutation paths, names, scopes and missing workspace identity", async () => { await fixture(async (files) => { - for (const directory of [ + const invalidPaths = [ "../outside", "/absolute", - "C:\\outside", "bad\0path", - ]) { + ...(process.platform === "win32" + ? ["C:\\outside", "C:outside", "\\outside"] + : []), + ]; + for (const directory of invalidPaths) { await expect( files.createWorkspaceDirectory({ ...params, @@ -439,14 +470,7 @@ if result == 0: files.moveWorkspaceFile({ ...params, path: "a", directory }), ).rejects.toThrow(); } - for (const path of [ - "", - ".", - "../outside", - "/absolute", - "C:\\outside", - "bad\0path", - ]) + for (const path of ["", ".", ...invalidPaths]) await expect( files.moveWorkspaceFile({ ...params, path }), ).rejects.toThrow(); diff --git a/server/src/workspace/file-paths.ts b/server/src/workspace/file-paths.ts index bea333ae..5adfdf46 100644 --- a/server/src/workspace/file-paths.ts +++ b/server/src/workspace/file-paths.ts @@ -12,7 +12,11 @@ export function sanitizeExplorerPath(value: unknown): string { } export function sanitizeMutationPath(value: unknown): string { - if (typeof value !== "string" || /^(?:[\\/]|[a-z]:)/i.test(value)) { + if ( + typeof value !== "string" || + isAbsolute(value) || + (process.platform === "win32" && /^[a-z]:/i.test(value)) + ) { throw new Error("file operations require a workspace-relative path"); } if (process.platform === "win32") return sanitizeExplorerPath(value); diff --git a/web/src/components/FileExplorerDialog.dom.test.ts b/web/src/components/FileExplorerDialog.dom.test.ts index 008e8a1a..29f9c464 100644 --- a/web/src/components/FileExplorerDialog.dom.test.ts +++ b/web/src/components/FileExplorerDialog.dom.test.ts @@ -576,6 +576,20 @@ if (process.env.ROAMGATE_FILE_EXPLORER_DOM_TEST !== "1") { ); expect(closes).toBe(0); expect(menuLabels()).toEqual([]); + await update(() => click(row("a.txt"))); + await update(() => click(row("c.txt"), { metaKey: true })); + expect(selected()).toEqual(["a.txt", "c.txt"]); + const clearSelection = new browser.KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + cancelable: true, + }); + await update(() => + row("c.txt").dispatchEvent(clearSelection as unknown as Event), + ); + expect(closes).toBe(0); + expect(selected()).toEqual([]); + expect(clearSelection.defaultPrevented).toBe(true); await update(() => click(container.querySelector("button[aria-label='New folder']")!), ); diff --git a/web/src/components/FileExplorerDialog.tsx b/web/src/components/FileExplorerDialog.tsx index 84375bed..ab1600cb 100644 --- a/web/src/components/FileExplorerDialog.tsx +++ b/web/src/components/FileExplorerDialog.tsx @@ -1754,6 +1754,8 @@ function FileExplorerContent({ return; } if (event.key === "Escape") { + event.preventDefault(); + event.stopPropagation(); setSelectedPaths(new Set()); return; }