diff --git a/docs/rfc/implemented/architecture/2026-06-17-filesystem-capability-seam.md b/docs/rfc/implemented/architecture/2026-06-17-filesystem-capability-seam.md index 55f5efe14b..4faae84d83 100644 --- a/docs/rfc/implemented/architecture/2026-06-17-filesystem-capability-seam.md +++ b/docs/rfc/implemented/architecture/2026-06-17-filesystem-capability-seam.md @@ -121,21 +121,22 @@ The root plugin registers the full suite by composing the per-tool registration ## Migration plan -This RFC starts from `origin/master`, where no filesystem tool package exists yet. The final implementation should add the new three-package topology directly: +This RFC starts from `origin/master`, where no filesystem tool package exists yet. The landed implementation adds the new three-package topology directly: 1. Add `packages/fs/fs` with the `ctx.fs` abstract service and vocabulary types. 2. Add `packages/fs/fs-local` with the local backend implementation and backend-level tests. 3. Add `packages/fs/tool-fs` with the model-facing `read`, `write`, and `edit` tools over `ctx.fs`. -4. Wire examples by loading a `ctx.fs` provider first (`dsh-fs-local`), then the consumer (`dsh-tool-fs` or one of its subpath plugins). -5. Update `docs/architecture.md`, `packages/README.md`, package READMEs, build/typecheck config, and aggregate maintenance scripts such as `scripts/publint-all.ts`. +4. Update `docs/architecture.md`, `packages/README.md`, package READMEs, build/typecheck config, and aggregate maintenance scripts such as `scripts/publint-all.ts`. This first pass does not add a separate `@deepseek-ai/dsh-file-context` package. The file-state store lives behind `ctx.fs` so root and subpath `tool-fs` plugins share the same read-before-write/edit policy automatically. +Example leaf configs stay bash-only in this landing. Wiring `examples/coding-agent` or `examples/acp-agent` to `dsh-fs-local` + `dsh-tool-fs` changes the model prompt, visible tool schemas, and ACP snapshot transcript, so it should land as a follow-up UX/example change with prompt and snapshot updates in the same PR. + If this work is split into multiple PRs, they should follow the seam order: 1. Interface PR: `dsh-fs` only, with service registration and contract tests. 2. Implementation PR: `dsh-fs-local`, with real filesystem behavior tests. -3. Consumer PR: `dsh-tool-fs`, examples, docs, and integration tests. +3. Consumer PR: `dsh-tool-fs`, docs, and integration tests; example wiring follows in a separate prompt/snapshot PR. The earlier combined package name `@deepseek-ai/dsh-fs-tools` should not become part of the new public surface. @@ -159,7 +160,7 @@ Beyond the happy/sad paths above, `dsh-fs-local` tests must cover the defensive- Integration tests should load `dsh-fs-local` plus `dsh-tool-fs` and execute `read`, `write`, and `edit` through `ctx.tools.execute()` to prove the three packages work together without bypassing the tool registry. They must verify the world, not the tool's self-report: after a `write`/`edit`, read the file back from disk and assert byte-identical content (and that untouched files are unchanged), rather than trusting the returned `ContentBlock[]`. Each integration/e2e test owns its resources — create the harness in the test, run against a per-test temporary directory, and dispose the harness and remove the directory in `afterEach` even on failure or timeout. -Repo gates for the implementation include the focused vitest suites, `yarn typecheck`, `yarn test:coverage` for runtime code, and build/publint coverage after adding package entrypoints. +Repo gates for the implementation include the focused vitest suites, `pnpm run typecheck`, `pnpm run test:coverage` for runtime code, and build/publint coverage after adding package entrypoints. ## Risks diff --git a/packages/fs/fs-local/README.md b/packages/fs/fs-local/README.md index 52dfa2e38b..794239ed2d 100644 --- a/packages/fs/fs-local/README.md +++ b/packages/fs/fs-local/README.md @@ -11,8 +11,8 @@ await ctx.plugin(LocalFileSystem, { cwd: process.cwd() }) ## Behavior -- **`resolve(path)`** — relative paths resolve from `config.cwd` (default `process.cwd()`). 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 keeps its absolute path as the key so creates still get a stable identity. `displayPath` is the absolute (un-resolved) path. -- **`readPage`** — UTF-8 only. A fast path (`readFile`) handles files under `FAST_PATH_MAX_SIZE` (10 MB); larger files stream with a capped line buffer so a newline-free giant file can't exhaust memory. NUL-byte samples are rejected (`FS_NOT_TEXT`). Output is bounded to `READ_LIMIT` (2000) lines, `READ_MAX_BYTES` (50 KB), and `READ_MAX_LINE_LENGTH` (2000) chars per line. The `version` is `mtimeMs:size`. +- **`resolve(path)`** — relative paths resolve from `config.cwd` (default `process.cwd()`). 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. +- **`readPage`** — UTF-8 only. A fast path (`readFile`) handles files under `FAST_PATH_MAX_SIZE` (10 MB); larger files stream with a capped line buffer so a newline-free giant file can't exhaust memory. Invalid UTF-8 and NUL-byte samples are rejected (`FS_NOT_TEXT`). Output is bounded to `READ_LIMIT` (2000) lines, `READ_MAX_BYTES` (50 KB), and `READ_MAX_LINE_LENGTH` (2000) chars per line; hitting any bound records a `partial` view. The `version` is `mtimeMs:size`. - **`createOrReplace`** — 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`. Honors the `FsExpectation`: an `observed` write must match the recorded version (else `FS_STALE_VERSION`); a `partial` write onto an existing file is rejected (`FS_PARTIAL_OBSERVATION`); an `unobserved` write onto an existing file is rejected (`FS_NOT_OBSERVED`). - **`applyEdit`** — atomic literal read-modify-write over the same primitive. Verifies the expected version, 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`). diff --git a/packages/fs/fs-local/src/fsio.ts b/packages/fs/fs-local/src/fsio.ts index 8c94abee24..a7e124db15 100644 --- a/packages/fs/fs-local/src/fsio.ts +++ b/packages/fs/fs-local/src/fsio.ts @@ -6,8 +6,8 @@ * The reader uses two code paths so a single huge line can never balloon * memory: a **fast path** (`readFile` + in-memory split) for files under * {@link FAST_PATH_MAX_SIZE}, and a **streaming path** (manual newline scan - * with a capped line buffer) for larger files. Both reject NUL-byte binary - * samples and keep only the requested page in memory. + * with a capped line buffer) for larger files. Both reject invalid UTF-8 and + * NUL-byte binary samples, and keep only the requested page in memory. * * Writes are atomic: content goes to a temp file opened exclusively (`wx`, * `0o600`, so a pre-existing path can never be clobbered and write-in-progress @@ -23,6 +23,7 @@ import { createReadStream } from 'node:fs' import { chmod, mkdir, open, readFile, realpath, rename, rm, stat } from 'node:fs/promises' import type { Stats } from 'node:fs' import { basename, dirname, join, resolve } from 'node:path' +import { TextDecoder } from 'node:util' import { FsError } from '@deepseek-ai/dsh-fs' import type { FsReadRequest, FsTextLine, FsView } from '@deepseek-ai/dsh-fs' @@ -41,7 +42,6 @@ export const FAST_PATH_MAX_SIZE = 10 * 1024 * 1024 const READ_MAX_BYTES_LABEL = `${READ_MAX_BYTES / 1024} KB` const READ_MAX_LINE_SUFFIX = `... (line truncated to ${READ_MAX_LINE_LENGTH} chars)` const BINARY_SAMPLE_BYTES = 8192 -const NUL_CHAR = String.fromCharCode(0) const LINE_BUFFER_CAP = READ_MAX_LINE_LENGTH + 1 /** @@ -145,17 +145,18 @@ interface PageAccumulator { totalLines: number outputBytes: number truncatedByBytes: boolean + truncatedByLine: boolean done: boolean } function newAccumulator(): PageAccumulator { - return { lines: [], totalLines: 0, outputBytes: 0, truncatedByBytes: false, done: false } + return { lines: [], totalLines: 0, outputBytes: 0, truncatedByBytes: false, truncatedByLine: false, done: false } } -function truncateReadLine(line: string): string { +function truncateReadLine(line: string): { text: string; truncated: boolean } { return line.length > READ_MAX_LINE_LENGTH - ? `${line.substring(0, READ_MAX_LINE_LENGTH)}${READ_MAX_LINE_SUFFIX}` - : line + ? { text: `${line.substring(0, READ_MAX_LINE_LENGTH)}${READ_MAX_LINE_SUFFIX}`, truncated: true } + : { text: line, truncated: false } } function lineByteSize(line: string, currentLineCount: number): number { @@ -166,7 +167,8 @@ function consumeLine(acc: PageAccumulator, rawLine: string, request: FsReadReque acc.totalLines += 1 if (acc.totalLines < request.offset || acc.lines.length >= request.limit) return - const text = truncateReadLine(rawLine) + const { text, truncated } = truncateReadLine(rawLine) + if (truncated) acc.truncatedByLine = true const bytes = lineByteSize(text, acc.lines.length) if (acc.outputBytes + bytes > READ_MAX_BYTES) { acc.truncatedByBytes = true @@ -195,13 +197,41 @@ function buildResult(acc: PageAccumulator, request: FsReadRequest, version: stri throw new FsError(`offset ${request.offset} is out of range for "${displayPath}" (${acc.totalLines} lines)`, 'FS_NOT_FOUND') } const endLine = acc.lines.at(-1)?.number ?? Math.max(0, request.offset - 1) - const view: FsView = request.offset === 1 && !acc.truncatedByBytes && endLine >= acc.totalLines ? 'full' : 'partial' + const view: FsView = request.offset === 1 && !acc.truncatedByBytes && !acc.truncatedByLine && endLine >= acc.totalLines ? 'full' : 'partial' return { lines: acc.lines, totalLines: acc.totalLines, truncatedByBytes: acc.truncatedByBytes, view, version } } +function notTextError(verb: 'read' | 'edit', displayPath: string): FsError { + return new FsError(`cannot ${verb} "${displayPath}": invalid UTF-8 text`, 'FS_NOT_TEXT') +} + +function decodeUtf8(buffer: Uint8Array, verb: 'read' | 'edit', displayPath: string): string { + try { + return new TextDecoder('utf-8', { fatal: true }).decode(buffer) + } catch (error: unknown) { + if (error instanceof TypeError) throw notTextError(verb, displayPath) + throw error + } +} + +function decodeUtf8Stream( + decoder: TextDecoder, + chunk: Uint8Array | undefined, + verb: 'read' | 'edit', + displayPath: string, +): string { + try { + return chunk ? decoder.decode(chunk, { stream: true }) : decoder.decode() + } catch (error: unknown) { + if (error instanceof TypeError) throw notTextError(verb, displayPath) + throw error + } +} + /** - * Read a bounded UTF-8 text-file page. Rejects non-regular files and NUL-byte - * binary samples; dispatches to the fast or streaming path by file size. + * Read a bounded UTF-8 text-file page. Rejects non-regular files, invalid + * UTF-8, and NUL-byte binary samples; dispatches to the fast or streaming path + * by file size. */ export async function readTextPage( target: LocalTarget, @@ -240,7 +270,7 @@ async function readTextPageFast( throw new FsError(`cannot read "${target.displayPath}": binary file`, 'FS_NOT_TEXT') } - const text = raw.toString('utf8') + const text = decodeUtf8(raw, 'read', target.displayPath) const acc = newAccumulator() let startPos = 0 let newlinePos: number @@ -261,10 +291,11 @@ async function readTextPageStreaming( version: string, signal?: AbortSignal, ): Promise { - const stream = createReadStream(target.targetKey, { encoding: 'utf8', ...signal ? { signal } : {} }) + const stream = createReadStream(target.targetKey, signal ? { signal } : {}) const acc = newAccumulator() let lineBuffer = '' - let firstChunk = true + let sampledBytes = 0 + const decoder = new TextDecoder('utf-8', { fatal: true }) function appendToLineBuffer(segment: string): void { if (lineBuffer.length >= LINE_BUFFER_CAP) return @@ -277,24 +308,36 @@ async function readTextPageStreaming( lineBuffer = '' } - try { - for await (const chunk of stream as AsyncIterable) { - if (firstChunk) { - firstChunk = false - if (chunk.slice(0, BINARY_SAMPLE_BYTES).includes(NUL_CHAR)) { - throw new FsError(`cannot read "${target.displayPath}": binary file`, 'FS_NOT_TEXT') - } - } - let startPos = 0 - let newlinePos: number - while ((newlinePos = chunk.indexOf('\n', startPos)) !== -1) { - appendToLineBuffer(chunk.slice(startPos, newlinePos)) - flushLine() - startPos = newlinePos + 1 - if (acc.done) return buildResult(acc, request, version, target.displayPath) - } - appendToLineBuffer(chunk.slice(startPos)) + function scanBinarySample(chunk: Buffer): void { + if (sampledBytes >= BINARY_SAMPLE_BYTES) return + const sample = chunk.subarray(0, Math.min(chunk.length, BINARY_SAMPLE_BYTES - sampledBytes)) + if (sample.includes(0)) { + throw new FsError(`cannot read "${target.displayPath}": binary file`, 'FS_NOT_TEXT') } + sampledBytes += sample.length + } + + function consumeChunk(chunk: string): ReadPageResult | undefined { + let startPos = 0 + let newlinePos: number + while ((newlinePos = chunk.indexOf('\n', startPos)) !== -1) { + appendToLineBuffer(chunk.slice(startPos, newlinePos)) + flushLine() + startPos = newlinePos + 1 + if (acc.done) return buildResult(acc, request, version, target.displayPath) + } + appendToLineBuffer(chunk.slice(startPos)) + return undefined + } + + try { + for await (const chunk of stream as AsyncIterable) { + scanBinarySample(chunk) + const result = consumeChunk(decodeUtf8Stream(decoder, chunk, 'read', target.displayPath)) + if (result) return result + } + const finalResult = consumeChunk(decodeUtf8Stream(decoder, undefined, 'read', target.displayPath)) + if (finalResult) return finalResult } catch (error: unknown) { /* v8 ignore next 4 -- mid-stream errors need an abort/IO fault racing the loop; pre-abort is caught by throwIfAborted. */ if (isAbortError(error)) throw new FsError('read aborted', 'FS_ABORTED') @@ -435,7 +478,7 @@ export async function readForEdit( const buffer = await readFile(absolutePath, signal ? { signal } : {}) throwIfAborted(signal, 'edit') if (buffer.includes(0)) throw new FsError(`cannot edit "${displayPath}": binary file`, 'FS_NOT_TEXT') - const raw = buffer.toString('utf8') + const raw = decodeUtf8(buffer, 'edit', displayPath) return { content: normalizeLineEndings(raw), lineEndings: detectLineEndings(raw) } } diff --git a/packages/fs/fs-local/tests/filesystem.spec.ts b/packages/fs/fs-local/tests/filesystem.spec.ts index ae1605892a..eb675472af 100644 --- a/packages/fs/fs-local/tests/filesystem.spec.ts +++ b/packages/fs/fs-local/tests/filesystem.spec.ts @@ -9,7 +9,7 @@ import { mkdtemp, readFile, rm, stat, symlink, writeFile, unlink } from 'node:fs import { tmpdir } from 'node:os' import { join } from 'node:path' import { Context } from 'cordis' -import { LocalFileSystem } from '@deepseek-ai/dsh-fs-local' +import { LocalFileSystem, probe } from '@deepseek-ai/dsh-fs-local' import type { FsExecContext } from '@deepseek-ai/dsh-fs' let dir: string @@ -89,6 +89,19 @@ describe('read → write → edit lifecycle', () => { expect(outcome.view).toBe('partial') }) + it('records an over-long-line read as partial, so write/edit stay blocked', async () => { + await writeFile(join(dir, 'long.txt'), 'x'.repeat(3000)) + const owner = exec() + const target = await fs.resolve('long.txt') + const outcome = await fs.read(target, READ_ALL, owner) + + expect(outcome.view).toBe('partial') + await expect(fs.write(target, 'new', owner)).rejects.toMatchObject({ code: 'FS_PARTIAL_OBSERVATION' }) + await expect( + fs.edit(target, { oldString: 'x', newString: 'y', replaceAll: false }, owner), + ).rejects.toMatchObject({ code: 'FS_PARTIAL_OBSERVATION' }) + }) + it('allows a follow-up edit without re-reading (write/edit refresh state)', async () => { await writeFile(join(dir, 'a.txt'), 'a b') const owner = exec() @@ -142,6 +155,22 @@ describe('read-before-write policy', () => { await expect(fs.edit(target, { oldString: 'old', newString: 'new', replaceAll: false }, exec())) .rejects.toMatchObject({ code: 'FS_NOT_OBSERVED' }) }) + + it('rejects invalid UTF-8 reads and edits without rewriting the file', async () => { + const path = join(dir, 'invalid-utf8.txt') + const bytes = Buffer.from([0x68, 0xff, 0x69]) + await writeFile(path, bytes) + const owner = exec() + const target = await fs.resolve('invalid-utf8.txt') + + await expect(fs.read(target, READ_ALL, owner)).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) + const existing = await probe(target.targetKey) + if (!existing) throw new Error('expected invalid UTF-8 fixture to exist') + await expect( + fs.applyEdit(target, { oldString: 'h', newString: 'H', replaceAll: false }, { version: existing.version }), + ).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) + expect(await readFile(path)).toEqual(bytes) + }) }) describe('stale-version guard + concurrency (defensive class B)', () => { diff --git a/packages/fs/fs-local/tests/fsio.spec.ts b/packages/fs/fs-local/tests/fsio.spec.ts index 77b8d8ccba..f25f9e9a0b 100644 --- a/packages/fs/fs-local/tests/fsio.spec.ts +++ b/packages/fs/fs-local/tests/fsio.spec.ts @@ -102,6 +102,7 @@ describe('readTextPage', () => { await writeFile(file, 'x'.repeat(3000)) const result = await readTextPage(localTarget(file), READ_ALL) expect(result.lines[0]?.text).toContain('... (line truncated to 2000 chars)') + expect(result.view).toBe('partial') }) it('caps output bytes and reports truncatedByBytes', async () => { @@ -141,6 +142,12 @@ describe('readTextPage', () => { await expect(readTextPage(localTarget(file), READ_ALL)).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) }) + it('rejects invalid UTF-8 bytes (fast path)', async () => { + const file = join(dir, 'invalid-utf8.txt') + await writeFile(file, Buffer.from([0x68, 0xff, 0x69])) + await expect(readTextPage(localTarget(file), READ_ALL)).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) + }) + it('rejects a missing file and a directory', async () => { await expect(readTextPage(localTarget(join(dir, 'nope')), READ_ALL)).rejects.toMatchObject({ code: 'FS_NOT_FOUND' }) await expect(readTextPage(localTarget(dir), READ_ALL)).rejects.toMatchObject({ code: 'FS_NOT_REGULAR_FILE' }) @@ -181,6 +188,13 @@ describe('readTextPage', () => { await writeFile(file, 'z'.repeat(5000)) const result = await readTextPage(localTarget(file), READ_ALL, undefined, stream) expect(result.lines[0]?.text).toContain('... (line truncated to 2000 chars)') + expect(result.view).toBe('partial') + }) + + it('rejects invalid UTF-8 bytes on the streaming path', async () => { + const file = join(dir, 'invalid-utf8.txt') + await writeFile(file, Buffer.from([0x68, 0xff, 0x69])) + await expect(readTextPage(localTarget(file), READ_ALL, undefined, stream)).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) }) it('honors abort on the streaming path', async () => { @@ -338,6 +352,12 @@ describe('readForEdit + restoreLineEndings', () => { await expect(readForEdit(file, file)).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) }) + it('rejects invalid UTF-8 bytes', async () => { + const file = join(dir, 'invalid-utf8.txt') + await writeFile(file, Buffer.from([0x68, 0xff, 0x69])) + await expect(readForEdit(file, file)).rejects.toMatchObject({ code: 'FS_NOT_TEXT' }) + }) + it('passes a live (non-aborted) signal through the read', async () => { const file = join(dir, 'a.txt') await writeFile(file, 'one\ntwo')