fix(host,client): review round 19 — home ships resolved on the wire; client mirror shrinks to the draft side; scoped focus guarantee
This commit is contained in:
@@ -19,7 +19,7 @@ export interface DirectoryEntry {
|
||||
export interface DirectoryListing {
|
||||
/** Absolute path of the listed directory. */
|
||||
path: string
|
||||
/** The host account's home directory (breadcrumb "Home" rooting). */
|
||||
/** The host account's home directory (breadcrumb "Home" rooting), in the same resolved shape as `path` and `crumbs[].path`. */
|
||||
home: string
|
||||
/**
|
||||
* Ancestor chain from the filesystem root to the listed directory
|
||||
|
||||
@@ -82,19 +82,17 @@ function foldSeparatorsFor(sep: '\\' | '/'): (value: string) => string {
|
||||
}
|
||||
|
||||
/**
|
||||
* Lexically normalizes an absolute host path for comparisons: folds
|
||||
* forward slashes on Windows (win32 accepts either), collapses repeated
|
||||
* and trailing separators, drops `.` segments, and applies `..` without
|
||||
* ever crossing the root — POSIX's `/`, a drive's `C:`, or UNC's
|
||||
* `\\server\share` pair — mirroring the backend's resolve() for the
|
||||
* shapes an environment-supplied HOME legally carries verbatim while the
|
||||
* backend's own paths arrive already resolved. A lexical mirror only:
|
||||
* symlinks are the backend's business.
|
||||
* Lexically normalizes a typed absolute path for comparisons against the
|
||||
* backend's resolved ones (the wire contract keeps `path`, `crumbs[].path`,
|
||||
* and `home` in resolved shape; only the DRAFT side needs this): collapses
|
||||
* repeated and trailing separators, drops `.` segments, and applies `..`
|
||||
* without ever crossing the root — POSIX's `/`, a drive's `C:`, or UNC's
|
||||
* `\\server\share` pair — mirroring resolve()'s lexical behavior. Expects
|
||||
* separators already folded to `sep` (foldSeparatorsFor); a lexical mirror
|
||||
* only, symlinks are the backend's business.
|
||||
*/
|
||||
function normalizePathFor(sep: '\\' | '/'): (value: string) => string {
|
||||
const foldSeparators = foldSeparatorsFor(sep)
|
||||
return (raw) => {
|
||||
const value = foldSeparators(raw)
|
||||
return (value) => {
|
||||
const unc = sep === '\\' && value.startsWith(`${sep}${sep}`)
|
||||
const rawSegments = (unc ? value.slice(2) : value).split(sep)
|
||||
// Empty segments are separator noise everywhere except POSIX's leading
|
||||
@@ -123,16 +121,14 @@ function normalizePathFor(sep: '\\' | '/'): (value: string) => string {
|
||||
/**
|
||||
* Breadcrumb rows for display: inside the home subtree the chain starts at a
|
||||
* localized Home crumb; outside it the full ancestry shows, the root labeled
|
||||
* by its own path. The home comparison folds per platform and lexically
|
||||
* normalizes both sides, so a typed-case Windows path or a `HOME=/home/u/.`
|
||||
* shape still collapses to the Home crumb.
|
||||
* by its own path. `home` and every crumb path arrive in the same resolved
|
||||
* shape (the wire contract), so only the platform case fold remains — a
|
||||
* typed-case Windows chain still collapses to the Home crumb.
|
||||
*/
|
||||
function displayCrumbs(listing: DirectoryListing, homeLabel: string): DirectoryEntry[] {
|
||||
const sep = separatorOf(listing)
|
||||
const fold = foldPathFor(sep)
|
||||
const normalize = normalizePathFor(sep)
|
||||
const home = fold(normalize(listing.home))
|
||||
const homeIndex = listing.crumbs.findIndex(crumb => fold(normalize(crumb.path)) === home)
|
||||
const fold = foldPathFor(separatorOf(listing))
|
||||
const home = fold(listing.home)
|
||||
const homeIndex = listing.crumbs.findIndex(crumb => fold(crumb.path) === home)
|
||||
if (homeIndex === -1) return listing.crumbs
|
||||
const tail = listing.crumbs.slice(homeIndex + 1)
|
||||
return [{ name: homeLabel, path: listing.home, hidden: false }, ...tail]
|
||||
@@ -161,13 +157,14 @@ function separatorOf(listing: DirectoryListing): '\\' | '/' {
|
||||
* The path draft's final segment, when its directory part names the level
|
||||
* `listing` lists — the segment the level prefix-filters on while the user
|
||||
* types. Any other draft (no separator yet, or naming some other directory)
|
||||
* leaves the level unfiltered. The directory part compares lexically
|
||||
* normalized (dot segments, repeated separators, and win32 forward
|
||||
* slashes all match what Enter would navigate to) and under the platform
|
||||
* case fold (exact on slash platforms; Windows folds, since an upgraded
|
||||
* selection may carry the actual entry's case while the level below still
|
||||
* carries the typed one); the name filter downstream is case-insensitive
|
||||
* everywhere.
|
||||
* leaves the level unfiltered. Only the directory part is lexically
|
||||
* normalized (dot segments, repeated separators, and win32 forward slashes
|
||||
* all match what Enter would navigate to) and platform-case-folded (exact
|
||||
* on slash platforms; Windows folds, since an upgraded selection may carry
|
||||
* the actual entry's case while the level below still carries the typed
|
||||
* one); the FINAL segment stays a literal name prefix — a lone `.` reads
|
||||
* as the dot-reveal, `..` matches no entry (Enter still navigates it) —
|
||||
* and the name filter downstream is case-insensitive everywhere.
|
||||
*/
|
||||
function draftPrefixFor(listing: DirectoryListing, draft: string | null): string | null {
|
||||
if (draft === null) return null
|
||||
@@ -177,7 +174,7 @@ function draftPrefixFor(listing: DirectoryListing, draft: string | null): string
|
||||
if (cut === -1) return null
|
||||
const fold = foldPathFor(sep)
|
||||
const normalize = normalizePathFor(sep)
|
||||
return fold(normalize(folded.slice(0, cut + 1))) === fold(normalize(listing.path))
|
||||
return fold(normalize(folded.slice(0, cut + 1))) === fold(listing.path)
|
||||
? folded.slice(cut + 1)
|
||||
: null
|
||||
}
|
||||
@@ -593,15 +590,16 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
||||
if (row !== null && childPath !== undefined) row.scrollLeft = row.scrollWidth
|
||||
}, [childPath])
|
||||
// Every pick and editor exit that would drop focus to body re-parks it
|
||||
// after commit, so keyboard traversal stays inside the dialog (the Modal
|
||||
// has no focus trap): a pick lands on the selection's row — aria-current
|
||||
// in the freshly rendered left pane, which survives even a right-pane
|
||||
// after commit, so THIS DIALOG'S OWN node replacements never leak focus
|
||||
// out of the card: a pick lands on the selection's row — aria-current in
|
||||
// the freshly rendered left pane, which survives even a right-pane
|
||||
// advance or a create landing replacing the picked button's column —
|
||||
// while the edit-zone exits enumerated at the flag declarations fall
|
||||
// back to the crumb edit zone. The one window outside this invariant is
|
||||
// the owner's adopt: busy inerts every control in the card (browsers
|
||||
// blur disabled elements to body) and no parking applies — the owner
|
||||
// closes the dialog either way.
|
||||
// back to the crumb edit zone. Outside the guarantee: the Modal has no
|
||||
// focus trap, so tabbing past the card's edge legitimately leaves, and
|
||||
// the owner's adopt window (busy inerts every control; browsers blur
|
||||
// disabled elements to body) gets no parking — the owner closes the
|
||||
// dialog either way.
|
||||
useEffect(() => {
|
||||
if (pathDraft !== null) return
|
||||
if (refocusPick.current) {
|
||||
|
||||
@@ -215,7 +215,12 @@ export default class BrowseDirectoryPicker extends DirectoryPicker {
|
||||
}
|
||||
|
||||
private async list(path?: string, signal?: AbortSignal): Promise<DirectoryListing> {
|
||||
const home = homedir()
|
||||
// Resolved like every other path in the listing: the environment may
|
||||
// decorate HOME (trailing or repeated separators, dot segments, win32
|
||||
// forward slashes) and homedir() ships it verbatim, while clients
|
||||
// compare home against the resolved `path`/`crumbs` — the wire contract
|
||||
// promises one canonical shape for all three.
|
||||
const home = resolve(homedir())
|
||||
// The seam contract takes fully qualified paths only; resolve() would
|
||||
// silently rebase a relative or empty wire value under the host process
|
||||
// cwd (or, for rooted drive-less Windows forms, its current drive).
|
||||
|
||||
@@ -240,19 +240,6 @@ describe('DirectoryBrowser', () => {
|
||||
expect(within(columns()[1]!).getByText('harness')).toBeTruthy()
|
||||
})
|
||||
|
||||
// HOME ships verbatim from the environment while listing paths arrive
|
||||
// resolved; each decoration (trailing, repeated, dot, dot-dot segments)
|
||||
// must still normalize to the display root — single pane, Home crumb,
|
||||
// and no parent leg launched.
|
||||
it.each(['/', '//', '/foo/../.'])('a home decorated with "%s" is still the display root', async (decoration) => {
|
||||
const listDirectory = vi.fn(async (path?: string) => ({ ...listingFor(path), home: `${HOME}${decoration}` }))
|
||||
mount({ listDirectory })
|
||||
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||
expect(columns()).toHaveLength(1)
|
||||
expect(screen.getByRole('button', { name: 'browser.home' })).toBeTruthy()
|
||||
expect(listDirectory).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('a landing whose new level dropped the focused row parks on the edit zone', async () => {
|
||||
mount()
|
||||
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||
@@ -407,42 +394,16 @@ describe('DirectoryBrowser', () => {
|
||||
expect(document.activeElement?.getAttribute('aria-current')).toBe('true')
|
||||
})
|
||||
|
||||
it('a UNC home with dot-dot never pops the share root and still collapses to Home', async () => {
|
||||
it('a UNC level is home-collapsed and filters decorated UNC drafts without popping the share root', async () => {
|
||||
const SHARE = '\\\\server\\share'
|
||||
const listing: DirectoryListing = {
|
||||
path: `${SHARE}\\x`,
|
||||
// USERPROFILE ships verbatim; win32.resolve keeps \\server\share as
|
||||
// the unpoppable root and folds the doubled separator, so this
|
||||
// normalizes to \\server\share\x.
|
||||
home: '\\\\server\\\\share\\..\\x',
|
||||
home: `${SHARE}\\x`,
|
||||
crumbs: [
|
||||
{ name: `${SHARE}\\`, path: `${SHARE}\\`, hidden: false },
|
||||
{ name: 'x', path: `${SHARE}\\x`, hidden: false },
|
||||
],
|
||||
entries: [],
|
||||
truncated: false,
|
||||
}
|
||||
const listDirectory = vi.fn(async () => listing)
|
||||
mount({ listDirectory })
|
||||
await waitFor(() => { expect(listDirectory).toHaveBeenCalled() })
|
||||
await waitFor(() => { expect(screen.getByRole('button', { name: 'browser.home' })).toBeTruthy() })
|
||||
expect(columns()).toHaveLength(1)
|
||||
expect(listDirectory).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('a forward-slash Windows home still reads as the display root', async () => {
|
||||
const listing: DirectoryListing = {
|
||||
path: 'C:\\Users\\Alice',
|
||||
// USERPROFILE may legally use forward slashes; the root crumb (not
|
||||
// the home text) carries the platform, and normalization folds the
|
||||
// slashes before comparing.
|
||||
home: 'C:/Users/Alice',
|
||||
crumbs: [
|
||||
{ name: 'C:\\', path: 'C:\\', hidden: false },
|
||||
{ name: 'Users', path: 'C:\\Users', hidden: false },
|
||||
{ name: 'Alice', path: 'C:\\Users\\Alice', hidden: false },
|
||||
],
|
||||
entries: [{ name: 'Desktop', path: 'C:\\Users\\Alice\\Desktop', hidden: false }],
|
||||
entries: [{ name: 'Alpha', path: `${SHARE}\\x\\Alpha`, hidden: false }],
|
||||
truncated: false,
|
||||
}
|
||||
const listDirectory = vi.fn(async () => listing)
|
||||
@@ -450,7 +411,15 @@ describe('DirectoryBrowser', () => {
|
||||
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||
expect(columns()).toHaveLength(1)
|
||||
expect(screen.getByRole('button', { name: 'browser.home' })).toBeTruthy()
|
||||
expect(listDirectory).toHaveBeenCalledTimes(1)
|
||||
// A decorated UNC draft (doubled separator, share-root-crossing dot-dot)
|
||||
// still normalizes to the listed level: the filter matches what Enter
|
||||
// would navigate to, and \\server\share stays unpoppable.
|
||||
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||
const input = screen.getByLabelText<HTMLInputElement>('browser.editPath')
|
||||
fireEvent.change(input, { target: { value: '\\\\server\\\\share\\..\\x\\a' } })
|
||||
expect(screen.getByText('Alpha')).toBeTruthy()
|
||||
fireEvent.change(input, { target: { value: `${SHARE}\\x\\z` } })
|
||||
expect(screen.queryByRole('listitem')).toBeNull()
|
||||
})
|
||||
|
||||
it('collapses a typed-case Windows home to the display root (single pane, Home crumb)', async () => {
|
||||
@@ -682,6 +651,8 @@ describe('DirectoryBrowser', () => {
|
||||
expect(screen.getByRole('listitem').textContent).toBe('Documents')
|
||||
fireEvent.change(input, { target: { value: `${HOME}//do` } })
|
||||
expect(screen.getByRole('listitem').textContent).toBe('Documents')
|
||||
fireEvent.change(input, { target: { value: `${HOME}/foo/../do` } })
|
||||
expect(screen.getByRole('listitem').textContent).toBe('Documents')
|
||||
})
|
||||
|
||||
it('filters the child pane in two-pane mode and follows the draft back up a level', async () => {
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
/**
|
||||
* The wire contract's home shape: a decorated HOME (trailing/repeated
|
||||
* separators, dot segments — homedir() ships it verbatim) still leaves the
|
||||
* listing carrying the resolved form, matching `path` and `crumbs[].path`.
|
||||
*/
|
||||
|
||||
import { resolve } from 'node:path'
|
||||
import { expect, it, vi } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
|
||||
vi.mock('node:os', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('node:os')>()
|
||||
return { ...actual, homedir: () => `${actual.homedir()}/.//.` }
|
||||
})
|
||||
|
||||
it('resolves a decorated homedir before stamping listing.home', async () => {
|
||||
const { homedir } = await vi.importActual<typeof import('node:os')>('node:os')
|
||||
const { default: BrowseDirectoryPicker } = await import('../src/index.ts')
|
||||
const ctx = new Context()
|
||||
const fiber = ctx.plugin(BrowseDirectoryPicker)
|
||||
await fiber.await()
|
||||
const picked = ctx.get('directoryPicker')!.capability()
|
||||
if (picked.kind !== 'browse') throw new Error('browse backend must advertise the browse capability')
|
||||
const listing = await picked.list()
|
||||
expect(listing.home).toBe(resolve(homedir()))
|
||||
expect(listing.path).toBe(listing.home)
|
||||
await fiber.dispose()
|
||||
})
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
|
||||
import { homedir, tmpdir } from 'node:os'
|
||||
import { basename, join } from 'node:path'
|
||||
import { basename, join, resolve } from 'node:path'
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import { DirectoryPickerError } from '@deepseek-ai/dsh-host-directory-picker'
|
||||
@@ -48,7 +48,9 @@ describe('BrowseDirectoryPicker', () => {
|
||||
it('lists directories only, flags hidden rows, follows symlinks, skips broken links, sorts by name', async () => {
|
||||
const listing = await capability.list(root)
|
||||
expect(listing.path).toBe(root)
|
||||
expect(listing.home).toBe(homedir())
|
||||
// Resolved like path and crumbs — the environment may decorate HOME,
|
||||
// and the wire contract promises one canonical shape for all three.
|
||||
expect(listing.home).toBe(resolve(homedir()))
|
||||
expect(listing.entries.map(entry => entry.name)).toEqual(['.hidden-dir', 'linked', 'projects'])
|
||||
expect(listing.entries.map(entry => entry.hidden)).toEqual([true, false, false])
|
||||
// Every entry path is absolute and host-joined — clients never join segments.
|
||||
|
||||
@@ -38,7 +38,7 @@ export interface DirectoryEntry {
|
||||
export interface DirectoryListing {
|
||||
/** Absolute path of the listed directory. */
|
||||
path: string
|
||||
/** The host account's home directory (breadcrumb "Home" rooting). */
|
||||
/** The host account's home directory (breadcrumb "Home" rooting), in the same resolved shape as `path` and `crumbs[].path`. */
|
||||
home: string
|
||||
/**
|
||||
* Ancestor chain from the filesystem root to the listed directory
|
||||
|
||||
Reference in New Issue
Block a user