refactor(fs): prune write-only fields and the dead routing knob from the seam
The fs seam split left four pieces of pre-split surface populated on
every call and read by nobody:
- STREAM_MIN_SIZE + FsIoInternals.streamMinSize in dsh-fs-local: the
backend has no read routing (readWholeText/streamWholeText are
separate primitives the caller picks), and the real 10 MiB routing
constant lives in dsh-tool-fs's read tool. Delete the dead mirror and
the knob whose JSDoc claimed an override that did not exist; the
remaining FsIoInternals knobs stay (the atomic-write tests use them).
- FsTarget.inputPath: a "diagnostics only" field every backend and test
fake had to fabricate, with zero production readers (policy and error
messages use targetKey/displayPath). listDir gave children the bare
entry name, which was nobody's input.
- FsEditOutcome.replacements/.replaceAll: replacements had no reader
(the single-match policy is enforced by the FS_AMBIGUOUS_EDIT /
FS_EDIT_NOT_FOUND throws, whose message keeps the internal count);
replaceAll only echoed the replace_all argument back to
formatEditOutput, which now takes it from the parsed args. The
outcome shrinks to { version, before, after }, parallel to
FsWriteOutcome's backend-discovered fields. Emitted text is unchanged
for both branches (no snapshot churn).
- FileReadOutcome.limit/.version: formatReadOutput renders
offset/lines/totalLines/truncatedByBytes only, and the fs/observed
emit uses info.version directly.
Backends shed four fabrication obligations and gain none. Doc pastes
(core-data-structures/filesystem.md), the dsh-fs README resolve row,
and the test fakes shrink with the types. RFC moved to
implemented/simplification and amended to the shipped shape
(FsEditSpec -> FsEditRequest name fix; manifest rows needed no change).
This commit is contained in:
@@ -26,9 +26,6 @@ import { basename, dirname, join, resolve } from 'node:path'
|
||||
import { TextDecoder } from 'node:util'
|
||||
import { FsError, FsTargetKey, FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
|
||||
/** Files at or above this size stream their text; smaller files read whole. */
|
||||
export const STREAM_MIN_SIZE = 10 * 1024 * 1024
|
||||
|
||||
const BINARY_SAMPLE_BYTES = 8192
|
||||
|
||||
function isENOENT(error: unknown): boolean {
|
||||
@@ -85,13 +82,11 @@ function versionOf(info: Stats): FsVersion {
|
||||
}
|
||||
|
||||
/**
|
||||
* Test seam: lets specs force the streaming read path (via a small
|
||||
* `streamMinSize`) and pin the temp-file name (to prove exclusive-open
|
||||
* behavior) without a 10 MB fixture or a name race.
|
||||
* Test seam: lets specs pin the atomic-write temp names (to prove
|
||||
* exclusive-open behavior without a name race) and observe the staged temp
|
||||
* file before it is renamed over the target.
|
||||
*/
|
||||
export interface FsIoInternals {
|
||||
/** Override {@link STREAM_MIN_SIZE} for read routing. */
|
||||
streamMinSize?: number
|
||||
/** Override the generated private staging-dir name (relative to the target dir). */
|
||||
tempDirName?: (writePath: string) => string
|
||||
/** Override the generated temp-file name (relative to the private staging dir). */
|
||||
|
||||
@@ -41,7 +41,6 @@ import {
|
||||
import type { FsIoInternals } from './fsio.ts'
|
||||
|
||||
export {
|
||||
STREAM_MIN_SIZE,
|
||||
applyLiteralEdit,
|
||||
listDirectory,
|
||||
probe,
|
||||
@@ -105,7 +104,7 @@ export class LocalFileSystem extends FileSystem {
|
||||
|
||||
override async resolve(path: string, opts?: { cwd?: string }): Promise<FsTarget> {
|
||||
const local = await resolveLocalTarget(opts?.cwd ?? this.config.cwd, path)
|
||||
return { inputPath: path, targetKey: local.targetKey, displayPath: local.displayPath }
|
||||
return { targetKey: local.targetKey, displayPath: local.displayPath }
|
||||
}
|
||||
|
||||
override async stat(target: FsTarget, signal?: AbortSignal): Promise<FsInfo | undefined> {
|
||||
@@ -128,7 +127,7 @@ export class LocalFileSystem extends FileSystem {
|
||||
return entries.map(entry => ({
|
||||
name: entry.name,
|
||||
type: entry.type,
|
||||
target: { inputPath: entry.name, targetKey: entry.target.targetKey, displayPath: entry.target.displayPath },
|
||||
target: { targetKey: entry.target.targetKey, displayPath: entry.target.displayPath },
|
||||
...(entry.version !== undefined ? { version: entry.version } : {}),
|
||||
...(entry.size !== undefined ? { size: entry.size } : {}),
|
||||
}))
|
||||
@@ -211,8 +210,6 @@ export class LocalFileSystem extends FileSystem {
|
||||
|
||||
const after = await probe(target.targetKey)
|
||||
return {
|
||||
replacements: edited.replacements,
|
||||
replaceAll: edit.replaceAll,
|
||||
version: this.versionAfterWrite(after, target),
|
||||
// The LF-normalized before/after text (the applied-hunk diff basis);
|
||||
// line-ending restoration is a storage detail the diff ignores.
|
||||
|
||||
@@ -138,12 +138,6 @@ 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))))
|
||||
@@ -335,7 +329,7 @@ describe('editText', () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'hello world')
|
||||
const target = await fs.resolve('a.txt')
|
||||
const outcome = await fs.editText(target, { oldString: 'world', newString: 'there', replaceAll: false }, { version: await versionOf(target) })
|
||||
expect(outcome.replacements).toBe(1)
|
||||
expect(outcome.after).toBe('hello there')
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('hello there')
|
||||
})
|
||||
|
||||
@@ -365,7 +359,7 @@ describe('editText', () => {
|
||||
const target = await fs.resolve('a.txt')
|
||||
// No version guard: any current content is edited, regardless of version.
|
||||
const outcome = await fs.editText(target, { oldString: 'world', newString: 'there', replaceAll: false })
|
||||
expect(outcome.replacements).toBe(1)
|
||||
expect(outcome.after).toBe('hello there')
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('hello there')
|
||||
})
|
||||
|
||||
@@ -411,7 +405,7 @@ describe('editText', () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'a a a')
|
||||
const target = await fs.resolve('a.txt')
|
||||
const outcome = await fs.editText(target, { oldString: 'a', newString: 'b', replaceAll: true }, { version: await versionOf(target) })
|
||||
expect(outcome.replacements).toBe(3)
|
||||
expect(outcome.after).toBe('b b b')
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('b b b')
|
||||
})
|
||||
|
||||
@@ -457,7 +451,7 @@ describe('editText', () => {
|
||||
// The version the first edit returned is a valid guard for a second edit —
|
||||
// no intervening re-stat needed.
|
||||
const second = await fs.editText(target, { oldString: 'two', newString: 'TWO', replaceAll: false }, { version: first.version })
|
||||
expect(second.replacements).toBe(1)
|
||||
expect(second.after).toBe('ONE TWO')
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('ONE TWO')
|
||||
})
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ import * as FsPolicy from '@deepseek-ai/dsh-fs-policy'
|
||||
import type { FsPolicyExec } from '@deepseek-ai/dsh-fs-policy'
|
||||
|
||||
function target(path: string): FsTarget {
|
||||
return { inputPath: path, targetKey: FsTargetKey(path), displayPath: path }
|
||||
return { targetKey: FsTargetKey(path), displayPath: path }
|
||||
}
|
||||
const ownerExec = (session: object): FsPolicyExec => ({ agent: { session } })
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ A backend subclasses `FileSystem` and implements seven primitives.
|
||||
|
||||
| Member | Semantics |
|
||||
|---|---|
|
||||
| `resolve(path, opts?)` | Resolve a path into a stable `FsTarget` (`inputPath`, opaque `targetKey`, `displayPath`). `opts.cwd` is the base a relative `path` resolves against (a caller supplies its session workspace; absolute paths ignore it; omitted ⇒ the backend default). Async — a remote backend may need I/O. The same file via different paths must yield the same `targetKey`. |
|
||||
| `resolve(path, opts?)` | Resolve a path into a stable `FsTarget` (opaque `targetKey`, `displayPath`). `opts.cwd` is the base a relative `path` resolves against (a caller supplies its session workspace; absolute paths ignore it; omitted ⇒ the backend default). Async — a remote backend may need I/O. The same file via different paths must yield the same `targetKey`. |
|
||||
| `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). |
|
||||
|
||||
@@ -52,8 +52,6 @@ export function FsVersion(v: string): FsVersion {
|
||||
* this; every other operation takes it.
|
||||
*/
|
||||
export interface FsTarget {
|
||||
/** The original model/plugin-supplied path, for diagnostics only. */
|
||||
inputPath: string
|
||||
/** Opaque key for stale guards and target lookup. */
|
||||
targetKey: FsTargetKey
|
||||
/**
|
||||
@@ -142,10 +140,6 @@ export interface FsEditRequest {
|
||||
|
||||
/** Outcome of a literal edit. */
|
||||
export interface FsEditOutcome {
|
||||
/** Number of literal replacements applied. */
|
||||
replacements: number
|
||||
/** Whether every match was replaced. */
|
||||
replaceAll: boolean
|
||||
/** Opaque version of the file after the edit. */
|
||||
version: FsVersion
|
||||
/**
|
||||
|
||||
@@ -23,7 +23,7 @@ class FakeFileSystem extends FileSystem {
|
||||
files = new Map<string, string>()
|
||||
|
||||
override async resolve(path: string): Promise<FsTarget> {
|
||||
return { inputPath: path, targetKey: FsTargetKey(path), displayPath: path }
|
||||
return { targetKey: FsTargetKey(path), displayPath: path }
|
||||
}
|
||||
override async stat(target: FsTarget): Promise<FsInfo | undefined> {
|
||||
const content = this.files.get(target.targetKey)
|
||||
@@ -45,7 +45,7 @@ class FakeFileSystem extends FileSystem {
|
||||
{
|
||||
name: 'alpha.md',
|
||||
type: 'file',
|
||||
target: { inputPath: 'skills/alpha.md', targetKey: FsTargetKey('skills/alpha.md'), displayPath: 'skills/alpha.md' },
|
||||
target: { targetKey: FsTargetKey('skills/alpha.md'), displayPath: 'skills/alpha.md' },
|
||||
size: 2,
|
||||
version: FsVersion('v1'),
|
||||
},
|
||||
@@ -60,7 +60,7 @@ class FakeFileSystem extends FileSystem {
|
||||
const content = this.files.get(target.targetKey) ?? ''
|
||||
const after = content.split(edit.oldString).join(edit.newString)
|
||||
this.files.set(target.targetKey, after)
|
||||
return { replacements: 1, replaceAll: edit.replaceAll, version: FsVersion('v3'), before: content, after }
|
||||
return { version: FsVersion('v3'), before: content, after }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -108,7 +108,7 @@ describe('FileSystem provider seam', () => {
|
||||
expect(entries).toEqual([{
|
||||
name: 'alpha.md',
|
||||
type: 'file',
|
||||
target: { inputPath: 'skills/alpha.md', targetKey: 'skills/alpha.md', displayPath: 'skills/alpha.md' },
|
||||
target: { targetKey: 'skills/alpha.md', displayPath: 'skills/alpha.md' },
|
||||
size: 2,
|
||||
version: 'v1',
|
||||
}])
|
||||
|
||||
@@ -16,7 +16,6 @@ import type { Context } from 'cordis'
|
||||
import { defineTool } from '@deepseek-ai/dsh-tools'
|
||||
import type { DiffCallView, DiffResultView, ToolResult } from '@deepseek-ai/dsh-tools'
|
||||
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import type { FsEditOutcome } from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { computeHunkDiffs, diffsFromMeta, type FsDiffMeta } from './diff.ts'
|
||||
@@ -43,9 +42,9 @@ export function parseEditArgs(args: { file_path: string; old_string: string; new
|
||||
}
|
||||
}
|
||||
|
||||
/** Format an edit outcome as a Claude-style model-facing success message. */
|
||||
export function formatEditOutput(displayPath: string, outcome: FsEditOutcome): string {
|
||||
return outcome.replaceAll
|
||||
/** Format an edit success (single-match or replace-all) as a Claude-style model-facing message. */
|
||||
export function formatEditOutput(displayPath: string, replaceAll: boolean): string {
|
||||
return replaceAll
|
||||
? `The file ${displayPath} has been updated. All occurrences were successfully replaced.`
|
||||
: `The file ${displayPath} has been updated successfully.`
|
||||
}
|
||||
@@ -91,7 +90,7 @@ export function applyEditTool(ctx: Context): void {
|
||||
// relativizes it).
|
||||
const diffs = computeHunkDiffs(input.filePath, outcome.before, outcome.after)
|
||||
return {
|
||||
content: [{ type: 'text', text: formatEditOutput(target.displayPath, outcome) }],
|
||||
content: [{ type: 'text', text: formatEditOutput(target.displayPath, input.replaceAll) }],
|
||||
meta: { diffs },
|
||||
}
|
||||
},
|
||||
|
||||
@@ -17,7 +17,6 @@
|
||||
*/
|
||||
|
||||
import { FsError } from '@deepseek-ai/dsh-fs'
|
||||
import type { FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
|
||||
/** Maximum characters returned for a single line. */
|
||||
export const READ_MAX_LINE_LENGTH = 2000
|
||||
@@ -58,16 +57,12 @@ export interface WindowResult {
|
||||
export interface FileReadOutcome {
|
||||
/** 1-based first line requested. */
|
||||
offset: number
|
||||
/** Maximum number of lines requested. */
|
||||
limit: number
|
||||
/** Returned lines, already numbered. */
|
||||
lines: FileTextLine[]
|
||||
/** Total line count in the file, unless `truncatedByBytes` stopped scanning early. */
|
||||
totalLines: number
|
||||
/** Whether selected output hit the byte cap before EOF or the requested limit. */
|
||||
truncatedByBytes?: true
|
||||
/** Opaque version of the file at read time. */
|
||||
version: FsVersion
|
||||
}
|
||||
|
||||
interface WindowAccumulator {
|
||||
|
||||
@@ -90,10 +90,8 @@ export function applyReadTool(ctx: Context): void {
|
||||
|
||||
const outcome: FileReadOutcome = {
|
||||
offset: input.offset,
|
||||
limit: input.limit,
|
||||
lines: window.lines,
|
||||
totalLines: window.totalLines,
|
||||
version: info.version,
|
||||
...window.truncatedByBytes ? { truncatedByBytes: true } : {},
|
||||
}
|
||||
// Record the observed version (a no-op when no policy plugin listens). The
|
||||
|
||||
@@ -40,7 +40,7 @@ class FakeFs extends FileSystem {
|
||||
}
|
||||
|
||||
override async resolve(path: string): Promise<FsTarget> {
|
||||
return { inputPath: path, targetKey: FsTargetKey(`key:${path}`), displayPath: `/abs/${path}` }
|
||||
return { targetKey: FsTargetKey(`key:${path}`), displayPath: `/abs/${path}` }
|
||||
}
|
||||
override async stat(target: FsTarget): Promise<FsInfo | undefined> {
|
||||
this.throwIfArmed()
|
||||
@@ -71,7 +71,7 @@ class FakeFs extends FileSystem {
|
||||
const content = this.files.get(target.targetKey) ?? ''
|
||||
const after = content.split(edit.oldString).join(edit.newString)
|
||||
this.files.set(target.targetKey, after)
|
||||
return { replacements: 1, replaceAll: edit.replaceAll, version: FsVersion('v3'), before: content, after }
|
||||
return { version: FsVersion('v3'), before: content, after }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -252,7 +252,7 @@ describe('read tool', () => {
|
||||
})
|
||||
|
||||
describe('formatReadOutput footer variants', () => {
|
||||
const base: FileReadOutcome = { offset: 1, limit: 2000, lines: [{ number: 1, text: 'x' }], totalLines: 1, version: FsVersion('v') }
|
||||
const base: FileReadOutcome = { offset: 1, lines: [{ number: 1, text: 'x' }], totalLines: 1 }
|
||||
|
||||
it('reports a byte-capped read', () => {
|
||||
const out = formatReadOutput('/f', { ...base, totalLines: 99, truncatedByBytes: true })
|
||||
|
||||
Reference in New Issue
Block a user