fix(host): review round 20 — canonical shape declared at the interface; hermetic home-shape spec; single resolve
This commit is contained in:
@@ -15,11 +15,18 @@ export interface DirectoryEntry {
|
||||
hidden: boolean
|
||||
}
|
||||
|
||||
/** host.listDirectory response value: one directory level plus its ancestry. */
|
||||
/**
|
||||
* host.listDirectory response value: one directory level plus its ancestry.
|
||||
* Every path in one listing — `path`, `crumbs[].path`, `entries[].path`,
|
||||
* and `home` — is host-resolved canonical form: no `.`/`..` segments, no
|
||||
* repeated or trailing separators (bare roots `/`, `C:\`, `\\server\share\`
|
||||
* excepted), one platform separator. Clients compare paths on this promise
|
||||
* without re-normalizing.
|
||||
*/
|
||||
export interface DirectoryListing {
|
||||
/** Absolute path of the listed directory. */
|
||||
path: string
|
||||
/** The host account's home directory (breadcrumb "Home" rooting), in the same resolved shape as `path` and `crumbs[].path`. */
|
||||
/** The host account's home directory (breadcrumb "Home" rooting), in the interface's canonical shape like every other path here. */
|
||||
home: string
|
||||
/**
|
||||
* Ancestor chain from the filesystem root to the listed directory
|
||||
|
||||
@@ -137,8 +137,9 @@ function displayCrumbs(listing: DirectoryListing, homeLabel: string): DirectoryE
|
||||
/**
|
||||
* The listing's platform separator, read from the host-resolved root crumb
|
||||
* (`/`, `C:\`, `\\server\share\`) — exact for every root form the backend
|
||||
* emits, immune both to a home delivered in the other slash flavor
|
||||
* (`USERPROFILE=C:/Users/Alice`) and to backslashes inside POSIX names.
|
||||
* emits, and immune to backslashes inside POSIX names (which the home text
|
||||
* may legally carry; the wire contract already excludes non-canonical
|
||||
* shapes elsewhere).
|
||||
* TODO: replace with a host-stamped `separator` field on the wire
|
||||
* DirectoryListing so the platform fact travels verbatim (the trade-off is
|
||||
* recorded in the directory-picker capability seam Agent Note).
|
||||
|
||||
@@ -217,9 +217,12 @@ export default class BrowseDirectoryPicker extends DirectoryPicker {
|
||||
private async list(path?: string, signal?: AbortSignal): Promise<DirectoryListing> {
|
||||
// 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.
|
||||
// forward slashes) and homedir() ships it verbatim, while the wire
|
||||
// contract promises one canonical shape for every listing path. A
|
||||
// relative or drive-less HOME rebases under the process cwd / current
|
||||
// drive here — the behavior the fullyQualified fence refuses for wire
|
||||
// values — accepted for the host's own environment, since the listed
|
||||
// target derives from home and stays consistent with it.
|
||||
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
|
||||
@@ -227,7 +230,7 @@ export default class BrowseDirectoryPicker extends DirectoryPicker {
|
||||
if (path !== undefined && !fullyQualified(path)) {
|
||||
throw new DirectoryPickerError('directory-unreadable', path, `cannot list "${path}": not a fully qualified path`)
|
||||
}
|
||||
const target = resolve(path ?? home)
|
||||
const target = path === undefined ? home : resolve(path)
|
||||
// Stream the level (opendir, one dirent at a time) into a name-sorted
|
||||
// window of maxEntries + 1 candidates: memory stays bounded no matter how
|
||||
// many children the directory holds, the window keeps the name-sorted
|
||||
|
||||
@@ -2,19 +2,33 @@
|
||||
* 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`.
|
||||
* The mock points homedir at a scratch tree so the probe never scans the
|
||||
* running machine's real home (same hermetic reasoning as service.spec's
|
||||
* temporary tree); the mock spreads the actual module, so tmpdir stays real.
|
||||
*/
|
||||
|
||||
import { resolve } from 'node:path'
|
||||
import { expect, it, vi } from 'vitest'
|
||||
import { mkdtemp, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { afterAll, beforeAll, expect, it, vi } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
|
||||
let scratch: string
|
||||
|
||||
vi.mock('node:os', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('node:os')>()
|
||||
return { ...actual, homedir: () => `${actual.homedir()}/.//.` }
|
||||
return { ...actual, homedir: () => `${scratch}/.//.` }
|
||||
})
|
||||
|
||||
beforeAll(async () => {
|
||||
scratch = await mkdtemp(join(tmpdir(), 'dsh-home-shape-'))
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
await rm(scratch, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
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)
|
||||
@@ -22,7 +36,7 @@ it('resolves a decorated homedir before stamping listing.home', async () => {
|
||||
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.home).toBe(resolve(scratch))
|
||||
expect(listing.path).toBe(listing.home)
|
||||
await fiber.dispose()
|
||||
})
|
||||
|
||||
@@ -34,11 +34,18 @@ export interface DirectoryEntry {
|
||||
hidden: boolean
|
||||
}
|
||||
|
||||
/** One directory level plus its ancestry, as a browse backend reports it. */
|
||||
/**
|
||||
* One directory level plus its ancestry, as a browse backend reports it.
|
||||
* Every path in one listing — `path`, `crumbs[].path`, `entries[].path`,
|
||||
* and `home` — is host-resolved canonical form: no `.`/`..` segments, no
|
||||
* repeated or trailing separators (bare roots `/`, `C:\`, `\\server\share\`
|
||||
* excepted), one platform separator. Clients compare paths on this promise
|
||||
* without re-normalizing; every backend must resolve before stamping.
|
||||
*/
|
||||
export interface DirectoryListing {
|
||||
/** Absolute path of the listed directory. */
|
||||
path: string
|
||||
/** The host account's home directory (breadcrumb "Home" rooting), in the same resolved shape as `path` and `crumbs[].path`. */
|
||||
/** The host account's home directory (breadcrumb "Home" rooting), in the interface's canonical shape like every other path here. */
|
||||
home: string
|
||||
/**
|
||||
* Ancestor chain from the filesystem root to the listed directory
|
||||
|
||||
Reference in New Issue
Block a user