Skip to content

fix(agent-chat): show ACP paths and diffs for tool calls - #15908

Merged
teamleaderleo merged 2 commits into
mainfrom
parity/acp-tool-detail
Sep 30, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
parity/acp-tool-detail

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

ACP tool-call transcript blocks now show the files an ACP tool touched and useful output for edits and terminals. The existing tool block and rendering path remain unchanged.

Summary

  • tool-start uses ACP location paths when present and keeps the raw-input JSON fallback when they are absent.
  • tool-end renders diff paths with added and removed line counts, includes the new text, and names terminal IDs. The existing 400-character truncation remains in place.
  • Added a stdio fake ACP process and regression coverage for location paths, raw-input fallback, added-file diffs, line counts, and mixed content.

Verification

The focused regression was committed before the fix and run with the same command.

Test commit 6e87bf05564 (bun test test/acp-tool-detail.test.ts):

FAIL 1 · Tests
error: expect(received).toEqual(expected)
0 pass
1 fail
Ran 1 test across 1 file.

Fix commit d3108259e98 (bun test test/acp-tool-detail.test.ts):

1 pass
0 fail
9 expect() calls
Ran 1 test across 1 file.

Full package verification with cd agent-chat && bun install && bun run check passed: 22 script checks and 43 tests across 13 Bun test files, with 0 failures.

The pull request CI job that executes the agent-chat suite is workflow-guard-tests / preflight, in the guards run.

Scope notes

files-changed is not affected and already works for ACP sessions. The block is produced from git in server.ts by filesChangedEvents, and adapters/acp.ts already threads generation into its done event, so the end-of-turn edited-files list is not the broken path here.

A clickable inline diff card for a single tool call is a separate feature and is deliberately not included in this pull request.

Changelog

Fixed ACP tool-call transcript details for file paths, diffs, and terminals.

🤖 Generated with Claude Code

teamleaderleo and others added 2 commits September 30, 2026 02:28
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 3d3c7ce9-5e25-4e5b-98c3-f5566cb43057

📥 Commits

Reviewing files that changed from the base of the PR and between 547340a and d310825.

📒 Files selected for processing (3)
  • agent-chat/adapters/acp.ts
  • agent-chat/test/acp-tool-detail.test.ts
  • agent-chat/test/fake-acp-tool-detail.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Checked the ACP adapter diff, focused regression, TypeScript compile, and full agent-chat suite. Found no correctness issues.
Fixed: Added location-path details and diff and terminal content formatting with the existing truncation limits.
Left: Did not add a new block kind, UI component, colour, inline diff card, or change files-changed handling because those are outside this fix.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 30, 2026 09:34
@teamleaderleo
teamleaderleo merged commit 1b06f84 into main Sep 30, 2026
63 of 65 checks passed
@teamleaderleo
teamleaderleo deleted the parity/acp-tool-detail branch September 30, 2026 09:36
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for d3108259e9: every check was green at merge (12 verified; 17 skipped by policy). Full suite runs on main after merge.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Post-merge review of this change, since it landed on green before I got to it.

Review: the two exported helpers do what the PR says. acpToolCallDetail prefers locations and falls back to the rawInput JSON, including when locations is present but empty, so a tool call with no paths still shows its command. diffText handles oldText: null by counting zero removed lines, which is the shape an ACP agent sends for a file it creates, and the added/removed counts in the test match by hand. The fake agent covers the three ToolCallContent variants and a mixed array, which is the part that was previously collapsing to an empty string.

Left: one cosmetic defect in contentText. The map still joins with the empty string, which was correct when every element was a text chunk of a streamed message, but is wrong now that elements can be whole blocks. A mixed array renders as text outputdiff src/mixed.ts (+1/-1) newterminal term-42: the words run together at each boundary. The test does not catch it because it asserts with toContain on each piece separately. The fix is to keep concatenating adjacent text chunks and put a separator only around the diff and terminal blocks.

Also worth knowing for the next change in this area: truncate in adapters/lines.ts collapses every run of whitespace to a single space before cutting, so the \n that diffText puts in front of newText never survives. The detail is always one line. That is fine for a transcript detail string, but it means a clickable inline diff card cannot be built by formatting more text into this field; it needs its own block kind, which is still the separate feature described on cmuxterm-hq#852.

I am folding the separator fix into a follow-up PR that also fixes the stop path noted on #15781, since both touch adapters/acp.ts and two open PRs on that file would conflict.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 30, 2026
ecba57a fix(sidebar): cut with an ellipsis character so a reference cannot re-parse (manaflow-ai#15893)
6d2b5d1 feat(terminal): browser-style navigation layout and a terminalAlternateScreen shortcut key (manaflow-ai#14863)
0d3fdb1 test: print the simulator pipe output when the EOF assertion fails (manaflow-ai#15857)
46fe41a Fix cloud dogfood pause link-down journey (manaflow-ai#15918)
7d246ed fix: open existing Cloud workspace rows optimistically (manaflow-ai#15747)
1b06f84 fix(agent-chat): show ACP paths and diffs for tool calls (manaflow-ai#15908)
b413b7a fix(agent-chat): preserve earlier ACP plans during updates (manaflow-ai#15907)
8b75678 Persist Cloud display membership across clients (manaflow-ai#15748)
547340a fix(cloud): carry the machine author from /api/vm to the machine row's snapshot (manaflow-ai#15309)
e30de3d test: probe cloud agent status in Cloud VM journey (manaflow-ai#15875)
296537c docs(agent-chat): correct provider claims and pin ACP argv (manaflow-ai#15901)
e1dc959 Count the renamed Agent spawn tool as a subagent in the pi bridge (manaflow-ai#15865)
14fae18 dogfood: record the hover steps as trees, not frames (manaflow-ai#15845)
4da3bb3 fix(agent-chat): scope ACP plans to their turn and refresh activity (manaflow-ai#15898)
64ec56d feat(terminal): right-click a link to choose where it opens (manaflow-ai#15325)
efb762c Make unsupported remote browser warning dismissible (manaflow-ai#15726)
666c77f Cloud Machines sidebar: add persistent create buttons (manaflow-ai#15680)

# Conflicts:
#	.github/workflows/cloud-vm-dogfood.yml
teamleaderleo added a commit that referenced this pull request Sep 30, 2026
* test(agent-chat): cover ACP stop cancellation races

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(agent-chat): let Stop cancel queued or starting ACP turns

The content separator follows up on #15908 review feedback.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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