From d8e325b17e1e1fadd144f6898b55b55a35f52eb5 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 6 Jun 2026 01:32:06 -0700 Subject: [PATCH] fix: empty folder selection reverts in DirectoryPicker + add create-folder button Fix bug where navigating into an empty folder in the DirectoryPicker would automatically revert to the previous folder. The useEffect that auto-fetches on open was using the stale 'value' prop instead of 'browser.currentPath' after navigation. Also adds a 'New folder' button to the DirectoryPicker toolbar so users can create folders directly when setting up a project. - DirectoryPicker.tsx: fix useEffect to use browser.currentPath || value - DirectoryPicker.tsx: add create folder UI state and handlers - DirectoryPicker.css: styles for create folder input and actions - routes.ts: add POST /api/create-directory endpoint - legacy.ts: add createDirectory API client - DirectoryPicker.test.tsx: add tests for empty folder nav and create folder --- packages/dashboard/app/api/legacy.ts | 8 ++ .../app/components/DirectoryPicker.css | 44 +++++++ .../app/components/DirectoryPicker.tsx | 105 +++++++++++++++- .../__tests__/DirectoryPicker.test.tsx | 118 +++++++++++++++++- packages/dashboard/src/routes.ts | 68 ++++++++++ 5 files changed, 339 insertions(+), 4 deletions(-) diff --git a/packages/dashboard/app/api/legacy.ts b/packages/dashboard/app/api/legacy.ts index 2380a6b428..d2f3bc107f 100644 --- a/packages/dashboard/app/api/legacy.ts +++ b/packages/dashboard/app/api/legacy.ts @@ -7000,6 +7000,14 @@ export function browseDirectory( return api(fullPath); } +/** Create a new directory */ +export function createDirectory(path: string): Promise<{ success: true; path: string }> { + return api<{ success: true; path: string }>("/create-directory", { + method: "POST", + body: JSON.stringify({ path }), + }); +} + /** Register a new project */ export function registerProject(input: ProjectCreateInput): Promise { return api("/projects", { diff --git a/packages/dashboard/app/components/DirectoryPicker.css b/packages/dashboard/app/components/DirectoryPicker.css index e12fc8c305..d33e5226ea 100644 --- a/packages/dashboard/app/components/DirectoryPicker.css +++ b/packages/dashboard/app/components/DirectoryPicker.css @@ -188,3 +188,47 @@ flex-shrink: 0; padding: var(--space-sm) var(--space-lg); } + +/* Create folder */ +.directory-picker-create-folder-toggle { + color: var(--text-muted); +} + +.directory-picker-create-folder-toggle:hover { + color: var(--text); +} + +.directory-picker-create-folder-toggle:focus-visible { + outline: none; + box-shadow: var(--focus-ring-strong); +} + +.directory-picker-create-folder { + display: flex; + flex-direction: column; + gap: var(--space-sm); + padding: var(--space-sm) var(--space-md); + border-top: 1px solid var(--border); + background: var(--surface); +} + +.directory-picker-create-folder-input { + width: 100%; +} + +.directory-picker-create-folder-actions { + display: flex; + gap: var(--space-sm); +} + +.directory-picker-create-folder-error { + display: flex; + align-items: center; + gap: var(--space-sm); + padding: var(--space-sm); + background: color-mix(in srgb, var(--color-error) 10%, transparent); + border: 1px solid var(--color-error); + border-radius: var(--radius-md); + color: var(--color-error); + font-size: var(--text-sm); +} diff --git a/packages/dashboard/app/components/DirectoryPicker.tsx b/packages/dashboard/app/components/DirectoryPicker.tsx index af3327f842..3b4fe09e27 100644 --- a/packages/dashboard/app/components/DirectoryPicker.tsx +++ b/packages/dashboard/app/components/DirectoryPicker.tsx @@ -1,7 +1,7 @@ import { useState, useCallback, useEffect } from "react"; import { useTranslation } from "react-i18next"; -import { Folder, FolderOpen, ChevronRight, ChevronUp, Loader2, Eye, EyeOff, AlertCircle } from "lucide-react"; -import { browseDirectory, type BrowseDirectoryResult } from "../api"; +import { Folder, FolderOpen, ChevronRight, ChevronUp, Loader2, Eye, EyeOff, AlertCircle, Plus } from "lucide-react"; +import { browseDirectory, createDirectory, type BrowseDirectoryResult } from "../api"; import { getPathBreadcrumbs } from "../utils/pathDisplay"; import "./DirectoryPicker.css"; @@ -25,6 +25,8 @@ interface BrowserState { parentPath: string | null; entries: BrowseDirectoryResult["entries"]; showHidden: boolean; + createFolderOpen: boolean; + createFolderError: string | null; } export function DirectoryPicker({ value, onChange, placeholder, onInputKeyDown, nodeId, localNodeId }: DirectoryPickerProps) { @@ -37,7 +39,10 @@ export function DirectoryPicker({ value, onChange, placeholder, onInputKeyDown, parentPath: null, entries: [], showHidden: false, + createFolderOpen: false, + createFolderError: null, }); + const [newFolderName, setNewFolderName] = useState(""); const fetchEntries = useCallback(async (path?: string, showHidden = false) => { setBrowser((prev) => ({ ...prev, loading: true, error: null })); @@ -72,7 +77,8 @@ export function DirectoryPicker({ value, onChange, placeholder, onInputKeyDown, // Fetch when browser opens useEffect(() => { if (browser.isOpen && !browser.loading && browser.entries.length === 0 && !browser.error) { - fetchEntries(value || undefined, browser.showHidden); + // Use browser.currentPath if available (user has navigated), otherwise fall back to value prop + fetchEntries(browser.currentPath || value || undefined, browser.showHidden); } }, [browser.isOpen, browser.loading, browser.entries.length, browser.error, value, browser.showHidden, fetchEntries, nodeId, localNodeId]); @@ -102,6 +108,51 @@ export function DirectoryPicker({ value, onChange, placeholder, onInputKeyDown, } }, [browser.showHidden]); + const handleToggleCreateFolder = useCallback(() => { + setBrowser((prev) => ({ + ...prev, + createFolderOpen: !prev.createFolderOpen, + createFolderError: null, + })); + setNewFolderName(""); + }, []); + + const handleCreateFolder = useCallback(async () => { + if (!newFolderName.trim() || !browser.currentPath) return; + + // Normalize path separator for the current platform by using the same + // separator already present in currentPath + const sep = browser.currentPath.includes("\\") ? "\\" : "/"; + const folderPath = browser.currentPath.endsWith(sep) + ? browser.currentPath + newFolderName.trim() + : browser.currentPath + sep + newFolderName.trim(); + + setBrowser((prev) => ({ ...prev, loading: true, createFolderError: null })); + try { + await createDirectory(folderPath); + setNewFolderName(""); + setBrowser((prev) => ({ ...prev, createFolderOpen: false })); + // Refresh entries to show the new folder + await fetchEntries(browser.currentPath, browser.showHidden); + } catch (err) { + setBrowser((prev) => ({ + ...prev, + loading: false, + createFolderError: err instanceof Error ? err.message : "Failed to create folder", + })); + } + }, [newFolderName, browser.currentPath, browser.showHidden, fetchEntries]); + + const handleCreateFolderKeyDown = useCallback((e: React.KeyboardEvent) => { + if (e.key === "Enter") { + e.preventDefault(); + void handleCreateFolder(); + } else if (e.key === "Escape") { + setBrowser((prev) => ({ ...prev, createFolderOpen: false, createFolderError: null })); + setNewFolderName(""); + } + }, [handleCreateFolder]); + const breadcrumbs = browser.currentPath ? getPathBreadcrumbs(browser.currentPath) : []; return ( @@ -171,6 +222,16 @@ export function DirectoryPicker({ value, onChange, placeholder, onInputKeyDown, {browser.showHidden ? : } {browser.showHidden ? t("dirPicker.hideHidden", "Hide hidden") : t("dirPicker.showHidden", "Show hidden")} + {/* Content */} @@ -209,6 +270,44 @@ export function DirectoryPicker({ value, onChange, placeholder, onInputKeyDown, )} + {/* Create folder input */} + {browser.createFolderOpen && ( +
+ setNewFolderName(e.target.value)} + onKeyDown={handleCreateFolderKeyDown} + placeholder={t("dirPicker.newFolderPlaceholder", "Folder name")} + autoFocus + /> +
+ + +
+ {browser.createFolderError && ( +
+ + {browser.createFolderError} +
+ )} +
+ )} + {/* Actions */}
diff --git a/packages/dashboard/app/components/__tests__/DirectoryPicker.test.tsx b/packages/dashboard/app/components/__tests__/DirectoryPicker.test.tsx index 845a857265..259fda7684 100644 --- a/packages/dashboard/app/components/__tests__/DirectoryPicker.test.tsx +++ b/packages/dashboard/app/components/__tests__/DirectoryPicker.test.tsx @@ -15,17 +15,20 @@ vi.mock("lucide-react", async () => { Eye: ({ size, ...props }: any) => 👁, EyeOff: ({ size, ...props }: any) => 🙈, AlertCircle: ({ size, ...props }: any) => ⚠, + Plus: ({ size, ...props }: any) => +, }; }); // Mock the API vi.mock("../../api", () => ({ browseDirectory: vi.fn(), + createDirectory: vi.fn(), })); -import { browseDirectory } from "../../api"; +import { browseDirectory, createDirectory } from "../../api"; const mockBrowseDirectory = vi.mocked(browseDirectory); +const mockCreateDirectory = vi.mocked(createDirectory); describe("DirectoryPicker", () => { beforeEach(() => { @@ -243,4 +246,117 @@ describe("DirectoryPicker", () => { expect(mockBrowseDirectory).toHaveBeenCalledWith("/home/user/projects", false, "remote-1", "local-1"); }); }); + + it("does not revert to previous folder when navigating into an empty directory (FN-XXXX)", async () => { + // First load: home directory with a folder + mockBrowseDirectory.mockResolvedValueOnce({ + currentPath: "/home/user", + parentPath: "/home", + entries: [ + { name: "empty-folder", path: "/home/user/empty-folder", hasChildren: false }, + ], + }); + + // Second load: the empty folder itself (no subdirectories) + mockBrowseDirectory.mockResolvedValueOnce({ + currentPath: "/home/user/empty-folder", + parentPath: "/home/user", + entries: [], + }); + + render(); + + fireEvent.click(screen.getByText("Browse")); + + await waitFor(() => { + expect(screen.getByText("empty-folder")).toBeDefined(); + }); + + // Navigate into the empty folder + fireEvent.click(screen.getByText("empty-folder")); + + // Should show "No subdirectories" in the empty folder, NOT re-fetch the parent + await waitFor(() => { + expect(screen.getByText("No subdirectories")).toBeDefined(); + }); + + // The current path should remain the empty folder, not revert to /home/user + expect(mockBrowseDirectory).toHaveBeenLastCalledWith( + "/home/user/empty-folder", + false, + undefined, + undefined, + ); + }); + + it("allows creating a new folder", async () => { + mockBrowseDirectory.mockResolvedValue({ + currentPath: "/home/user", + parentPath: "/home", + entries: [ + { name: "projects", path: "/home/user/projects", hasChildren: true }, + ], + }); + + mockCreateDirectory.mockResolvedValue({ success: true, path: "/home/user/my-new-folder" }); + + render(); + + fireEvent.click(screen.getByText("Browse")); + + await waitFor(() => { + expect(screen.getByText("projects")).toBeDefined(); + }); + + // Click "New folder" button + fireEvent.click(screen.getByRole("button", { name: "Create new folder" })); + + // Type folder name + const input = screen.getByPlaceholderText("Folder name"); + fireEvent.change(input, { target: { value: "my-new-folder" } }); + + // Click Create + fireEvent.click(screen.getByRole("button", { name: "Create" })); + + await waitFor(() => { + expect(mockCreateDirectory).toHaveBeenCalledWith("/home/user/my-new-folder"); + }); + + // Should refresh the directory listing + await waitFor(() => { + expect(mockBrowseDirectory).toHaveBeenLastCalledWith("/home/user", false, undefined, undefined); + }); + }); + + it("shows error when creating folder fails", async () => { + mockBrowseDirectory.mockResolvedValue({ + currentPath: "/home/user", + parentPath: "/home", + entries: [], + }); + + mockCreateDirectory.mockRejectedValue(new Error("Permission denied")); + + render(); + + fireEvent.click(screen.getByText("Browse")); + + await waitFor(() => { + expect(screen.getByText("No subdirectories")).toBeDefined(); + }); + + // Click "New folder" button + fireEvent.click(screen.getByRole("button", { name: "Create new folder" })); + + // Type folder name + const input = screen.getByPlaceholderText("Folder name"); + fireEvent.change(input, { target: { value: "test-folder" } }); + + // Click Create + fireEvent.click(screen.getByRole("button", { name: "Create" })); + + await waitFor(() => { + expect(screen.getByText("Permission denied")).toBeDefined(); + }); + }); }); diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts index 4cd992a077..cfcc6c4868 100644 --- a/packages/dashboard/src/routes.ts +++ b/packages/dashboard/src/routes.ts @@ -4455,6 +4455,74 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout } }); + /** + * POST /api/create-directory + * Create a new directory at the specified path. + * Body: { path: string } + * Returns: { success: true, path: string } + */ + router.post("/create-directory", async (req, res) => { + try { + const { resolve } = await import("node:path"); + const { mkdir, stat } = await import("node:fs/promises"); + + const rawPath = req.body?.path as string | undefined; + if (!rawPath) { + throw badRequest("Path is required"); + } + + // Validate: must be absolute, no .. traversal + const resolvedPath = resolve(rawPath); + if (rawPath.includes("..")) { + throw badRequest("Path must not contain '..' traversal"); + } + if (resolvedPath !== resolve(resolvedPath)) { + throw badRequest("Path must be absolute"); + } + + // Check if path already exists + try { + const existingStat = await stat(resolvedPath); + if (existingStat.isDirectory()) { + throw badRequest("Directory already exists"); + } + throw badRequest("A file already exists at this path"); + } catch (err: unknown) { + const e = err as { code?: string }; + if (e.code !== "ENOENT") { + throw err; + } + // ENOENT means it doesn't exist — proceed + } + + // Ensure parent directory exists + const { dirname } = await import("node:path"); + const parentPath = dirname(resolvedPath); + try { + const parentStat = await stat(parentPath); + if (!parentStat.isDirectory()) { + throw badRequest("Parent path is not a directory"); + } + } catch (err: unknown) { + const e = err as { code?: string }; + if (e.code === "ENOENT") { + throw badRequest("Parent directory does not exist"); + } + throw err; + } + + // Create the directory + await mkdir(resolvedPath); + + res.json({ success: true, path: resolvedPath }); + } catch (err: unknown) { + if (err instanceof ApiError) { + throw err; + } + rethrowAsApiError(err); + } + }); + // Registrar order is API-contract sensitive. Keep this domain sequence stable: // 1) project routes (`/projects/across-nodes|detect` before `/projects/:id`) // 2) node CRUD/operational routes