diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 3e70c22fbc..42eb9c2325 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:127`](../packages/host/directory-picker-browse/src/index.ts) +Source: [`packages/host/directory-picker-browse/src/index.ts:170`](../packages/host/directory-picker-browse/src/index.ts) ## `@deepseek-ai/dsh-host-webserver` diff --git a/packages/host/apiproxy/src/api-proxy.ts b/packages/host/apiproxy/src/api-proxy.ts index d4c4268088..a2b2a887cd 100644 --- a/packages/host/apiproxy/src/api-proxy.ts +++ b/packages/host/apiproxy/src/api-proxy.ts @@ -1090,6 +1090,11 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro // stops the backend's directory scan instead of outliving it. return ok(request, await capability.list(request.payload.path, signal)) } catch (error: unknown) { + // An abort is the caller's own timeout/disconnect, not a server + // failure — same code pickDirectory and command.execute report. + if (signal.aborted) { + return err(request, { code: 'cancelled', message: 'directory listing was aborted', details: {} }) + } return err(request, directoryError(error)) } }, diff --git a/packages/host/apiproxy/tests/api-proxy-workspace.spec.ts b/packages/host/apiproxy/tests/api-proxy-workspace.spec.ts index f04850c34d..3968cba5c7 100644 --- a/packages/host/apiproxy/tests/api-proxy-workspace.spec.ts +++ b/packages/host/apiproxy/tests/api-proxy-workspace.spec.ts @@ -193,6 +193,20 @@ describe('host.listDirectory / host.createDirectory', () => { }) }) + it('reports an aborted listing as cancelled, like the other signal-following RPCs', async () => { + const { api } = await harness(undefined, { + kind: 'browse', + list: (_path, signal) => new Promise((_resolve, reject) => { + signal?.addEventListener('abort', () => { reject(new Error('scan aborted')) }, { once: true }) + }), + createDirectory: async () => '/never', + }) + const abort = new AbortController() + const pending = api.host.listDirectory(request({}), abort.signal) + abort.abort() + expect((await pending).result).toMatchObject({ ok: false, error: { code: 'cancelled' } }) + }) + it('refuses the browse RPCs under a native composition', async () => { const { api } = await harness() expect((await api.host.listDirectory(request({}), new AbortController().signal)).result).toMatchObject({ diff --git a/packages/host/directory-picker-browse/src/index.ts b/packages/host/directory-picker-browse/src/index.ts index be27c5a2d4..dbb3a03a5b 100644 --- a/packages/host/directory-picker-browse/src/index.ts +++ b/packages/host/directory-picker-browse/src/index.ts @@ -95,6 +95,49 @@ export function boundedInsert(window: ListingCandidate[], candidate: ListingCand return true } +/** + * Await `operation`, but reject with the signal's reason the moment it + * aborts. Node's filesystem reads are not retractable, so the operation + * itself keeps running against a handle the caller then closes — its late + * settlement is swallowed here so an abandoned read cannot surface as an + * unhandled rejection. + * @param operation - the in-flight filesystem step. + * @param signal - caller lifetime; absent means plain awaiting. + * @returns the operation's value. + */ +export function raceAbort(operation: Promise, signal: AbortSignal | undefined): Promise { + if (signal === undefined) return operation + return new Promise((resolve, reject) => { + const onAbort = (): void => { + operation.catch(() => { + // Abandoned read: its handle is being closed by the aborting caller, + // and the abort reason already carried the outcome. + }) + reject(asError(signal.reason)) + } + if (signal.aborted) { + onAbort() + return + } + signal.addEventListener('abort', onAbort, { once: true }) + operation.then( + (value) => { + signal.removeEventListener('abort', onAbort) + resolve(value) + }, + (reason: unknown) => { + signal.removeEventListener('abort', onAbort) + reject(asError(reason)) + }, + ) + }) +} + +/** The thrown value as an Error (wire/abort reasons may be anything). */ +function asError(reason: unknown): Error { + return reason instanceof Error ? reason : new Error(String(reason)) +} + /** 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. */ @@ -180,16 +223,35 @@ export default class BrowseDirectoryPicker extends DirectoryPicker { const window: ListingCandidate[] = [] let evicted = false try { - const level = await opendir(target) - for await (const dirent of level) { - // A disconnected/timed-out caller stops the scan here; throwing out - // of the loop closes the directory handle via the iterator's return. - signal?.throwIfAborted() - // Only rows a browser could enter contend for the window; dirent - // says "directory" outright, a symlink needs the later stat probe. - if (!dirent.isDirectory() && !dirent.isSymbolicLink()) continue - const candidate = { name: dirent.name, isDirectory: dirent.isDirectory(), isSymbolicLink: dirent.isSymbolicLink() } - if (boundedInsert(window, candidate, keep)) evicted = true + // Every filesystem await races the caller's signal: a stalled + // opendir/read on a network filesystem must not keep a departed + // caller's scan alive, and an already-aborted request rejects even + // when the level is empty. + const opening = opendir(target) + 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() }, () => { + // Already rejected: raceAbort surfaced or swallowed it. + }) + throw error + }) + try { + for (;;) { + const dirent = await raceAbort(level.read(), signal) + if (dirent === null) break + // Only rows a browser could enter contend for the window; dirent + // says "directory" outright, a symlink needs the later stat probe. + if (!dirent.isDirectory() && !dirent.isSymbolicLink()) continue + const candidate = { name: dirent.name, isDirectory: dirent.isDirectory(), isSymbolicLink: dirent.isSymbolicLink() } + 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() } } catch (error: unknown) { // An abort is the caller's own reason, not an unreadable directory. diff --git a/packages/host/directory-picker-browse/tests/service.spec.ts b/packages/host/directory-picker-browse/tests/service.spec.ts index 99366e8754..835948d9fe 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, { boundedInsert, fullyQualified } from '../src/index.ts' +import BrowseDirectoryPicker, { boundedInsert, fullyQualified, raceAbort } from '../src/index.ts' import type { ListingCandidate } from '../src/index.ts' let root: string @@ -86,8 +86,15 @@ describe('BrowseDirectoryPicker', () => { it('stops the scan with the caller: an aborted signal rejects with its own reason', async () => { const gone = new AbortController() gone.abort(new Error('caller left')) - // The abort surfaces as-is, not dressed as an unreadable directory. + // The abort surfaces as-is, not dressed as an unreadable directory — + // and rejects even before any level row is read. await expect(capability.list(root, gone.signal)).rejects.toThrow('caller left') + // The abandoned open that still succeeds is closed, not leaked. + await new Promise(resolve => setTimeout(resolve, 10)) + // Aborted against a missing target: the abandoned open rejects on its + // 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. const live = new AbortController() const missing = join(root, 'no-such-dir') @@ -96,6 +103,34 @@ describe('BrowseDirectoryPicker', () => { expect((failure as DirectoryPickerError).code).toBe('directory-unreadable') }) + it('raceAbort follows the operation until the signal wins, and swallows the abandoned settlement', async () => { + // No signal / settled operations: plain passthrough, listener removed. + await expect(raceAbort(Promise.resolve('ok'), undefined)).resolves.toBe('ok') + const live = new AbortController() + await expect(raceAbort(Promise.resolve('ok'), live.signal)).resolves.toBe('ok') + // Failure passthrough keeps the operation's own error. + await expect(raceAbort(Promise.reject(new Error('raw failure')), live.signal)).rejects.toThrow('raw failure') + // The abort wins over a pending operation and carries its own reason; + // the operation's late rejection is swallowed, never unhandled. + const rejections: unknown[] = [] + const onUnhandled = (reason: unknown): void => { rejections.push(reason) } + process.on('unhandledRejection', onUnhandled) + try { + let rejectLate!: (reason: unknown) => void + const pending = new Promise((_resolve, reject) => { rejectLate = reject }) + const controller = new AbortController() + const raced = raceAbort(pending, controller.signal) + // A bare-string abort reason exercises the Error wrap. + controller.abort('caller left') + await expect(raced).rejects.toThrow('caller left') + rejectLate(new Error('late read failure')) + await new Promise(resolve => setTimeout(resolve, 10)) + expect(rejections).toEqual([]) + } finally { + process.off('unhandledRejection', onUnhandled) + } + }) + it('boundedInsert keeps the window name-sorted and bounded, reporting evictions', () => { const candidate = (name: string): ListingCandidate => ({ name, isDirectory: true, isSymbolicLink: false }) const window: ListingCandidate[] = []