Skip to content

feat(desktop): Changes panel — review findings and the working tree - #220

Merged
oratis merged 2 commits into
mainfrom
feat/desktop-review-panel
Aug 3, 2026
Merged

feat(desktop): Changes panel — review findings and the working tree#220
oratis merged 2 commits into
mainfrom
feat/desktop-review-panel

Conversation

@oratis

@oratis oratis commented Aug 3, 2026

Copy link
Copy Markdown
Owner

The protocol has carried workspace/diff and review apply/revert since 0.2.0. protocol-agent.ts exposes diff(), applyFindings(), revertAction(). 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 calls review/apply, which runs 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. A finding with no suggested replacement shows 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 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:

this.emit({ kind: 'event', turnId, type: 'review_action', ...event.item.payload });

A review_action payload carries its own kind ('apply' | 'revert'), so the spread overwrote the envelope discriminator. Every consumer filtering on kind === '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/apply and a review_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:check clean; 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:check covers 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

t and others added 2 commits August 3, 2026 08:59
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>
@oratis
oratis merged commit 452a718 into main Aug 3, 2026
5 checks passed
oratis pushed a commit that referenced this pull request Aug 3, 2026
The conflict boundary fell inside a describe(), so the file lost its closing
braces and esbuild refused it. Same shape as the CSS seam in #220 — resolving
a conflict by keeping both sides needs a structural check, not just "no
markers left".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oratis added a commit that referenced this pull request Aug 3, 2026
* feat(protocol): carry reasoning, and show it in the desktop

The protocol carried reasoningTokens and nothing else, so DeepSeek's reasoner
produced its most distinctive output and every client dropped it. The CLI got
this in #218; the desktop could not, because there was nothing on the wire to
render.

Adds a `reasoning.delta` transient event behind a `reasoningDeltas` capability.
Separate from `item.delta` rather than a flag on it: reasoning is not the
answer, it is never persisted as a completed item, and a client that doesn't
understand the type has to be able to drop it rather than accidentally render
it as assistant text.

The app-server forwards the agent loop's thinking_delta events; the desktop
projects them onto the assistant turn as a distinct `reasoning` field — kept
out of `text` precisely so it can be rendered as its own channel — and shows a
collapsed `▸ thinking · N lines` block above the answer. Collapsed because
reasoner output is long and is not the response; the line count is there
because while a turn streams it is often the only thing to look at.

VS Code and the LSP bridge forward protocol events unchanged, so they receive
the new event without changes.

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

* fix(test): restore the describe block the rebase truncated

The conflict boundary fell inside a describe(), so the file lost its closing
braces and esbuild refused it. Same shape as the CSS seam in #220 — resolving
a conflict by keeping both sides needs a structural check, not just "no
markers left".

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

---------

Co-authored-by: t <t@t>
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