fix(fs): translate listDir metadata failures

This commit is contained in:
Yichen Jiang
2026-07-03 15:12:40 +08:00
parent 803ed4bd95
commit 6fc2cee837
11 changed files with 144 additions and 33 deletions

View File

@@ -15,7 +15,7 @@ await ctx.plugin(LocalFileSystem, { cwd: process.cwd() })
- **`resolve(path, opts?)`** — a relative `path` resolves against `opts.cwd` when the caller supplies one (the model-facing tools pass the calling agent's session cwd — see [the per-session cwd RFC](../../../docs/rfc/implemented/architecture/2026-07-02-fs-per-session-cwd.md)), else `config.cwd` (default `process.cwd()`); an absolute `path` ignores both. The `targetKey` is the file's `realpath`, so two input paths reaching the same file through symlinks share one identity, and writes/edits land on the link target (preserving the link). A not-yet-existing path uses the realpathed parent directory plus basename when the parent exists; only an unresolvable parent falls back to the absolute path. `displayPath` is the absolute (un-resolved) path.
- **`stat`** — returns `FsInfo` (`version` = `mtimeMs:size`, `type` of `file`/`directory`/`other`, byte `size`) or `undefined` when the target is absent.
- **`readText` / `streamText`** — UTF-8 only. `readText` reads the whole file; `streamText` streams it in chunks (cross-chunk decoding) so a huge file never has to be held whole in memory. Both reject invalid UTF-8 and NUL-byte binary samples (`FS_NOT_TEXT`) and non-regular targets. The `read` tool (`@deepseek-ai/dsh-tool-fs`) decides which to call by size and owns the line windowing.
- **`listDir`** — lists one directory level in stable `name.localeCompare()` order. Each entry carries the child basename, type, resolved child target (`displayPath` under the listed directory, `targetKey` as the realpath identity), and cheap stat metadata (`version`, plus `size` for regular files). It never opens or decodes file contents. Missing targets report `FS_NOT_FOUND`, file/special-file targets report `FS_NOT_DIRECTORY`, aborted calls report `FS_ABORTED`, permission failures report `FS_PERMISSION_DENIED`, and other `readdir` I/O failures report `FS_IO_ERROR`.
- **`listDir`** — lists one directory level in stable `name.localeCompare()` order. Each entry carries the child basename, type, resolved child target (`displayPath` under the listed directory, `targetKey` as the realpath identity), and cheap stat metadata (`version`, plus `size` for regular files). It never opens or decodes file contents. Missing targets report `FS_NOT_FOUND`, file/special-file targets report `FS_NOT_DIRECTORY`, aborted calls report `FS_ABORTED`, permission failures report `FS_PERMISSION_DENIED`, and other listing or child metadata I/O failures report `FS_IO_ERROR`. Broken/disappeared children are returned as `other` without metadata, but permission/IO failures while resolving a child fail the whole listing with a structured `FsError`.
- **`writeText`** — atomic: writes to a temp file opened exclusively (`wx`, `0o600`) inside a randomly-named private staging dir (`0o700`) next to the target, fsyncs, then renames over the target. An existing file's mode is preserved, while new files default to `0o600`. The `expected` guard is OPTIONAL: omitting it unconditionally creates-or-overwrites; `createIfAbsent` creates a missing target and rejects an existing one (`FS_NOT_OBSERVED`); `replaceIfVersion` replaces only at the observed version (a missing target or mismatch is `FS_STALE_VERSION`).
- **`editText`** — atomic literal read-modify-write over the same primitive, serialized per target by a mutation lock. The `expected` guard is OPTIONAL: when supplied it verifies the version BEFORE literal matching (a stale edit reports `FS_STALE_VERSION`, never `FS_EDIT_NOT_FOUND`/`FS_AMBIGUOUS_EDIT` against newer content); omitting it edits the current content unconditionally. A missing target reports `FS_STALE_VERSION` either way. LF-normalizes for matching, restores the file's dominant CRLF/LF style, and rejects empty `oldString` / zero matches (`FS_EDIT_NOT_FOUND`) or ambiguous multi-matches without `replace_all` (`FS_AMBIGUOUS_EDIT`).

View File

@@ -55,11 +55,9 @@ function errorMessage(error: unknown): string {
}
/* v8 ignore stop */
/* v8 ignore start -- requires permission/kernel failures from readdir after a successful directory stat. */
function isPermissionError(error: unknown): boolean {
return error instanceof Error && 'code' in error && (error.code === 'EACCES' || error.code === 'EPERM')
}
/* v8 ignore stop */
function throwIfAborted(signal: AbortSignal | undefined, verb: string): void {
if (signal?.aborted) throw new FsError(`${verb} aborted`, 'FS_ABORTED')
@@ -189,13 +187,14 @@ export async function probe(absolutePath: string): Promise<PathInfo | null> {
// --- Directory listing ---
/* v8 ignore start -- requires permission/kernel failures from readdir after a successful directory stat. */
function listingIoError(displayPath: string, error: unknown): FsError {
/* v8 ignore next -- defensive pass-through for races where a child resolver has already produced a structured FsError. */
if (error instanceof FsError) return error
/* v8 ignore next -- requires the listed target/parent to disappear between successful preflight and listing/child resolution. */
if (isENOENT(error) || isENOTDIR(error)) return new FsError(`cannot list "${displayPath}": not found`, 'FS_NOT_FOUND', { cause: error })
if (isPermissionError(error)) return new FsError(`cannot list "${displayPath}": permission denied`, 'FS_PERMISSION_DENIED', { cause: error })
return new FsError(`cannot list "${displayPath}": ${errorMessage(error)}`, 'FS_IO_ERROR', { cause: error })
}
/* v8 ignore stop */
/**
* List direct children of a directory in stable name order. Each child includes
@@ -204,7 +203,12 @@ function listingIoError(displayPath: string, error: unknown): FsError {
*/
export async function listDirectory(target: LocalTarget, signal?: AbortSignal): Promise<LocalDirEntry[]> {
throwIfAborted(signal, 'list')
const info = await probe(target.targetKey)
let info: PathInfo | null
try {
info = await probe(target.targetKey)
} catch (error: unknown) {
throw listingIoError(target.displayPath, error)
}
if (!info) throw new FsError(`cannot list "${target.displayPath}": not found`, 'FS_NOT_FOUND')
if (info.type !== 'directory') throw new FsError(`cannot list "${target.displayPath}": not a directory`, 'FS_NOT_DIRECTORY')
@@ -217,19 +221,25 @@ export async function listDirectory(target: LocalTarget, signal?: AbortSignal):
}
throwIfAborted(signal, 'list')
return await Promise.all(entries
.sort((left, right) => left.name.localeCompare(right.name))
.map(async (entry): Promise<LocalDirEntry> => {
const result: LocalDirEntry[] = []
for (const entry of entries.sort((left, right) => left.name.localeCompare(right.name))) {
throwIfAborted(signal, 'list')
try {
const childTarget = await resolveLocalTarget(target.displayPath, entry.name)
const childInfo = await probe(childTarget.targetKey)
return {
result.push({
name: entry.name,
type: childInfo?.type ?? 'other',
target: childTarget,
...(childInfo ? { version: childInfo.version } : {}),
...(childInfo?.type === 'file' ? { size: childInfo.size } : {}),
}
}))
})
} catch (error: unknown) {
throw listingIoError(join(target.displayPath, entry.name), error)
}
throwIfAborted(signal, 'list')
}
return result
}
// --- Reading ---

View File

@@ -125,7 +125,7 @@ export class LocalFileSystem extends FileSystem {
return entries.map(entry => ({
name: entry.name,
type: entry.type,
target: { inputPath: entry.target.displayPath, targetKey: entry.target.targetKey, displayPath: entry.target.displayPath },
target: { inputPath: entry.name, targetKey: entry.target.targetKey, displayPath: entry.target.displayPath },
...(entry.version !== undefined ? { version: entry.version } : {}),
...(entry.size !== undefined ? { size: entry.size } : {}),
}))

View File

@@ -138,6 +138,12 @@ describe('listDir', () => {
join(dir, 'skills', 'dir-skill'),
join(dir, 'skills', 'zeta.md'),
])
expect(entries.map(entry => entry.target.inputPath)).toEqual([
'alpha.md',
'broken-link',
'dir-skill',
'zeta.md',
])
const materializedEntries = entries.filter(entry => entry.version !== undefined)
expect(materializedEntries.map(entry => entry.target.targetKey))
.toEqual(await Promise.all(materializedEntries.map(entry => realpath(entry.target.displayPath))))

View File

@@ -6,7 +6,7 @@
*/
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { mkdtemp, readFile, rm, stat, symlink, writeFile, mkdir, readdir, realpath } from 'node:fs/promises'
import { chmod, mkdtemp, readFile, rm, stat, symlink, writeFile, mkdir, readdir, realpath } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { createServer } from 'node:net'
@@ -174,6 +174,54 @@ describe('listDirectory', () => {
await expect(listDirectory(localTarget(file))).rejects.toMatchObject({ code: 'FS_NOT_DIRECTORY' })
await expect(listDirectory(localTarget(dir), AbortSignal.abort())).rejects.toMatchObject({ code: 'FS_ABORTED' })
})
it('translates directory permission failures into FS_PERMISSION_DENIED', async () => {
const root = join(dir, 'restricted')
await mkdir(root)
await chmod(root, 0o000)
try {
const error = await listDirectory(localTarget(root)).then(() => undefined, (caught: unknown) => caught)
// Root-like environments may still be able to list mode-000 directories.
if (error === undefined) return
expect(error).toBeInstanceOf(FsError)
expect(error).toMatchObject({ code: 'FS_PERMISSION_DENIED' })
} finally {
await chmod(root, 0o700)
}
})
it('translates preflight metadata IO failures into FS_IO_ERROR', async () => {
const loop = join(dir, 'loop')
await symlink(loop, loop)
await expect(listDirectory(localTarget(loop))).rejects.toMatchObject({ code: 'FS_IO_ERROR' })
})
it('translates child resolution failures into structured listing errors', async () => {
const root = join(dir, 'listed')
await mkdir(root)
const loop = join(root, 'loop')
await symlink(loop, loop)
await expect(listDirectory(localTarget(root))).rejects.toMatchObject({ code: 'FS_IO_ERROR' })
})
it('translates child permission failures into FS_PERMISSION_DENIED', async () => {
const root = join(dir, 'listed')
const protectedRoot = join(dir, 'protected')
const secret = join(protectedRoot, 'secret')
await mkdir(root)
await mkdir(secret, { recursive: true })
await symlink(secret, join(root, 'secret-link'))
await chmod(protectedRoot, 0o000)
try {
const error = await listDirectory(localTarget(root)).then(() => undefined, (caught: unknown) => caught)
// Root-like environments may still resolve through mode-000 directories.
if (error === undefined) return
expect(error).toBeInstanceOf(FsError)
expect(error).toMatchObject({ code: 'FS_PERMISSION_DENIED' })
} finally {
await chmod(protectedRoot, 0o700)
}
})
})
describe('readWholeText', () => {

View File

@@ -23,7 +23,7 @@ A backend subclasses `FileSystem` and implements seven primitives.
| `stat(target, signal?)` | Return `FsInfo` metadata (`version`, `type`, optional `size`), or `undefined` when the target is absent. Never content. |
| `readText(target, signal?)` | Read the whole regular text file as one decoded string. Owns regular-file checks, UTF-8 decoding, binary/NUL rejection (`FS_NOT_TEXT`). |
| `streamText(target, signal?)` | Stream the same text as decoded chunks for large files (cross-chunk UTF-8 decoding stays here). |
| `listDir(target, signal?)` | List direct directory children in stable name order. Returns entry names, entry types, resolved child targets, and cheap metadata (`version`/file `size` when available); never reads file contents. Missing targets throw `FS_NOT_FOUND`, non-directories throw `FS_NOT_DIRECTORY`, permission failures throw `FS_PERMISSION_DENIED`, and other backend I/O failures throw `FS_IO_ERROR`. |
| `listDir(target, signal?)` | List direct directory children in stable name order. Returns entry names, entry types, resolved child targets, and cheap metadata (`version`/file `size` when available); never reads file contents. Missing targets throw `FS_NOT_FOUND`, non-directories throw `FS_NOT_DIRECTORY`, permission failures throw `FS_PERMISSION_DENIED`, and other backend I/O failures throw `FS_IO_ERROR`. Broken/disappeared children may be returned as `other` without metadata; child permission/IO failures fail the whole listing with the same structured codes. |
| `writeText(target, content, expected?, signal?)` | Atomic create/replace. `expected` is OPTIONAL: omit ⇒ unconditional create-or-overwrite; supply an `FsWriteIntent` (`createIfAbsent`/`replaceIfVersion`) to guard. |
| `editText(target, edit, expected?, signal?)` | Literal edit. `expected` is OPTIONAL: omit ⇒ unconditional edit of the current content; supply `{ version }` to guard (verified BEFORE matching). A missing target reports `FS_STALE_VERSION` either way. Applies and writes atomically — one mutation critical section. |