Skip to content

fix(agent-chat): show ACP plans as structured step lists - #15889

Merged
teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
teamleaderleo:parity/acp-plan
Sep 30, 2026
Merged

teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
teamleaderleo:parity/acp-plan

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The ACP parity review found that session/update plan snapshots were reduced to one status line and truncated at 300 characters. Agent chat now carries each plan entry with its text, normalized status, and optional priority, then renders the current snapshot as a small step list. Repeated snapshots replace the prior plan in the client session fold so the transcript does not accumulate copies, while the server event history remains available for reconnects.

The visual treatment is deliberately minimal and uses the existing activity styling. The in_progress status is more prominent, and the treatment is open to change as plan presentation evolves.

Claude and Codex use their own adapters and emit no plan events here, so their provider paths are unchanged.

The ACP tool_call update with kind: "edit" is an obvious next step: its diff is in the call content, but nothing currently maps that content to the existing files-changed payload. This PR records that gap without changing it.

Testing

  • Regression before the fix, commit 8fab1189b15: cd agent-chat && bun test test/acp-plan.test.ts returned FAIL, with Expected length: 2 and Received length: 0.
  • The same focused command after the fix, commit f69fb037273: 1 pass, 0 fail, 5 expect() calls.
  • cd agent-chat && bun install && bun run check: 38 pass, 0 fail, 103 expect() calls.
  • ./scripts/localize-changes: localization audit passed with 0 changed message keys and 0 catalog parity errors.

Changelog

Changed: Agent chat now shows an ACP agent's plan with per-step status instead of a single truncated line.

Demo Video

Not included because this portable agent-chat change does not require a native app build, and the repository instructions prohibit running one for this verification.

Checklist

  • Behavior changes have added or updated tests, with the failing and passing focused results recorded above
  • UI behavior change has been localization audited, with the result recorded above
  • User-facing docs are not needed for this change
  • Self-reviewed the adapter, session fold, renderer, provider paths, and focused regression

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Shows ACP session/update plans as structured step lists instead of collapsing each snapshot into a single 300-character status line.

  • Carries each plan entry's text, normalized status, and optional priority into the transcript renderer.
  • Replaces the prior plan snapshot on subsequent updates so the transcript no longer accumulates copies; server event history stays intact for reconnects.
  • Unrecognized statuses normalize to unknown, and empty plan updates are dropped.
  • Notes that ACP tool_call updates with kind: "edit" still aren't mapped to the existing files-changed payload; that gap is recorded but not changed here.

Written for commit f69fb03. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 30, 2026 01:48
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d status line

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 6 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: e00486f8-7a19-4b9e-9bde-888b68fb0d87

📥 Commits

Reviewing files that changed from the base of the PR and between d87c3be and f69fb03.

📒 Files selected for processing (8)
  • agent-chat/adapters/acp.ts
  • agent-chat/public/app.css
  • agent-chat/src/components/Transcript.tsx
  • agent-chat/src/session.ts
  • agent-chat/src/turns.ts
  • agent-chat/test/acp-plan.test.ts
  • agent-chat/test/fake-acp-plan.ts
  • agent-chat/types.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: I checked the ACP adapter normalization, client plan replacement, renderer, provider isolation, and regression coverage. I found no additional issues.
