From de24461b9147b0b40e701ca9d93b71094a471412 Mon Sep 17 00:00:00 2001 From: creatixchu Date: Thu, 30 Jul 2026 02:53:45 +0800 Subject: [PATCH] =?UTF-8?q?fix(host):=20review=20round=2022=20=E2=80=94=20?= =?UTF-8?q?note=20enumeration=20includes=20the=20create=20path;=20cross-me?= =?UTF-8?q?thod=20equality=20pinned;=20NFD-volume=20boundary=20recorded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-07-28-directory-picker-capability-seam.i18n.yaml | 4 ++-- .../2026-07-28-directory-picker-capability-seam.md | 2 +- ...2026-07-28-directory-picker-capability-seam.zh.md | 2 +- packages/client/test-runtime/src/workspaces.ts | 4 +++- .../host/directory-picker-browse/README.i18n.yaml | 4 ++-- packages/host/directory-picker-browse/README.md | 1 + packages/host/directory-picker-browse/README.zh.md | 1 + .../src/client/DirectoryBrowser.tsx | 12 ++++++------ .../directory-picker-browse/tests/home-shape.spec.ts | 1 + .../directory-picker-browse/tests/service.spec.ts | 4 ++++ 10 files changed, 22 insertions(+), 13 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.i18n.yaml index cee7a5f948..bc62880e58 100644 --- a/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.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 .agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md -2026-07-28-directory-picker-capability-seam.md: ebd40b9dbc89630c5cbbfacdcfb8be2b6a65fd49 -2026-07-28-directory-picker-capability-seam.zh.md: 6ba8eb347bf906f05ac6192cf941e76d3bc4f44a +2026-07-28-directory-picker-capability-seam.md: 6768ed8f898765396d6a4549a661674b5336c1f3 +2026-07-28-directory-picker-capability-seam.zh.md: f20ea0af278151cb6f9ebd963185701cc3f3f2fd diff --git a/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md b/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md index ebd40b9dbc..6768ed8f89 100644 --- a/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md +++ b/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.md @@ -22,7 +22,7 @@ Placement and policy rulings folded into this decision: - **Path-editor cancel scope: the dialog card.** The browse client's path editor cancels on Escape and on focus leaving the card, both observed at a card-scope wrapper rather than the input — after Tab parks focus on a filtered row the input is off the event path, yet Escape must collapse the editor (not the dialog) and a later focus departure must still cancel. Non-cancel exemptions: window/tab focus loss, in-card focus moves, and pointer paths (rows and the toggle suppress focus steal on mousedown while editing). Separators for seeding and draft-tail filtering are read from the host-resolved root crumb (exact for every root form this backend emits: `/`, `C:\`, `\\server\share\`); the wire-field alternative below records the deferred authoritative form. Combobox semantics between the editor and the list it filters (`aria-expanded`/`aria-controls`/active-descendant, result announcements) are likewise deferred — today they read to assistive tech as separate widgets. Focus parking is a card-wide invariant, not an editor-only one: every pick — editing or not, including right-pane advances and create landings whose columns are replaced — re-parks focus on the selection's row after commit, while every other displacing exit (Enter, Escape, a navigation landing whose new level dropped the focused row, a failed pick or create relist, and the nested create dialog closing) falls back to the crumb edit zone whenever focus actually fell to body, and a show-hidden toggle click that finds focus among the rows parks synchronously on the toggle itself. The guarantee is scoped to the dialog's own node replacements — the Modal has no focus trap, so tabbing past the card's edge legitimately leaves, and the owner's adopt window (where `busy` inerts every control and the dialog is closing either way) is likewise outside it. - **Navigation lands selection-anchored, progressively.** Away from the display root (the same collapse the crumb header renders, so crumbs and pane shape never disagree), the browse client's navigate commits the target level the moment it arrives — the editor closes and loading ends on that first settlement, so an Enter-submitted navigation is never withdrawn waiting on more — and a parent leg then upgrades the landing in place: the target's actual parent-level entry re-selected (platform case folding on Windows), its children on the right, so a crumb jump reads as stepping back one pane rather than collapsing to a single column. The parent leg runs under the landing's supersession scope and is aborted on the wire by any newer intent; a failed parent leg, or a truncated parent window lacking the target, leaves the committed single-pane landing — the upgrade must never orphan the selection it exists to anchor. Known boundaries of the progressive shape: a pointer press landing exactly inside the one-RTT upgrade window can lose its click when the pressed row node is replaced (keyboard focus is re-parked on the re-selected row; the pointer window is accepted); on slash platforms (macOS) only a final-segment case drift misses the parent-entry match and keeps the single-pane landing, while ancestor-segment drift still matches — parent entry paths inherit the typed prefix — and lands two panes at the cost of the Home collapse; and navigate always relists both legs even when the target is the currently shown level — a crumb tap doubles as the refresh gesture, so freshness wins over reusing possibly-stale in-hand listings at the cost of up to two host scans. - **Symlinks: follow for enterability.** `stat` probes symlinks (broken/cyclic → skipped); crumbs keep the logical path the operator navigated, and `workspace.create` already canonicalizes via realpath at adoption. -- **One canonical path shape on the wire.** A listing's `path`, `crumbs[].path`, `entries[].path`, and `home` all ship host-resolved — homedir() output included, since the environment may decorate HOME and the backend resolves it before stamping. Clients compare listing paths verbatim on that promise; the only lexical mirror left in the browse client serves the draft side, the one path a user types and hence naturally non-canonical. Normalizing at the source replaces a client-side mirror of resolve() that had to anticipate every decoration (trailing and repeated separators, dot segments, UNC roots, forward slashes), and the promise binds every browse backend. +- **One canonical path shape on the wire.** A listing's `path`, `crumbs[].path`, `entries[].path`, and `home` — and `createDirectory`'s returned path, which clients compare verbatim against the child's next `entries[].path` to anchor the create landing — all ship host-resolved: lexical `resolve()`, never realpath (the symlink ruling above keeps ancestries logical), with homedir() output included since the environment may decorate HOME and the backend resolves it before stamping. Clients compare listing paths verbatim on that promise; the only lexical mirror left in the browse client serves the draft side, the one path a user types and hence naturally non-canonical. Normalizing at the source replaces a client-side mirror of resolve() that had to anticipate every decoration (trailing and repeated separators, dot segments, UNC roots, forward slashes), and the promise binds every browse backend. - **Listing levels are bounded, and streamed.** One `list` call returns at most `maxEntries` rows (config, default 1000 — GitHub's web-UI directory-listing bound). The level streams via `opendir` into a name-sorted window of `maxEntries + 1` candidates, so memory stays O(maxEntries) and enterability probing touches only windowed candidates; the wire `DirectoryListing` carries a required `truncated` flag so the client states incompleteness instead of silently missing tail entries. A windowed broken symlink is not backfilled from beyond the window — the eviction already marks the level truncated. Window insertion is binary with an O(1) full-window tail rejection (an oversized level must not pay a window scan per dirent), and `list(path, signal)` threads the carrier's request signal so a scan of a stalled network directory cannot outlive a disconnected caller — every await in the scan (open, each read, each symlink probe) races the signal, an aborted exit abandons rather than awaits the close (Node queues close behind in-flight reads), and abandoned settlements are swallowed so cleanup can never surface as an unhandled rejection. An unbounded level is a memory/responsiveness hole for large or adversarial directories. - **Whole-filesystem scope, no roots config.** `workspace.create` accepts arbitrary paths and the API serves bash-driving methods, so a browse root would be UX scoping, not a boundary; configurability without a consumer fails the evidence bar. Deferred until a deployment needs it. - **The native backend stays.** Plugin-form was the point: multiple providers can serve the seam (an Electron shell would provide the `native` interaction through its own dialog API). Kind naming: `dialog` was the first pick and was dropped — the browse interaction also presents a dialog (the in-app modal), so the word failed to discriminate; `native` names where the chooser runs. diff --git a/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.zh.md b/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.zh.md index 6ba8eb347b..f20ea0af27 100644 --- a/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-28-directory-picker-capability-seam.zh.md @@ -22,7 +22,7 @@ web GUI 的"打开本地文件夹"流程被焊死在一种交互上:`host.pick - **路径编辑器的取消范围:对话框卡片。** browse 客户端的路径编辑器在按 Escape 与焦点离开卡片时取消,两者都在卡片范围的包装层而非输入框上监听——Tab 把焦点停到某个过滤命中的行之后,输入框已不在事件路径上,但 Escape 仍须收起编辑器(而非对话框),其后的焦点离开也仍须取消。不取消的豁免:窗口/标签页失焦、卡片内焦点移动,以及指针路径(编辑期间行与开关在 mousedown 时抑制焦点夺取)。预填与草稿末段过滤所用的分隔符从宿主解析的根 crumb 读取(对该后端发出的每种根形态都精确:`/`、`C:\`、`\\server\share\`);下文的线上字段替代方案记录了被延期的权威形态。编辑器与其过滤的列表之间的 combobox 语义(`aria-expanded`/`aria-controls`/active-descendant、结果播报)同样被延期——目前二者在辅助技术看来是彼此独立的控件。焦点停靠是卡片全域的不变量,而非编辑器独有:每次选取——无论是否处于编辑态,包括右栏推进与各列被替换的创建落地——提交后都把焦点重新停靠到选中项所在的行上,而其余所有会顶离焦点的退出(Enter、Escape、新层级已不含焦点所在行的导航落地、选取或创建失败后的重新列举,以及嵌套创建对话框的关闭)只要焦点确实落到了 body 上,就回落到 crumb 编辑区,而点击"显示隐藏"开关时若发现焦点落在行间,则把焦点同步停靠到开关自身。该保证的范围仅限对话框自身的节点替换——Modal 没有焦点陷阱,所以 Tab 越过卡片边缘属于正当离开,而 owner 的接纳窗口(其间 `busy` 把每个控件置为惰性,且对话框反正正在关闭)同样在此范围之外。 - **导航以选中项为锚、渐进落地。** 在展示根之外(与 crumb 头部渲染的是同一塌缩,因此 crumb 与分栏形态永不相左),browse 客户端的导航在目标层级到达的那一刻即提交它——这次首个落定即关闭编辑器并结束加载,因此 Enter 提交的导航绝不会为等待更多内容而被撤回——随后父层级这一程就地升级这次落地:重新选中目标在父层级中的实际条目(Windows 上按平台惯例折叠大小写),右侧展示其子项,因此 crumb 跳转读作后退一栏,而不是塌缩成单列。父层级这一程在落地的 supersession 范围下运行,任何较新的意图都会在线上将其中止;父层级这一程失败,或被截断的父窗口缺少目标时,都保留已提交的单栏落地——升级的存在正是为了锚定选中项,绝不能反而让它悬空。渐进形态的已知边界:指针按压若恰好落在单个 RTT 的升级窗口内,可能因所按行节点被替换而丢失点击(键盘焦点会重新停靠到重新选中的行上;指针的这段窗口则被接受);斜杠平台(macOS)上仅末段的大小写偏差会错过父层级条目匹配,保留单栏落地,而祖先段的偏差仍能匹配——父层级条目路径继承键入的前缀——并落地双栏,代价是 Home 塌缩;且导航总是重新列举两程,哪怕目标就是当前展示的层级——crumb 点按兼作刷新手势,因此宁要新鲜度也不复用手头可能已陈旧的列举,代价是至多两次宿主扫描。 - **符号链接:为可进入性而跟随。** 用 `stat` 探测符号链接(断链/循环→跳过);面包屑保留操作者导航的逻辑路径,`workspace.create` 在接纳时本就做 realpath 规范化。 -- **线上只有一种规范路径形态。** 列举的 `path`、`crumbs[].path`、`entries[].path` 与 `home` 一律以宿主解析后的形态发出——homedir() 的输出也不例外,因为环境可能修饰 HOME,后端在标注前先行解析。客户端凭这一承诺逐字比较列举路径;browse 客户端仅剩的词法镜像服务于草稿一侧——用户键入的那一条路径,因而天然非规范。在源头做规范化,取代了客户端侧那份必须预判每种修饰(末尾与重复的分隔符、点段、UNC 根、正斜杠)的 resolve() 镜像,且这一承诺约束每一个 browse 后端。 +- **线上只有一种规范路径形态。** 列举的 `path`、`crumbs[].path`、`entries[].path` 与 `home`——连同 `createDirectory` 返回的路径,客户端拿它与该子项下一次的 `entries[].path` 逐字比较以锚定创建落地——一律以宿主解析后的形态发出:词法 `resolve()`,绝不做 realpath(上文的符号链接裁决保持祖先链为逻辑路径);homedir() 的输出也不例外,因为环境可能修饰 HOME,后端在标注前先行解析。客户端凭这一承诺逐字比较列举路径;browse 客户端仅剩的词法镜像服务于草稿一侧——用户键入的那一条路径,因而天然非规范。在源头做规范化,取代了客户端侧那份必须预判每种修饰(末尾与重复的分隔符、点段、UNC 根、正斜杠)的 resolve() 镜像,且这一承诺约束每一个 browse 后端。 - **列举层级有上限,且流式处理。** 单次 `list` 至多返回 `maxEntries` 行(配置项,默认 1000——GitHub 网页端目录列举的同一上限)。层级经 `opendir` 流入一个按名排序、容量 `maxEntries + 1` 的候选窗口,内存保持 O(maxEntries),可进入性探测只触及窗口内候选;线上 `DirectoryListing` 携带必填的 `truncated` 标志,让客户端明示不完整而不是静默缺尾。窗口内的断链符号链接不从窗口外回填——发生过驱逐本身已把层级标记为截断。窗口插入为二分查找、满窗尾部单次比较即拒绝(超大层级不能为每个 dirent 付出一次全窗扫描),且 `list(path, signal)` 透传载体的请求信号,滞塞网络目录的扫描不会在调用方断连后继续存活——扫描中的每个 await(打开、每次读取、每次符号链接探测)都与信号赛跑,中止路径放弃而非等待 close(Node 会把 close 排在在飞读取之后),被放弃的 settlement 全部吞掉,清理不会以未处理拒绝的形式冒出。无上限的层级对超大或恶意构造的目录就是内存/响应性漏洞。 - **全盘可浏览,不做 roots 配置。** `workspace.create` 接受任意路径且 API 本就提供驱动 bash 的方法,浏览根只会是 UX 范围而非边界;没有消费方的可配置性过不了证据门槛。等到有部署需要再做。 - **native 后端保留。** 插件化正是目的:多方都能提供该 seam(Electron 壳可以经自己的对话框 API 提供 `native` 交互)。kind 命名:最初选了 `dialog` 后被放弃——browse 交互同样以对话框呈现(应用内弹窗),这个词起不到判别作用;`native` 命名的是选择器运行的位置。 diff --git a/packages/client/test-runtime/src/workspaces.ts b/packages/client/test-runtime/src/workspaces.ts index 6c1a9d0aad..d24e7b8c8a 100644 --- a/packages/client/test-runtime/src/workspaces.ts +++ b/packages/client/test-runtime/src/workspaces.ts @@ -142,7 +142,9 @@ export class TestWorkspaces implements IWorkspaces { * Browse child creation (recorded). The default joins parent and name. * @param path - absolute existing parent directory. * @param name - single path segment. - * @returns the created directory's absolute path. + * @returns the created directory's absolute path, in the shape + * `DirectoryPickerBrowseCapability.createDirectory` contracts (verbatim + * equal to the child's `entries[].path` in the parent's next listing). */ async createDirectory(path: string, name: string): Promise { this.calls.push({ method: 'createDirectory', args: [path, name] }) diff --git a/packages/host/directory-picker-browse/README.i18n.yaml b/packages/host/directory-picker-browse/README.i18n.yaml index d807bd737a..7d37891821 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: 23153881b84dcb71dfb05d4f297a5818c410ca77 -README.zh.md: d7010e2941a801ba6358082824330eaae46e42b7 +README.md: 01e2b9e5afcfd7c47a3f42a76cc1388a25477334 +README.zh.md: 8cb63048713dc964f92762546de783d2ce6ce5a7 diff --git a/packages/host/directory-picker-browse/README.md b/packages/host/directory-picker-browse/README.md index 23153881b8..01e2b9e5af 100644 --- a/packages/host/directory-picker-browse/README.md +++ b/packages/host/directory-picker-browse/README.md @@ -19,5 +19,6 @@ None; this package neither assembles nor sends a provider request. ## Known Limitations and Deferred Work - **Windows hidden attribute is not read** — Node dirents do not expose `FILE_ATTRIBUTE_HIDDEN`, so `hidden` means dot-prefixed on every platform until a native probe is worth its cost. +- **Name-normalizing volumes void the create-path equality** — `createDirectory` promises its return verbatim-equal to the child's next `entries[].path`; Node's namespaced Win32 paths store even trailing-dot/space segments literally, but a volume that rewrites names on storage (NFD normalization on HFS+-style volumes) breaks the match, and the create landing degrades to the documented single-pane / edit-zone fallback. - **No drive-root enumeration** — on Windows the ancestry stops at the drive root; crossing drives waits for the browser UI's path-entry affordance rather than an enumeration primitive here. - **Whole-filesystem scope** — no per-deployment browse-root restriction; `workspace.create` accepts arbitrary paths today, so a root here would be UX scoping, not a boundary — deferred until a deployment needs it. diff --git a/packages/host/directory-picker-browse/README.zh.md b/packages/host/directory-picker-browse/README.zh.md index d7010e2941..8cb6304871 100644 --- a/packages/host/directory-picker-browse/README.zh.md +++ b/packages/host/directory-picker-browse/README.zh.md @@ -19,5 +19,6 @@ ## 已知限制与延期工作 - **不读取 Windows 隐藏属性**——Node 的 dirent 不暴露 `FILE_ATTRIBUTE_HIDDEN`,因此在所有平台上 `hidden` 都意味着点前缀,直到原生探测值回其成本为止。 +- **名称规范化的卷会使创建路径等式失效**——`createDirectory` 承诺其返回值与该子项下一次的 `entries[].path` 逐字相等;Node 带命名空间的 Win32 路径连末尾点/空格段都按字面存储,但在存储时改写名称的卷(HFS+ 风格卷上的 NFD 规范化)会破坏这一匹配,创建落地随之退化为文档所述的单栏/编辑区回退。 - **不枚举盘符根**——Windows 上祖先链止于盘符根;跨盘依赖浏览器 UI 的路径输入入口,而不是这里的枚举原语。 - **全盘可浏览**——没有按部署限定的浏览根;`workspace.create` 今天就接受任意路径,这里的根只会是 UX 范围而非边界——等到有部署需要时再做。 diff --git a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx index 7614d77f50..3fff931685 100644 --- a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx +++ b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx @@ -85,12 +85,12 @@ function foldSeparatorsFor(sep: '\\' | '/'): (value: string) => string { * Lexically normalizes a typed absolute path for comparisons against the * backend's resolved ones (every listing path arrives in the * DirectoryListing contract's canonical shape; only the DRAFT side, the - * one path a user types, 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. + * one path a user types, 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 { return (value) => { diff --git a/packages/host/directory-picker-browse/tests/home-shape.spec.ts b/packages/host/directory-picker-browse/tests/home-shape.spec.ts index 569a631388..b0ae26a921 100644 --- a/packages/host/directory-picker-browse/tests/home-shape.spec.ts +++ b/packages/host/directory-picker-browse/tests/home-shape.spec.ts @@ -38,5 +38,6 @@ it('resolves a decorated homedir before stamping listing.home', async () => { const listing = await picked.list() expect(listing.home).toBe(resolve(scratch)) expect(listing.path).toBe(listing.home) + expect(listing.crumbs.at(-1)!.path).toBe(listing.home) await fiber.dispose() }) diff --git a/packages/host/directory-picker-browse/tests/service.spec.ts b/packages/host/directory-picker-browse/tests/service.spec.ts index 0ace7811f1..cfb65af627 100644 --- a/packages/host/directory-picker-browse/tests/service.spec.ts +++ b/packages/host/directory-picker-browse/tests/service.spec.ts @@ -207,6 +207,10 @@ describe('BrowseDirectoryPicker', () => { expect(created).toBe(join(root, 'fresh')) const listing = await capability.list(root) expect(listing.entries.map(entry => entry.name)).toContain('fresh') + // The contract's cross-method equality: the returned path is verbatim + // the child's entries[].path (clients anchor the create landing's + // selection and focus on it). + expect(listing.entries.find(entry => entry.name === 'fresh')!.path).toBe(created) }) it('refuses an existing child with directory-exists', async () => {