From b211a80b1fa8d3eccd1d0c5ab0cb2f7b5b1755d6 Mon Sep 17 00:00:00 2001 From: creatixchu Date: Tue, 28 Jul 2026 18:13:20 +0800 Subject: [PATCH] fix(host,client): require fully qualified browse paths; clear the picker kind on close ds-review-bot round 4. On Windows, isAbsolute admits rooted drive-less forms (\foo, /foo) that resolve() then rebases onto the process's current drive; both browse primitives now gate on a fullyQualified check (drive letter or UNC on win32, POSIX-absolute elsewhere) with a platform test seam, per-platform unit cases, and the contract wording updated on the seam, the backend README pair, and the error messages. The picker-kind effect also kept a resolved 'dialog' across close, so a backend swapped while the menu was closed could paint the stale entry for one frame on reopen; the close arm now clears the state, pinned by a reopen-under-pending-read race test. --- docs/cordis-catalog/services.md | 2 +- .../src/client/WorkspacePicker.tsx | 16 ++++++--- .../tests/workspace-picker.spec.tsx | 14 ++++++++ .../directory-picker-browse/README.i18n.yaml | 4 +-- .../host/directory-picker-browse/README.md | 2 +- .../host/directory-picker-browse/README.zh.md | 2 +- .../host/directory-picker-browse/src/index.ts | 34 ++++++++++++++----- .../tests/service.spec.ts | 15 +++++++- packages/host/directory-picker/src/index.ts | 7 ++-- 9 files changed, 74 insertions(+), 22 deletions(-) diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 3b47e405f0..ce24921fe8 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -481,7 +481,7 @@ Abstract directory-picking service. Subclass, implement `capability()`, and load abstract capability(): DirectoryPickerCapability ``` -Source: [`packages/host/directory-picker/src/index.ts:120`](../../packages/host/directory-picker/src/index.ts) +Source: [`packages/host/directory-picker/src/index.ts:121`](../../packages/host/directory-picker/src/index.ts) ## `ctx.fs` — `FileSystem` (abstract seam) diff --git a/packages/client/ui-workspace/src/client/WorkspacePicker.tsx b/packages/client/ui-workspace/src/client/WorkspacePicker.tsx index 2e65fc6b72..786c5cf305 100644 --- a/packages/client/ui-workspace/src/client/WorkspacePicker.tsx +++ b/packages/client/ui-workspace/src/client/WorkspacePicker.tsx @@ -79,11 +79,17 @@ export function WorkspaceCreateFlow({ // flow open — no cache to go stale across reconnects. const [dialogPicker, setDialogPicker] = useState(false) useEffect(() => { - if (!open) return - // Reset before each read: a reconnect can change the composed backend, so - // a previous open's answer must not leak into this one; and a settlement - // from a superseded open (flow closed, or a newer read started) is - // discarded via the cleanup-toggled flag. + if (!open) { + // Close discards the answer: a reconnect or HMR can swap the composed + // backend while the menu is closed, and the reopened menu must never + // paint the previous host's entry before the fresh read lands. + setDialogPicker(false) + return + } + // Reset before each read: the injected reader can also change identity + // while the flow stays open, and that prior answer must not leak either; + // a settlement from a superseded read is discarded via the + // cleanup-toggled flag. setDialogPicker(false) let stale = false void directoryPickerKind() diff --git a/packages/client/ui-workspace/tests/workspace-picker.spec.tsx b/packages/client/ui-workspace/tests/workspace-picker.spec.tsx index 9ca5304fa4..d8b3ab5a54 100644 --- a/packages/client/ui-workspace/tests/workspace-picker.spec.tsx +++ b/packages/client/ui-workspace/tests/workspace-picker.spec.tsx @@ -300,6 +300,20 @@ describe('WorkspacePicker', () => { expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() }) + it('clears the advertised kind on close so a reopen cannot paint the previous host entry', async () => { + const directoryPickerKind = vi.fn<() => Promise>() + .mockImplementationOnce(async () => 'dialog') + // The reopened read never settles: the assertion below sees the paint + // that precedes any fresh answer. + .mockImplementation(() => new Promise(() => {})) + const t = togglable(directoryPickerKind) + await screen.findByRole('menuitem', { name: 'Open local folder…' }) + t.setOpen(false) + t.setOpen(true) + await screen.findByRole('menuitem', { name: 'Create a new workspace' }) + expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() + }) + it('discards a stale describe failure after a newer open already answered', async () => { let rejectFirst!: (reason: Error) => void const first = new Promise((_settle, reject) => { rejectFirst = reject }) diff --git a/packages/host/directory-picker-browse/README.i18n.yaml b/packages/host/directory-picker-browse/README.i18n.yaml index 916db3c321..4dfc363903 100644 --- a/packages/host/directory-picker-browse/README.i18n.yaml +++ b/packages/host/directory-picker-browse/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/host/directory-picker-browse/README.md -README.md: 81357269e1d4b075f7e31b5f3ac5d4721811024a -README.zh.md: 06a7f7651b2abc0aa73eba41042f3d1a86661b76 +README.md: 160a12a8594400c9e6c565881d8e9ca0517b4e24 +README.zh.md: 4cfc4a611fb17cb30ac841c8af8498b51a6388b0 diff --git a/packages/host/directory-picker-browse/README.md b/packages/host/directory-picker-browse/README.md index 81357269e1..160a12a859 100644 --- a/packages/host/directory-picker-browse/README.md +++ b/packages/host/directory-picker-browse/README.md @@ -4,7 +4,7 @@ English | [中文](README.zh.md) The **in-app browsing backend** of the [directory-picker seam](../directory-picker/README.md): `BrowseDirectoryPicker` registers `ctx.directoryPicker` with the `browse` capability — one-level directory listing and child-directory creation over Node's stdlib, which already carries the per-OS adaptation. Nothing renders on the host display, so this backend serves remote clients the dialog backend cannot. -Behavior facts: listings return **directories only**, name-sorted, with symlinks-to-directories followed (broken/cyclic links skipped — the probe `stat` failing means "not enterable") and a host-owned `hidden` flag (POSIX dot convention) left for the client to act on; `crumbs` is the root-to-target ancestor chain, the root crumb labeled by its full path (`/`, `C:\`); an absent `list` path means the host account's home directory. `createDirectory` is non-recursive (a missing parent is a real failure, not a level to invent) and validates the name as a single non-blank segment even when called directly, mirroring the wire schema's fence. Both primitives reject a non-absolute explicit path (`directory-unreadable`/`directory-create-failed`) instead of letting `resolve` rebase it under the host process cwd. Failures throw the seam's typed `DirectoryPickerError`. Policy rationale: [the directory-picker capability seam Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md). +Behavior facts: listings return **directories only**, name-sorted, with symlinks-to-directories followed (broken/cyclic links skipped — the probe `stat` failing means "not enterable") and a host-owned `hidden` flag (POSIX dot convention) left for the client to act on; `crumbs` is the root-to-target ancestor chain, the root crumb labeled by its full path (`/`, `C:\`); an absent `list` path means the host account's home directory. `createDirectory` is non-recursive (a missing parent is a real failure, not a level to invent) and validates the name as a single non-blank segment even when called directly, mirroring the wire schema's fence. Both primitives reject an explicit path that is not fully qualified — relative forms, and on Windows the rooted drive-less forms (`\foo`, `/foo`) that `isAbsolute` accepts — with `directory-unreadable`/`directory-create-failed`, instead of letting `resolve` rebase it under the host process cwd or current drive. Failures throw the seam's typed `DirectoryPickerError`. Policy rationale: [the directory-picker capability seam Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md). ## Model Experience diff --git a/packages/host/directory-picker-browse/README.zh.md b/packages/host/directory-picker-browse/README.zh.md index 06a7f7651b..4cfc4a611f 100644 --- a/packages/host/directory-picker-browse/README.zh.md +++ b/packages/host/directory-picker-browse/README.zh.md @@ -4,7 +4,7 @@ [目录选择 seam](../directory-picker/README.md) 的**应用内浏览后端**：`BrowseDirectoryPicker` 以 `browse` 能力注册 `ctx.directoryPicker`——基于 Node 标准库（跨 OS 适配本就由它承担）提供单层目录列举与子目录创建。宿主屏幕上不渲染任何东西，因此该后端能服务 dialog 后端无法触及的远程客户端。 -行为事实：列举**只返回目录**、按名称排序，指向目录的符号链接会被跟随（断链／循环链接被跳过——探测 `stat` 失败即"不可进入"），并携带宿主判定的 `hidden` 标志（POSIX 点前缀约定），展示决策留给客户端；`crumbs` 是从根到目标的祖先链，根 crumb 以完整路径标注（`/`、`C:\`）；`list` 不带路径即列举宿主账户的家目录。`createDirectory` 不递归（父目录缺失是真实失败，不是要补造的层级），且即便被直接调用也把名称校验为单个非空段，与协议 schema 的栅栏一致。两个原语都拒绝非绝对的显式路径（`directory-unreadable`／`directory-create-failed`），而不是任由 `resolve` 把它重定位到宿主进程 cwd 之下。失败抛出 seam 的类型化 `DirectoryPickerError`。策略依据：[目录选择能力 seam Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md)。 +行为事实：列举**只返回目录**、按名称排序，指向目录的符号链接会被跟随（断链／循环链接被跳过——探测 `stat` 失败即"不可进入"），并携带宿主判定的 `hidden` 标志（POSIX 点前缀约定），展示决策留给客户端；`crumbs` 是从根到目标的祖先链，根 crumb 以完整路径标注（`/`、`C:\`）；`list` 不带路径即列举宿主账户的家目录。`createDirectory` 不递归（父目录缺失是真实失败，不是要补造的层级），且即便被直接调用也把名称校验为单个非空段，与协议 schema 的栅栏一致。两个原语都拒绝非完全限定的显式路径——相对形态，以及 Windows 上 `isAbsolute` 会放行的无盘符有根形态（`\foo`、`/foo`）——报 `directory-unreadable`／`directory-create-failed`，而不是任由 `resolve` 把它重定位到宿主进程 cwd 或当前盘符之下。失败抛出 seam 的类型化 `DirectoryPickerError`。策略依据：[目录选择能力 seam Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md)。 ## 模型体验 diff --git a/packages/host/directory-picker-browse/src/index.ts b/packages/host/directory-picker-browse/src/index.ts index 83a3f6762e..d3566e9415 100644 --- a/packages/host/directory-picker-browse/src/index.ts +++ b/packages/host/directory-picker-browse/src/index.ts @@ -11,7 +11,7 @@ import { mkdir, readdir, stat } from 'node:fs/promises' import { homedir } from 'node:os' -import { basename, dirname, isAbsolute, join, resolve } from 'node:path' +import { basename, dirname, join, posix, resolve, win32 } from 'node:path' import { DirectoryPicker, DirectoryPickerError, } from '@deepseek-ai/dsh-host-directory-picker' @@ -35,6 +35,22 @@ function ancestryCrumbs(target: string): DirectoryEntry[] { } } +/** + * True when the path names one fixed filesystem location regardless of + * process state: POSIX-absolute on POSIX; on Windows only drive-qualified + * (`C:\…`) or UNC (`\\server\…`) forms — rooted drive-less forms (`\foo`, + * `/foo`) pass `isAbsolute` yet still resolve against the process's current + * drive. + * @param path - candidate path. + * @param platform - replaces `process.platform` for deterministic tests. + * @returns whether the path is fully qualified on the platform. + */ +export function fullyQualified(path: string, platform: NodeJS.Platform = process.platform): boolean { + return platform === 'win32' + ? win32.isAbsolute(path) && /^(?:[A-Za-z]:[\\/]|[\\/]{2})/.test(path) + : posix.isAbsolute(path) +} + /** Message text of an unknown thrown value. */ function messageOf(error: unknown): string { /* v8 ignore next -- node:fs rejects with Error instances; the String arm only satisfies the unknown narrowing. */ @@ -81,10 +97,11 @@ export default class BrowseDirectoryPicker extends DirectoryPicker { private async list(path?: string): Promise { const home = homedir() - // The seam contract takes absolute paths only; resolve() would silently - // rebase a relative or empty wire value under the host process cwd. - if (path !== undefined && !isAbsolute(path)) { - throw new DirectoryPickerError('directory-unreadable', path, `cannot list "${path}": not an absolute path`) + // 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). + if (path !== undefined && !fullyQualified(path)) { + throw new DirectoryPickerError('directory-unreadable', path, `cannot list "${path}": not a fully qualified path`) } const target = resolve(path ?? home) let names: { name: string; isDirectory: boolean; isSymbolicLink: boolean }[] @@ -105,9 +122,10 @@ export default class BrowseDirectoryPicker extends DirectoryPicker { } private async createDirectory(path: string, name: string): Promise { - // Same absolute-path fence as list: never rebase a parent under the cwd. - if (!isAbsolute(path)) { - throw new DirectoryPickerError('directory-create-failed', path, `cannot create under "${path}": not an absolute parent path`) + // Same fully-qualified fence as list: never rebase a parent under the + // cwd or the current drive. + if (!fullyQualified(path)) { + throw new DirectoryPickerError('directory-create-failed', path, `cannot create under "${path}": not a fully qualified parent path`) } const parent = resolve(path) // The backend owns segment validation (the wire schema also refuses these, diff --git a/packages/host/directory-picker-browse/tests/service.spec.ts b/packages/host/directory-picker-browse/tests/service.spec.ts index 608fe89348..4cf16107f8 100644 --- a/packages/host/directory-picker-browse/tests/service.spec.ts +++ b/packages/host/directory-picker-browse/tests/service.spec.ts @@ -7,7 +7,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { Context } from 'cordis' import { DirectoryPickerError } from '@deepseek-ai/dsh-host-directory-picker' import type { DirectoryPickerBrowseCapability } from '@deepseek-ai/dsh-host-directory-picker' -import BrowseDirectoryPicker from '../src/index.ts' +import BrowseDirectoryPicker, { fullyQualified } from '../src/index.ts' let root: string let capability: DirectoryPickerBrowseCapability @@ -70,6 +70,19 @@ describe('BrowseDirectoryPicker', () => { expect((failure as DirectoryPickerError).path).toBe(missing) }) + it('classifies fully qualified paths per platform (drive-less rooted Windows forms rejected)', () => { + expect(fullyQualified('/home/x', 'linux')).toBe(true) + expect(fullyQualified('x/y', 'darwin')).toBe(false) + expect(fullyQualified('C:\\projects', 'win32')).toBe(true) + expect(fullyQualified('C:/projects', 'win32')).toBe(true) + expect(fullyQualified('\\\\server\\share', 'win32')).toBe(true) + // Rooted but drive-less: isAbsolute accepts these, yet resolve() would + // inject the process's current drive. + expect(fullyQualified('\\foo', 'win32')).toBe(false) + expect(fullyQualified('/foo', 'win32')).toBe(false) + expect(fullyQualified('C:relative', 'win32')).toBe(false) + }) + it('rejects non-absolute paths instead of rebasing them under the process cwd', async () => { for (const relative of ['', 'projects', './projects', '..']) { const listFailure = await capability.list(relative).catch((error: unknown) => error) diff --git a/packages/host/directory-picker/src/index.ts b/packages/host/directory-picker/src/index.ts index 1c4f5e32fb..77a63bb568 100644 --- a/packages/host/directory-picker/src/index.ts +++ b/packages/host/directory-picker/src/index.ts @@ -60,8 +60,9 @@ export interface DirectoryPickerBrowseCapability { * List one directory level. * @param path - absolute directory to list; absent lists the home directory. * @returns the level's listing with ancestry. - * @throws {DirectoryPickerError} `directory-unreadable` when the target is not absolute - * (a wire value must never rebase under the host cwd) or cannot be listed. + * @throws {DirectoryPickerError} `directory-unreadable` when the target is not fully + * qualified (a wire value must never resolve against the host cwd or, on + * Windows, its current drive) or cannot be listed. */ list(path?: string): Promise /** @@ -70,7 +71,7 @@ export interface DirectoryPickerBrowseCapability { * @param name - single non-blank path segment (no separators, not `.`/`..`). * @returns the created directory's absolute path. * @throws {DirectoryPickerError} `directory-exists` for an existing child, - * `directory-create-failed` for a non-absolute parent or any other failure. + * `directory-create-failed` for a parent that is not fully qualified or any other failure. */ createDirectory(path: string, name: string): Promise }