Fixed: I changed the ACP plan update from a truncated status event to structured entries and added the step-list rendering and tests.
Left: I left ACP edit diff attribution untouched because it is outside this ACP plan fix and needs separate files-changed mapping.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 30, 2026 08:56
@teamleaderleo
teamleaderleo merged commit a192a14 into manaflow-ai:main Sep 30, 2026
60 of 61 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

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

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 30, 2026
0e44675 test: bound remote bootstrap subprocess waits (manaflow-ai#15608)
a192a14 fix(agent-chat): show ACP plans as structured step lists (manaflow-ai#15889)
d7f59a3 ci: place attempt 2 like attempt 1, owned minis first (manaflow-ai#15406)
d87c3be feat(agent-chat): register Cursor Agent as an ACP provider (manaflow-ai#15877)
1bd5083 fix: preserve Codex provider for workspace auto-naming (manaflow-ai#15635)
03e1245 fix(worktree-seed): budget each pattern and refuse dangling escapes (manaflow-ai#15860)
5c28fcb fix(agent-chat): stop a disposed ACP session from resurrecting its agent (manaflow-ai#15872)
11216d2 Fix Codex Agent Chat Stop interrupt request (manaflow-ai#15837)
d6b8c15 ci: watch Unix cmux-tui installer changes (manaflow-ai#15874)
0fc35d6 feat(agent-chat): register goose as an ACP provider (manaflow-ai#15871)
7f27bfc cmux ssh: security hardening from the ssh audit (manaflow-ai#15768)
8599250 fix(agent-chat): launch gemini with --experimental-acp (manaflow-ai#15868)
849376a docs: classify contributor issue difficulty (manaflow-ai#15627)
2761cc9 Keep agents with live background work out of hibernation (manaflow-ai#15278)
eae02a6 Cloud: rebake the devbox ladder with cmux-tui 02dac3c (manaflow-ai#15866)
7ed2f6b ci: bound open pull-request media revisions (manaflow-ai#15861)
13c417c Notify on SubagentStop in the notifications hook docs (manaflow-ai#15854)
5cfc6a6 fix: make cmux-tui installs immutable across release uploads (manaflow-ai#15859)
4eee1b1 fix: preserve longest Claude upstream cooldown (manaflow-ai#15856)
204b936 Pin Cloud panes to the daemon's terminal grid (manaflow-ai#15792)
fc13b7c cmux-tui: fix the replay row scroll and stale hook fence tests breaking the full gate (manaflow-ai#15240)
87d66af Add Cloud to the menu bar extra and a main-menu Cloud menu (manaflow-ai#15822)

# Conflicts:
#	.github/workflows/ci-failure-attribution.yml
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/ci-queue-janitor.yml
#	.github/workflows/ci.yml
#	.github/workflows/cmux-tui-artifacts.yml
#	.github/workflows/cmux-tui-build-package.yml
#	.github/workflows/cmux-tui-sdks.yml
#	.github/workflows/pr-media-prune.yml
#	.github/workflows/remote-daemon.yml
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review, posted after the merge. The session that opened this PR did not post one, so this is mine. The change is good and the test is the strongest of the four agent-chat PRs this week: it drives a fake ACP agent over stdio rather than asserting on a literal.

Verified. Commit order is right, test before fix. The fake at agent-chat/test/fake-acp-plan.ts covers all five cases worth covering: several entries with mixed statuses, an entry of 350 characters surviving whole where the old 300-character cut would have eaten it, an unrecognised status arriving as waiting and being kept as unknown rather than dropped, a second plan replacing the first, and both entries: [] and a plan with no entries key at all producing no event, which toHaveLength(2) pins. The visual treatment stays inside the existing token set and adds no new colours or animation.

Review: a plan from a later turn lands in an earlier turn's position. src/session.ts foldEvent finds the first plan block anywhere in the transcript and replaces it in place. Within one turn that is exactly right. Across turns it is not: an agent that plans in turn 1, works, and plans again in turn 3 has turn 3's plan rendered inside turn 1's activity group, and turn 3 shows no plan at all. The fix is to scope the search to the current turn rather than the whole block list. Left: follow-up PR.

Review: an in-place plan update does not invalidate the activity key. src/activity.ts activityTailKey keys on blocks.length plus a per-kind discriminator, and has no plan case, so a plan tail returns <n>:unknown. Because a plan update replaces a block instead of appending one, the count does not change and neither does the key. An agent moving from step 2 to step 3 with nothing else appended therefore leaves the activity row stale, which is the one thing per-step status is for. Left: same follow-up, and it wants a plan case that includes the statuses.

Review: AgentEvent is declared twice and this PR had to edit both copies. agent-chat/types.ts and agent-chat/src/session.ts each carry a full copy of the union, and now each carries its own AgentPlanStatus and AgentPlanEntry as well. Nothing checks that they agree, so the next variant can be added to one and silently missed in the other. Not this PR's doing, and worth a separate cleanup rather than smuggling it into a feature. Left: noted for the follow-up to propose.

Review: none of these tests run in CI. .github/workflows/ci-guards.yml runs one agent-chat file and nothing calls bun run test, so test/acp-plan.test.ts is local-only like the other three added this week. Left: a PR wiring the suite in is already open.

Smaller things, all left: normalizeAcpPlanEntries is exported and nothing imports it. String(entry.priority) and String(entry?.content ?? "") would render a structured value as [object Object]; ACP specifies both as strings so this is a hedge that cannot currently fire, but a thrown-away value is better than a misleading one. The test has to shim globalThis.location and dynamically import src/session.ts because the fold logic reaches a DOM global at module scope, which will keep costing every future unit test in that file. And summarizeTurnActivity counts a plan under other, so a turn with a plan reads "1 other".

Fixed: nothing here, it had already merged. The follow-up covers the first two findings, which are the ones a user would notice. :)

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Follow-up review fixes are in PR #15898: #15898

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