fix(windows): list logical drives in folder picker (#1870)

This commit is contained in:
Carlos Florêncio
2026-07-23 14:45:49 +01:00
committed by GitHub
parent 5d889478ea
commit e53fd038a8
12 changed files with 360 additions and 26 deletions
+5 -2
View File
@@ -255,5 +255,8 @@ jobs:
# Keep this targeted: the service package has broad subprocess coverage,
# while this native test is the Windows path-semantics regression.
- name: Test explicit local repository paths
run: go test -race -v -run '^TestExplicitLocalRepositoryWindowsPathSemantics$' ./internal/task/service
- name: Test Windows path semantics
run: >-
go test -race -v
-run '^(TestExplicitLocalRepositoryWindowsPathSemantics|TestListDirectory.*|TestDriveRootsFromMask|TestSplitAbsForRoot)$'
./internal/task/service
@@ -159,9 +159,10 @@ func (h *RepositoryHandlers) httpListDirectory(c *gin.Context) {
entries = append(entries, gin.H{"name": e.Name, "path": e.Path})
}
c.JSON(http.StatusOK, gin.H{
"path": result.Path,
"parent": result.Parent,
"entries": entries,
"path": result.Path,
"parent": result.Parent,
"entries": entries,
"choosable": result.Choosable,
})
}
@@ -91,6 +91,31 @@ func TestHTTPListRepositoryBranchesUsesRepositoryIdentity(t *testing.T) {
}
}
func TestHTTPListDirectoryIncludesChoosableContract(t *testing.T) {
router, _ := newRepositoryHTTPTestRouter(t)
request := httptest.NewRequest(
http.MethodGet,
"/api/v1/fs/list-dir?path="+url.QueryEscape(t.TempDir()),
nil,
)
response := httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusOK {
t.Fatalf("status = %d, want %d; body = %s", response.Code, http.StatusOK, response.Body.String())
}
var body struct {
Choosable *bool `json:"choosable"`
}
if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil {
t.Fatalf("decode response: %v", err)
}
if body.Choosable == nil || !*body.Choosable {
t.Fatalf("choosable = %v, want true; body = %s", body.Choosable, response.Body.String())
}
}
func TestHTTPListBranchesRejectsRepositoryFromAnotherWorkspace(t *testing.T) {
router, repo, svc := newRepositoryHTTPTestRouterWithService(t)
for _, workspaceID := range []string{"ws-a", "ws-b"} {
@@ -16,12 +16,13 @@ type DirectoryEntry struct {
}
// DirectoryListing is the result of ListDirectory: the absolute path that was
// listed, the parent (empty when at the filesystem root), and the immediate
// subdirectory children sorted alphabetically.
// listed, whether it may be selected, the parent (empty when at the filesystem
// root), and the immediate subdirectory children sorted alphabetically.
type DirectoryListing struct {
Path string
Parent string
Entries []DirectoryEntry
Path string
Parent string
Entries []DirectoryEntry
Choosable bool
}
// ListDirectory returns the immediate subdirectories of path. When path is
@@ -41,6 +42,10 @@ type DirectoryListing struct {
//
// Hidden (".") directories are excluded.
func (s *Service) ListDirectory(ctx context.Context, path string) (DirectoryListing, error) {
if listing, handled, err := listVirtualDirectoryRoot(path); handled {
return listing, err
}
abs, err := resolveListingPath(path)
if err != nil {
return DirectoryListing{}, err
@@ -53,9 +58,10 @@ func (s *Service) ListDirectory(ctx context.Context, path string) (DirectoryList
_ = ctx
return DirectoryListing{
Path: abs,
Parent: parentPath(abs),
Entries: collectSubdirs(abs, entries),
Path: abs,
Parent: parentPath(abs),
Entries: collectSubdirs(abs, entries),
Choosable: true,
}, nil
}
@@ -49,6 +49,22 @@ func TestListDirectory_ListsImmediateSubdirsOnly(t *testing.T) {
}
}
func TestDriveRootsFromMask(t *testing.T) {
got := driveRootsFromMask((1 << 2) | (1 << 4))
want := []DirectoryEntry{
{Name: `C:\`, Path: `C:\`},
{Name: `E:\`, Path: `E:\`},
}
if len(got) != len(want) {
t.Fatalf("drive roots = %+v, want %+v", got, want)
}
for i := range want {
if got[i] != want[i] {
t.Errorf("drive root[%d] = %+v, want %+v", i, got[i], want[i])
}
}
}
func TestListDirectory_DefaultsToHome(t *testing.T) {
svc := &Service{}
got, err := svc.ListDirectory(context.Background(), "")
@@ -148,4 +164,7 @@ func TestListDirectory_ParentEmptyAtFilesystemRoot(t *testing.T) {
if got.Parent != "" {
t.Errorf("expected empty parent at %q, got %q", target, got.Parent)
}
if !got.Choosable {
t.Errorf("filesystem root %q should be choosable", target)
}
}
@@ -0,0 +1,33 @@
//go:build windows
package service
import (
"context"
"strings"
"testing"
)
func TestListDirectoryWindowsVirtualRootListsLogicalDrives(t *testing.T) {
got, err := (&Service{}).ListDirectory(context.Background(), "/")
if err != nil {
t.Fatalf("ListDirectory virtual root: %v", err)
}
if got.Path != "/" {
t.Errorf("Path = %q, want virtual root /", got.Path)
}
if got.Parent != "" {
t.Errorf("Parent = %q, want empty", got.Parent)
}
if got.Choosable {
t.Error("Windows virtual root must not be choosable")
}
if len(got.Entries) == 0 {
t.Fatal("virtual root returned no logical drives")
}
for _, entry := range got.Entries {
if !strings.HasSuffix(entry.Path, `:\`) {
t.Errorf("drive path = %q, want native absolute drive root", entry.Path)
}
}
}
@@ -0,0 +1,13 @@
package service
func driveRootsFromMask(mask uint32) []DirectoryEntry {
roots := make([]DirectoryEntry, 0, 26)
for drive := 0; drive < 26; drive++ {
if mask&(1<<drive) == 0 {
continue
}
root := string(rune('A'+drive)) + `:\`
roots = append(roots, DirectoryEntry{Name: root, Path: root})
}
return roots
}
@@ -0,0 +1,7 @@
//go:build !windows
package service
func listVirtualDirectoryRoot(_ string) (DirectoryListing, bool, error) {
return DirectoryListing{}, false, nil
}
@@ -0,0 +1,24 @@
//go:build windows
package service
import (
"fmt"
"golang.org/x/sys/windows"
)
func listVirtualDirectoryRoot(path string) (DirectoryListing, bool, error) {
if path != "/" {
return DirectoryListing{}, false, nil
}
mask, err := windows.GetLogicalDrives()
if err != nil {
return DirectoryListing{}, true, fmt.Errorf("list logical drives: %w", err)
}
return DirectoryListing{
Path: "/",
Entries: driveRootsFromMask(mask),
Choosable: false,
}, true, nil
}
+163
View File
@@ -0,0 +1,163 @@
import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
import { type ReactElement, type ReactNode, cloneElement } from "react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { FolderPicker } from "./folder-picker";
import { listDirectory, type DirectoryListing } from "@/lib/api/domains/fs-api";
let setPopoverOpen: ((open: boolean) => void) | undefined;
vi.mock("@kandev/ui/popover", () => ({
Popover: ({
children,
onOpenChange,
}: {
children: ReactNode;
onOpenChange: (open: boolean) => void;
}) => {
setPopoverOpen = onOpenChange;
return <div>{children}</div>;
},
PopoverTrigger: ({ children }: { children: ReactElement<{ onClick?: () => void }> }) =>
cloneElement(children, { onClick: () => setPopoverOpen?.(true) }),
PopoverContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
}));
vi.mock("@kandev/ui/tooltip", () => ({
Tooltip: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: ReactNode }) => <>{children}</>,
}));
vi.mock("@/lib/api/domains/fs-api", async (importOriginal) => {
const original = await importOriginal<typeof import("@/lib/api/domains/fs-api")>();
return { ...original, listDirectory: vi.fn() };
});
const mockedListDirectory = vi.mocked(listDirectory);
const BUTTON_ROLE = "button";
const TRIGGER_TEST_ID = "folder-picker-trigger";
const VIRTUAL_ROOT = "/";
const DRIVE_E_ROOT = "E:\\";
const SUCCESS_PATH = "E:\\Success";
const SUCCESS_NAME = "Success";
afterEach(() => {
cleanup();
mockedListDirectory.mockReset();
setPopoverOpen = undefined;
});
describe("FolderPicker", () => {
it("preserves the separator when displaying the POSIX root", () => {
render(<FolderPicker value="/" onChange={vi.fn()} placeholder="Pick a folder" />);
expect(screen.getByTestId(TRIGGER_TEST_ID).textContent).toBe("/");
});
it("preserves the separator when displaying a Windows drive root", () => {
render(<FolderPicker value="C:\\" onChange={vi.fn()} />);
expect(screen.getByTestId(TRIGGER_TEST_ID).textContent).toBe("C:\\");
});
it("builds Windows breadcrumbs with a virtual root and native drive paths", async () => {
mockedListDirectory
.mockResolvedValueOnce(
listing(VIRTUAL_ROOT, false, [{ name: DRIVE_E_ROOT, path: DRIVE_E_ROOT }]),
)
.mockResolvedValueOnce(listing("E:\\Projects\\Kandev", true))
.mockResolvedValueOnce(listing(VIRTUAL_ROOT, false));
render(<FolderPicker value="" onChange={vi.fn()} />);
fireEvent.click(screen.getByTestId(TRIGGER_TEST_ID));
fireEvent.click(await screen.findByRole(BUTTON_ROLE, { name: DRIVE_E_ROOT }));
await screen.findByRole(BUTTON_ROLE, { name: "Kandev" });
expect(screen.getByRole(BUTTON_ROLE, { name: VIRTUAL_ROOT })).toBeTruthy();
expect(screen.getByRole(BUTTON_ROLE, { name: DRIVE_E_ROOT })).toBeTruthy();
expect(screen.getByRole(BUTTON_ROLE, { name: "Projects" })).toBeTruthy();
fireEvent.click(screen.getByRole(BUTTON_ROLE, { name: VIRTUAL_ROOT }));
expect(mockedListDirectory).toHaveBeenLastCalledWith(VIRTUAL_ROOT);
});
it("clears a stale selectable listing when navigation fails", async () => {
mockedListDirectory
.mockResolvedValueOnce(listing("C:\\", true, [{ name: "Denied", path: "C:\\Denied" }]))
.mockRejectedValueOnce(new Error("failed to list directory"));
render(<FolderPicker value="" onChange={vi.fn()} />);
fireEvent.click(screen.getByTestId(TRIGGER_TEST_ID));
fireEvent.click(await screen.findByRole(BUTTON_ROLE, { name: "Denied" }));
await screen.findByTestId("folder-picker-error");
await waitFor(() =>
expect(screen.getByTestId("folder-picker-choose").hasAttribute("disabled")).toBe(true),
);
expect(screen.queryByRole(BUTTON_ROLE, { name: "C:\\" })).toBeNull();
});
it("ignores an old success after a value change starts a successor load", async () => {
const oldLoad = deferred<DirectoryListing>();
const successorLoad = deferred<DirectoryListing>();
mockedListDirectory
.mockReturnValueOnce(oldLoad.promise)
.mockReturnValueOnce(successorLoad.promise);
const { rerender } = render(<FolderPicker value="" onChange={vi.fn()} />);
fireEvent.click(screen.getByTestId(TRIGGER_TEST_ID));
await waitFor(() => expect(mockedListDirectory).toHaveBeenCalledWith(""));
rerender(<FolderPicker value={SUCCESS_PATH} onChange={vi.fn()} />);
await waitFor(() => expect(mockedListDirectory).toHaveBeenCalledWith(SUCCESS_PATH));
await act(async () => successorLoad.resolve(listing(SUCCESS_PATH, true)));
await waitFor(() =>
expect(screen.getAllByRole(BUTTON_ROLE, { name: SUCCESS_NAME })).toHaveLength(2),
);
await act(async () => oldLoad.resolve(listing("C:\\Stale", true)));
expect(screen.queryByRole(BUTTON_ROLE, { name: "Stale" })).toBeNull();
expect(screen.getAllByRole(BUTTON_ROLE, { name: SUCCESS_NAME })).toHaveLength(2);
});
it("ignores an old error after close and reopen completes a successor load", async () => {
const oldLoad = deferred<DirectoryListing>();
const successorLoad = deferred<DirectoryListing>();
mockedListDirectory
.mockReturnValueOnce(oldLoad.promise)
.mockReturnValueOnce(successorLoad.promise);
render(<FolderPicker value="" onChange={vi.fn()} />);
fireEvent.click(screen.getByTestId(TRIGGER_TEST_ID));
await waitFor(() => expect(mockedListDirectory).toHaveBeenCalledTimes(1));
act(() => setPopoverOpen?.(false));
act(() => setPopoverOpen?.(true));
await waitFor(() => expect(mockedListDirectory).toHaveBeenCalledTimes(2));
await act(async () => successorLoad.resolve(listing(SUCCESS_PATH, true)));
await screen.findByRole(BUTTON_ROLE, { name: SUCCESS_NAME });
await act(async () => oldLoad.reject(new Error("stale failure")));
expect(screen.queryByTestId("folder-picker-error")).toBeNull();
expect(screen.getByRole(BUTTON_ROLE, { name: SUCCESS_NAME })).toBeTruthy();
expect(screen.getByTestId("folder-picker-choose").hasAttribute("disabled")).toBe(false);
});
});
function listing(
path: string,
choosable: boolean,
entries: DirectoryListing["entries"] = [],
): DirectoryListing {
return { path, parent: "", entries, choosable };
}
function deferred<T>() {
let resolve!: (value: T) => void;
let reject!: (reason?: unknown) => void;
const promise = new Promise<T>((resolvePromise, rejectPromise) => {
resolve = resolvePromise;
reject = rejectPromise;
});
return { promise, resolve, reject };
}
+52 -13
View File
@@ -1,6 +1,6 @@
"use client";
import { Fragment, useCallback, useEffect, useState } from "react";
import { Fragment, useCallback, useEffect, useRef, useState } from "react";
import { IconFolder, IconBox, IconChevronRight } from "@tabler/icons-react";
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
@@ -59,7 +59,7 @@ export function FolderPicker({ value, onChange, placeholder }: FolderPickerProps
)}
</PopoverTrigger>
<PopoverContent
className="w-[440px] gap-0 p-0 overflow-hidden"
className="w-[440px] max-w-[calc(100vw-2rem)] gap-0 p-0 overflow-hidden"
align="start"
sideOffset={4}
data-testid="folder-picker-popover"
@@ -72,7 +72,7 @@ export function FolderPicker({ value, onChange, placeholder }: FolderPickerProps
onDescend={(p) => void load(p)}
/>
<Footer
choosable={!!listing?.path}
choosable={listing?.choosable === true}
onUseScratch={() => {
onChange("");
setOpen(false);
@@ -90,8 +90,10 @@ export function FolderPicker({ value, onChange, placeholder }: FolderPickerProps
function leafName(path: string): string {
if (!path) return "";
const trimmed = path.replace(/\/+$/, "");
const idx = trimmed.lastIndexOf("/");
if (path === "/") return "/";
const trimmed = path.replace(/[\\/]+$/, "");
if (/^[A-Za-z]:$/.test(trimmed)) return `${trimmed}\\`;
const idx = Math.max(trimmed.lastIndexOf("/"), trimmed.lastIndexOf("\\"));
return idx === -1 ? trimmed : trimmed.slice(idx + 1) || "/";
}
@@ -99,16 +101,22 @@ function useDirectoryListing(open: boolean, value: string) {
const [listing, setListing] = useState<DirectoryListing | null>(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
const requestGeneration = useRef(0);
const load = useCallback(async (path: string) => {
const generation = ++requestGeneration.current;
setLoading(true);
setError(null);
setListing(null);
try {
setListing(await listDirectory(path));
const nextListing = await listDirectory(path);
if (generation !== requestGeneration.current) return;
setListing(nextListing);
} catch (err) {
if (generation !== requestGeneration.current) return;
setError(err instanceof Error ? err.message : "Failed to load directory");
} finally {
setLoading(false);
if (generation === requestGeneration.current) setLoading(false);
}
}, []);
@@ -118,12 +126,17 @@ function useDirectoryListing(open: boolean, value: string) {
// closed without committing, and reopened would still see the stale
// last-browsed directory instead of their picked folder (or the root).
if (!open) {
requestGeneration.current++;
setListing(null);
setLoading(false);
setError(null);
return;
}
if (listing) return;
void load(value || "");
}, [open, listing, load, value]);
return () => {
requestGeneration.current++;
};
}, [open, load, value]);
return { listing, loading, error, load };
}
@@ -131,14 +144,40 @@ function useDirectoryListing(open: boolean, value: string) {
/** Splits an absolute path into clickable breadcrumb segments. */
function pathSegments(path: string): Array<{ name: string; path: string }> {
if (!path) return [];
const parts = path.split("/").filter(Boolean);
const segs: Array<{ name: string; path: string }> = [{ name: "/", path: "/" }];
let acc = "";
const driveMatch = /^([A-Za-z]:)[\\/]*(.*)$/.exec(path);
if (driveMatch) {
const driveRoot = `${driveMatch[1]}\\`;
segs.push({ name: driveRoot, path: driveRoot });
appendPathSegments(segs, driveRoot, driveMatch[2], "\\");
return segs;
}
const uncMatch = /^\\\\([^\\/]+)[\\/]([^\\/]+)[\\/]*(.*)$/.exec(path);
if (uncMatch) {
const shareRoot = `\\\\${uncMatch[1]}\\${uncMatch[2]}\\`;
segs.push({ name: shareRoot, path: shareRoot });
appendPathSegments(segs, shareRoot, uncMatch[3], "\\");
return segs;
}
appendPathSegments(segs, "", path, "/");
return segs;
}
function appendPathSegments(
segs: Array<{ name: string; path: string }>,
root: string,
path: string,
separator: "/" | "\\",
) {
const parts = path.split(/[\\/]+/).filter(Boolean);
let acc = root.replace(/[\\/]+$/, "");
for (const part of parts) {
acc += `/${part}`;
acc += `${separator}${part}`;
segs.push({ name: part, path: acc });
}
return segs;
}
function Breadcrumb({ path, onNavigate }: { path: string; onNavigate: (p: string) => void }) {
+1
View File
@@ -9,6 +9,7 @@ export type DirectoryListing = {
path: string;
parent: string;
entries: DirectoryEntry[];
choosable: boolean;
};
/**