Skip to content

feat(app): align review change sources with the TUI - #59608

Open
opencode-agent[bot] wants to merge 2 commits into
devfrom
review-diff-sources-tui
Open

opencode-agent[bot] wants to merge 2 commits into
devfrom
review-diff-sources-tui

Conversation

@opencode-agent

@opencode-agent opencode-agent Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Desktop's review panel now offers the same change sources as the TUI diff viewer, with the same names, the same default and the same comparison base.

  • Sources: All changes (branch + local changes), Committed (branch commits only), Uncommitted (local changes only) and Last turn. They replace Git changes / Branch changes / Last turn changes.
  • Default: All changes, as in the TUI's branch source. Before, every session started on Git changes, so you had to switch to Branch changes once the agent committed.
  • One choice for the app: the selected source is stored once (plugin.review.source) instead of per session, like the TUI's review memory. It persists across restarts because desktop has no diffs.source config. The file selection and open files are still stored per session, and older reviewMode values are no longer imported.
  • Base: All changes and Committed resolve the base with vcs.base, the ref the branch was created from, and pass it to vcs.diff, as the TUI does. If the server cannot resolve a base, the diff falls back to the default branch. The base is not shown in the header.
  • Availability: All changes and Committed are offered whenever the repository has a default branch, including on the default branch itself, so the dropdown is the same everywhere. A source the session does not offer falls back to the first one that is offered.

Not included: the TUI's base picker. The three ui.sessionReview.title.git|branch|lastTurn keys in packages/ui are now unused but are left alone here, so this PR doesn't touch 60 locale files.

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72" fill="none">
  <style>
    .label{font:600 11px sans-serif;fill:currentColor}
    .hl{animation:hl 6s infinite steps(1)}
    @keyframes hl{0%{transform:translateY(0)}33%{transform:translateY(18px)}66%{transform:translateY(36px)}}
    @media (prefers-reduced-motion:reduce){.hl{animation:none}}
  </style>
  <rect x="8" y="6" width="224" height="60" rx="6" stroke="currentColor" stroke-opacity=".2"/>
  <rect class="hl" x="12" y="9" width="216" height="18" rx="4" fill="currentColor" fill-opacity=".08"/>
  <text x="20" y="22" class="label">All changes</text>
  <text x="20" y="40" class="label">Committed</text>
  <text x="20" y="58" class="label">Uncommitted</text>
</svg>

Checks

  • bun run typecheck in packages/gui-plugins and packages/app. typecheck:e2e reports only the 11 errors that already exist on dev.
  • bun run lint:changed, plus prettier on the changed files.
  • Playwright regression/review.spec.ts (24 passed), mobile-session.spec.ts and mobile-plugins.spec.ts. mobile-session › "drawer navigation preserve session identity" also fails intermittently on origin/dev (2 of 5 runs).
  • Demo recorded against a real local backend and a Git repo with a feature branch that has two commits and local edits.

Requested by: @vimtor (victor via Slack)

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The approach is sound: the sources, default and base resolution match the TUI diff viewer, and the server's Git adapter already supports committed and base. I couldn't run it here. Type-checks pass in packages/gui-plugins and packages/app, and the review unit tests pass. Playwright couldn't start Chromium in this sandbox, so I have no e2e results and no screenshots.

Public API: unchanged. Only app storage changes: plugin.review.source is new, and the per-session mode and the old reviewMode import are dropped. The client calls vcs.base / vcs.diff, which already exist.

One concern, inline: All isn't offered until the repository's branch info has loaded, so the panel can briefly show Uncommitted and fetch that diff before switching to All.

const chosen = source.value?.mode ?? "branch"

// Until the server and the project answer, what the session offers is unknown.
if (!view().server.connected || !view().project) return chosen

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All and Committed are offered only once vcs()?.branch.default is known. Until vcs.info loads for this directory, options() is ["working", "turn"]. So with the new default (All), mode() resolves to working: the header shows Uncommitted and a working diff is requested, then it switches to All once branch info arrives. This already happened for a stored Branch choice before, but now it's the default for everyone. Could mode() (and loaded() / the diff query's enabled) treat the offered sources as unknown while vcs() is undefined for a VCS project, the same way it waits for the server and project?

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The new commit removes the source descriptions and the vs <base> header label, and renames the first source to "All changes". It looks fine. Type-checks pass in packages/gui-plugins and packages/app, and the review unit tests pass. I couldn't run the Playwright tests in this sandbox.

The earlier point about the brief switch to Uncommitted while branch info loads isn't addressed by this commit; its thread stays open.

Public API: unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant