Skip to content

fix(acp): report tool names and full-file diffs - #53360

Merged
nexxeln merged 2 commits into
v2from
acp-tool-content
Oct 6, 2026
Merged

nexxeln merged 2 commits into
v2from
acp-tool-content

Conversation

@nexxeln

@nexxeln nexxeln commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Part of #52636

Why the change

Completed tool calls built their ACP diff from the edit tool's oldString/newString snippets, so clients that draw inline diffs from that content showed partial or wrong diffs for edits and nothing for the patch tool or plugin edit tools. After this change, diffs are built from the file changes core already reports, and the first tool_call carries the tool's name, which the spec says agents SHOULD include.

Special things to note

  • The write tool still gets no completed diff. Core's write tool returns no metadata.files on success, unlike edit (core/src/tool/plugin/write.ts). Fixing that needs a small core change, so this PR leaves ACP: completed tool_call_update diff content is derived from tool input schema, not result metadata — write/patch and plugin edit tools never emit diffs #52636 open. Plugins that report diffs in another metadata shape also still get none.
  • On session/load, diffs are rebuilt against the file as it is now. If the file changed near an edit since then, reversing the patch fails and that call replays without a diff. If it changed elsewhere, the highlighted change is still exactly the tool's, but the unchanged lines around it come from today's file. Snippet diffs are gone, so edits migrated from V1, which store no files, replay without a diff.
  • Core trims indentation from the patch tool's recorded diffs, so applyPatch can't reverse them on indented code. For those files, the diff is rebuilt from the tool's own hunks and kept only if its @@ ranges match the recorded patch. A completed patch-tool delete of a trimmed file gets no diff, because delete hunks have no body.

Change outline

A completed update's diff now comes from core's FileDiff metadata, read against the file on disk. After a successful tool call, the disk holds the new text, so the old text is rebuilt by reversing the recorded patch.

withCompletedDiffs(update, source: { toolName, input, metadata }, cwd)
  for each metadata.files entry, independently (a failure drops only that file)
    added    -> oldText null, newText from disk
    deleted  -> oldText from reversing the patch against "", newText ""
    modified -> newText from disk
                oldText = applyPatch(newText, reversePatch(recorded))
                if that fails and the tool is patch:
                  rebuild from the tool's hunks, keep only if @@ ranges match
  append the diffs to the update's content

translate.fold stays pure. It attaches the source to the output for session.tool.success, and the file read happens where updates are sent. Permission previews don't change.

 ACPTranslate.fold
   session.tool.success
-    send(completedToolUpdate(...))          # diff from oldString/newString
+    send(completedToolUpdate(...)) + diff source   # no diff yet

 ACPTurn.interpret (SessionUpdate and child "update")
+  ACPPermission.withCompletedDiffs(update, source, cwd)
   send

 ACPReplay.history
+  withCompletedDiffs(update, source from the stored tool part, cwd)
   send

name is the raw tool name, not the alias-mapped one. It goes on the first tool_call, both live and on replay. A permission ask for a known tool repeats it. An ask without a known tool leaves it out, because a permission action such as external_directory isn't a tool.

 pendingToolCall -> ToolCall
   toolCallId, title, kind, status: "pending", ...
+  name: toolName

Tests build fixture diffs with createTwoFilesPatch, as core does, through a shared fileDiff helper in wire-fixture.ts:

packages/cli/test/acp/
├── prompt.test.ts       # edit diff (CRLF); patch with a trimmed indented hunk; formatter change;
│                        # added and deleted files; a missing file; snippet input sends no diff
├── replay.test.ts       # name and an edit diff on session/load
├── permission.test.ts   # name only when the tool is known
├── translate.test.ts    # name on the pending tool call
└── wire-fixture.ts      # shared fileDiff helper

Completed tool diffs come from FileDiff metadata, reversed against the file already on disk. Patch tools still use their own hunks. The first tool_call carries the raw tool name.
Completed diffs apply the recorded FileDiff patch first. Patch tools rebuild from hunks only when that reverse fails and the hunk ranges still match.
@nexxeln
nexxeln merged commit f6ae986 into v2 Oct 6, 2026
11 checks passed
@nexxeln
nexxeln deleted the acp-tool-content branch October 6, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant