From 7d07ab0c9dc2983c7476d7648b30289dfaee6a38 Mon Sep 17 00:00:00 2001 From: creatixchu Date: Wed, 29 Jul 2026 05:32:12 +0800 Subject: [PATCH] fix(host): abandon close behind a stalled read; race symlink probes; observe cleanup failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The aborted exit no longer awaits close (Node queues it behind any in-flight read, chaining the departed caller back onto the very stall the abort escaped) — the abandoned close's failure is swallowed, it has no consumer. Symlink stat probes race the signal too, with a per-candidate abort check between probes, so a stalled probe target cannot keep a departed request alive. The deferred handle cleanup after a lost opendir race now consumes its own close failure instead of leaking it as an unhandled rejection. --- ...directory-picker-capability-seam.i18n.yaml | 4 +- ...-07-28-directory-picker-capability-seam.md | 2 +- ...-28-directory-picker-capability-seam.zh.md | 2 +- docs/config-catalog.md | 2 +- .../host/directory-picker-browse/src/index.ts | 42 +++++++++++++++---- .../tests/service.spec.ts | 7 +++- 6 files changed, 44 insertions(+), 15 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 5696f35720..cd1441f46f 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: 9d885abe394cfd30a174f07f6ac9432b774188b6 -2026-07-28-directory-picker-capability-seam.zh.md: 6dd1b66b508f5f06d6a150b6c45f0da7d36bf2ce +2026-07-28-directory-picker-capability-seam.md: 9f2703b7499870bf2d5ff8735e3c8cb2c351a503 +2026-07-28-directory-picker-capability-seam.zh.md: 946156a5d06cea30c833c768147b36b30f258192 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 9d885abe39..9f2703b749 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 @@ -20,7 +20,7 @@ Placement and policy rulings folded into this decision: - **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 planned show-hidden toggle becomes a client-only change. Windows' `FILE_ATTRIBUTE_HIDDEN` is not exposed by dirents — documented limitation until a native probe pays for itself. - **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. An unbounded level is a memory/responsiveness hole for large or adversarial directories. +- **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 6dd1b66b50..946156a5d0 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 @@ -20,7 +20,7 @@ web GUI 的"打开本地文件夹"流程被焊死在一种交互上：`host.pick - **依赖调研（手写 vs 引入）。** Node 标准库本身就是维护中的跨平台 OS 层（`readdir(withFileTypes)`、`homedir`、路径语义）；调研过的替代品都过不了依赖门槛——文件管理器包（`node-file-manager`、`files-and-folders`、Syncfusion 的 provider）是整套 HTTP 应用（契合度不过），盘符工具（原生插件 `drivelist`、约七年未更的 `windows-drive-letters`）健康度／比例失当。browse 后端是标准库上的薄适配。 - **隐藏条目：返回并打标。** 宿主标注 `hidden`（POSIX 点前缀约定）并返回全部条目；客户端过滤。展示策略留在客户端，计划中的"显示隐藏"开关变成纯客户端改动。Windows 的 `FILE_ATTRIBUTE_HIDDEN` 不被 dirent 暴露——记为限制，直到原生探测值回其成本。 - **符号链接：为可进入性而跟随。** 用 `stat` 探测符号链接（断链／循环→跳过）；面包屑保留操作者导航的逻辑路径，`workspace.create` 在接纳时本就做 realpath 规范化。 -- **列举层级有上限，且流式处理。** 单次 `list` 至多返回 `maxEntries` 行（配置项，默认 1000——GitHub 网页端目录列举的同一上限）。层级经 `opendir` 流入一个按名排序、容量 `maxEntries + 1` 的候选窗口，内存保持 O(maxEntries)，可进入性探测只触及窗口内候选；线上 `DirectoryListing` 携带必填的 `truncated` 标志，让客户端明示不完整而不是静默缺尾。窗口内的断链符号链接不从窗口外回填——发生过驱逐本身已把层级标记为截断。窗口插入为二分查找、满窗尾部单次比较即拒绝（超大层级不能为每个 dirent 付出一次全窗扫描），且 `list(path, signal)` 透传载体的请求信号，滞塞网络目录的扫描不会在调用方断连后继续存活。无上限的层级对超大或恶意构造的目录就是内存／响应性漏洞。 +- **列举层级有上限，且流式处理。** 单次 `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/docs/config-catalog.md b/docs/config-catalog.md index 42eb9c2325..075597d502 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -533,7 +533,7 @@ export interface Config { } ``` -Source: [`packages/host/directory-picker-browse/src/index.ts:170`](../packages/host/directory-picker-browse/src/index.ts) +Source: [`packages/host/directory-picker-browse/src/index.ts:181`](../packages/host/directory-picker-browse/src/index.ts) ## `@deepseek-ai/dsh-host-webserver` diff --git a/packages/host/directory-picker-browse/src/index.ts b/packages/host/directory-picker-browse/src/index.ts index dbb3a03a5b..6fa157ea87 100644 --- a/packages/host/directory-picker-browse/src/index.ts +++ b/packages/host/directory-picker-browse/src/index.ts @@ -138,6 +138,11 @@ function asError(reason: unknown): Error { return reason instanceof Error ? reason : new Error(String(reason)) } +/* v8 ignore start -- a close failure of an abandoned handle has no consumer, and forcing one needs a filesystem torn down mid-request. */ +/** Swallow the close failure of a handle its caller already departed. */ +function swallowCloseFailure(): void {} +/* v8 ignore stop */ + /** 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. */ @@ -149,13 +154,19 @@ function messageOf(error: unknown): string { * non-directories and broken/cyclic links (skipped silently — the browser * shows what can be entered, and a broken link cannot). */ -async function directoryRow(parent: string, name: string, isDirectory: boolean, isSymbolicLink: boolean): Promise { +async function directoryRow( + parent: string, name: string, isDirectory: boolean, isSymbolicLink: boolean, signal: AbortSignal | undefined, +): Promise { const path = join(parent, name) let enterable = isDirectory if (!enterable && isSymbolicLink) { try { - enterable = (await stat(path)).isDirectory() + // The probe races the caller too: a symlink target on a stalled + // network filesystem must not keep a departed caller's request alive. + enterable = (await raceAbort(stat(path), signal)).isDirectory() } catch { + /* v8 ignore next 2 -- an abort landing mid-probe needs a stalled stat; the per-candidate check in list covers the settled path. */ + if (signal?.aborted) throw asError(signal.reason) // Broken or cyclic symlink: stat is the probe, failure means "not enterable". return null } @@ -231,8 +242,10 @@ export default class BrowseDirectoryPicker extends DirectoryPicker { const level = await raceAbort(opening, signal).catch((error: unknown) => { // The abandoned open can still mint a handle after the abort won; // close it so a departed caller cannot leak a descriptor. (A lost - // race against opendir's own rejection has nothing to close.) - void opening.then(async (dir) => { await dir.close() }, () => { + // race against opendir's own rejection has nothing to close, and + // the close's own failure is swallowed — the request already + // returned, so a cleanup error has no consumer.) + void opening.then(dir => dir.close().catch(swallowCloseFailure), () => { // Already rejected: raceAbort surfaced or swallowed it. }) throw error @@ -248,10 +261,18 @@ export default class BrowseDirectoryPicker extends DirectoryPicker { if (boundedInsert(window, candidate, keep)) evicted = true } } finally { - // Manual read() never auto-closes; close on every exit, the aborted - // one included (its abandoned read settles against the closed handle - // and raceAbort already swallowed that settlement). - await level.close() + // Manual read() never auto-closes; close on every exit. The aborted + // exit must not await it — Node queues close behind any in-flight + // read, so awaiting would chain the departed caller back onto the + // very stall the abort escaped (the abandoned read's settlement is + // already swallowed by raceAbort). + const closing = level.close() + /* v8 ignore next 3 -- an abort between open and close needs a stalled read; the abandoned-close arm has no observable outcome. */ + if (signal?.aborted) { + closing.catch(swallowCloseFailure) + } else { + await closing + } } } catch (error: unknown) { // An abort is the caller's own reason, not an unreadable directory. @@ -261,7 +282,10 @@ export default class BrowseDirectoryPicker extends DirectoryPicker { const entries: DirectoryEntry[] = [] let truncated = evicted for (const candidate of window) { - const row = await directoryRow(target, candidate.name, candidate.isDirectory, candidate.isSymbolicLink) + // A caller that departed between reads and probes stops before the + // next probe (each probe's own await is raced inside directoryRow). + signal?.throwIfAborted() + const row = await directoryRow(target, candidate.name, candidate.isDirectory, candidate.isSymbolicLink, signal) if (row === null) continue if (entries.length === this.config.maxEntries) { truncated = true diff --git a/packages/host/directory-picker-browse/tests/service.spec.ts b/packages/host/directory-picker-browse/tests/service.spec.ts index 835948d9fe..002d42e516 100644 --- a/packages/host/directory-picker-browse/tests/service.spec.ts +++ b/packages/host/directory-picker-browse/tests/service.spec.ts @@ -95,8 +95,13 @@ describe('BrowseDirectoryPicker', () => { // own and there is nothing to close. await expect(capability.list(join(root, 'no-such-dir'), gone.signal)).rejects.toThrow('caller left') await new Promise(resolve => setTimeout(resolve, 10)) - // A live signal changes nothing about ordinary failures. + // A live signal leaves a normal listing untouched — the reads and the + // symlink probes race it without ever losing. const live = new AbortController() + const complete = await capability.list(root, live.signal) + expect(complete.truncated).toBe(false) + expect(complete.entries.map(entry => entry.name)).toContain('linked') + // A live signal changes nothing about ordinary failures. const missing = join(root, 'no-such-dir') const failure = await capability.list(missing, live.signal).catch((error: unknown) => error) expect(failure).toBeInstanceOf(DirectoryPickerError)