refactor(tool-fs): consolidate read rendering; drop the fs/observed try-catch
Two cohesion cleanups on the filesystem tool package: - Fold window.ts + types.ts + formatReadOutput into one cordis-free read-render.ts. Line windowing, the FileReadOutcome shape, and output formatting are one concern (the read tool's rendering); splitting them across three files added no value. read.ts is now just the tool (schema + I/O). - Drop observe.ts and emit fs/observed with a plain ctx.emit in read/write/edit. The event is contractually a synchronous, side-effect-only recorder (file-context's listener is a WeakMap.set), so the per-call try/catch guarded against a contract violation that cannot happen under the shipped listener — defensive code for an impossible case. The event contract (dsh-fs JSDoc, README, RFC) is updated to state the fire-and-forget semantics plainly.
This commit is contained in:
@@ -117,13 +117,13 @@ declare module 'cordis' {
|
||||
'fs/edit-expectation'(target: FsTarget, actor: object | undefined, next: () => { version: FsVersion } | undefined | Promise<{ version: FsVersion } | undefined>): Promise<{ version: FsVersion } | undefined>
|
||||
/**
|
||||
* Record that an actor observed a target at a version, after a successful
|
||||
* read/write/edit. Fire-and-forget. A listener MUST be a synchronous,
|
||||
* side-effect-only recorder (`@deepseek-ai/dsh-file-context`'s is a
|
||||
* `WeakMap.set`); the tool wraps the emit in a try/catch so a synchronous
|
||||
* listener bug is logged and swallowed, never failing the already-completed
|
||||
* mutation. cordis `emit` does not await listener promises, so this is not an
|
||||
* async-error containment seam — async audit/telemetry does not belong here.
|
||||
* No listener ⇒ nothing recorded. `actor` is the opaque tool-execution context.
|
||||
* read/write/edit. Fire-and-forget (plain `emit`). A listener MUST be a
|
||||
* synchronous, side-effect-only recorder (`@deepseek-ai/dsh-file-context`'s
|
||||
* is a `WeakMap.set`): the tool does not guard the emit, so a listener that
|
||||
* throws surfaces as the tool's `isError` result, and cordis `emit` does not
|
||||
* await listener promises — async or fallible audit/telemetry does not
|
||||
* belong here. No listener ⇒ nothing recorded. `actor` is the opaque
|
||||
* tool-execution context.
|
||||
* @mode emit
|
||||
*/
|
||||
'fs/observed'(target: FsTarget, version: FsVersion, actor: object | undefined): void
|
||||
|
||||
@@ -31,8 +31,8 @@ The tools do **not** inject a policy service or inspect any cache. Each tool res
|
||||
|
||||
The tool passes `exec` (the tool-execution context) as the opaque `actor` on every dispatch. The default thunks return `undefined` (the unconstrained bare provider). When `@deepseek-ai/dsh-file-context` is loaded it occupies the single decision slot — returning `createIfAbsent`/`replaceIfVersion`/`{ version }` or throwing `FS_NOT_OBSERVED` — and records on `fs/observed`. Backend errors (`FsError`) and a thrown `FS_NOT_OBSERVED` flow through `ToolRegistry.execute()` and become `isError` tool results with their `{ name, code }` attached.
|
||||
|
||||
## `fs/observed` never fails the tool
|
||||
## `fs/observed` is fire-and-forget
|
||||
|
||||
`fs/observed` fires AFTER the read/write/edit already succeeded, so the tool wraps the emit in a try/catch (`src/observe.ts`) that logs and swallows a synchronous listener bug — otherwise a recording failure would turn a completed mutation into an `isError`. The event contract requires synchronous, side-effect-only listeners; this is the synchronous backstop, not async-error handling.
|
||||
`fs/observed` fires AFTER the read/write/edit already succeeded, via a plain `ctx.emit`. A listener is contractually a synchronous, side-effect-only recorder (`@deepseek-ai/dsh-file-context`'s is a `WeakMap.set`); the tool does not guard the emit, so a listener that throws would surface as the tool's `isError` result — async or fallible observation does not belong on this event.
|
||||
|
||||
The line-windowing mechanics live in `src/window.ts` (Cordis-free, independently unit-tested); `src/read.ts`/`write.ts`/`edit.ts` are the tool executors and `src/index.ts` composes them.
|
||||
The read rendering (line windowing + output formatting) lives in `src/read-render.ts` (Cordis-free, independently unit-tested); `src/read.ts`/`write.ts`/`edit.ts` are the tool executors and `src/index.ts` composes them.
|
||||
|
||||
@@ -2,13 +2,12 @@
|
||||
* The model-facing `edit` tool: update an existing UTF-8 text file by replacing
|
||||
* literal text, requiring a unique match by default. The tool is the executor:
|
||||
* it dispatches the `fs/edit-expectation` waterfall to obtain the optional
|
||||
* version guard, calls `ctx.fs.editText` directly, and emits a contained
|
||||
* `fs/observed`. The default thunk returns `undefined` (unconditional edit of
|
||||
* the current content — the bare provider); a policy plugin
|
||||
* (`@deepseek-ai/dsh-file-context`) occupies the single decision slot, returning
|
||||
* `{ version: vObserved }` or throwing `FS_NOT_OBSERVED` for an unread file. The
|
||||
* tool stats ZERO times either way; a missing target is reported by the provider
|
||||
* as `FS_STALE_VERSION`.
|
||||
* version guard, calls `ctx.fs.editText` directly, and emits `fs/observed`. The
|
||||
* default thunk returns `undefined` (unconditional edit of the current content
|
||||
* — the bare provider); a policy plugin (`@deepseek-ai/dsh-file-context`)
|
||||
* occupies the single decision slot, returning `{ version: vObserved }` or
|
||||
* throwing `FS_NOT_OBSERVED` for an unread file. The tool stats ZERO times
|
||||
* either way; a missing target is reported by the provider as `FS_STALE_VERSION`.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/src/edit
|
||||
*/
|
||||
@@ -19,7 +18,6 @@ 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 { emitObserved } from './observe.ts'
|
||||
|
||||
/** Validated `edit` arguments after defaulting. */
|
||||
interface EditInput {
|
||||
@@ -79,7 +77,8 @@ export function applyEditTool(ctx: Context): void {
|
||||
expectation,
|
||||
exec.signal,
|
||||
)
|
||||
emitObserved(ctx, target, outcome.version, exec)
|
||||
// Record the observed version (a no-op when no policy plugin listens).
|
||||
ctx.emit('fs/observed', target, outcome.version, exec)
|
||||
return [{ type: 'text', text: formatEditOutput(target.displayPath, outcome) }]
|
||||
},
|
||||
}))
|
||||
|
||||
@@ -26,13 +26,11 @@ import { applyReadTool } from './read.ts'
|
||||
import { applyWriteTool } from './write.ts'
|
||||
import { applyEditTool } from './edit.ts'
|
||||
|
||||
export { READ_LIMIT, STREAM_MIN_SIZE, applyReadTool, formatReadOutput, parseReadArgs } from './read.ts'
|
||||
export { READ_LIMIT, STREAM_MIN_SIZE, applyReadTool, parseReadArgs } from './read.ts'
|
||||
export { applyWriteTool, formatWriteOutput, parseWriteArgs } from './write.ts'
|
||||
export { applyEditTool, formatEditOutput, parseEditArgs } from './edit.ts'
|
||||
export { emitObserved } from './observe.ts'
|
||||
export type { FileTextLine, ReadWindow, WindowResult } from './window.ts'
|
||||
export { READ_MAX_BYTES, READ_MAX_LINE_LENGTH, buildWindow } from './window.ts'
|
||||
export type { FileReadOutcome } from './types.ts'
|
||||
export { READ_MAX_BYTES, READ_MAX_LINE_LENGTH, buildWindow, formatReadOutput } from './read-render.ts'
|
||||
export type { FileReadOutcome, FileTextLine, ReadWindow, WindowResult } from './read-render.ts'
|
||||
|
||||
/** Cordis plugin name used by loader diagnostics. */
|
||||
export const name = 'tool-fs'
|
||||
|
||||
@@ -1,34 +0,0 @@
|
||||
/**
|
||||
* The contained `fs/observed` emit shared by the `read`/`write`/`edit` tools.
|
||||
*
|
||||
* `fs/observed` fires AFTER a mutation/read already succeeded, so a throwing
|
||||
* listener must never turn the completed operation into an `isError` result
|
||||
* (the tool registry catches a tool throw into an error result). The event
|
||||
* contract requires a synchronous, side-effect-only listener (the policy
|
||||
* plugin's is a `WeakMap.set`); this try/catch is the synchronous backstop —
|
||||
* it logs and swallows a listener bug, mirroring the fire-and-forget pattern in
|
||||
* the agent loop. It is NOT async-error containment: cordis `emit` does not
|
||||
* await listener promises, so async observation does not belong on this event.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/observe
|
||||
*/
|
||||
|
||||
import type { Context } from 'cordis'
|
||||
import type { FsTarget, FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-fs'
|
||||
|
||||
/**
|
||||
* Emit `fs/observed` for a just-completed read/write/edit, containing any
|
||||
* synchronous listener throw so the already-successful operation still reports
|
||||
* success.
|
||||
*/
|
||||
export function emitObserved(ctx: Context, target: FsTarget, version: FsVersion, actor: object | undefined): void {
|
||||
try {
|
||||
ctx.emit('fs/observed', target, version, actor)
|
||||
} catch (error: unknown) {
|
||||
// Contained: the read/write/edit already succeeded. An `fs/observed` listener
|
||||
// MUST be synchronous and side-effect-only; a synchronous bug is logged and
|
||||
// swallowed so a recording failure never fails the completed operation.
|
||||
ctx.logger.warn(`fs/observed listener threw for "${target.displayPath}": ${String(error)}`)
|
||||
}
|
||||
}
|
||||
@@ -1,19 +1,23 @@
|
||||
/**
|
||||
* Cordis-free line-windowing for `@deepseek-ai/dsh-tool-fs`. Turning a file's
|
||||
* Cordis-free read rendering for `@deepseek-ai/dsh-tool-fs`: turn a file's
|
||||
* decoded text into a bounded, line-numbered window (offset/limit, byte cap,
|
||||
* per-line truncation) is the model-facing READ-RENDERING detail the tool owns
|
||||
* now that the tool reads through `ctx.fs` directly — it is not a storage
|
||||
* primitive and not freshness policy.
|
||||
* per-line truncation) and format it as the model-facing text block. This is
|
||||
* the `read` tool's RENDERING detail — not a storage primitive, not freshness
|
||||
* policy — so it lives apart from the tool's I/O and event wiring as a pure,
|
||||
* independently-testable module (no cordis, no filesystem).
|
||||
*
|
||||
* The provider (`ctx.fs.readText`/`streamText`) hands back already-decoded text
|
||||
* (UTF-8 validated, binary rejected); this module only scans that text for
|
||||
* newlines and builds the requested window. A capped line buffer means a
|
||||
* (UTF-8 validated, binary rejected); {@link buildWindow} only scans that text
|
||||
* for newlines and builds the requested window. A capped line buffer means a
|
||||
* newline-free giant line can never balloon memory even when streamed.
|
||||
* {@link formatReadOutput} turns the resulting {@link FileReadOutcome} into the
|
||||
* `<path>/<content>` envelope the model sees.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/window
|
||||
* @module @deepseek-ai/dsh-tool-fs/read-render
|
||||
*/
|
||||
|
||||
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
|
||||
@@ -40,7 +44,7 @@ export interface FileTextLine {
|
||||
text: string
|
||||
}
|
||||
|
||||
/** The windowed result this module builds from a file's decoded text. */
|
||||
/** The windowed result {@link buildWindow} produces from a file's decoded text. */
|
||||
export interface WindowResult {
|
||||
/** Returned lines, already numbered. */
|
||||
lines: FileTextLine[]
|
||||
@@ -50,6 +54,22 @@ export interface WindowResult {
|
||||
truncatedByBytes: boolean
|
||||
}
|
||||
|
||||
/** Outcome of a bounded text read — what {@link formatReadOutput} renders. */
|
||||
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 {
|
||||
lines: FileTextLine[]
|
||||
totalLines: number
|
||||
@@ -137,3 +157,24 @@ export async function buildWindow(
|
||||
if (lineBuffer.length > 0) flushLine()
|
||||
return finish(acc, request, displayPath)
|
||||
}
|
||||
|
||||
/** Format a read outcome as one OpenCode-style line-numbered text block body. */
|
||||
export function formatReadOutput(displayPath: string, outcome: FileReadOutcome): string {
|
||||
const endLine = outcome.lines.at(-1)?.number ?? Math.max(0, outcome.offset - 1)
|
||||
let footer: string
|
||||
if (outcome.truncatedByBytes) {
|
||||
footer = `(Output capped. Showing lines ${outcome.offset}-${endLine}. Use offset=${endLine + 1} to continue.)`
|
||||
} else if (endLine < outcome.totalLines) {
|
||||
footer = `(Showing lines ${outcome.offset}-${endLine} of ${outcome.totalLines}. Use offset=${endLine + 1} to continue.)`
|
||||
} else {
|
||||
footer = `(End of file - total ${outcome.totalLines} lines)`
|
||||
}
|
||||
const body = outcome.lines.length > 0
|
||||
? `${outcome.lines.map(line => `${line.number}: ${line.text}`).join('\n')}\n\n${footer}`
|
||||
: footer
|
||||
return `<path>${displayPath}</path>
|
||||
<type>file</type>
|
||||
<content>
|
||||
${body}
|
||||
</content>`
|
||||
}
|
||||
@@ -2,11 +2,12 @@
|
||||
* The model-facing `read` tool: inspect a UTF-8 text file and return
|
||||
* line-numbered content with pagination guidance. The tool is the executor — it
|
||||
* stats and reads through `ctx.fs` directly, builds the line window
|
||||
* ({@link module:@deepseek-ai/dsh-tool-fs/window}), and emits a contained
|
||||
* `fs/observed` so a policy plugin (`@deepseek-ai/dsh-file-context`) can record
|
||||
* the read. With no policy plugin the emit is simply unheard. This module owns
|
||||
* the model-facing schema, argument validation, read windowing, and result
|
||||
* formatting; the freshness/observation policy is not its concern.
|
||||
* ({@link module:@deepseek-ai/dsh-tool-fs/read-render}), and emits `fs/observed`
|
||||
* so a policy plugin (`@deepseek-ai/dsh-file-context`) can record the read. With
|
||||
* no policy plugin the emit is simply unheard. This module owns the
|
||||
* model-facing schema, argument validation, and the read I/O; the rendering
|
||||
* (windowing + formatting) lives in `read-render.ts` and the
|
||||
* freshness/observation policy is not its concern.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/src/read
|
||||
*/
|
||||
@@ -17,9 +18,8 @@ import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import { FsError } from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { buildWindow } from './window.ts'
|
||||
import { emitObserved } from './observe.ts'
|
||||
import type { FileReadOutcome } from './types.ts'
|
||||
import { buildWindow, formatReadOutput } from './read-render.ts'
|
||||
import type { FileReadOutcome } from './read-render.ts'
|
||||
|
||||
/** Default and maximum number of lines returned by one `read` call. */
|
||||
export const READ_LIMIT = 2000
|
||||
@@ -50,27 +50,6 @@ export function parseReadArgs(args: { file_path: string; offset?: number; limit?
|
||||
return { filePath: args.file_path, offset, limit }
|
||||
}
|
||||
|
||||
/** Format a read outcome as one OpenCode-style line-numbered text block body. */
|
||||
export function formatReadOutput(displayPath: string, outcome: FileReadOutcome): string {
|
||||
const endLine = outcome.lines.at(-1)?.number ?? Math.max(0, outcome.offset - 1)
|
||||
let footer: string
|
||||
if (outcome.truncatedByBytes) {
|
||||
footer = `(Output capped. Showing lines ${outcome.offset}-${endLine}. Use offset=${endLine + 1} to continue.)`
|
||||
} else if (endLine < outcome.totalLines) {
|
||||
footer = `(Showing lines ${outcome.offset}-${endLine} of ${outcome.totalLines}. Use offset=${endLine + 1} to continue.)`
|
||||
} else {
|
||||
footer = `(End of file - total ${outcome.totalLines} lines)`
|
||||
}
|
||||
const body = outcome.lines.length > 0
|
||||
? `${outcome.lines.map(line => `${line.number}: ${line.text}`).join('\n')}\n\n${footer}`
|
||||
: footer
|
||||
return `<path>${displayPath}</path>
|
||||
<type>file</type>
|
||||
<content>
|
||||
${body}
|
||||
</content>`
|
||||
}
|
||||
|
||||
/** Register the `read` tool and its system-prompt guidance. */
|
||||
export function applyReadTool(ctx: Context): void {
|
||||
ctx.systemPrompt.section({
|
||||
@@ -114,7 +93,10 @@ export function applyReadTool(ctx: Context): void {
|
||||
version: info.version,
|
||||
...window.truncatedByBytes ? { truncatedByBytes: true } : {},
|
||||
}
|
||||
emitObserved(ctx, target, info.version, exec)
|
||||
// Record the observed version (a no-op when no policy plugin listens). The
|
||||
// read already succeeded; an fs/observed listener is contractually a
|
||||
// synchronous, side-effect-only recorder.
|
||||
ctx.emit('fs/observed', target, info.version, exec)
|
||||
return [{ type: 'text', text: formatReadOutput(target.displayPath, outcome) }]
|
||||
},
|
||||
}))
|
||||
|
||||
@@ -1,32 +0,0 @@
|
||||
/**
|
||||
* Vocabulary for the model-facing filesystem tools (`@deepseek-ai/dsh-tool-fs`):
|
||||
* the structured read outcome the `read` tool renders. The read window
|
||||
* (`offset`/`limit`) and per-line shape live in
|
||||
* {@link module:@deepseek-ai/dsh-tool-fs/window}; this file owns the assembled
|
||||
* outcome the tool formats.
|
||||
*
|
||||
* The provider vocabulary (`FsTarget`, `FsVersion`, write/edit shapes) is
|
||||
* re-used from `@deepseek-ai/dsh-fs` — this package owns only the model-facing
|
||||
* read-rendering shape on top of it.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/types
|
||||
*/
|
||||
|
||||
import type { FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
import type { FileTextLine } from './window.ts'
|
||||
|
||||
/** Outcome of a bounded text read — what the model-facing `read` tool renders. */
|
||||
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
|
||||
}
|
||||
@@ -2,8 +2,8 @@
|
||||
* The model-facing `write` tool: create or fully replace a UTF-8 text file. The
|
||||
* tool is the executor: it dispatches the `fs/write-expectation` waterfall to
|
||||
* obtain the optional version guard, calls `ctx.fs.writeText` directly, and
|
||||
* emits a contained `fs/observed`. The default thunk returns `undefined`
|
||||
* (unconditional create-or-overwrite — the bare provider); a policy plugin
|
||||
* emits `fs/observed`. The default thunk returns `undefined` (unconditional
|
||||
* create-or-overwrite — the bare provider); a policy plugin
|
||||
* (`@deepseek-ai/dsh-file-context`) occupies the single decision slot and
|
||||
* returns `createIfAbsent`/`replaceIfVersion` instead. The tool stats ZERO
|
||||
* times either way.
|
||||
@@ -17,7 +17,6 @@ import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import type { FsWriteOutcome } from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { emitObserved } from './observe.ts'
|
||||
|
||||
/** Validate value constraints the schema DSL can't express. */
|
||||
export function parseWriteArgs(args: { file_path: string; content: string }): { filePath: string; content: string } {
|
||||
@@ -57,7 +56,8 @@ export function applyWriteTool(ctx: Context): void {
|
||||
// replaceIfVersion; the bare default is undefined (unconditional). No stat.
|
||||
const expectation = await ctx.waterfall('fs/write-expectation', target, exec, () => undefined)
|
||||
const outcome = await ctx.fs.writeText(target, input.content, expectation, exec.signal)
|
||||
emitObserved(ctx, target, outcome.version, exec)
|
||||
// Record the observed version (a no-op when no policy plugin listens).
|
||||
ctx.emit('fs/observed', target, outcome.version, exec)
|
||||
return [{ type: 'text', text: formatWriteOutput(target.displayPath, outcome) }]
|
||||
},
|
||||
}))
|
||||
|
||||
@@ -223,19 +223,6 @@ describe('default deployment (with dsh-file-context)', () => {
|
||||
statSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
describe('contained fs/observed recording', () => {
|
||||
it('a synchronously throwing fs/observed listener does not fail the completed write', async () => {
|
||||
ctx.on('fs/observed', () => { throw new Error('listener boom') })
|
||||
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {})
|
||||
const result = await call('write', { file_path: 'a.txt', content: 'hi' })
|
||||
// The write succeeded on disk; the listener throw was logged and swallowed.
|
||||
expect(result.isError).toBe(false)
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('hi')
|
||||
expect(warn).toHaveBeenCalled()
|
||||
warn.mockRestore()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
// --------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user