feat(tool-fs): result-time applied-hunk diffs for write/edit
fs write/edit now emit a result-time contextual-diff tool_call_update
(the applied hunk with ±3 context lines, one hunk per replace_all site),
matching what claude-agent-acp sends and what makes an editor render the
change in place. The call-time snippet diff stays; the result hunk
supersedes it (ACP content-replace).
Mechanism:
- A persisted tool-private `meta` channel: execute may return
`{ content, meta }`; `meta` (JsonValue) rides on the tool/result event
and is handed back to presentResult, so the diff reproduces on replay
(event-sourced). JsonValue is now exported from dsh-session.
- The backend returns raw before/after text (storage facts) on
FsWriteOutcome/FsEditOutcome; the tool computes the hunk via the npm
`diff` package's structuredPatch. A create has no before → no result
diff; a failed/aborted mutation carries no meta.
- ToolResultView gains a DiffResultView; the bridge's result-side switch
renders it as {type:'diff'} content blocks.
RFC: docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md
(justifies the npm `diff` runtime dep over vendoring and the meta channel);
the render-intent-union RFC's Non-goal is updated to record this shipped.
All fs snapshot goldens re-recorded; edit/overwrite gain the contextual
result diff, create/read/policy-reject unchanged in structure.
This commit is contained in:
@@ -382,6 +382,25 @@ export async function readForEdit(
|
||||
return { content: normalizeLineEndings(raw), lineEndings: detectLineEndings(raw) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort read of a file's current text for a before/after diff basis, used
|
||||
* by an overwrite. Returns the LF-normalized decoded content, or `null` when the
|
||||
* file is binary or not valid UTF-8 — a write must succeed regardless of the
|
||||
* prior bytes, so an undiffable prior file simply yields no contextual diff
|
||||
* (the caller treats `null` the same as an absent file: call-time card only).
|
||||
*/
|
||||
export async function readTextForDiff(absolutePath: string, signal?: AbortSignal): Promise<string | null> {
|
||||
const buffer = await readFileAbortable(absolutePath, 'read', signal)
|
||||
if (buffer.includes(0)) return null
|
||||
try {
|
||||
return normalizeLineEndings(new TextDecoder('utf-8', { fatal: true }).decode(buffer))
|
||||
} catch (error: unknown) {
|
||||
/* v8 ignore next 2 -- TextDecoder({fatal}) only throws TypeError on invalid bytes; any other throw is an unreachable runtime fault. */
|
||||
if (!(error instanceof TypeError)) throw error
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply a literal replacement to LF-normalized content. Throws
|
||||
* `FS_EDIT_NOT_FOUND` on empty `oldString` or zero matches and
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
applyLiteralEdit,
|
||||
probe,
|
||||
readForEdit,
|
||||
readTextForDiff,
|
||||
readWholeText,
|
||||
resolveLocalTarget,
|
||||
restoreLineEndings,
|
||||
@@ -41,6 +42,7 @@ export {
|
||||
applyLiteralEdit,
|
||||
probe,
|
||||
readForEdit,
|
||||
readTextForDiff,
|
||||
readWholeText,
|
||||
resolveLocalTarget,
|
||||
restoreLineEndings,
|
||||
@@ -143,11 +145,18 @@ export class LocalFileSystem extends FileSystem {
|
||||
// provider) — no version guard, no read-first requirement. Still atomic
|
||||
// (the per-target lock is unconditional), so the write is never torn.
|
||||
|
||||
// Capture the prior text (the before/after diff basis) BEFORE the write.
|
||||
// `null` for a create (no existing file) OR an existing-but-undiffable
|
||||
// file (binary/invalid-UTF-8) — a consumer renders no result-time diff for
|
||||
// either, only the call-time whole-file card.
|
||||
const before = existing ? await readTextForDiff(target.targetKey, signal) : null
|
||||
await writeFileAtomic(target.targetKey, content, existing?.mode, signal, this.internals)
|
||||
const after = await probe(target.targetKey)
|
||||
return {
|
||||
operation: existing ? 'update' : 'create',
|
||||
version: this.versionAfterWrite(after, target),
|
||||
before,
|
||||
after: content,
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -183,6 +192,10 @@ export class LocalFileSystem extends FileSystem {
|
||||
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.
|
||||
before: original.content,
|
||||
after: edited.content,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -188,6 +188,49 @@ describe('writeText', () => {
|
||||
await expect(fs.writeText(target, 'x')).rejects.toMatchObject({ code: 'FS_NOT_REGULAR_FILE' })
|
||||
})
|
||||
|
||||
it('a create reports before:null and after = the written content (no prior file)', async () => {
|
||||
const target = await fs.resolve('new.txt')
|
||||
const outcome = await fs.writeText(target, 'fresh')
|
||||
expect(outcome.before).toBeNull()
|
||||
expect(outcome.after).toBe('fresh')
|
||||
})
|
||||
|
||||
it('an overwrite reports before = the OLD content and after = the new content', async () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'old body')
|
||||
const target = await fs.resolve('a.txt')
|
||||
const outcome = await fs.writeText(target, 'new body')
|
||||
expect(outcome.before).toBe('old body')
|
||||
expect(outcome.after).toBe('new body')
|
||||
})
|
||||
|
||||
it('an overwrite of a CRLF file returns LF-normalized before content', async () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'a\r\nb\r\n')
|
||||
const target = await fs.resolve('a.txt')
|
||||
const outcome = await fs.writeText(target, 'a\nB\n')
|
||||
expect(outcome.before).toBe('a\nb\n')
|
||||
})
|
||||
|
||||
it('an overwrite of a BINARY prior file reports before:null (undiffable), still succeeds', async () => {
|
||||
await writeFile(join(dir, 'a.bin'), Buffer.from([0x00, 0x01, 0x02]))
|
||||
const target = await fs.resolve('a.bin')
|
||||
const outcome = await fs.writeText(target, 'now text')
|
||||
expect(outcome.operation).toBe('update')
|
||||
expect(outcome.before).toBeNull()
|
||||
expect(outcome.after).toBe('now text')
|
||||
})
|
||||
|
||||
it('an overwrite of an INVALID-UTF-8 (non-NUL) prior file reports before:null, still succeeds', async () => {
|
||||
// 0xff is never valid UTF-8 but is not a NUL, so it exercises the decoder's
|
||||
// fatal-throw path (not the NUL-scan short-circuit): an undiffable prior file
|
||||
// still yields a successful write with no before-content basis.
|
||||
await writeFile(join(dir, 'a.bin'), Buffer.from([0x68, 0xff, 0x69]))
|
||||
const target = await fs.resolve('a.bin')
|
||||
const outcome = await fs.writeText(target, 'now valid')
|
||||
expect(outcome.operation).toBe('update')
|
||||
expect(outcome.before).toBeNull()
|
||||
expect(outcome.after).toBe('now valid')
|
||||
})
|
||||
|
||||
it('releases per-target mutation locks after success and failure', async () => {
|
||||
const target = await fs.resolve('a.txt')
|
||||
await fs.writeText(target, 'created', { kind: 'createIfAbsent' })
|
||||
@@ -242,6 +285,17 @@ describe('editText', () => {
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('hello there')
|
||||
})
|
||||
|
||||
it('reports before/after content (the applied-hunk basis), LF-normalized', async () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'a\r\nOLD\r\nb\r\n')
|
||||
const target = await fs.resolve('a.txt')
|
||||
const outcome = await fs.editText(target, { oldString: 'OLD', newString: 'NEW', replaceAll: false })
|
||||
expect(outcome.before).toBe('a\nOLD\nb\n')
|
||||
expect(outcome.after).toBe('a\nNEW\nb\n')
|
||||
// The written file keeps the original CRLF endings (before/after are the
|
||||
// LF-normalized diff basis, not the on-disk bytes).
|
||||
expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('a\r\nNEW\r\nb\r\n')
|
||||
})
|
||||
|
||||
it('checks the stale version BEFORE literal matching', async () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'hello world')
|
||||
const target = await fs.resolve('a.txt')
|
||||
|
||||
@@ -101,6 +101,14 @@ export interface FsWriteOutcome {
|
||||
operation: 'create' | 'update'
|
||||
/** Opaque version of the file after the write. */
|
||||
version: FsVersion
|
||||
/**
|
||||
* The file's content BEFORE the write, or `null` when the file did not exist
|
||||
* (a create). Raw storage text (LF-normalized by the backend), never a diff —
|
||||
* a consumer computes the result-time contextual diff from `before`/`after`.
|
||||
*/
|
||||
before: string | null
|
||||
/** The file's content AFTER the write (the text that was written). */
|
||||
after: string
|
||||
}
|
||||
|
||||
/** A literal-replacement edit request. */
|
||||
@@ -121,6 +129,14 @@ export interface FsEditOutcome {
|
||||
replaceAll: boolean
|
||||
/** Opaque version of the file after the edit. */
|
||||
version: FsVersion
|
||||
/**
|
||||
* The file's content BEFORE the edit. Raw storage text (LF-normalized by the
|
||||
* backend), never a diff — a consumer computes the result-time contextual diff
|
||||
* (the applied hunk with context) from `before`/`after`.
|
||||
*/
|
||||
before: string
|
||||
/** The file's content AFTER the edit. */
|
||||
after: string
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -39,14 +39,15 @@ class FakeFileSystem extends FileSystem {
|
||||
return (async function* () { yield content })()
|
||||
}
|
||||
override async writeText(target: FsTarget, content: string, _expected?: FsWriteIntent): Promise<FsWriteOutcome> {
|
||||
const existed = this.files.has(target.targetKey)
|
||||
const before = this.files.get(target.targetKey) ?? null
|
||||
this.files.set(target.targetKey, content)
|
||||
return { operation: existed ? 'update' : 'create', version: FsVersion('v2') }
|
||||
return { operation: before !== null ? 'update' : 'create', version: FsVersion('v2'), before, after: content }
|
||||
}
|
||||
override async editText(target: FsTarget, edit: FsEditRequest): Promise<FsEditOutcome> {
|
||||
const content = this.files.get(target.targetKey) ?? ''
|
||||
this.files.set(target.targetKey, content.split(edit.oldString).join(edit.newString))
|
||||
return { replacements: 1, replaceAll: edit.replaceAll, version: FsVersion('v3') }
|
||||
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 }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -21,9 +21,13 @@
|
||||
"src"
|
||||
],
|
||||
"license": "BSD-3-Clause",
|
||||
"dependencies": {
|
||||
"diff": "^9.0.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@deepseek-ai/dsh-fs": "^0.0.1",
|
||||
"@deepseek-ai/dsh-llm": "^0.0.1",
|
||||
"@deepseek-ai/dsh-session": "^0.0.1",
|
||||
"@deepseek-ai/dsh-system-prompt": "^0.0.1",
|
||||
"@deepseek-ai/dsh-tools": "^0.0.1",
|
||||
"cordis": "^4.0.0-rc.6"
|
||||
|
||||
92
packages/fs/tool-fs/src/diff.ts
Normal file
92
packages/fs/tool-fs/src/diff.ts
Normal file
@@ -0,0 +1,92 @@
|
||||
/**
|
||||
* Result-time contextual-diff computation for the `write`/`edit` tools. Turns a
|
||||
* before/after pair of file texts into one {@link FileDiff} per applied hunk —
|
||||
* each hunk's `oldText`/`newText` reconstructed from the unified-diff lines with
|
||||
* ±{@link DIFF_CONTEXT} surrounding context lines, matching how claude-agent-acp
|
||||
* renders an editor inline diff.
|
||||
*
|
||||
* This is display-only presentation vocabulary (a UI concern), so it lives in
|
||||
* the model-facing tool, NOT the `dsh-fs` storage seam — the backend returns
|
||||
* only the raw before/after text (storage facts) and the tool computes the diff.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/src/diff
|
||||
*/
|
||||
|
||||
import { structuredPatch } from 'diff'
|
||||
import type { FileDiff } from '@deepseek-ai/dsh-tools'
|
||||
import type { JsonValue } from '@deepseek-ai/dsh-session'
|
||||
|
||||
/** Context lines shown on each side of an applied hunk (matches claude-agent-acp). */
|
||||
export const DIFF_CONTEXT = 3
|
||||
|
||||
/**
|
||||
* The `write`/`edit` tools' private `tool/result` `meta` payload: the applied
|
||||
* contextual-diff hunks. A {@link JsonValue} (persisted with the session log, so
|
||||
* `presentResult` reproduces the diff card on replay). The producing tool owns
|
||||
* this shape; the bridge only sees the opaque `meta` and the tool narrows it back
|
||||
* via {@link diffsFromMeta}.
|
||||
*/
|
||||
export type FsDiffMeta = { diffs: FileDiff[] }
|
||||
|
||||
/**
|
||||
* Compute one {@link FileDiff} per hunk between `before` and `after`, each
|
||||
* carrying the applied change plus {@link DIFF_CONTEXT} context lines. Returns an
|
||||
* empty array when the texts are identical (no hunks). For a scattered
|
||||
* `replace_all` edit the patch yields multiple hunks, so multiple `FileDiff`s
|
||||
* come back — matching the editor rendering one diff block per site.
|
||||
*
|
||||
* Each hunk's `oldText` is its `-` (removed) and context lines joined by `\n`;
|
||||
* `newText` is its `+` (added) and context lines. A hunk with no old lines
|
||||
* (a pure insertion) reports `oldText: null` (nothing to diff against), mirroring
|
||||
* the call-time card's new-file convention. The unified-diff "\ No newline at end
|
||||
* of file" markers are dropped — they annotate the patch, not file content.
|
||||
*/
|
||||
export function computeHunkDiffs(path: string, before: string, after: string): FileDiff[] {
|
||||
const patch = structuredPatch('', '', before, after, undefined, undefined, { context: DIFF_CONTEXT })
|
||||
const diffs: FileDiff[] = []
|
||||
for (const hunk of patch.hunks) {
|
||||
const oldLines: string[] = []
|
||||
const newLines: string[] = []
|
||||
for (const line of hunk.lines) {
|
||||
// The unified-diff marker for a missing trailing newline annotates the
|
||||
// patch, not the content — skip it so it never leaks into a diff block.
|
||||
if (line.startsWith('\\')) continue
|
||||
const text = line.slice(1)
|
||||
if (line.startsWith('-')) {
|
||||
oldLines.push(text)
|
||||
} else if (line.startsWith('+')) {
|
||||
newLines.push(text)
|
||||
} else {
|
||||
// A context (unchanged) line appears on both sides.
|
||||
oldLines.push(text)
|
||||
newLines.push(text)
|
||||
}
|
||||
}
|
||||
diffs.push({ path, oldText: oldLines.length > 0 ? oldLines.join('\n') : null, newText: newLines.join('\n') })
|
||||
}
|
||||
return diffs
|
||||
}
|
||||
|
||||
/** Whether `value` is a valid {@link FileDiff} (defensive narrowing from opaque `meta`). */
|
||||
function isFileDiff(value: JsonValue): value is FileDiff & JsonValue {
|
||||
if (typeof value !== 'object' || value === null || Array.isArray(value)) return false
|
||||
const { path, oldText, newText } = value
|
||||
return typeof path === 'string'
|
||||
&& (oldText === null || typeof oldText === 'string')
|
||||
&& typeof newText === 'string'
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrow an opaque `tool/result` `meta` back to this tool's {@link FileDiff}
|
||||
* hunks, or `undefined` when it is absent/malformed. `presentResult` runs on
|
||||
* arbitrary logged `meta` (possibly from an older shape or a hand-edited log), so
|
||||
* it validates defensively rather than trusting the payload — a bad `meta` yields
|
||||
* no diff card (the generic result rendering) instead of a thrown presenter.
|
||||
*/
|
||||
export function diffsFromMeta(meta: JsonValue | undefined): FileDiff[] | undefined {
|
||||
if (typeof meta !== 'object' || meta === null || Array.isArray(meta)) return undefined
|
||||
const diffs = meta.diffs
|
||||
if (!Array.isArray(diffs) || diffs.length === 0 || !diffs.every(isFileDiff)) return undefined
|
||||
return diffs
|
||||
}
|
||||
|
||||
@@ -14,11 +14,12 @@
|
||||
|
||||
import type { Context } from 'cordis'
|
||||
import { defineTool } from '@deepseek-ai/dsh-tools'
|
||||
import type { DiffCallView } 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'
|
||||
import { sessionCwd } from './session-cwd.ts'
|
||||
|
||||
/** Validated `edit` arguments after defaulting. */
|
||||
@@ -66,7 +67,7 @@ export function applyEditTool(ctx: Context): void {
|
||||
new_string: { type: 'string', required: true, description: 'Literal replacement text. Use an empty string to delete the match.' },
|
||||
replace_all: { type: 'boolean', description: 'Replace all matches. Defaults to false; when false, old_string must appear exactly once.' },
|
||||
},
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
async execute(args, exec): Promise<{ content: ContentBlock[]; meta?: FsDiffMeta }> {
|
||||
const input = parseEditArgs(args)
|
||||
const cwd = sessionCwd(exec)
|
||||
const target = await ctx.fs.resolve(input.filePath, cwd !== undefined ? { cwd } : undefined)
|
||||
@@ -82,7 +83,17 @@ export function applyEditTool(ctx: Context): void {
|
||||
)
|
||||
// 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) }]
|
||||
// The result-time applied-hunk diff (before→after with context lines). An
|
||||
// edit always changes content (parseEditArgs requires old_string to differ
|
||||
// and editText matches at least once), so there is always at least one hunk.
|
||||
// The bridge renders these as an inline diff that supersedes the call-time
|
||||
// snippet; the display path is the model-facing `file_path` (the bridge
|
||||
// relativizes it).
|
||||
const diffs = computeHunkDiffs(input.filePath, outcome.before, outcome.after)
|
||||
return {
|
||||
content: [{ type: 'text', text: formatEditOutput(target.displayPath, outcome) }],
|
||||
meta: { diffs },
|
||||
}
|
||||
},
|
||||
// Pure display: a diff card of the literal replacement (old_string →
|
||||
// new_string), derived from the call args. `oldText: old_string || null`
|
||||
@@ -96,5 +107,15 @@ export function applyEditTool(ctx: Context): void {
|
||||
locations: [{ path: args.file_path }],
|
||||
}
|
||||
},
|
||||
// Result-time display: the applied contextual-diff hunks carried on `meta`.
|
||||
// On success with diffs, a `diff` result card supersedes the call-time
|
||||
// snippet; on error (nothing applied) or malformed meta, fall through to the
|
||||
// generic "updated successfully" rendering.
|
||||
presentResult(args, result: ToolResult): DiffResultView | undefined {
|
||||
if (result.isError) return undefined
|
||||
const diffs = diffsFromMeta(result.meta)
|
||||
if (diffs === undefined) return undefined
|
||||
return { card: 'diff', title: `Edit ${args.file_path}`, diffs }
|
||||
},
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -32,6 +32,8 @@ export { applyWriteTool, formatWriteOutput, parseWriteArgs } from './write.ts'
|
||||
export { applyEditTool, formatEditOutput, parseEditArgs } from './edit.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'
|
||||
export { DIFF_CONTEXT, computeHunkDiffs, diffsFromMeta } from './diff.ts'
|
||||
export type { FsDiffMeta } from './diff.ts'
|
||||
|
||||
/** Cordis plugin name used by loader diagnostics. */
|
||||
export const name = 'tool-fs'
|
||||
|
||||
@@ -13,11 +13,12 @@
|
||||
|
||||
import type { Context } from 'cordis'
|
||||
import { defineTool } from '@deepseek-ai/dsh-tools'
|
||||
import type { DiffCallView } 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 { FsWriteOutcome } 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'
|
||||
import { sessionCwd } from './session-cwd.ts'
|
||||
|
||||
/** Validate value constraints the schema DSL can't express. */
|
||||
@@ -51,7 +52,7 @@ export function applyWriteTool(ctx: Context): void {
|
||||
file_path: { type: 'string', required: true, description: 'Path to write, resolved by the filesystem backend.' },
|
||||
content: { type: 'string', required: true, description: 'Full UTF-8 text content to write.' },
|
||||
},
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
async execute(args, exec): Promise<{ content: ContentBlock[]; meta?: FsDiffMeta }> {
|
||||
const input = parseWriteArgs(args)
|
||||
const cwd = sessionCwd(exec)
|
||||
const target = await ctx.fs.resolve(input.filePath, cwd !== undefined ? { cwd } : undefined)
|
||||
@@ -61,7 +62,14 @@ export function applyWriteTool(ctx: Context): void {
|
||||
const outcome = await ctx.fs.writeText(target, input.content, intent, exec.signal)
|
||||
// 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) }]
|
||||
// Result-time contextual diff ONLY for an overwrite (a before-version
|
||||
// exists). A create has no "before" — `outcome.before` is null — so it
|
||||
// carries no result diff, leaving just the call-time whole-file card.
|
||||
const diffs = outcome.before !== null ? computeHunkDiffs(input.filePath, outcome.before, outcome.after) : []
|
||||
return {
|
||||
content: [{ type: 'text', text: formatWriteOutput(target.displayPath, outcome) }],
|
||||
...diffs.length > 0 ? { meta: { diffs } } : {},
|
||||
}
|
||||
},
|
||||
// Pure display: a diff card (an editor renders write as a new-file / full-
|
||||
// replace diff). `oldText: null` — a call-time presenter has no access to the
|
||||
@@ -75,5 +83,15 @@ export function applyWriteTool(ctx: Context): void {
|
||||
locations: [{ path: args.file_path }],
|
||||
}
|
||||
},
|
||||
// Result-time display: for an OVERWRITE, the applied contextual-diff hunks on
|
||||
// `meta` supersede the call-time whole-file snippet. A create carries no meta
|
||||
// (no "before"), so this returns undefined and the call-time new-file card
|
||||
// stands; an error or malformed meta also falls through to generic rendering.
|
||||
presentResult(args, result: ToolResult): DiffResultView | undefined {
|
||||
if (result.isError) return undefined
|
||||
const diffs = diffsFromMeta(result.meta)
|
||||
if (diffs === undefined) return undefined
|
||||
return { card: 'diff', title: `Write ${args.file_path}`, diffs }
|
||||
},
|
||||
}))
|
||||
}
|
||||
|
||||
113
packages/fs/tool-fs/tests/diff.spec.ts
Normal file
113
packages/fs/tool-fs/tests/diff.spec.ts
Normal file
@@ -0,0 +1,113 @@
|
||||
/**
|
||||
* Unit tests for the result-time contextual-diff computation (`src/diff.ts`):
|
||||
* the pure before/after → {@link FileDiff}[] hunk builder and the defensive
|
||||
* `meta` narrowing. These pin the exact hunk reconstruction (context lines,
|
||||
* multi-hunk replaceAll, pure insertion/deletion, no-op) the ACP bridge renders.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { computeHunkDiffs, diffsFromMeta, DIFF_CONTEXT } from '@deepseek-ai/dsh-tool-fs'
|
||||
import type { JsonValue } from '@deepseek-ai/dsh-session'
|
||||
|
||||
const lines = (n: number): string => Array.from({ length: n }, (_, i) => `line${i + 1}`).join('\n') + '\n'
|
||||
|
||||
describe('computeHunkDiffs', () => {
|
||||
it('a single-line change yields one hunk with ±context lines on both sides', () => {
|
||||
const before = lines(8)
|
||||
const after = before.replace('line4', 'CHANGED')
|
||||
const diffs = computeHunkDiffs('f.txt', before, after)
|
||||
expect(diffs).toEqual([{
|
||||
path: 'f.txt',
|
||||
oldText: 'line1\nline2\nline3\nline4\nline5\nline6\nline7',
|
||||
newText: 'line1\nline2\nline3\nCHANGED\nline5\nline6\nline7',
|
||||
}])
|
||||
})
|
||||
|
||||
it('a scattered replace_all yields one FileDiff PER hunk (matching per-site editor blocks)', () => {
|
||||
const before = lines(20)
|
||||
const after = before.replace('line3', 'A').replace('line16', 'B')
|
||||
const diffs = computeHunkDiffs('f.txt', before, after)
|
||||
expect(diffs).toHaveLength(2)
|
||||
expect(diffs[0]?.path).toBe('f.txt')
|
||||
expect(diffs[0]?.oldText).toContain('line3')
|
||||
expect(diffs[0]?.newText).toContain('A')
|
||||
expect(diffs[1]?.oldText).toContain('line16')
|
||||
expect(diffs[1]?.newText).toContain('B')
|
||||
// The two hunks are distinct sites, not one merged block.
|
||||
expect(diffs[0]?.newText).not.toContain('B')
|
||||
expect(diffs[1]?.newText).not.toContain('A')
|
||||
})
|
||||
|
||||
it('identical before/after (a no-op) yields no hunks', () => {
|
||||
expect(computeHunkDiffs('f.txt', 'same\n', 'same\n')).toEqual([])
|
||||
})
|
||||
|
||||
it('a pure insertion into empty content reports oldText null (nothing to diff against)', () => {
|
||||
const diffs = computeHunkDiffs('f.txt', '', 'brand new\n')
|
||||
expect(diffs).toEqual([{ path: 'f.txt', oldText: null, newText: 'brand new' }])
|
||||
})
|
||||
|
||||
it('a pure deletion of the whole file reports newText empty', () => {
|
||||
const diffs = computeHunkDiffs('f.txt', 'gone\n', '')
|
||||
expect(diffs).toEqual([{ path: 'f.txt', oldText: 'gone', newText: '' }])
|
||||
})
|
||||
|
||||
it('drops the "\\ No newline at end of file" marker from a no-trailing-newline change', () => {
|
||||
const diffs = computeHunkDiffs('f.txt', 'x', 'y')
|
||||
// The marker line (starting with "\\") must never leak into a diff block.
|
||||
expect(diffs).toEqual([{ path: 'f.txt', oldText: 'x', newText: 'y' }])
|
||||
expect(diffs[0]?.oldText).not.toContain('\\')
|
||||
expect(diffs[0]?.newText).not.toContain('\\')
|
||||
})
|
||||
|
||||
it('uses DIFF_CONTEXT (3) surrounding lines', () => {
|
||||
expect(DIFF_CONTEXT).toBe(3)
|
||||
const before = lines(20)
|
||||
const after = before.replace('line10', 'CHANGED')
|
||||
const [diff] = computeHunkDiffs('f.txt', before, after)
|
||||
// 3 context above (7,8,9) + the change + 3 below (11,12,13) = 7 lines a side.
|
||||
expect(diff?.oldText?.split('\n')).toHaveLength(7)
|
||||
expect(diff?.newText.split('\n')).toHaveLength(7)
|
||||
expect(diff?.oldText?.split('\n')[0]).toBe('line7')
|
||||
})
|
||||
})
|
||||
|
||||
describe('diffsFromMeta (defensive narrowing)', () => {
|
||||
// The narrowing accepts an opaque JsonValue; a malformed payload is not a
|
||||
// statically-valid JsonValue, so route every case through one cast helper that
|
||||
// mirrors how a hand-edited/older session log delivers arbitrary shapes.
|
||||
const m = (value: unknown): JsonValue | undefined => value as JsonValue | undefined
|
||||
const good = { diffs: [{ path: 'f.txt', oldText: 'a', newText: 'b' }] }
|
||||
|
||||
it('narrows a well-formed { diffs } payload', () => {
|
||||
expect(diffsFromMeta(m(good))).toEqual(good.diffs)
|
||||
})
|
||||
|
||||
it('accepts a diff whose oldText is null (a create-style hunk)', () => {
|
||||
const meta = { diffs: [{ path: 'f.txt', oldText: null, newText: 'x' }] }
|
||||
expect(diffsFromMeta(m(meta))).toEqual(meta.diffs)
|
||||
})
|
||||
|
||||
it('rejects undefined / non-object / array meta', () => {
|
||||
expect(diffsFromMeta(undefined)).toBeUndefined()
|
||||
expect(diffsFromMeta(null)).toBeUndefined()
|
||||
expect(diffsFromMeta(m('nope'))).toBeUndefined()
|
||||
expect(diffsFromMeta(m([]))).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects a missing / empty / non-array diffs field', () => {
|
||||
expect(diffsFromMeta(m({}))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: [] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: 'x' }))).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects a diffs array containing a malformed entry', () => {
|
||||
expect(diffsFromMeta(m({ diffs: [{ path: 'f.txt', oldText: 'a' }] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: [{ path: 1, oldText: 'a', newText: 'b' }] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: [{ path: 'f', oldText: 5, newText: 'b' }] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: [{ path: 'f', oldText: 'a', newText: 7 }] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: [null] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: ['x'] }))).toBeUndefined()
|
||||
expect(diffsFromMeta(m({ diffs: [[]] }))).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -57,16 +57,17 @@ class FakeFs extends FileSystem {
|
||||
override async writeText(target: FsTarget, content: string, expected?: FsWriteIntent): Promise<FsWriteOutcome> {
|
||||
this.throwIfArmed()
|
||||
this.writeIntents.push(expected)
|
||||
const existed = this.files.has(target.targetKey)
|
||||
const before = this.files.get(target.targetKey) ?? null
|
||||
this.files.set(target.targetKey, content)
|
||||
return { operation: existed ? 'update' : 'create', version: FsVersion('v2') }
|
||||
return { operation: before !== null ? 'update' : 'create', version: FsVersion('v2'), before, after: content }
|
||||
}
|
||||
override async editText(target: FsTarget, edit: FsEditRequest, expected?: { version: FsVersion }): Promise<FsEditOutcome> {
|
||||
this.throwIfArmed()
|
||||
this.editIntents.push(expected)
|
||||
const content = this.files.get(target.targetKey) ?? ''
|
||||
this.files.set(target.targetKey, content.split(edit.oldString).join(edit.newString))
|
||||
return { replacements: 1, replaceAll: edit.replaceAll, version: FsVersion('v3') }
|
||||
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 }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -395,3 +396,80 @@ describe('tool-owned presentation (pure presentCall)', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('result-time contextual diff (meta + presentResult)', () => {
|
||||
// An edit records the applied contextual hunk on `tool/result` meta, and the
|
||||
// tool's presentResult narrows it back into a `diff` result card the bridge
|
||||
// renders. Drive execute end-to-end so the meta is the REAL computed hunk.
|
||||
const withContext = 'a\nb\nc\nOLD\nd\ne\nf\n'
|
||||
|
||||
it('edit: execute attaches the applied hunk as meta { diffs }', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const session = { header: {} }
|
||||
fs.files.set('key:a.txt', withContext)
|
||||
await call(ctx, 'read', { file_path: 'a.txt' }, { session })
|
||||
const result = await call(ctx, 'edit', { file_path: 'a.txt', old_string: 'OLD', new_string: 'NEW' }, { session })
|
||||
expect(result.isError).toBe(false)
|
||||
expect(result.meta).toEqual({
|
||||
diffs: [{ path: 'a.txt', oldText: 'a\nb\nc\nOLD\nd\ne\nf', newText: 'a\nb\nc\nNEW\nd\ne\nf' }],
|
||||
})
|
||||
})
|
||||
|
||||
it('edit: presentResult turns the meta into a diff result card', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const session = { header: {} }
|
||||
fs.files.set('key:a.txt', withContext)
|
||||
await call(ctx, 'read', { file_path: 'a.txt' }, { session })
|
||||
const result = await call(ctx, 'edit', { file_path: 'a.txt', old_string: 'OLD', new_string: 'NEW' }, { session })
|
||||
const view = ctx.tools.get('edit')?.presentResult?.({ file_path: 'a.txt', old_string: 'OLD', new_string: 'NEW' }, result)
|
||||
expect(view).toEqual({
|
||||
card: 'diff', title: 'Edit a.txt',
|
||||
diffs: [{ path: 'a.txt', oldText: 'a\nb\nc\nOLD\nd\ne\nf', newText: 'a\nb\nc\nNEW\nd\ne\nf' }],
|
||||
})
|
||||
})
|
||||
|
||||
it('write OVERWRITE: execute attaches a contextual hunk; presentResult renders a diff card', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const session = { header: {} }
|
||||
fs.files.set('key:a.txt', withContext)
|
||||
await call(ctx, 'read', { file_path: 'a.txt' }, { session })
|
||||
const result = await call(ctx, 'write', { file_path: 'a.txt', content: 'a\nb\nc\nNEW\nd\ne\nf\n' }, { session })
|
||||
expect(result.isError).toBe(false)
|
||||
expect(result.meta).toEqual({ diffs: [{ path: 'a.txt', oldText: 'a\nb\nc\nOLD\nd\ne\nf', newText: 'a\nb\nc\nNEW\nd\ne\nf' }] })
|
||||
const view = ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: 'x' }, result)
|
||||
expect(view).toEqual({ card: 'diff', title: 'Write a.txt', diffs: [{ path: 'a.txt', oldText: 'a\nb\nc\nOLD\nd\ne\nf', newText: 'a\nb\nc\nNEW\nd\ne\nf' }] })
|
||||
})
|
||||
|
||||
it('write CREATE: no before-version → no meta, presentResult returns undefined (call-time card stands)', async () => {
|
||||
const { ctx } = await setup()
|
||||
const session = { header: {} }
|
||||
const result = await call(ctx, 'write', { file_path: 'new.txt', content: 'fresh\n' }, { session })
|
||||
expect(result.isError).toBe(false)
|
||||
expect(result.meta).toBeUndefined()
|
||||
expect(ctx.tools.get('write')?.presentResult?.({ file_path: 'new.txt', content: 'fresh\n' }, result)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('write OVERWRITE with identical content: a before exists but yields no hunk → no meta', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const session = { header: {} }
|
||||
fs.files.set('key:a.txt', 'same\n')
|
||||
await call(ctx, 'read', { file_path: 'a.txt' }, { session })
|
||||
const result = await call(ctx, 'write', { file_path: 'a.txt', content: 'same\n' }, { session })
|
||||
expect(result.isError).toBe(false)
|
||||
expect(result.meta).toBeUndefined()
|
||||
})
|
||||
|
||||
it('presentResult returns undefined on an error result (nothing applied)', async () => {
|
||||
const { ctx } = await setup()
|
||||
const errorResult = { content: [{ type: 'text' as const, text: 'Error: boom' }], isError: true }
|
||||
expect(ctx.tools.get('edit')?.presentResult?.({ file_path: 'a.txt', old_string: 'x', new_string: 'y' }, errorResult)).toBeUndefined()
|
||||
expect(ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: 'y' }, errorResult)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('presentResult returns undefined on malformed meta (defensive narrowing)', async () => {
|
||||
const { ctx } = await setup()
|
||||
const badMeta = { content: [{ type: 'text' as const, text: 'ok' }], isError: false, meta: { diffs: 'nope' } }
|
||||
expect(ctx.tools.get('edit')?.presentResult?.({ file_path: 'a.txt', old_string: 'x', new_string: 'y' }, badMeta)).toBeUndefined()
|
||||
expect(ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: 'y' }, badMeta)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user