Merge remote-tracking branch 'origin/master' into codex/fs-directory-listing
# Conflicts: # docs/rfc/README.md # packages/fs/fs-local/src/index.ts
This commit is contained in:
Tianyi Cui
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()
|
||||
})
|
||||
})
|
||||
@@ -61,16 +61,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 }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -356,34 +357,141 @@ describe('tool-owned presentation (pure presentCall)', () => {
|
||||
return ctx.tools.get(name)?.presentCall?.(args)
|
||||
}
|
||||
|
||||
it('read: titles by file, read kind, location with the offset line', async () => {
|
||||
it('read: generic card titled by file with the read window, read kind, location with the offset line', async () => {
|
||||
expect(await presentCall('read', { file_path: 'src/a.ts', offset: 12, limit: 40 })).toEqual({
|
||||
title: 'Read src/a.ts', kind: 'read', rawInput: 'offset 12, limit 40',
|
||||
card: 'generic', title: 'Read src/a.ts (12 - 51)', kind: 'read',
|
||||
locations: [{ path: 'src/a.ts', line: 12 }],
|
||||
})
|
||||
})
|
||||
|
||||
it('read: omits rawInput and the location line when offset/limit are unset', async () => {
|
||||
it('read: bare title and line-1 location when offset/limit are unset', async () => {
|
||||
expect(await presentCall('read', { file_path: 'a.txt' })).toEqual({
|
||||
title: 'Read a.txt', kind: 'read', locations: [{ path: 'a.txt' }],
|
||||
card: 'generic', title: 'Read a.txt', kind: 'read', locations: [{ path: 'a.txt', line: 1 }],
|
||||
})
|
||||
})
|
||||
|
||||
it('write: titles by file, edit kind, location', async () => {
|
||||
expect(await presentCall('write', { file_path: 'out.txt', content: 'x' })).toEqual({
|
||||
title: 'Write out.txt', kind: 'edit', locations: [{ path: 'out.txt' }],
|
||||
it('read: "from line N" window when only offset is set', async () => {
|
||||
expect(await presentCall('read', { file_path: 'a.txt', offset: 5 })).toEqual({
|
||||
card: 'generic', title: 'Read a.txt (from line 5)', kind: 'read', locations: [{ path: 'a.txt', line: 5 }],
|
||||
})
|
||||
})
|
||||
|
||||
it('edit: titles by file, edit kind, an old→new rawInput summary, location', async () => {
|
||||
expect(await presentCall('edit', { file_path: 'a.txt', old_string: 'foo', new_string: 'bar' })).toEqual({
|
||||
title: 'Edit a.txt', kind: 'edit', rawInput: '"foo" → "bar"', locations: [{ path: 'a.txt' }],
|
||||
it('write: diff card (new-file style, oldText null), location', async () => {
|
||||
expect(await presentCall('write', { file_path: 'out.txt', content: 'hello' })).toEqual({
|
||||
card: 'diff', title: 'Write out.txt',
|
||||
diffs: [{ path: 'out.txt', oldText: null, newText: 'hello' }],
|
||||
locations: [{ path: 'out.txt' }],
|
||||
})
|
||||
})
|
||||
|
||||
it('edit: clips a long old/new string in the rawInput summary', async () => {
|
||||
const long = 'a'.repeat(60)
|
||||
const p = await presentCall('edit', { file_path: 'a.txt', old_string: long, new_string: 'b' })
|
||||
expect((p as { rawInput: string }).rawInput).toBe(`${JSON.stringify(`${'a'.repeat(40)}…`)} → ${JSON.stringify('b')}`)
|
||||
it('read: a limit with no offset windows from line 1', async () => {
|
||||
expect(await presentCall('read', { file_path: 'a.txt', limit: 10 })).toEqual({
|
||||
card: 'generic', title: 'Read a.txt (1 - 10)', kind: 'read', locations: [{ path: 'a.txt', line: 1 }],
|
||||
})
|
||||
})
|
||||
|
||||
it('edit: an empty old_string maps to oldText null (a whole-file replace diff)', async () => {
|
||||
// presentCall runs on replay of raw logged args, which parseEditArgs does not
|
||||
// gate — an empty old_string must still produce a valid diff (oldText null).
|
||||
expect(await presentCall('edit', { file_path: 'a.txt', old_string: '', new_string: 'seed' })).toEqual({
|
||||
card: 'diff', title: 'Edit a.txt',
|
||||
diffs: [{ path: 'a.txt', oldText: null, newText: 'seed' }],
|
||||
locations: [{ path: 'a.txt' }],
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
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, but presentResult still renders a whole-file diff card', async () => {
|
||||
// A create has no prior content (no `meta`), yet the completed card must be a
|
||||
// `diff` — an ACP tool_call_update.content REPLACES the call's content, so a
|
||||
// non-diff result would clobber the pending new-file diff. The whole-file diff
|
||||
// is derived from the args (oldText:null), replay-safe.
|
||||
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()
|
||||
const view = ctx.tools.get('write')?.presentResult?.({ file_path: 'new.txt', content: 'fresh\n' }, result)
|
||||
expect(view).toEqual({ card: 'diff', title: 'Write new.txt', diffs: [{ path: 'new.txt', oldText: null, newText: 'fresh\n' }] })
|
||||
})
|
||||
|
||||
it('write OVERWRITE with identical content: a before exists but yields no hunk → no meta, presentResult falls back to a whole-file diff', 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()
|
||||
const view = ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: 'same\n' }, result)
|
||||
expect(view).toEqual({ card: 'diff', title: 'Write a.txt', diffs: [{ path: 'a.txt', oldText: null, newText: 'same\n' }] })
|
||||
})
|
||||
|
||||
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('edit presentResult returns undefined on malformed meta (defensive narrowing)', async () => {
|
||||
// edit has no whole-file fallback (only a literal replacement), so a malformed
|
||||
// meta yields the generic "updated successfully" rendering.
|
||||
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()
|
||||
})
|
||||
|
||||
it('write presentResult falls back to a whole-file diff on malformed meta (never leaks the result text)', async () => {
|
||||
// write always renders a diff card so the completed update can't clobber the
|
||||
// pending diff with the model-facing text; a malformed meta falls back to the
|
||||
// args-derived whole-file diff, same as a create.
|
||||
const { ctx } = await setup()
|
||||
const badMeta = { content: [{ type: 'text' as const, text: 'ok' }], isError: false, meta: { diffs: 'nope' } }
|
||||
const view = ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: 'y' }, badMeta)
|
||||
expect(view).toEqual({ card: 'diff', title: 'Write a.txt', diffs: [{ path: 'a.txt', oldText: null, newText: 'y' }] })
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user