fix(host): review round 4 — post-commit refocus covers right-pane picks; combobox semantics recorded as deferred

This commit is contained in:
creatixchu
2026-07-29 15:26:00 +08:00
parent 355d505b89
commit 9c9c32ed59
6 files changed
+56 -20

No files matched your search

@@ -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: f633cd60b32c10e63814bf06a06773d2dba396f2
2026-07-28-directory-picker-capability-seam.zh.md: b708e6fffa46cf51fd35154bfc2b947d955e5e9c
2026-07-28-directory-picker-capability-seam.md: 550d51cf2e4e5b70b0844401d42bfea2342612e7
2026-07-28-directory-picker-capability-seam.zh.md: afaf277e866349b20d370fe1fd638b0c71e1a3e7
@@ -19,7 +19,7 @@ Placement and policy rulings folded into this decision:
- **Not the `ctx.fs` seam.** `packages/fs/` is the model/session-facing storage stack (policy events, sandbox-swappable backends). Riding it would couple GUI browsing to the model's confinement backend — swapping `fs-sandbox` for the model must never change GUI behavior — and OS facts (home anchoring, hidden conventions) are not storage primitives. The picker seam stays presentation-free and model-free; `packages/host/` is its consumer-domain home.
- **Dependency survey (hand-roll vs adopt).** Node's stdlib *is* the maintained cross-platform OS layer (`readdir(withFileTypes)`, `homedir`, path semantics); surveyed alternatives fail the dependency bar — file-manager packages (`node-file-manager`, `files-and-folders`, Syncfusion's provider) are whole HTTP apps (fit), drive-letter helpers (`drivelist` native addon, `windows-drive-letters` ~7y stale) fail health/proportionality. The browse backend is a thin adapter over stdlib.
- **Hidden entries: return-and-flag.** The host stamps `hidden` (POSIX dot convention) and returns everything; the client filters. Display policy stays client-side, and the show-hidden toggle shipped as exactly that client-only change: a fixed-label footer toggle whose state lives in the pressed presentation (`aria-pressed` + check glyph), a dot-led path-draft prefix reveals the hidden entries it names, and the current selection is exempt from both the hidden and the prefix filter (it anchors the two-pane view). Windows' `FILE_ATTRIBUTE_HIDDEN` is not exposed by dirents — documented limitation until a native probe pays for itself.
- **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 inferred from `listing.home`; the wire-field alternative below records the deferred authoritative form.
- **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 inferred from `listing.home`; 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.
- **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.
- **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.
@@ -19,7 +19,7 @@ web GUI 的"打开本地文件夹"流程被焊死在一种交互上:`host.pick
- **不用 `ctx.fs` seam。** `packages/fs/` 是面向模型/会话的存储栈(policy 事件、sandbox 可换后端)。骑上去会把 GUI 浏览耦合进模型的限制后端——为模型换 `fs-sandbox` 绝不能改变 GUI 行为——而 OS 事实(home 锚定、隐藏约定)也不是存储原语。picker seam 保持无展示、无模型;`packages/host/` 是它消费方域的家。
- **依赖调研(手写 vs 引入)。** Node 标准库本身就是维护中的跨平台 OS 层(`readdir(withFileTypes)``homedir`、路径语义);调研过的替代品都过不了依赖门槛——文件管理器包(`node-file-manager``files-and-folders`、Syncfusion 的 provider)是整套 HTTP 应用(契合度不过),盘符工具(原生插件 `drivelist`、约七年未更的 `windows-drive-letters`)健康度/比例失当。browse 后端是标准库上的薄适配。
- **隐藏条目:返回并打标。** 宿主标注 `hidden`(POSIX 点前缀约定)并返回全部条目;客户端过滤。展示策略留在客户端,"显示隐藏"开关正是作为这一纯客户端改动落地:标签固定的 footer 开关,其状态由按下态呈现承载(`aria-pressed` + 勾选符号);以点开头的路径草稿前缀会显出它所指名的隐藏条目;当前选中项则不受隐藏与前缀两种过滤影响(它锚定着双栏视图)。Windows 的 `FILE_ATTRIBUTE_HIDDEN` 不被 dirent 暴露——记为限制,直到原生探测值回其成本。
- **路径编辑器的取消范围:对话框卡片。** browse 客户端的路径编辑器在按 Escape 与焦点离开卡片时取消,两者都在卡片范围的包装层而非输入框上监听——Tab 把焦点停到某个过滤命中的行之后,输入框已不在事件路径上,但 Escape 仍须收起编辑器(而非对话框),其后的焦点离开也仍须取消。不取消的豁免:窗口/标签页失焦、卡片内焦点移动,以及指针路径(编辑期间行与开关在 mousedown 时抑制焦点夺取)。预填与草稿末段过滤所用的分隔符从 `listing.home` 推断;下文的线上字段替代方案记录了被延期的权威形态。
- **路径编辑器的取消范围:对话框卡片。** browse 客户端的路径编辑器在按 Escape 与焦点离开卡片时取消,两者都在卡片范围的包装层而非输入框上监听——Tab 把焦点停到某个过滤命中的行之后,输入框已不在事件路径上,但 Escape 仍须收起编辑器(而非对话框),其后的焦点离开也仍须取消。不取消的豁免:窗口/标签页失焦、卡片内焦点移动,以及指针路径(编辑期间行与开关在 mousedown 时抑制焦点夺取)。预填与草稿末段过滤所用的分隔符从 `listing.home` 推断;下文的线上字段替代方案记录了被延期的权威形态。编辑器与其过滤的列表之间的 combobox 语义(`aria-expanded``aria-controls`active-descendant、结果播报)同样被延期——目前二者在辅助技术看来是彼此独立的控件。
- **符号链接:为可进入性而跟随。** 用 `stat` 探测符号链接(断链/循环→跳过);面包屑保留操作者导航的逻辑路径,`workspace.create` 在接纳时本就做 realpath 规范化。
- **列举层级有上限,且流式处理。** 单次 `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 范围而非边界;没有消费方的可配置性过不了证据门槛。等到有部署需要再做。
@@ -15,6 +15,13 @@
gap: 0;
}
/* Card-scope wrapper hosting the path editor's Escape and focus-leave
* observers; display:contents keeps header/content/footer as direct flex
* children of the Modal card. */
.editorScope {
display: contents;
}
/* Header block: pl24 pr14 pt16 pb8, 8px between title row and crumb row. */
.header {
display: flex;
@@ -284,13 +291,6 @@
color: var(--dsw-alias-label-primary);
}
/* Card-scope wrapper hosting the path editor's Escape and focus-leave
* observers; display:contents keeps header/content/footer as direct flex
* children of the Modal card. */
.editorScope {
display: contents;
}
.footerGap {
flex: 1 1 0;
}
@@ -132,13 +132,11 @@ function LevelColumn({ entries, selectedPath, busy, onPick, showHidden, filterPr
// where the blur lands before our guards) drop this click.
// Outside editing, rows keep native focus behavior.
onMouseDown={pathEditing ? (event) => { event.preventDefault() } : undefined}
onClick={(event) => {
// A pick during editing is about to unmount the focused
// input; park focus on the picked row so keyboard traversal
// stays inside the dialog (the Modal has no focus trap).
if (pathEditing) event.currentTarget.focus()
onPick(entry)
}}
// Editing-time focus parking happens after commit (the
// DirectoryBrowser refocus effect): a right-pane pick replaces
// this very column, so focusing the clicked node here would
// still fall to body.
onClick={() => { onPick(entry) }}
>
{selected
? <IconFolderOpen16 size={16} className={css.rowIconSelected} />
@@ -235,11 +233,18 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
})
}, [launchListing])
// An editing-time pick parks focus on the selection after commit; the
// flag is set by select() and consumed by the refocus effect below the
// miller-row ref.
const refocusPick = useRef(false)
/** Select a row of the listed level and preview its children on the right. */
const select = useCallback((entry: DirectoryEntry) => {
const { seq, scan } = launchListing(entry.path)
// A pick while the path editor is open adopts the (filtered) row and
// closes the editor — the draft served its purpose.
// closes the editor — the draft served its purpose. Focus re-parks on
// the selection after commit (see the refocus effect below).
if (pathDraft !== null) refocusPick.current = true
setPathDraft(null)
setSelected(entry)
setChild(null)
@@ -257,7 +262,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
// breadcrumb still names the level: fall back to the single pane.
setSelected(null)
})
}, [launchListing])
}, [launchListing, pathDraft])
/** Abandon path editing (Escape or clicking away) and restore the crumb view. */
const cancelPathEdit = useCallback(() => {
@@ -368,6 +373,18 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
const row = millerRowRef.current
if (row !== null && childPath !== undefined) row.scrollLeft = row.scrollWidth
}, [childPath])
// An editing-time pick unmounts the focused input, and a right-pane pick
// additionally replaces the picked button's whole column (advance swaps
// both panes): park focus on the selection's row — aria-current in the
// freshly rendered left pane — after commit, so keyboard traversal stays
// inside the dialog (the Modal has no focus trap).
useEffect(() => {
if (!refocusPick.current) return
refocusPick.current = false
/* v8 ignore next 2 -- narrowing guard: the pick that set the flag just rendered its aria-current row inside the miller row. */
const row = millerRowRef.current?.querySelector<HTMLButtonElement>('button[aria-current="true"]')
row?.focus()
})
if (!open) return null
const twoPane = selected !== null
@@ -369,6 +369,25 @@ describe('DirectoryBrowser', () => {
expect(screen.getByRole('button', { name: 'browser.home' })).toBeTruthy()
})
it('a right-pane pick while editing parks focus on the advanced selection', async () => {
mount()
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
fireEvent.click(rowButton(screen.getByRole('listitem')))
await waitFor(() => { expect(columns()).toHaveLength(2) })
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
const input = screen.getByLabelText<HTMLInputElement>('browser.editPath')
fireEvent.change(input, { target: { value: `${DOCS}/h` } })
// The advance replaces BOTH panes (the picked button's own column
// unmounts), so focus is re-parked on the selection's aria-current row
// in the freshly rendered left pane rather than the clicked node.
const row = rowButton(within(columns()[1]!).getByRole('listitem'))
fireEvent.mouseDown(row)
fireEvent.click(row)
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
await waitFor(() => { expect(document.activeElement?.textContent).toBe('harness') })
expect(document.activeElement?.getAttribute('aria-current')).toBe('true')
})
it('seeds and filters with backslashes on a Windows-rooted listing', async () => {
const ROOT = 'C:\\'
const windowsListing: DirectoryListing = {