feat(desktop): Changes panel — review findings and the working tree - #220
Merged
Conversation
The protocol has carried workspace/diff and review apply/revert since 0.2.0,
protocol-agent.ts exposed diff()/applyFindings()/revertAction(), and the
desktop called none of them: an engine with no steering wheel. Finding C /
F7 in docs/THREE_WAY_REVIEW.md.
A fourth rail icon opens a Changes panel with two sections:
Review findings — every review_finding the agent submits, with priority, file
and line, and per-finding Apply (or Apply all). Applying runs review/apply as
an ordinary turn, so it keeps permissions, approvals, hooks, sandboxing and
snapshots; the resulting review_action flips the row to applied and offers
Revert. Findings with no suggested replacement show a disabled Apply that
says why, rather than failing on click.
Working tree — files from workspace/diff with add/delete counts, collapsed
until asked for, then hunks rendered line by line. Binary and truncated files
say so instead of rendering an empty box.
The badge shows findings still to act on, falling back to the changed-file
count.
Fixes a real bug found by becoming the first consumer of these events. The
bus envelope was built as `{ kind: 'event', ..., ...payload }` — and a
review_action payload carries its own `kind` ('apply' | 'revert'), so the
spread overwrote the envelope discriminator and every consumer filtering on
`kind === 'event'` silently dropped the event. The payload is now nested. The
existing projection test passed straight through this because it only checked
fields the spread preserved.
The preview fixture gains workspace/diff, review/apply and a review_finding
so the Playwright journey exercises the panel for real.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The conflict boundary fell inside a CSS rule; `format:check` covers ts/tsx/json/md/yml/yaml but not css, so a stylesheet that postcss cannot parse still passed the format gate. The Playwright journey caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The protocol has carried
workspace/diffand review apply/revert since 0.2.0.protocol-agent.tsexposesdiff(),applyFindings(),revertAction(). The desktop called none of them — an engine with no steering wheel. Finding C / F7 indocs/THREE_WAY_REVIEW.md.A fourth rail icon opens a Changes panel with two sections.
Review findings — every
review_findingthe agent submits, with priority, file and line, and per-finding Apply (or Apply all). Applying callsreview/apply, which runs as an ordinary turn, so it keeps permissions, approvals, hooks, sandboxing and snapshots; the resultingreview_actionflips the row to applied and offers Revert. A finding with no suggested replacement shows a disabled Apply that says why, rather than failing on click.Working tree — files from
workspace/diffwith add/delete counts, collapsed until asked for, then hunks line by line. Binary and truncated files say so instead of rendering an empty box.The rail badge shows findings still to act on, falling back to the changed-file count.
A real bug, found by being the first consumer
The event envelope was built as:
A
review_actionpayload carries its ownkind('apply' | 'revert'), so the spread overwrote the envelope discriminator. Every consumer filtering onkind === 'event'— which is the documented shape — silently dropped these events. Nothing consumed them, so nothing noticed. The payload is now nested, and the existing projection test (which passed straight through the bug, because it only asserted fields the spread happened to preserve) now pins the envelope.Verification
The preview fixture gains
workspace/diff,review/applyand areview_finding, so the Playwright journey exercises the panel against real protocol traffic: findings render, hunks expand with the right add/delete counts, the binary file is labelled, Apply produces an action and the row flips to Revert.15 unit tests over the pure reducer (accumulation across turns, replay de-duplication, batched applies, revert returning a finding to pending, badge precedence).
pnpm typecheck·lint·format:checkclean; desktop 90 · core 718/16 skipped · cli 204 · server 41 · protocol 24 · vscode 12 · lsp 13 · scripts 21;playwright test→ 6 passed.One follow-up worth doing separately:
format:checkcovers ts/tsx/json/md/yml/yaml but not css, so a stylesheet postcss cannot parse passes the format gate. A rebase seam produced exactly that here, and only the browser journey caught it.🤖 Generated with Claude Code