From 901df106f1e0d84d16a225b8442efae3277e7755 Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 00:01:32 +0800 Subject: [PATCH 1/3] Fix workspace paths and add directory suggestions --- FEATURES.md | 3 + docs/ARCHITECTURE.md | 8 + .../src/connections/profile-process.test.ts | 32 +- server/src/index.ts | 28 +- server/src/workspace/path.test.ts | 223 ++++++++++++++ server/src/workspace/path.ts | 166 +++++++++++ shared/workspacePath.ts | 5 + web/src/components/CreateWorkspaceDialog.css | 31 ++ web/src/components/CreateWorkspaceDialog.tsx | 163 +++++++++- web/src/components/ModalDialogs.test.ts | 279 +++++++++++++++++- web/src/components/WorkspaceTree.tsx | 2 + 11 files changed, 922 insertions(+), 18 deletions(-) create mode 100644 server/src/workspace/path.test.ts create mode 100644 server/src/workspace/path.ts create mode 100644 shared/workspacePath.ts create mode 100644 web/src/components/CreateWorkspaceDialog.css diff --git a/FEATURES.md b/FEATURES.md index 1b452d0f..74c33adc 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -9,6 +9,9 @@ A Web/PWA client for a running [Herdr](https://herdr.dev) server. - Create, rename, pin, switch, and close workspaces/tabs. Linked Git worktrees group under their repository; pins and collapsed groups stay in this browser. Double-click a tab to rename it, or use its context menu. +- Workspace creation expands `~/` on the connected host, checks that the + directory exists, and suggests subdirectories as you type. Use an absolute + path or `~/`; leave the field blank to preserve Herdr's default cwd policy. - Pinned tabs lead the tab strip and cannot be closed from Roamgate; their panes still close while another pane remains. Herdr keeps tab ids across restarts, so pins persist until the tab closes elsewhere, such as in the TUI. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0c7a2705..bd349972 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -230,6 +230,14 @@ serialized focus/scroll lane without another endpoint or focus call. Omitting synthetic cwd preserves Herdr's `terminal.new_cwd`; explicit cwd wins. Shared same-tab focus still controls the `follow` source. +`workspace.path` expands `~`/`~/` on the selected connection's host, validates +directory access, and returns up to 50 directory suggestions for a directory +or partial basename. Explicit workspace creation paths must be absolute or +home-relative; the bridge resolves and checks them again before dispatch, so +invalid paths fail instead of silently falling back to Herdr's home directory. +Empty cwd still preserves Herdr's configured policy. These checks use native +filesystem access locally and quoted Bash/PowerShell commands over SSH. + Missing source attachments fail. Only first-workspace bootstrap uses control creation, serializing an empty-topology check per runtime. Competing requests retry when topology becomes nonempty. A 20-second admission deadline covers diff --git a/server/src/connections/profile-process.test.ts b/server/src/connections/profile-process.test.ts index c569aedc..4a8dadd6 100644 --- a/server/src/connections/profile-process.test.ts +++ b/server/src/connections/profile-process.test.ts @@ -1,5 +1,11 @@ import { afterEach, expect, test } from "bun:test"; -import { chmodSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + mkdirSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; import * as net from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -823,7 +829,9 @@ test("production routing bootstraps only a verified empty session and serializes JSON.stringify({ id, method, - connection_id: method === "workspace.create" ? "empty" : undefined, + connection_id: method.startsWith("workspace.") + ? "empty" + : undefined, ...(generation === undefined ? {} : { connection_generation: generation }), @@ -842,6 +850,15 @@ test("production routing bootstraps only a verified empty session and serializes message: "Cannot verify an empty session. Refresh and retry creation.", }); expect(mutations).toHaveLength(0); + const chosen = join(realpathSync(root), "chosen"); + mkdirSync(chosen); + expect( + (await rpc(browsers[0], "workspace.path", { path: "~/chosen" })).result, + ).toEqual({ + path: chosen, + error: null, + directories: [], + }); valid = true; const results = await Promise.all( browsers.map((ws) => @@ -858,21 +875,24 @@ test("production routing bootstraps only a verified empty session and serializes ( await rpc(browsers[0], "workspace.create", { browser_source: null, - cwd: "/wrong", + cwd: join(root, "missing"), }) ).error, - ).toBeDefined(); + ).toMatchObject({ + message: expect.stringContaining("Directory does not exist."), + }); + expect(mutations).toHaveLength(1); workspaces = []; expect( ( await rpc(browsers[0], "workspace.create", { browser_source: null, - cwd: "/explicit", + cwd: "~/chosen", focus: true, }) ).result.type, ).toBe("workspace_created"); - expect(mutations[1]).toEqual({ cwd: "/explicit", focus: false }); + expect(mutations[1]).toEqual({ cwd: chosen, focus: false }); workspaces = []; expect( ( diff --git a/server/src/index.ts b/server/src/index.ts index 032df829..de91d30f 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -104,6 +104,7 @@ import { import { runProcessWithCodeTimeout, shQuote } from "./utils/process-utils"; import { rpcLogLevel } from "./utils/rpc-logging"; import { createWorkspaceWorktree } from "./worktree/create"; +import { inspectWorkspacePath } from "./workspace/path"; import { removeWorktreeWithRecovery, WORKTREE_REMOVE_TIMEOUT_MS, @@ -689,7 +690,8 @@ async function handleRpc(ws: ServerWebSocket, raw: string) { return; } const req = parsed as RpcRequest; - const { id, method, params } = req; + const { id, method } = req; + let { params } = req; let validationConnectionId: string | undefined; let connectionIdValidationFailed = false; if (Object.hasOwn(req, "connection_id")) { @@ -974,6 +976,30 @@ async function handleRpc(ws: ServerWebSocket, raw: string) { readEntry: readAgentHistoryEntry, } = connection.agentSessions; + if ( + method === "workspace.path" || + (method === "workspace.create" && params?.cwd != null && params.cwd !== "") + ) { + try { + const info = await inspectWorkspacePath( + method === "workspace.path" ? params?.path : params?.cwd, + { host: sshHost(), remotePlatform: connection.remotePlatform }, + method === "workspace.path", + ); + if (method === "workspace.path") { + sendReply({ id, result: info }, method); + return; + } + if (info.error) throw new Error(`${info.error} ${info.path}`); + if (!requestIsCurrent()) + throw new Error("Creation connection changed. Reconnect and retry."); + params = { ...params, cwd: info.path }; + } catch (error) { + sendError(`${method}-error`, error); + return; + } + } + if ( (method === "tab.create" || method === "workspace.create") && params && diff --git a/server/src/workspace/path.test.ts b/server/src/workspace/path.test.ts new file mode 100644 index 00000000..d7a33c6b --- /dev/null +++ b/server/src/workspace/path.test.ts @@ -0,0 +1,223 @@ +import { expect, spyOn, test } from "bun:test"; +import { + mkdir, + mkdtemp, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { homedir, tmpdir } from "node:os"; +import { join, parse, relative } from "node:path"; +import * as processUtils from "../utils/process-utils"; +import { inspectWorkspacePath } from "./path"; + +for (const remote of [false, true]) { + test(`${remote ? "SSH" : "local"} paths expand home, validate directories and complete prefixes safely`, async () => { + if (remote && process.platform === "win32") return; + const root = await realpath( + await mkdtemp(join(tmpdir(), "workspace-path-")), + ); + const run = processUtils.runProcessWithCodeTimeout; + const ssh = remote + ? spyOn(processUtils, "runProcessWithCodeTimeout").mockImplementation( + (argv, timeout) => { + expect(argv[0]).toBe("ssh"); + expect(argv.at(-2)).toBe("remote-host"); + return run(["bash", "-c", argv.at(-1)!], timeout, { cwd: root }); + }, + ) + : null; + const connection = remote ? { host: "remote-host" } : {}; + try { + await mkdir(join(root, "project")); + await mkdir(join(root, "project", "child")); + await mkdir(join(root, "ProjectCase")); + await mkdir(join(root, ".hidden")); + if (process.platform !== "win32") { + await mkdir(join(root, "literal\\")); + await mkdir(join(root, "literal")); + await mkdir(join(root, "literal\\", "child")); + await mkdir(join(root, "literal", "other")); + expect( + ( + await inspectWorkspacePath( + join(root, "literal\\"), + connection, + false, + ) + ).path, + ).toBe(join(root, "literal\\")); + expect( + (await inspectWorkspacePath(join(root, "literal\\"), connection)) + .directories, + ).toEqual([join(root, "literal\\", "child")]); + expect( + ( + await inspectWorkspacePath( + join(root, "literal\\", "ch"), + connection, + ) + ).directories, + ).toEqual([join(root, "literal\\", "child")]); + } + const quoted = "quote' $(touch SENTINEL)"; + await mkdir(join(root, quoted)); + await writeFile(join(root, "file"), "text"); + await symlink(join(root, "project"), join(root, "linked"), "dir"); + + expect(await inspectWorkspacePath("~", connection, false)).toMatchObject({ + path: await realpath(homedir()), + error: null, + directories: [], + }); + if (process.platform === "win32") { + const driveRoot = parse(root).root; + for (const path of [driveRoot, `${driveRoot}/`]) { + expect(await inspectWorkspacePath(path, connection, false)).toEqual({ + path: driveRoot, + error: null, + directories: [], + }); + } + } + expect( + await inspectWorkspacePath( + `~/${relative(homedir(), root)}/project`, + connection, + ), + ).toEqual({ + path: join(root, "project"), + error: null, + directories: [join(root, "project", "child")], + }); + const list = await inspectWorkspacePath(`${root}/`, connection); + expect(list.error).toBeNull(); + expect(list.directories).toContain(join(root, "project")); + expect(list.directories).toContain(join(root, "linked")); + expect(list.directories).not.toContain(join(root, ".hidden")); + expect(list.directories).not.toContain(join(root, "file")); + expect( + (await inspectWorkspacePath(join(root, "projectc"), connection)) + .directories, + ).toEqual( + process.platform === "win32" ? [join(root, "ProjectCase")] : [], + ); + expect(await inspectWorkspacePath(join(root, "pro"), connection)).toEqual( + { + path: join(root, "pro"), + error: "Directory does not exist.", + directories: + process.platform === "win32" + ? [join(root, "project"), join(root, "ProjectCase")] + : [join(root, "project")], + }, + ); + expect( + (await inspectWorkspacePath(join(root, ".hid"), connection)) + .directories, + ).toEqual([join(root, ".hidden")]); + expect( + (await inspectWorkspacePath(join(root, "file"), connection)).error, + ).toBe("Path is not a directory."); + expect( + (await inspectWorkspacePath(join(root, "absent", "child"), connection)) + .error, + ).toBe("Directory does not exist."); + expect( + (await inspectWorkspacePath(join(root, quoted), connection)).error, + ).toBeNull(); + expect( + (await inspectWorkspacePath(join(root, "SENTINEL"), connection)).error, + ).toBe("Directory does not exist."); + expect( + (await inspectWorkspacePath(join(root, "linked"), connection, false)) + .path, + ).toBe(join(root, "project")); + } finally { + ssh?.mockRestore(); + await rm(root, { recursive: true, force: true }); + } + }); +} + +test("local completion filters the entire directory before limiting suggestions", async () => { + const root = await realpath( + await mkdtemp(join(tmpdir(), "workspace-path-limit-")), + ); + try { + await Promise.all( + Array.from({ length: 1000 }, (_, i) => + mkdir(join(root, `a-${i.toString().padStart(4, "0")}`)), + ), + ); + const matches = Array.from({ length: 53 }, (_, i) => + join(root, `z-project-${i.toString().padStart(3, "0")}`), + ); + await Promise.all(matches.map((directory) => mkdir(directory))); + const info = await inspectWorkspacePath(join(root, "z-project-"), {}); + expect(info.error).toBe("Directory does not exist."); + expect(info.directories).toEqual(matches.slice(0, 50)); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("invalid paths fail before any host command", async () => { + const command = spyOn(processUtils, "runProcessWithCodeTimeout"); + try { + for (const path of [ + null, + {}, + "", + "relative", + "~someone/project", + "/tmp/\0", + "/tmp/\n", + "x".repeat(4097), + ]) { + await expect( + inspectWorkspacePath(path, { host: "remote-host" }), + ).rejects.toThrow(); + } + expect(command).not.toHaveBeenCalled(); + } finally { + command.mockRestore(); + } +}); + +test("Windows SSH paths use quoted PowerShell and the remote user's home", async () => { + const command = spyOn( + processUtils, + "runProcessWithCodeTimeout", + ).mockImplementation(async (argv) => { + const wire = argv.at(-1)!; + expect(wire).toStartWith("powershell.exe "); + const script = Buffer.from(wire.split(" ").at(-1)!, "base64").toString( + "utf16le", + ); + expect(script).toContain("$env:USERPROFILE"); + expect(script).toContain("$path = '~/quote'' $(throw)' ".trimEnd()); + expect(script).toContain("-LiteralPath"); + return { + code: 0, + stdout: `PATH\t${Buffer.from("C:\\Users\\remote\\quote' $(throw)").toString("base64")}\nERROR\t\n`, + stderr: "", + }; + }); + try { + expect( + await inspectWorkspacePath( + "~/quote' $(throw)", + { host: "remote-host", remotePlatform: "windows" }, + false, + ), + ).toEqual({ + path: "C:\\Users\\remote\\quote' $(throw)", + error: null, + directories: [], + }); + } finally { + command.mockRestore(); + } +}); diff --git a/server/src/workspace/path.ts b/server/src/workspace/path.ts new file mode 100644 index 00000000..3253e3a3 --- /dev/null +++ b/server/src/workspace/path.ts @@ -0,0 +1,166 @@ +import { access, readdir, realpath, stat } from "node:fs/promises"; +import { constants } from "node:fs"; +import { homedir } from "node:os"; +import { basename, dirname, isAbsolute, join, resolve } from "node:path"; +import type { WorkspacePathInfo } from "../../../shared/workspacePath"; +import { sshCommandArgv } from "../bridge/ssh-command"; +import { powershellCommand } from "../bridge/ssh-windows"; +import type { RemotePlatform } from "../connections/unix-only"; +import { runProcessWithCodeTimeout, shQuote } from "../utils/process-utils"; + +const DIRECTORY_LIMIT = 50; + +async function realDirectoryPath(path: string): Promise { + // Bun realpath drops the separator from Windows drive roots, changing C:\ into C:. + if (process.platform === "win32" && /^[a-z]:[/\\]+$/i.test(path)) return path; + // shortcut: retain symlink spelling for trailing Unix backslashes until Bun realpath preserves them. + return process.platform !== "win32" && path.endsWith("\\") + ? path + : realpath(path); +} + +export async function inspectWorkspacePath( + value: unknown, + connection: { host?: string; remotePlatform?: RemotePlatform }, + suggestions = true, +): Promise { + if ( + typeof value !== "string" || + !value.trim() || + value.length > 4096 || + /[\x00-\x1f\x7f]/.test(value) + ) + throw new Error("Enter a valid directory path."); + const input = value.trim(); + const windows = connection.host + ? connection.remotePlatform === "windows" + : process.platform === "win32"; + const homePath = (windows ? /^~(?:[/\\]|$)/ : /^~(?:\/|$)/).test(input); + if ( + !(windows ? /^(?:[a-z]:[/\\]|[/\\]{2})/i.test(input) : isAbsolute(input)) && + !homePath + ) + throw new Error("Use an absolute path or a path starting with ~/."); + + if (connection.host) { + const command = windows + ? powershellCommand(` +$ErrorActionPreference = 'Stop' +function Emit($kind, $value) { Write-Output ($kind + "\t" + [Convert]::ToBase64String([Text.Encoding]::UTF8.GetBytes($value))) } +$path = '${input.replaceAll("'", "''")}' +if ($path -match '^~([/\\\\]|$)') { $path = $env:USERPROFILE + $path.Substring(1) } +$path = [IO.Path]::GetFullPath($path) +$errorText = '' +if (Test-Path -LiteralPath $path -PathType Container) { $path = (Get-Item -Force -LiteralPath $path).FullName } +elseif (Test-Path -LiteralPath $path) { $errorText = 'Path is not a directory.' } +else { $errorText = 'Directory does not exist.' } +Emit 'PATH' $path +Emit 'ERROR' $errorText +if (${suggestions ? "$true" : "$false"}) { + $directory = $path + $prefix = '' + if ($errorText) { $directory = [IO.Path]::GetDirectoryName($path); $prefix = [IO.Path]::GetFileName($path) } + Get-ChildItem -Force -Directory -LiteralPath $directory -ErrorAction SilentlyContinue | + Where-Object { $_.Name.StartsWith($prefix, [StringComparison]::OrdinalIgnoreCase) -and ($prefix.StartsWith('.') -or -not $_.Name.StartsWith('.')) -and $_.Name -notmatch '[\\x00-\\x1f\\x7f]' } | + Sort-Object Name | Select-Object -First ${DIRECTORY_LIMIT} | ForEach-Object { Emit 'DIRECTORY' $_.FullName } +} +`) + : `bash -c ${shQuote(` +set -eu +path=${shQuote(input)} +case "$path" in '~'|'~/'*) path="$HOME\${path#\\~}" ;; esac +error_text='' +if [ -d "$path" ]; then + if [ -x "$path" ]; then path="$(cd -- "$path" && pwd -P)"; else error_text='Directory is not accessible.'; fi +elif [ -e "$path" ]; then error_text='Path is not a directory.' +else error_text='Directory does not exist.'; fi +emit() { printf '%s\\t%s\\n' "$1" "$(printf '%s' "$2" | base64 | tr -d '\\n')"; } +emit PATH "$path" +emit ERROR "$error_text" +if [ ${suggestions ? "1" : "0"} = 1 ]; then + directory="$path" + prefix='' + if [ -n "$error_text" ]; then directory="$(dirname -- "$path")"; prefix="$(basename -- "$path")"; fi + count=0 + for child in "$directory"/* "$directory"/.[!.]* "$directory"/..?*; do + [ -d "$child" ] || continue + name="\${child##*/}" + case "$name" in "$prefix"*) ;; *) continue ;; esac + case "$name" in .*) case "$prefix" in .*) ;; *) continue ;; esac ;; esac + [[ "$name" =~ [[:cntrl:]] ]] && continue + emit DIRECTORY "$child" + count=$((count + 1)) + [ "$count" -lt ${DIRECTORY_LIMIT} ] || break + done +fi +`)}`; + const result = await runProcessWithCodeTimeout( + sshCommandArgv(connection.host, command), + 8000, + ); + if (result.code !== 0) + throw new Error("Could not check the directory on the connected host."); + const info: WorkspacePathInfo = { path: "", error: null, directories: [] }; + for (const line of result.stdout.split(/\r?\n/)) { + const [kind, encoded = ""] = line.split("\t"); + const text = Buffer.from(encoded, "base64").toString("utf8"); + if (kind === "PATH") info.path = text; + if (kind === "ERROR") info.error = text || null; + if (kind === "DIRECTORY") info.directories.push(text); + } + if (!info.path) + throw new Error("Invalid directory response from the connected host."); + return info; + } + + const info: WorkspacePathInfo = { + path: resolve(homePath ? join(homedir(), input.slice(2)) : input), + error: null, + directories: [], + }; + try { + if (!(await stat(info.path)).isDirectory()) { + info.error = "Path is not a directory."; + } else { + await access(info.path, constants.X_OK); + info.path = await realDirectoryPath(info.path); + } + } catch (error) { + info.error = ["ENOENT", "ENOTDIR"].includes( + (error as NodeJS.ErrnoException).code ?? "", + ) + ? "Directory does not exist." + : "Directory is not accessible."; + } + if (suggestions) { + const directory = await realDirectoryPath( + info.error ? dirname(info.path) : info.path, + ).catch(() => null); + if (!directory) return info; + const prefix = info.error ? basename(info.path) : ""; + const entries = await readdir(directory, { withFileTypes: true }).catch( + () => [], + ); + const matches = entries + .filter( + (entry) => + (windows + ? entry.name.toLowerCase().startsWith(prefix.toLowerCase()) + : entry.name.startsWith(prefix)) && + (prefix.startsWith(".") || !entry.name.startsWith(".")) && + !/[\x00-\x1f\x7f]/.test(entry.name), + ) + .sort((a, b) => a.name.localeCompare(b.name)); + for (const entry of matches) { + const path = join(directory, entry.name); + if ( + entry.isDirectory() || + (entry.isSymbolicLink() && + (await stat(path).catch(() => null))?.isDirectory()) + ) + info.directories.push(path); + if (info.directories.length === DIRECTORY_LIMIT) break; + } + } + return info; +} diff --git a/shared/workspacePath.ts b/shared/workspacePath.ts new file mode 100644 index 00000000..4cd757d6 --- /dev/null +++ b/shared/workspacePath.ts @@ -0,0 +1,5 @@ +export interface WorkspacePathInfo { + path: string; + error: string | null; + directories: string[]; +} diff --git a/web/src/components/CreateWorkspaceDialog.css b/web/src/components/CreateWorkspaceDialog.css new file mode 100644 index 00000000..b6f29f82 --- /dev/null +++ b/web/src/components/CreateWorkspaceDialog.css @@ -0,0 +1,31 @@ +.workspace-path-hint { + color: var(--muted); + font-size: 12px; + overflow-wrap: anywhere; +} + +.workspace-path-hint p { + margin: 8px 0 0; +} + +.workspace-path-directories { + max-height: 160px; + overflow-y: auto; + margin-top: 8px; + border: 1px solid var(--border); + border-radius: 6px; +} + +.workspace-path-directories button { + display: flex; + align-items: center; + gap: 8px; + width: 100%; + text-align: left; +} + +.workspace-path-directories button span { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} diff --git a/web/src/components/CreateWorkspaceDialog.tsx b/web/src/components/CreateWorkspaceDialog.tsx index 852d4841..9a9118d1 100644 --- a/web/src/components/CreateWorkspaceDialog.tsx +++ b/web/src/components/CreateWorkspaceDialog.tsx @@ -1,8 +1,13 @@ -import { useEffect, useEffectEvent, useRef, useState } from "react"; +import { useEffect, useEffectEvent, useId, useRef, useState } from "react"; +import { Folder } from "lucide-react"; +import type { WorkspacePathInfo } from "../../../shared/workspacePath"; +import type { ConnectionClient } from "../api"; import { luckyWorkspaceName } from "../luckyName"; import { store, useEndpointCreationReason } from "../store"; +import { useConnectionClient } from "../useConnectionClient"; import { CloseButton } from "./CloseButton"; import { focusDialogElement } from "./dialogFocus"; +import "./CreateWorkspaceDialog.css"; export function CreateWorkspaceDialog({ open, @@ -16,9 +21,21 @@ export function CreateWorkspaceDialog({ onClose: () => void; }) { const createReason = useEndpointCreationReason("workspace.create"); + const client = useConnectionClient(); const [label, setLabel] = useState(""); const [cwd, setCwd] = useState(""); + const [pathState, setPathState] = useState<{ + client: ConnectionClient; + input: string; + info?: WorkspacePathInfo; + error?: string; + } | null>(null); + const [creating, setCreating] = useState(false); + const [createError, setCreateError] = useState(""); + const pendingSubmit = useRef(null); const labelRef = useRef(null); + const cwdRef = useRef(null); + const pathHintId = useId(); const onCloseEvent = useEffectEvent(onClose); useEffect(() => { @@ -36,13 +53,72 @@ export function CreateWorkspaceDialog({ }; }, [open, initialName, initialCwd]); + useEffect(() => { + setCreating(false); + setCreateError(""); + setPathState(null); + return () => { + pendingSubmit.current = null; + }; + }, [open, client]); + + const input = cwd.trim(); + useEffect(() => { + if (!open || !input) return; + let cancelled = false; + const timer = setTimeout(() => { + void client.call("workspace.path", { path: input }).then( + (info: WorkspacePathInfo) => { + if (!cancelled && client.isCurrent()) + setPathState({ client, input, info }); + }, + (error: Error) => { + if (!cancelled && client.isCurrent()) + setPathState({ client, input, error: error.message }); + }, + ); + }, 200); + return () => { + cancelled = true; + clearTimeout(timer); + }; + }, [open, client, input]); + if (!open) return null; - const submit = (e: React.FormEvent) => { + const checked = + pathState?.client === client && pathState.input === input + ? pathState + : null; + const pathError = checked?.error || checked?.info?.error; + const checking = !!input && !checked; + const submit = async (e: React.FormEvent) => { e.preventDefault(); - if (createReason) return; - store.createWorkspace(label.trim() || undefined, cwd.trim() || undefined); - onClose(); + if ( + createReason || + pendingSubmit.current || + checking || + pathError || + !client.isCurrent() + ) + return; + const submission = Symbol(); + pendingSubmit.current = submission; + setCreating(true); + setCreateError(""); + const result = await store.createWorkspace( + label.trim() || undefined, + input ? checked?.info?.path : undefined, + ); + if (pendingSubmit.current !== submission || !client.isCurrent()) return; + pendingSubmit.current = null; + setCreating(false); + if (result) onClose(); + else + setCreateError( + store.get().error || + "Workspace creation failed. Check the directory and retry.", + ); }; return ( @@ -65,20 +141,87 @@ export function CreateWorkspaceDialog({ setLabel(e.currentTarget.value)} placeholder="Optional" /> +
+ {checking ? ( +

Checking directory...

+ ) : pathError ? ( +

+ {pathError} +

+ ) : checked?.info ? ( +

+ Directory: {checked.info.path} +

+ ) : ( +

+ Use an absolute path or ~/ on the connected host. Leave blank to + use the default directory. +

+ )} +
+ {checked?.info?.directories.length ? ( +
+ {checked.info.directories.map((directory) => ( +
+ +
+ ))} +
+ ) : null} + {createError ? ( +

+ {createError} +

+ ) : null} {createReason ?

{createReason}

: null}
diff --git a/web/src/components/ModalDialogs.test.ts b/web/src/components/ModalDialogs.test.ts index 5c6f958e..99afe46d 100644 --- a/web/src/components/ModalDialogs.test.ts +++ b/web/src/components/ModalDialogs.test.ts @@ -1,4 +1,4 @@ -import { afterAll, afterEach, expect, spyOn, test } from "bun:test"; +import { afterAll, afterEach, expect, jest, spyOn, test } from "bun:test"; import { Window } from "happy-dom"; import type { ReactElement } from "react"; import type { Root } from "react-dom/client"; @@ -61,6 +61,8 @@ async function registerDomTests() { const { CreateWorkspaceDialog } = await import("./CreateWorkspaceDialog"); const { WorktreeOpenDialog } = await import("./WorktreeOpenDialog"); const { bridge } = await import("../api"); + const { __storeTesting, store } = await import("../store"); + const { WorkspaceTree } = await import("./WorkspaceTree"); let listCalls = 0; const client: ConnectionClient = { connectionId: "dialog-test", @@ -148,6 +150,281 @@ async function registerDomTests() { }, ]; + test("first workspace creation stays pending across an early topology refresh", async () => { + const previousState = store.get(); + let finish!: (result: unknown) => void; + const create = spyOn(store, "createWorkspace").mockImplementation( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + const container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + const render = () => root!.render(h(WorkspaceTree)); + try { + __storeTesting.replaceState({ + ...previousState, + status: "connected", + navigationMode: "shared", + workspaces: [], + panes: [], + tabs: [], + }); + await act(async () => render()); + await act(async () => + container + .querySelector('[title="New workspace"]')! + .click(), + ); + const form = container.querySelector("form")!; + const submit = () => + form.dispatchEvent( + new Event("submit", { bubbles: true, cancelable: true }), + ); + await act(async () => submit()); + expect( + form.querySelector('button[type="submit"]')! + .disabled, + ).toBe(true); + + __storeTesting.replaceState({ + ...store.get(), + workspaces: [ + { + workspace_id: "first", + number: 1, + label: "First", + focused: true, + pane_count: 0, + tab_count: 0, + agent_status: "unknown", + }, + ], + }); + await act(async () => render()); + expect(container.querySelector("form")).toBe(form); + expect( + form.querySelector('button[type="submit"]')! + .disabled, + ).toBe(true); + await act(async () => submit()); + expect(create).toHaveBeenCalledTimes(1); + await act(async () => finish({ type: "workspace_created" })); + expect( + container.querySelector('[aria-label="Create workspace"]'), + ).toBeNull(); + } finally { + create.mockRestore(); + await act(async () => root!.unmount()); + root = null; + __storeTesting.replaceState(previousState); + } + }); + + for (const [directory, label] of [ + ["/parent/project\\", "project\\/"], + ["C:\\Projects\\Example", "Example/"], + ["C:\\", "C:/"], + ]) { + test(`selecting ${directory} preserves the suggested directory`, async () => { + const pathCall = spyOn(client, "call").mockImplementation( + async (_method, params) => ({ + path: params?.path === "/parent" ? "/parent" : directory, + error: null, + directories: params?.path === "/parent" ? [directory] : [], + }), + ); + const create = spyOn(store, "createWorkspace").mockResolvedValue({ + type: "workspace_created", + }); + const container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + jest.useFakeTimers(); + try { + await act(async () => + root!.render( + h(CreateWorkspaceDialog, { + open: true, + initialName: "Path test", + initialCwd: "/parent", + onClose: () => {}, + }), + ), + ); + await act(async () => { + jest.advanceTimersByTime(200); + }); + const suggestion = container.querySelector( + '[aria-label="Directory suggestions"] button', + )!; + expect(suggestion.textContent).toBe(label); + await act(async () => suggestion.click()); + expect( + container.querySelectorAll("input")[1].value, + ).toBe(`${directory}/`); + await act(async () => { + jest.advanceTimersByTime(200); + }); + expect(pathCall).toHaveBeenLastCalledWith("workspace.path", { + path: `${directory}/`, + }); + await act(async () => + container + .querySelector("form")! + .dispatchEvent( + new Event("submit", { bubbles: true, cancelable: true }), + ), + ); + expect(create).toHaveBeenLastCalledWith("Path test", directory); + } finally { + jest.useRealTimers(); + pathCall.mockRestore(); + create.mockRestore(); + } + }); + } + + test("workspace paths show expansion, navigate suggestions and keep failed creation open", async () => { + const calls: string[] = []; + let fail = true; + const pathCall = spyOn(client, "call").mockImplementation( + async (method, params) => { + expect(method).toBe("workspace.path"); + calls.push(String(params?.path)); + if (params?.path === "~/missing") + return { + path: "/home/test/missing", + error: "Directory does not exist.", + directories: [], + }; + if (params?.path === "/home/test/project/") + return { path: "/home/test/project", error: null, directories: [] }; + return { + path: "/home/test", + error: null, + directories: ["/home/test/project"], + }; + }, + ); + const create = spyOn(store, "createWorkspace").mockImplementation( + async () => (fail ? undefined : { type: "workspace_created" }), + ); + let closed = 0; + const container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + const render = (initialCwd: string) => + root!.render( + h(CreateWorkspaceDialog, { + open: true, + initialName: "Test", + initialCwd, + onClose: () => { + closed += 1; + }, + }), + ); + jest.useFakeTimers(); + try { + await act(async () => render("~/missing")); + const submit = () => + container + .querySelector("form")! + .dispatchEvent( + new Event("submit", { bubbles: true, cancelable: true }), + ); + expect( + container.querySelector('button[type="submit"]')! + .disabled, + ).toBe(true); + await act(async () => { + jest.advanceTimersByTime(200); + }); + expect(container.textContent).toContain("Directory does not exist."); + await act(async () => submit()); + expect(create).not.toHaveBeenCalled(); + expect(closed).toBe(0); + + await act(async () => render("~/")); + await act(async () => { + jest.advanceTimersByTime(200); + }); + expect(container.textContent).toContain("/home/test"); + const suggestion = container.querySelector( + '[aria-label="Directory suggestions"] button', + )!; + await act(async () => suggestion.click()); + expect( + container.querySelectorAll("input")[1].value, + ).toBe("/home/test/project/"); + expect(document.activeElement).toBe( + container.querySelectorAll("input")[1], + ); + await act(async () => { + jest.advanceTimersByTime(200); + }); + await act(async () => submit()); + expect(create).toHaveBeenLastCalledWith("Test", "/home/test/project"); + expect(closed).toBe(0); + expect(container.querySelector('[role="alert"]')).not.toBeNull(); + fail = false; + await act(async () => submit()); + expect(closed).toBe(1); + expect(calls).toEqual(["~/missing", "~/", "/home/test/project/"]); + } finally { + jest.useRealTimers(); + pathCall.mockRestore(); + create.mockRestore(); + } + }); + + test("workspace path checks discard replies for an older input", async () => { + let resolveOld!: (value: unknown) => void; + const pathCall = spyOn(client, "call").mockImplementation( + async (_method, params) => { + if (params?.path === "~/old") + return new Promise((resolve) => { + resolveOld = resolve; + }); + return { path: "/home/test/new", error: null, directories: [] }; + }, + ); + const container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + const render = (initialCwd: string) => + root!.render( + h(CreateWorkspaceDialog, { open: true, initialCwd, onClose: () => {} }), + ); + jest.useFakeTimers(); + try { + await act(async () => render("~/old")); + await act(async () => { + jest.advanceTimersByTime(200); + }); + await act(async () => render("~/new")); + await act(async () => { + jest.advanceTimersByTime(200); + }); + await act(async () => + resolveOld({ + path: "/home/test/old", + error: "Old error", + directories: ["/home/test/old/child"], + }), + ); + expect(container.textContent).toContain("/home/test/new"); + expect(container.textContent).not.toContain("Old error"); + expect(container.textContent).not.toContain("old/child"); + } finally { + jest.useRealTimers(); + pathCall.mockRestore(); + } + }); + for (const dialog of dialogs) { test(`${dialog.name} keeps committed keyboard callbacks and cleans up focus/listeners`, async () => { const container = document.createElement("div"); diff --git a/web/src/components/WorkspaceTree.tsx b/web/src/components/WorkspaceTree.tsx index f93a30f0..2f2d5cde 100644 --- a/web/src/components/WorkspaceTree.tsx +++ b/web/src/components/WorkspaceTree.tsx @@ -578,6 +578,7 @@ export function WorkspaceTree({ setCreateOpen(false)} /> @@ -853,6 +854,7 @@ export function WorkspaceTree({ }} /> setCreateOpen(false)} /> From 493e0367d5b87b93d737df3acc448133ce79f9ff Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 00:22:52 +0800 Subject: [PATCH 2/3] Validate assistant workspace directories before creation --- server/src/assistant/actions.test.ts | 128 ++++++++++++++++++++++----- server/src/assistant/actions.ts | 13 +++ 2 files changed, 121 insertions(+), 20 deletions(-) diff --git a/server/src/assistant/actions.test.ts b/server/src/assistant/actions.test.ts index 812dd65a..d9f5b196 100644 --- a/server/src/assistant/actions.test.ts +++ b/server/src/assistant/actions.test.ts @@ -1,9 +1,24 @@ -import { describe, expect, jest, mock, test } from "bun:test"; +import { afterAll, describe, expect, jest, mock, spyOn, test } from "bun:test"; +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import type { AssistantActionKind } from "../../../shared/assistant"; import type { LegacyConnectionRuntime } from "../connections/runtime"; +import * as processUtils from "../utils/process-utils"; import { createWorkspaceWorktree } from "../worktree/create"; import { createAssistantContext } from "./context"; +const fixtureDirectory = realpathSync( + mkdtempSync(join(tmpdir(), "ranger-actions-")), +); +afterAll(() => rmSync(fixtureDirectory, { recursive: true, force: true })); + const ref = { connection_id: "local", workspace_id: "w1" }; function deferred() { let resolve!: (value: T) => void; @@ -24,7 +39,7 @@ function fixture(paneId = "w1:p1") { { workspace_id: ref.workspace_id, label: "Source", - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, }, ]; const panes: Record[] = [ @@ -33,7 +48,7 @@ function fixture(paneId = "w1:p1") { pane_id: paneId, tab_id: `${ref.workspace_id}:t1`, terminal_id: "term1", - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, focused: true, agent: null, agent_session: null, @@ -213,12 +228,12 @@ function fixture(paneId = "w1:p1") { ? null : { config: { setup }, - path: "/ranger-actions-fixture/roamgate.json", + path: join(fixtureDirectory, "roamgate.json"), source: "roamgate" as const, }, ); const resolveWorkspaceGitRoot = mock(async () => ({ - root: "/ranger-actions-fixture", + root: fixtureDirectory, workspace: workspaces[0], })); const runtime = { @@ -515,7 +530,7 @@ describe("confirmed Ranger action targets", () => { const prepared = await f.prepare("create_tab", {}); expect(f.mutations).toEqual([]); expect(prepared.preview.params).toEqual({ - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, }); const [first, repeated] = await Promise.all([ prepared.execute(), @@ -528,7 +543,7 @@ describe("confirmed Ranger action targets", () => { expect(f.mutations).toEqual(["tab.create"]); expect(f.call).toHaveBeenCalledWith( "tab.create", - { workspace_id: "w1", cwd: "/ranger-actions-fixture", focus: false }, + { workspace_id: "w1", cwd: fixtureDirectory, focus: false }, 30_000, expect.any(Function), ); @@ -553,7 +568,7 @@ describe("confirmed Ranger action targets", () => { delete f.workspaces[0]!.cwd; const prepared = await f.prepare("create_tab", {}); expect(prepared.preview.params).toEqual({ - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, pane_id: "w1:p1", tab_id: "w1:t1", terminal_id: "term1", @@ -570,7 +585,7 @@ describe("confirmed Ranger action targets", () => { expect((await prepared.execute()).status).toBe("succeeded"); expect(f.call).toHaveBeenCalledWith( "tab.create", - { workspace_id: "w1", cwd: "/ranger-actions-fixture", focus: false }, + { workspace_id: "w1", cwd: fixtureDirectory, focus: false }, 30_000, expect.any(Function), ); @@ -589,7 +604,7 @@ describe("confirmed Ranger action targets", () => { pane_id: "w1:p1", tab_id: "w1:t1", terminal_id: "term1", - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, direction, }); const result = await prepared.execute(); @@ -602,7 +617,7 @@ describe("confirmed Ranger action targets", () => { { target_pane_id: "w1:p1", direction, - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, focus: false, }, 30_000, @@ -741,7 +756,7 @@ describe("confirmed Ranger action targets", () => { ...ref, runtime_generation: 7, workspace_label: "Source", - params: { label: "New workspace", cwd: "/ranger-actions-fixture" }, + params: { label: "New workspace", cwd: fixtureDirectory }, }); expect(Object.isFrozen(prepared)).toBe(true); expect(Object.isFrozen(prepared.preview.params)).toBe(true); @@ -757,7 +772,7 @@ describe("confirmed Ranger action targets", () => { "workspace.create", { label: "New workspace", - cwd: "/ranger-actions-fixture", + cwd: fixtureDirectory, source_workspace_id: "w1", focus: false, }, @@ -766,6 +781,79 @@ describe("confirmed Ranger action targets", () => { ); }); + test.each(["explicit", "inferred", "removed", "file"] as const)( + "workspace creation checks directories before dispatch: %s", + async (kind) => { + const f = fixture(); + const root = mkdtempSync(join(fixtureDirectory, "cwd-")); + const cwd = join(root, "project"); + try { + if (kind === "removed") mkdirSync(cwd); + if (kind === "file") writeFileSync(cwd, "not a directory"); + if (kind === "inferred") f.workspaces[0]!.cwd = cwd; + const prepared = await f.prepare("create_workspace", { + label: "New", + ...(kind === "inferred" ? {} : { cwd }), + }); + if (kind === "removed") rmSync(cwd, { recursive: true }); + const result = await prepared.execute(); + expect(result.status).toBe("failed"); + expect(result.detail).toContain( + kind === "file" + ? "Path is not a directory." + : "Directory does not exist.", + ); + expect(f.mutations).toEqual([]); + expect(await prepared.execute()).toBe(result); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, + ); + + test.each(["valid", "missing", "retired", "unreachable"] as const)( + "workspace creation validates the selected Windows SSH host: %s", + async (state) => { + const f = fixture(); + Object.assign(f.runtime, { + sshHost: () => "windows-host", + remotePlatform: "windows", + }); + const cwd = "C:\\Users\\remote\\project"; + const command = spyOn( + processUtils, + "runProcessWithCodeTimeout", + ).mockImplementation(async (argv) => { + expect(argv.at(-2)).toBe("windows-host"); + expect(argv.at(-1)).toStartWith("powershell.exe "); + if (state === "retired") f.retire(); + if (state === "unreachable") throw new Error("SECRET /private/socket"); + const error = state === "missing" ? "Directory does not exist." : ""; + return { + code: 0, + stdout: `PATH\t${Buffer.from(cwd).toString("base64")}\nERROR\t${Buffer.from(error).toString("base64")}\n`, + stderr: "", + }; + }); + try { + const prepared = await f.prepare("create_workspace", { + label: "New", + cwd, + }); + const result = await prepared.execute(); + expect(result.status).toBe(state === "valid" ? "succeeded" : "failed"); + expect(f.mutations).toEqual( + state === "valid" ? ["workspace.create"] : [], + ); + expect(command).toHaveBeenCalledTimes(1); + expect(JSON.stringify(result)).not.toContain("SECRET"); + expect(await prepared.execute()).toBe(result); + } finally { + command.mockRestore(); + } + }, + ); + test("a completed preview survives disposal of the model tool signal", async () => { const f = fixture(); const controller = new AbortController(); @@ -918,7 +1006,7 @@ describe("confirmed Ranger action targets", () => { focus: false, expected_setup_hook: "bun install", expected_hooks_enabled: true, - expected_source_root: "/ranger-actions-fixture", + expected_source_root: fixtureDirectory, }); expect( f.call.mock.calls.some(([method]) => method === "worktree.create"), @@ -1259,7 +1347,7 @@ describe("confirmed Ranger close actions", () => { const closeParams = (kind: (typeof closeKinds)[number]) => kind === "close_workspace" ? {} : { pane_id: "w1:p1" }; const addSibling = (f: ReturnType) => - f.createPane({ cwd: "/ranger-actions-fixture" }, "w1:t1"); + f.createPane({ cwd: fixtureDirectory }, "w1:t1"); test.each(["close_pane", "close_agent"] as const)( "%s verifies a successful close after the native automatic label changes", @@ -1319,7 +1407,7 @@ describe("confirmed Ranger close actions", () => { const f = fixture(); f.useAgent(); addSibling(f); - f.worktrees.push({ branch: "feature", path: "/ranger-actions-fixture" }); + f.worktrees.push({ branch: "feature", path: fixtureDirectory }); const prepared = await f.prepare(kind, closeParams(kind)); expect(prepared.preview.kind).toBe(kind); expect(prepared.preview.summary).toContain( @@ -1377,8 +1465,8 @@ describe("confirmed Ranger close actions", () => { addSibling(f); f.workspaces[0]!.worktree = { repo_key: "repo", - repo_root: "/ranger-actions-fixture", - checkout_path: "/ranger-actions-fixture", + repo_root: fixtureDirectory, + checkout_path: fixtureDirectory, is_linked_worktree: linked, }; if (linked === true) { @@ -1399,8 +1487,8 @@ describe("confirmed Ranger close actions", () => { addSibling(f); f.workspaces[0]!.worktree = { repo_key: "repo", - repo_root: "/ranger-actions-fixture", - checkout_path: "/ranger-actions-fixture", + repo_root: fixtureDirectory, + checkout_path: fixtureDirectory, is_linked_worktree: true, }; const prepared = await f.prepare(kind, closeParams(kind)); diff --git a/server/src/assistant/actions.ts b/server/src/assistant/actions.ts index 69b27f9a..9a8e5a6e 100644 --- a/server/src/assistant/actions.ts +++ b/server/src/assistant/actions.ts @@ -13,6 +13,7 @@ import { import { validateConnectionId } from "../connections/protocol"; import type { LegacyConnectionRuntime } from "../connections/runtime"; import { assertUnixHost } from "../connections/unix-only"; +import { inspectWorkspacePath } from "../workspace/path"; import { checkoutPath } from "../workspace/utils"; import { surfaceAssistantUserError } from "./errors"; @@ -379,6 +380,18 @@ export async function prepareAssistantAction(args: { detail: "A workspace with this label already exists. No duplicate was created.", }; + const directory = await read(() => + inspectWorkspacePath( + cwd, + { host: runtime.sshHost(), remotePlatform: runtime.remotePlatform }, + false, + ), + ); + if (directory.error) + return { + status: "failed", + detail: `${directory.error} Nothing was sent; prepare a new preview.`, + }; const previousIds = new Set(before.map((item) => item.workspace_id)); let response: unknown; try { From 76ffb28b2fe0125001f487328740ad1178c12771 Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Thu, 15 Oct 2026 00:42:43 +0800 Subject: [PATCH 3/3] Recheck workspace directories after creation queues --- server/src/bridge/endpoint-creation.test.ts | 57 ++++++++++++ server/src/bridge/endpoint-creation.ts | 2 + .../bridge/endpoint-terminal-session.test.ts | 93 +++++++++++++++++++ server/src/bridge/terminal-bridge.ts | 4 + .../src/connections/profile-process.test.ts | 27 +++++- server/src/index.ts | 14 +++ 6 files changed, 196 insertions(+), 1 deletion(-) diff --git a/server/src/bridge/endpoint-creation.test.ts b/server/src/bridge/endpoint-creation.test.ts index 49199a9d..69cc6b56 100644 --- a/server/src/bridge/endpoint-creation.test.ts +++ b/server/src/bridge/endpoint-creation.test.ts @@ -1,4 +1,8 @@ import { expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { inspectWorkspacePath } from "../workspace/path"; import { assertEndpointCreationSource, createEmptyWorkspaceCreator, @@ -96,3 +100,56 @@ test("empty bootstrap rejects malformed and nonempty topology without mutation", expect(calls).toEqual(["workspace.list"]); } }); + +test("empty bootstrap rechecks the directory after waiting for topology", async () => { + const root = realpathSync(mkdtempSync(join(tmpdir(), "bootstrap-path-"))); + const cwd = join(root, "project"); + mkdirSync(cwd); + const entered = Promise.withResolvers(); + const held = Promise.withResolvers(); + const calls: string[] = []; + const create = createEmptyWorkspaceCreator(async (method) => { + calls.push(method); + if (method === "workspace.list") { + entered.resolve(); + await held.promise; + } + return { type: "workspace_list", workspaces: [] }; + }); + try { + expect((await inspectWorkspacePath(cwd, {}, false)).error).toBeNull(); + const checkDirectory = async () => { + const info = await inspectWorkspacePath(cwd, {}, false); + if (info.error) throw new Error(info.error); + }; + const pending = create( + { cwd, browser_source: null }, + () => true, + new EndpointCreationDeadline(), + checkDirectory, + ).then( + () => null, + (error: Error) => error, + ); + await entered.promise; + rmSync(cwd, { recursive: true }); + held.resolve(); + expect((await pending)?.message).toContain("Directory does not exist."); + expect(calls).toEqual(["workspace.list"]); + mkdirSync(cwd); + await create( + { cwd, browser_source: null }, + () => true, + new EndpointCreationDeadline(), + checkDirectory, + ); + expect(calls).toEqual([ + "workspace.list", + "workspace.list", + "workspace.create", + ]); + } finally { + held.resolve(); + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/server/src/bridge/endpoint-creation.ts b/server/src/bridge/endpoint-creation.ts index d7166159..2dbfaa83 100644 --- a/server/src/bridge/endpoint-creation.ts +++ b/server/src/bridge/endpoint-creation.ts @@ -119,6 +119,7 @@ export function createEmptyWorkspaceCreator( params: Record, isCurrent: () => boolean, deadline: EndpointCreationDeadline, + validatePath: () => Promise = async () => {}, ) => { const check = () => { deadline.assertBeforeDispatch(); @@ -143,6 +144,7 @@ export function createEmptyWorkspaceCreator( throw new Error( "Session is no longer empty. Open a terminal in the desired workspace and retry creation.", ); + await validatePath(); check(); const creationParams: Record = { ...params, diff --git a/server/src/bridge/endpoint-terminal-session.test.ts b/server/src/bridge/endpoint-terminal-session.test.ts index 521d87cc..4ed2e409 100644 --- a/server/src/bridge/endpoint-terminal-session.test.ts +++ b/server/src/bridge/endpoint-terminal-session.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, jest, spyOn, test } from "bun:test"; import { EventEmitter, once } from "node:events"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync } from "node:fs"; import * as net from "node:net"; import * as path from "node:path"; import { tmpdir } from "node:os"; @@ -15,6 +16,7 @@ import type { Popup } from "./endpoint-surface"; import type { ServerWebSocket } from "bun"; import { createTerminalBridge } from "./terminal-bridge"; import { silentLogger } from "../utils/logger"; +import { inspectWorkspacePath } from "../workspace/path"; import { EndpointCreationDeadline } from "./endpoint-creation"; import { dropCoalescedMessage, @@ -2801,6 +2803,97 @@ test("two browser source endpoints create concurrently without changing each oth } }); +for (const stage of ["readiness", "queue", "source validation"] as const) { + test(`workspace directory is rechecked after ${stage} before dispatch`, async () => { + const root = realpathSync( + mkdtempSync(path.join(tmpdir(), "creation-path-")), + ); + const cwd = path.join(root, "project"); + mkdirSync(cwd); + const held = deferred(); + let waiting = false; + const requests: string[] = []; + const socketPath = await startSessionServer({ + methods: creationMethods, + onRequest: async (method) => { + requests.push(method); + if (stage === "queue" && method === "tab.create") await held.promise; + }, + }); + const { bridge, ws, attach } = creationBridge(socketPath, { + lookup: async () => { + if (stage === "readiness") { + waiting = true; + await held.promise; + } + return "w1:p1"; + }, + validate: async () => { + if (stage === "source validation") { + waiting = true; + await held.promise; + } + }, + }); + let prior: Promise | undefined; + try { + expect((await inspectWorkspacePath(cwd, {}, false)).error).toBeNull(); + const attaching = attach(); + if (stage === "readiness") await settleUntil(() => waiting); + else await attaching; + if (stage === "queue") { + prior = bridge.createFromTerminal( + ws, + "tab.create", + { workspace_id: "w1", browser_source: creationSource }, + () => true, + ); + await settleUntil(() => requests.includes("tab.create")); + } + const params = { cwd, browser_source: creationSource }; + const checkDirectory = async () => { + const info = await inspectWorkspacePath(params.cwd, {}, false); + if (info.error) throw new Error(info.error); + }; + const pending = bridge + .createFromTerminal( + ws, + "workspace.create", + params, + () => true, + checkDirectory, + ) + .then( + () => null, + (error: Error) => error, + ); + if (stage === "source validation") await settleUntil(() => waiting); + rmSync(cwd, { recursive: true }); + held.resolve(); + await attaching; + await prior; + expect((await pending)?.message).toContain("Directory does not exist."); + expect(requests).not.toContain("workspace.create"); + mkdirSync(cwd); + await bridge.createFromTerminal( + ws, + "workspace.create", + params, + () => true, + checkDirectory, + ); + expect( + requests.filter((method) => method === "workspace.create"), + ).toHaveLength(1); + } finally { + held.resolve(); + await prior; + bridge.dispose(); + rmSync(root, { recursive: true, force: true }); + } + }); +} + test("creation queued behind readiness or validation cannot mutate a detached source", async () => { const validation = deferred(); let validating = false; diff --git a/server/src/bridge/terminal-bridge.ts b/server/src/bridge/terminal-bridge.ts index 8c1784ff..12603d35 100644 --- a/server/src/bridge/terminal-bridge.ts +++ b/server/src/bridge/terminal-bridge.ts @@ -87,6 +87,7 @@ export function createTerminalBridge(args: { params: Record, isCurrent: () => boolean, deadline: EndpointCreationDeadline, + validatePath?: () => Promise, ) => Promise; safeSend: ( ws: ServerWebSocket, @@ -1061,6 +1062,7 @@ export function createTerminalBridge(args: { method: "tab.create" | "workspace.create", params: Record, requestIsCurrent: () => boolean, + validatePath: () => Promise = async () => {}, ) { const deadline = new EndpointCreationDeadline(); return deadline.wait( @@ -1075,6 +1077,7 @@ export function createTerminalBridge(args: { params, () => !disposed && requestIsCurrent(), deadline, + validatePath, ); } const source = parseEndpointCreationSource(params.browser_source); @@ -1115,6 +1118,7 @@ export function createTerminalBridge(args: { if (!args.validateCreationSource) throw new Error("Creation source validation is unavailable."); await args.validateCreationSource(source); + await validatePath(); validateAttachment(); }, deadline, diff --git a/server/src/connections/profile-process.test.ts b/server/src/connections/profile-process.test.ts index 4a8dadd6..94688078 100644 --- a/server/src/connections/profile-process.test.ts +++ b/server/src/connections/profile-process.test.ts @@ -730,6 +730,7 @@ test("production routing bootstraps only a verified empty session and serializes mkdirSync(root, { recursive: true, mode: 0o700 }); let valid = false; let workspaces: Array<{ workspace_id: string }> = []; + let beforeTopology: (() => Promise) | undefined; const mutations: Record[] = []; const profile = await fakeHerdr( root, @@ -737,10 +738,12 @@ test("production routing bootstraps only a verified empty session and serializes 22, undefined, async (request) => { - if (request.method === "workspace.list") + if (request.method === "workspace.list") { + await beforeTopology?.(); return valid ? { type: "workspace_list", workspaces } : { workspaces: [] }; + } if (request.method === "workspace.create") { mutations.push(request.params ?? {}); await Bun.sleep(20); @@ -894,6 +897,28 @@ test("production routing bootstraps only a verified empty session and serializes ).toBe("workspace_created"); expect(mutations[1]).toEqual({ cwd: chosen, focus: false }); workspaces = []; + const topologyEntered = Promise.withResolvers(); + const releaseTopology = Promise.withResolvers(); + beforeTopology = async () => { + topologyEntered.resolve(); + await releaseTopology.promise; + }; + try { + const creating = rpc(browsers[0], "workspace.create", { + browser_source: null, + cwd: "~/chosen", + }); + await topologyEntered.promise; + rmSync(chosen, { recursive: true }); + releaseTopology.resolve(); + expect((await creating).error).toMatchObject({ + message: expect.stringContaining("Directory does not exist."), + }); + expect(mutations).toHaveLength(2); + } finally { + releaseTopology.resolve(); + beforeTopology = undefined; + } expect( ( await rpc( diff --git a/server/src/index.ts b/server/src/index.ts index de91d30f..72a21bc3 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -1011,6 +1011,20 @@ async function handleRpc(ws: ServerWebSocket, raw: string) { method, params, requestIsCurrent, + async () => { + if ( + method === "workspace.create" && + params?.cwd != null && + params.cwd !== "" + ) { + const info = await inspectWorkspacePath( + params.cwd, + { host: sshHost(), remotePlatform: connection.remotePlatform }, + false, + ); + if (info.error) throw new Error(`${info.error} ${info.path}`); + } + }, ); sendReply({ id, result }, method); } catch (error) {