Fix the review dialog's buttons that are hard to see in the dark theme - #697

…n the editor (#677)

## Why

After a patch is applied or a pull request is checked out, the app says
how many files changed and nothing more. Unless a contributor already
knows which files to look at, they can't find the change, and their own
edits on top of it aren't listed anywhere. The only way to undo one of
their edits was **Discard all changes**, which also took the patch away.
This is #669 in one PR. It was a stack of five (#677, #678, #679, #682,
#683); they are folded together here because each builds the same
section, and a reviewer was already testing the top of the stack.

## What changes

- The site's details gain a **Changed files** section, last in the
sidebar after **Build watch**, so a long list grows without pushing the
server and the watch out of reach.
- It has up to two groups, each under its own heading and drawn as a
tree of folders:
- **From the patch** / **From PR #N**: the change's own files, each with
a pencil when the contributor has edited it since.
- **Your changes**: every other file changed on the branch, with a plus
when the contributor created it. On a ticket with nothing applied, this
is the only group.
- Each folder starts open, and a chain of folders that hold only one
another shares one row (`tests/phpunit/tests/wp`), as GitHub and VS Code
draw them. A folder row's button, or the Left and Right arrows, close
and open it. Each file shows its name with a page icon and its full path
as a tooltip. Drawn with `__experimentalTreeGrid` from
`@wordpress/components`, which gives the tree roles a screen reader
needs and the arrow keys between rows.
- One click on a file opens it in the first editor the app detects,
inside the site's folder (`open -a App <site> <file>`, `Code.exe <site>
<file>`). With no editor detected, the file is selected in Finder or
Explorer instead. "Choose application…" after a file fails to open opens
that file in the application picked.
- **A revert button** on each file the contributor changed, at the end
of its row. It asks first, and leaves the file as **Discard your
changes** would: a file of theirs back to the base, a new file deleted,
a pull request's file back to the pull request's version, a patch's file
back to how the patch left it with the patch still applied. A patch file
they did not touch, and either end of a rename, have no button. While an
install, build, trunk update or dev server runs, the button is held and
says why.
- **Discard your changes** (renamed from "Discard all changes") now
removes only the contributor's own work. With a patch applied, the tree
goes back to the base and the patch is re-applied from its stored text,
so the patch and **Revert this patch** stay. If the text was not kept,
the patch goes too and the dialog says so first. **Discard this ticket
to its base** still removes the patch. The ticket card's discard button
from #696 calls the same discard, so every "discard your changes"
control keeps the patch.
- How the pencil is decided: a pull request is its branch's base, so any
of its files with changes on top was edited. A patch is an uncommitted
layer, so each file it wrote is fingerprinted when applied (SHA-256,
line endings normalised, `src/change-fingerprint.cjs`) and compared with
the disk.
- The list refreshes when the window regains focus, which is when a
contributor comes back from their editor, and whenever the app itself
changes the tree.
- New IPC `git:revert-file`, which accepts only paths Git reported as
changed. `editor:open` and `dir:show` take an optional file inside the
site, checked by its real path, so a symbolic link that leads out of the
site is refused.

Not in this PR: an editor preference, per-file diffs, marking deleted
files, live updates while the app keeps focus, undo after a revert,
reverting several files at once, and remembering which folders were
closed across a relaunch.

## How to test this

Platforms: macOS and Windows. The launch arguments, path checks and the
revert's writes (`core.autocrlf`) differ per platform.

The journeys cover the flow with stubbed editors and fixture
repositories. Run from the repository root:

```
npm run test:e2e -- tests/e2e/journeys/patch-apply.spec.js tests/e2e/journeys/pr-checkout.spec.js tests/e2e/journeys/i18n.spec.js
```

They check opening a file in the editor or the file manager, "Choose
application…", the groups and their marks, the list during a build,
after a discard, after a refused apply and after a refocus, each kind of
revert, and that a folder closes and opens and the revert button is
reached with the arrow keys. What they can't reach is a real editor, a
real `wordpress-develop` clone and a real pull request, so that part is
by hand.

**Starting state:** a site with a real `wordpress-develop` clone, VS
Code installed, the dev server stopped, and a ticket linked whose Trac
attachment applies cleanly, for example #23602 with
`23602_pr14112_20261008.diff`.

1. Apply the attachment. **Changed files** appears at the bottom of the
sidebar, under **From the patch**, as a tree of two folder rows:
`src/wp-includes` over `class-wp.php`, and `tests/phpunit/tests/wp` over
`queryPosts.php`. No pencil, no revert button.
2. Click the `src/wp-includes` folder. Its file is hidden and its
chevron points right. Click it again. The file is back.
3. Click `class-wp.php`. VS Code opens the site folder with that file
open. Click `queryPosts.php`. It opens in the same VS Code window.
4. In VS Code, change `class-wp.php` and `src/wp-login.php`, and create
`src/new-helper.php`. Switch back to the app. `class-wp.php` has a
pencil; under **Your changes**, `wp-login.php` has none and
`new-helper.php` has a plus. All three have a revert button.
5. Revert `new-helper.php`. The dialog asks "Delete
src/new-helper.php?". Confirm. The file is gone from disk and from the
list.
6. Revert `class-wp.php`. The dialog says it goes back to how the patch
left it. Confirm. The file has the patch's change and not yours, and the
pencil is gone.
7. Change `class-wp.php` again and switch back. Click **Review & submit
changes**, then **Discard your changes**. The dialog says the patch
stays applied. Confirm. `class-wp.php` has the patch's change only,
`wp-login.php` is back to trunk, and **Revert this patch** is still
there.
8. Click **Revert this patch**. The section is gone.
9. Link a ticket with a GitHub pull request and check it out. **Changed
files** lists its files under **From PR #N**, the same files its preview
did.
10. On Windows, repeat steps 1, 3 and 6 with a site whose path has a
space in it.

**What must not have happened:**

- A second VS Code window for the same site.
- Anything opened outside the site folder.
- A file listed in both groups.
- Any patched file with a pencil right after applying, or right after
step 6, before you changed anything. On Windows this would point at line
endings.
- The patch's change missing from `class-wp.php` after step 6 or step 7.
- Any file other than the one reverted changed by a revert.
- After checking out a different pull request, the list from the
previous one.
- A focus ring cut off at either edge of the sidebar, on a folder, a
file or a revert button, after tabbing in or after cancelling a revert.

## Risks and limitations

- Review of the combined branch: 7 [fix here] fixed, 3 [follow-up]
deferred (see the review outcome). CodeRabbit's one finding, on patches
applied before this PR, is fixed.
- Rebased onto trunk after #696 and #697. The ticket card's discard
button that #696 added now keeps an applied patch or pull request, like
the note's link and the review modal's; before the rebase it would have
removed the patch.
- `__experimentalTreeGrid` is not a stable export yet; the window
already uses another `__experimental` control the same way
(`create-site-dialog.jsx`).
- In the tree, the pencil and plus are the file button's description
rather than focusable stops of their own, because only buttons are in
the tree's reach of the arrow keys. A screen reader hears them; a
sighted keyboard user sees the icon but can't open its tooltip.
Follow-up.
- Verified by hand with VS Code on macOS. PhpStorm, Zed, and Windows
generally are untested with a file argument, and the revert and discard
have not been run by hand on a real clone or on Windows.
- A chain of folders shares one row, so deep paths indent less, but a
long merged folder name, like a long file name, can wrap in a narrow
sidebar.
- A move onto trunk that updates the branch and then fails at the
checkout keeps the patch's old text and fingerprints. A pencil can then
mark a file trunk changed, and a discard or revert re-applies text
written for the old trunk. Older than this PR; follow-up.
- Neither a discard nor a revert is held while an apply or pull request
checkout runs. Older than this PR (the discard link had the same gate);
follow-up.
- A pull request checked out before this ships has no list until it is
checked out again. A patch applied before this ships marks every one of
its files as edited, since the app did not record what it wrote:
**Discard your changes** and each revert stay offered, reverting a file
unmarks it, and a discard records the patch afresh. Covered by unit
tests only; staging it by hand needs a site from an older build. A file
the patch or the contributor deleted is still listed, and says it is
missing when clicked.
- The diff is about 3,000 lines across 32 files, well over the size
AGENTS.md flags. It is one feature in five steps; the commits keep each
step apart for reading one at a time, in this order: patches, pull
requests, your changes, discard, per-file revert, then the tree.
- In one of three local runs of the full journey suite, four journeys
failed while the run took three times its usual length; the two runs
either side passed all of them. I read it as load on the machine, not
the branch.

## Related

Part of #669. Replaces #678, #679, #682 and #683, which are folded in
here.

A pull request's list including files trunk changed, when the site's
trunk is behind the pull request, is older than this PR and tracked in
#701.

---

<details>
<summary>Design decisions and alternatives considered</summary>

- **One PR, not a stack of five.** Every step builds one section, and a
reviewer was testing the top of the stack while commenting on the
bottom.
- **Trees, one per group.** A reviewer asked for `TreeGrid` because full
paths filled the sidebar's width. One tree per group keeps the headings.
- **A chain of single folders on one row** (`tests/phpunit/tests/wp`),
as GitHub and VS Code draw them. The first version gave every folder its
own row; a reviewer pointed out it took five rows to show one file.
- **Last in the sidebar**, not first: the list has no fixed height, and
first it pushed the server and the build watch out of view.
- **A revert leaves the file as Discard your changes would,** so the two
never disagree about what "your changes" means. A patch file therefore
goes back to the patch's version, not to trunk.
- **Discard keeps the patch.** One button that always means "remove my
edits, keep the patch or PR" matches what it already did for a PR.
Removing the patch stays **Revert this patch**; re-applying the stored
text is the only way back to "as the patch left it".
- **Renames refused, not modelled.** One change spread over two paths
cannot go back one path at a time.
- **One click, no per-file menu.** There is no editor preference yet, so
the click takes the first detected editor. A preference belongs in the
Settings dialog from #638.
- **The real path, not just the text.** A lexical check let a symbolic
link inside the site point outside it.

</details>

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

Earlier reviews of each folded PR are recorded on it (#678 at `8ddcb27`,
#679 at `eb7be4f`, #682 at `85b47f9`, #683 at `ff23819`, before the
rebase); every finding there was fixed or deferred, and the deferrals
are in Risks. The reviews below cover this combined branch: reviews 1 to
3 on base `60078fe`, review 4 on trunk `819575a` after the rebase.

- **Review 1:** completed. A separate agent context reviewed
`60078fe..73b47df` (#677 to #679 plus the tree) against
`.github/instructions/code-review.instructions.md`. `npm run lint`
clean, `npm test` 2147 passed. 1 [fix here] · 1 [follow-up].
- Fixed in `eb93d07` (🟡): the file list was walked as soon as an apply,
revert or pull request switch changed the tree, but the status it is
grouped by was read again only after the install and the build, so for
those minutes a patch's files were listed as the contributor's. The
status is now read with the walk. The journey "while the build after an
apply runs, the patch's files are the patch's (#669)" holds the build
open; it fails without the fix.
- Deferred (🔵): the marks and the rule for whether the section shows are
decided in `site-details.jsx` rather than `changedFileGroups`. One
branch each.
- **Review 2:** completed. Separate agent context, the fix above. No
findings; confirmed on every path into the install and build. Its
comment note was taken in `52ecde4`.
- **Review 3:** completed. Separate agent context, `8400845..52ecde4`
(#682, #683 and the tree redrawn over them). 1 [fix here] · 2
[follow-up].
- Fixed in `288b7c8` (🟡): the tree remembers the last button focused in
it and gives every other one no tab stop, so a file that left the list
on a refocus took the whole tree out of the tab order. The tree is now
keyed by its files. The journey "a file that leaves the list does not
take its tree out of the tab order (#669)" fails without the fix.
- Fixed in `288b7c8` (🔵, raised as a follow-up): the treegrid itself had
no accessible name; it now has its group's.
- Deferred (🔵): a sighted keyboard user cannot open a mark's tooltip
(see Risks).
- **Review 4 (rebase onto trunk):** completed. Separate agent context,
`819575a...455d0e8`, on the conflict resolution with #696: the ticket
card's discard button pointed at `discardYourChanges`, `discardLabel` at
its question, and `nothingOfYours` in the shared reason. 1 [fix here] ·
0 [follow-up]. Fixed in `a54b394` (🔵): a unit test still tied the label
to the full-discard question, passing only because both say "Discard
changes". Nothing from #696 or #697 lost; no `discardAllChanges` left.
- **CodeRabbit** reviewed `a54b394`: 1 finding (🟡), fixed in `d55ba08`.
A kept patch applied before fingerprints marked none of its files as
edited, so **Discard your changes** was disabled and no file could be
reverted. Its files still changed now count as possibly edited.
- **Since then:** `c7f4ad9` merges a chain of single folders into one
row, at @zaerl's request; `299bc2d` and `9a28812` give the revert
button's focus ring room at the sidebar's edge.
- **Review 5:** completed. Separate agent context, `a54b394..d55ba08`. 3
[fix here] · 1 [follow-up], all fixed:
- Fixed in `1f93225` (🟡): a folder closed before a file left the list
could stop being a row of its own and still hide everything under it,
leaving an empty tree. Unit test.
- Fixed in `1f93225` (🟡): the window dropped `renamedByChange` from the
reply, so both ends of a rename were offered a revert the handler
refuses. Now carried by `unsubmittedFromReply`, unit-tested.
- Fixed in `0dea62b` (🔵): on a patch from before fingerprints, a
reverted file still looked edited. Its first revert now records a full
set. Unit test.
- Fixed in `4a77c79` (🔵, raised as a follow-up): a first-level folder's
focus ring was clipped at the sidebar's other edge. Every cell now has
room for the ring; the journey checks a folder, a file and a revert
button, and fails without it.
- **On `4a77c79`:** `npm run lint` clean, `npm test` 2193 passed, full
journey suite 163 passed hidden on macOS.

</details>

<details>
<summary>Implementation notes</summary>

- `resolveLaunch(editorPath, sitePath, { platform, file })` appends the
file after the folder in the argument vector. Never a shell.
- `existingSiteFile(sitePath, relPath, { platform, realPath })` runs
realpath on both the site and the file, so a site behind a link (macOS's
`/var`) still holds its files.
- `src/renderer/affected-files.cjs` holds the decisions, unit-tested:
`affectedFiles`, `changedFileGroups` (groups, labels, `canRevert`),
`unsubmittedFromReply` (the reply as the list reads it),
`affectedFileTree` (paths to tree rows, merging single-folder chains),
`visibleAffectedRows` (what a closed folder hides),
`revertFileQuestion`, `revertOutcome` and `fileOpenTarget`.
- `src/git-write.cjs`: `restoreWorktreePaths(dir, source, paths)`,
worktree only, literal pathspecs on stdin. `src/trunk-update.js`:
`discardFile`. `src/patch-apply.js`: `applyPatchToDir({ paths })` keeps
only those files' sections.
- `src/main.js`: `git:revert-file`; `git:discard-to-base(sitePath, {
keepChange })` re-applies the kept patch and fingerprints it again;
`git:unsubmitted-work` adds `entries`, `editedSinceChange` and
`renamedByChange`.
- The tree's indent is read from each row's `aria-level` in CSS
(`attr(aria-level type(<integer>))`), since the repository's lint bans
inline styles. The revert button gets the tree's item props through
`Tooltip.Trigger`'s render prop.
- `forgetChangedFiles` in `src/renderer/index.jsx` bumps the dirty
probe's generation, so an answer still in flight from before a change is
dropped, then clears the list and asks again.

</details>

<details>
<summary>Screenshots or recording</summary>

To add.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)