Coder/deepseek-harness
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tool-fs): write always renders a diff card on the completed update

Browse Source 
A Write CREATE rendered its completed tool_call_update as the model-facing
result TEXT (`<path>…</path>…Created file`), which — because an ACP
tool_call_update.content REPLACES the call's content — clobbered the
new-file diff the pending call installed. So Zed showed the diff, then
replaced it with raw XML-ish text; only overwrite/edit looked right
(their result re-sends a diff).

write's presentResult now ALWAYS returns a diff card for a successful
write: the applied contextual hunk from `meta` when there is one
(overwrite), else an args-derived whole-file diff (`oldText: null`) for a
create or an unchanged-content overwrite. This matches claude-agent-acp,
where the create diff rides on the update and no result text replaces it.
An error still falls through to generic rendering so its message shows.
edit is unchanged (it always has a hunk; no whole-file fallback).

Re-recorded fs-write / fs-write-overwrite goldens; the create's completed
update is now a {type:'diff'} block, not the XML result text.
This commit is contained in:
Tianyi Cui
2026-07-03 20:52:36 +08:00
parent 57923c68ee
commit 53b215c646
7 changed files with 292 additions and 308 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
packages/fs/tool-fs/src/write.ts
View File

@@ -83,14 +83,18 @@ 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.
// Result-time display: a `diff` card so the completed `tool_call_update`
// re-installs the diff rather than the model-facing result text (an ACP
// `tool_call_update.content` REPLACES the call's content, so a text result
// would clobber the pending diff card). An OVERWRITE uses the applied
// contextual hunks on `meta`; a CREATE has no `meta` (no prior content), so
// its whole-file new-file diff is derived from `args.content` (replay-safe,
// matching the call-time card). An error falls through to generic rendering
// so its message shows.
presentResult(args, result: ToolResult): DiffResultView | undefined {
if (result.isError) return undefined
const diffs = diffsFromMeta(result.meta)
if (diffs === undefined) return undefined
?? [{ path: args.file_path, oldText: null, newText: args.content }]
return { card: 'diff', title: `Write ${args.file_path}`, diffs }
},
}))

28
packages/fs/tool-fs/tests/tools.spec.ts
View File

@@ -440,16 +440,21 @@ describe('result-time contextual diff (meta + presentResult)', () => {
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 () => {
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()
expect(ctx.tools.get('write')?.presentResult?.({ file_path: 'new.txt', content: 'fresh\n' }, result)).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', async () => {
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')
@@ -457,6 +462,8 @@ describe('result-time contextual diff (meta + presentResult)', () => {
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 () => {
@@ -466,10 +473,21 @@ describe('result-time contextual diff (meta + presentResult)', () => {
expect(ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: 'y' }, errorResult)).toBeUndefined()
})
it('presentResult returns undefined on malformed meta (defensive narrowing)', async () => {
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()
expect(ctx.tools.get('write')?.presentResult?.({ file_path: 'a.txt', content: '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' }] })
})
})