Skip to content

Say what a patch deleted, and name what it could not carry - #198

Merged
juanmaguitar merged 11 commits into
juanmaguitar/ticket-branches-uifrom
juanmaguitar/patch-deletions
Aug 10, 2026
Merged

Say what a patch deleted, and name what it could not carry#198
juanmaguitar merged 11 commits into
juanmaguitar/ticket-branches-uifrom
juanmaguitar/patch-deletions

Conversation

@juanmaguitar

@juanmaguitar juanmaguitar commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Why

The patch is what a contributor hands over, so a change it does not mention did not happen as far as anyone reviewing it can tell. It was not saying what happened, in three ways:

  • A deleted file fell out of the patch entirely. Remove a file for your ticket and the diff simply never mentions it, with no warning. Its status row survives the scan, but with no working copy to read, the new side was defaulted to the old one — so the file compared equal to itself and was skipped.
  • Both sides were always named a/<path> and b/<path>. That filename is the only thing this app's own parser reads an add or a delete from. So a deletion came back as a modification, and the applier would have written an empty file where the patch said remove. Additions had the mirror problem and were refused with "is not in this checkout".
  • Binary files were dropped in silence — a patch missing a file the contributor changed, and no way for them to know.

Two more surfaced while testing, neither reported, both live before this PR:

  • Any patch for a file without a trailing newline failed to re-apply, modifications included. jsdiff keeps reading past the \ No newline at end of file marker, so the blank line this generator put between sections became a phantom context line.
  • git apply rejected every deletion — see Risks; this one was found by the review pass after the first commit here introduced deletions, and it would have been worse than the bug it fixed.

What changes

Root cause in all of it is one loop, createMinimalPatchForDir in src/main.js:

  • The status matrix decides what happened, not the buffers. workdir === 0 is the file being gone; head is whether a base side exists. This also keeps "deleted" apart from "present but will not open" — emitting a deletion for the second would tell the next checkout to remove a file nobody removed.
  • /dev/null names whichever side does not exist, which is what makes an add an add and a delete a delete to every reader, this app's included.
  • The section separator is gone. Each section already ends in a newline.
  • Files the patch cannot carry are named in # lines above the diff — binaries, and files whose contents would not read. That is the placement the mentor-handoff header (Patch destination: name where the patch goes, and let a mentor carry it #166) already established: git apply and patch skip leading comment lines, and this app's parser starts at the first ---.

The panel's "is there anything to attach" test moves from comparing against the No changes. sentinel to asking whether a diff survives under the commentary (hasDiffLines in src/renderer/diff-highlight.cjs), since the notice now sits above it.

Deliberately not in this PR: adding or deleting an empty file is still dropped — a === b === '' skips before the naming, and jsdiff emits no hunks for it, so representing it needs more than a naming change. Filed as #199 rather than bolted on here.

How to test this

Platforms: any. Stacked on #185 — that branch is the base, so this diff is only the patch generator.

Starting state: a site with a linked ticket, freshly on trunk.

  1. Delete a file (say src/wp-login.php), edit a second, and create a third. Click Submit patch. → All three appear. The deletion reads --- a/src/wp-login.php / +++ /dev/null; the new file reads --- /dev/null.
  2. Save that patch, and apply it to a second site through Apply a patch or PR. → The deleted file is really gone there, the edit landed, the new file exists.
  3. The step that matters most, and the one the app cannot do for you: run git apply --check patch-file against a real wordpress-develop checkout. → Exit 0. This is what a core committer does with a Trac attachment, and it is where the deletion bug in the first commit here showed up.
  4. Change only an image in the site, nothing else. Click Submit patch. → The panel names the file above the diff — 1 file is not in this patch — a text diff cannot carry binary content — and offers no destination, because there is no diff to attach.

What must not have happened:

  • No patch that applies only partway. git apply is all-or-nothing: if the deletion section is malformed, the contributor's unrelated edits are refused with it, which is worse than the deletion being missing.
  • Nothing staged. Generating a patch still leaves git status in the site exactly as it was.
  • No file silently absent. Anything changed and not in the diff must be named in the # lines.

Risks and limitations

Review outcome: 3 [fix here] · 2 [follow-up] — all 3 fixed. One was serious enough to name here: the first commit's deletions each carried a \ No newline at end of file marker jsdiff invents for the empty new side, which asserts something false about the removed file and makes git apply refuse the entire patch. It would have shipped a fix that broke more than the bug did. Verified against real git in both directions — refused with the marker on a file that ended in a newline, clean without it, and still required when the file genuinely lacked one.

  • Empty-file adds and deletes are still droppedA patch still leaves out an empty file that was added or removed #199.
  • A file present but unreadable is named, not carried. There is no test for it: provoking it needs a permission or lock state that does not reproduce across macOS and Windows, and the repo's standard rules out a platform-skipped test.
  • The binary notice puts a non-ASCII em dash in a plain patch for the first time. Fine for git apply and for Trac's UTF-8, worth knowing if anything downstream guesses encodings.

Related

Closes #85, and #174 with it — the deleted-file half was reported twice.

Stacked on #185#168, all part of #108's line of work.

Since trunk moved: #179 introduced collectChangedFiles, one walk read by both the .diff and the pull request, so this branch's changes moved into that shared shape rather than sitting beside it. It converged well — that walk already reports inHead/inWorkdir per file, with a comment saying the status codes and not the buffers are what decide whether a file is gone. That is exactly the distinction this fix needed, so the deletion test now reads a field the PR path had already established rather than one invented here.


Design decisions and alternatives considered

Why not emit diff --git headers and be conventionally git-shaped? The app's own parser is documented as reading createTwoFilesPatch output without them, and scanSections in patch-provenance.cjs keys on their absence. Adding them is a bigger change to the reading side than to the writing side, and /dev/null alone is what both git apply and this app need to tell an add from a delete.

Why a notice rather than dropping binaries silently, or refusing the patch? Refusing would make one image block a working text patch. Silence is the bug. Naming them costs three lines and tells the contributor exactly what to attach by hand.

Why strip jsdiff's marker instead of hand-writing deletion sections? The marker is wrong only in one case — a deletion whose old side ended in a newline — and it is load-bearing in the adjacent case. A targeted strip keeps jsdiff as the single producer of hunk text; hand-rolling the section would put this app in the business of emitting unified diffs by hand for one branch of the loop.

Review outcome (required — see AGENTS.md)

3 [fix here] · 2 [follow-up] — all 3 fixed. Judgement pass run in a fresh context per the skill, on the first commit here.

# Dimension What was wrong
1 🔴 Cross-platform Every deletion claimed the removed file had no trailing newline, and git apply refuses the whole patch over it. Fixed by stripping the marker only when jsdiff invented it; both directions verified against real git.
2 🟡 Architecture Filenames were chosen from whether a buffer loaded, so an unreadable base blob retyped an edit as an addition that applies nowhere. They now follow the status matrix, and unreadable files are named above the diff.
3 🟡 Tests The round-trip test only exercised this app's own applier, which tolerates the bad marker — which is why finding 1 survived a green suite. Now asserted on the bytes, with the adjacent "marker is correct here" case pinned too.

[follow-up]: empty-file adds/deletes (#199), and the unreadable-file path having no test (both in Risks).

Worth flagging rather than burying: finding 1 was introduced by this PR, not inherited. Emitting deletions at all is what exposed it, and only a check against real git — not the suite, not the app — caught it.

Implementation notes

Verified by running the code rather than reasoning about it:

  • jsdiff emits the no-newline marker correctly for additions and modifications; only the deletion case is wrong, because the empty new side reads as "no trailing newline".
  • git apply --check on a four-section patch (add, modify, delete-with-newline, delete-without) → exit 0, and applying it produces exactly the expected tree.
  • statusMatrix takes a stat shortcut: a same-size rewrite in the same millisecond reads as unchanged. Two tests here depend on differing file sizes and say so — matching sizes would make them green while testing nothing.

src/renderer/index.js is gitignored and built by npm run build:once, so there is no bundle in the diff.

juanmaguitar and others added 2 commits August 8, 2026 23:33
The patch is what a contributor hands over, so a change it does not
mention did not happen as far as any reviewer can tell. Three ways it
was not saying what happened:

- A removed file fell out entirely. Its status row survives the filter,
  but with no working copy to read the new side was defaulted to the
  old one, so the file compared equal to itself and was skipped. The
  worktree column now decides, which also keeps "deleted" apart from
  "present but will not open" — emitting a deletion for the second
  would tell the next checkout to remove a file nobody removed.

- Both sides were always named a/<path> and b/<path>. That filename is
  the only thing this app's own parser reads an add or a delete from,
  so a deletion came back as a modification and the applier would have
  written an empty file where the patch said remove. The absent side is
  now /dev/null, which fixes additions the same way — they were being
  refused with "is not in this checkout".

- Binary files were dropped in silence. They are named in # lines above
  the diff now, the placement the handoff header already established.

One more, found while testing rather than reported: the blank line
between sections made any patch for a file without a trailing newline
fail to re-apply, modifications included. jsdiff keeps reading past the
"\ No newline at end of file" marker, so the separator became a phantom
context line. Each section already ends in a newline.

The panel's "is there anything to attach" test moves from comparing
against the No changes. sentinel to asking whether a diff survives
under the commentary, since the notice sits above it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rix (self-review)

Three findings from the pre-PR review, all in the code the previous
commit added.

The serious one: jsdiff decides the "\ No newline at end of file"
marker from the new side, and a deletion's new side is the empty
string — so every deletion came out asserting the removed file had
lacked a trailing newline. The marker attaches to the preceding minus
line, so on the ordinary case, a file that did end in one, it says
something false about the old side and git apply refuses the patch.
All of it: git apply is all-or-nothing, so one deleted file would have
stopped a contributor's unrelated edits from applying too. Verified
against real git both ways — refused with the marker, clean without,
and still required when the file genuinely lacked the newline.

The other two: the emitted filenames were chosen from whether a buffer
loaded rather than from the status matrix, so a base blob that would
not read turned an edit into an addition that applies nowhere; those
files, and files present but unreadable, are now named above the diff
like binaries are. And the round-trip test only ever exercised this
app's own applier, which tolerates the marker — which is exactly why
the first finding survived a green suite. It is now asserted on the
bytes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juanmaguitar
juanmaguitar merged commit b658af3 into trunk Aug 10, 2026
3 checks passed
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
## Why

Switching tickets is a worktree scan and a full `git.checkout`. On a
real `wordpress-develop` that is seconds, and the window says nothing
for all of them.

Every other long operation here streams — installs, scripts, the dev
server — so this was the one place where *it is taking a while* looked
exactly like *it has hung*. The natural responses are clicking again, or
force-quitting. And force-quitting part-way through a checkout is
precisely what leaves the half-swapped worktree the mid-switch marker
exists to recover from: the app was silently inviting the state it then
has to rescue people from.

#185 turned switching into a link contributors actually click, so this
stopped being theoretical.

## What changes

The panel names each stage while it happens: saving your work on the
ticket you are leaving, then swapping the files, then ready.

**The park stages are the point, and they are entirely synthetic.**
`statusMatrix` reports nothing while it runs and is about a third of a
switch — and it covers the stretch where the contributor's edits are
committed *nowhere*. "Saving your work on #59234…" is the sentence that
stops someone quitting there. `git.checkout` reports its own progress;
the scan and the commit had to be given theirs.

Three decisions a reviewer would otherwise have to reverse-engineer, all
measured first:

- **Progress is additive.** The handlers still await and return exactly
as they did. The obvious alternative — return an id, stream, finish on a
`:done` channel — would mean the renderer subscribes *after* the invoke
answers, and the first event lands ~4ms in. The beginning of every
switch would be lost. That ordering gap is already documented in
`test/preload-listeners.test.cjs`.
- **Events are throttled**, one per stage change plus one per 100ms. A
checkout of 1500 files called back **4395 times in 143ms**; unthrottled
that is not a channel, it is a flood. The rule that earns its keep is
flushing whatever was held when the stage turns over — a line that stops
at 87% and jumps to done reads as a hang, which is the failure this
exists to prevent.
- **The subscription lives in `App`, not `SiteRow`.** Every row stays
mounted (the parent hides inactive ones), so subscribing per row would
open one listener per registered site and wake all of them for each
other's events. `App` already owns this shape for setup logs.

The trunk update feeds its **own** log rather than this channel — one
operation with two progress surfaces is how the two end up disagreeing.

**Deliberately not in this PR:** `deleteTicketBranch` also checks out
and stays silent; it runs under a different busy flag, so covering it
means a second progress surface for a rarely-used destructive action.

## How to test this

**Platforms:** any, but the effect is proportional to the checkout, so a
real `wordpress-develop` shows it and a toy repo will not. Stacked on
#198.

**Starting state:** a site with two ticket branches that have work on
them, sitting on one of them with uncommitted edits.

1. Click **switch** on the other ticket. → A spinner and a line appear
under the panel. The first thing it says is that it is **saving your
work on #<the ticket you are leaving>** — by number, not `ticket/59234`.
2. Watch it through. → The line moves: saving → checking which files
change → swapping files, with a percentage → gone. It never sits on one
sentence for the whole switch, and never freezes partway and then jumps.
3. Switch on a site with **no** changes to park. → The line still
appears for the checkout half.
4. Trigger a switch from the **Attach to Trac** card's *Link ticket*
field instead. → The same line appears there.
5. Open a second site in the sidebar, then switch tickets on the first.
→ Nothing renders on the second site's panel.
6. Run **Update to latest trunk** on a site with a linked ticket. → Its
terminal shows a handful of park/return stage lines among the update
output — and the ticket panel shows nothing, because that flow owns its
own log.

**What must not have happened:**

- **No silent stretch.** If any part of the switch shows no line, that
is the bug — particularly the beginning, which is the part a
`:done`-channel design would have lost.
- **The line never freezes.** A sentence that stops moving and is then
replaced by the finished panel means a suppressed frame was dropped.
- **No spinner on an idle panel.** After the switch completes (or
fails), the line is gone.
- **The switch is no slower.** The progress callback is on the
checkout's critical path — if a switch takes noticeably longer than
before, the throttle is not doing its job.

**What could not be tested by hand:** a checkout that dies part-way,
which needs an `EPERM` from an editor or antivirus holding a file.
Covered by a wiring test asserting the last held frame is still
delivered.

## Risks and limitations

**Review outcome: 3 `[fix here]` · 2 `[follow-up]` — all 3 fixed.** The
first is worth naming here because it is the feature's whole point: the
staging stage carried no `from`, so the sentence dropped to "Saving your
work… 41%" for the *bulk* of the park — losing the ticket number for
exactly the stretch where naming it is what stops a force-quit. The scan
and commit either side read correctly, which is why it would have looked
right in any screenshot.

- **`deleteTicketBranch` stays silent** (above).
- **`branches:switch` streams to a channel no renderer reaches yet.**
Deliberate: the send-only channel is invisible to the IPC classification
guard, so the day the switcher is wired to that door, silence would
arrive with nothing failing.
- **The percentage covers only part of the work.** `Analyzing workdir`
reports a count with no total, so that phase gets a sentence and no
number rather than an invented one.
- **When a site has no linked ticket and a patch exists, the line
renders in both the ticket card and the destination card** — two copies
of the same true sentence. Cosmetic; noted rather than fixed because
this component has no test coverage for its output.

## Related

Closes #173. Stacked on #198#185#168, all part of #108's line of
work.

---

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

**Why not `nonBlocking`/`batchSize` on the checkout?** They yield to the
event loop between batches, which sounds like it would help the events
reach the renderer. Measured, they are not needed — the events already
arrive spread across the whole checkout (first at 4ms of 143ms, last at
141ms). And they are not free: yielding *widens* the window in which the
worktree is half-swapped, which is the state this issue is trying to
make less likely.

**Why our own stage vocabulary instead of passing isomorphic-git's
phases through?** `Analyzing workdir` is not a sentence for a
contributor, and the park stages have no library phase at all. Mapping
in one place means a version bump breaks a lookup, not the UI.

**Why a shared `src/switch-progress.cjs` rather than main-side text?**
The throttle belongs to the sender and the sentence to the renderer, but
they are two ends of one contract — payload shape and stage names.
Keeping them in one dependency-free module means `node --test` pins
both, and the trunk-update log reuses the throttle in a different mode
rather than growing its own.

</details>

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

**3 [fix here] · 2 [follow-up] — all 3 fixed.** Judgement pass in a
fresh context per the skill.

| # | Dimension | What was wrong |
|---|---|---|
| 1 🟡 | Architecture | The staging stage omitted `from`, so the sentence
lost the ticket number for the longest stretch of the park. Fixed at the
one call site; now asserted on the rendered sentence, not on the payload
fields — the fields were what let it through. |
| 2 🟡 | Architecture | A refused switch left its last frame in the map,
so the discard-and-switch that follows a dirty-trunk refusal ran under a
spinner describing the attempt that never happened. |
| 3 🔵 | Architecture | Every `flush()` was a bare statement after an
`await`, so a switch that threw mid-checkout dropped what the throttle
held — contradicting the module's own contract, and in the trunk update
losing the last line of the stage that failed. Now in a `finally`, with
a test that fails without it. |

`[follow-up]`: `deleteTicketBranch` staying silent, and the line
rendering twice when both link controls are on screen. Both in Risks.

The review also confirmed the parts most likely to be wrong and were
not: the throttle's stage-change flush, `emit` returning no thenable
(isomorphic-git awaits it), the `stageWorktree` filter being exactly
equivalent to the guard it replaced, and the subscription being
leak-free.

</details>

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

Measured before designing, on 1500 files: `statusMatrix` 91ms · `commit`
14ms · `checkout` 143ms, and 4395 checkout callbacks — 3895 of them
`Analyzing workdir` with no `total`. `wordpress-develop` is several
times that.

`test/preload-listeners.test.cjs` gains the first coverage of the
`subscribe*` family — one listener per subscribe, removal by handler
rather than by channel, independent unsubscribes, payload without the
Electron event, double-unsubscribe safe. Pinned for the whole family,
not just the new member, because the leak shape (`App` subscribing once
per app rather than once per run) is what makes removal-by-handler
load-bearing.

</details>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
## Why

Found by driving the app, not by the suite.

Link a ticket while you have uncommitted changes on trunk and one of two
things happens, depending on something you cannot see:

- **The ticket already has a branch on this site** → the switch is
refused, and the panel offers to save the changes as a patch or discard
them.
- **The ticket is new here** → the changes come along into the new
branch, **silently**.

Same starting state, same click, opposite outcomes, and nothing on
screen telling you which one you are in. The reporter's words were "algo
nos hemos cargado por el camino" — it read as a regression, because the
app had asked the last time. It had not regressed; they had linked a
ticket the other site happened to already have a branch for.

Carrying the work is the right behaviour and stays. *I started editing,
then realised which ticket this is* is a real sequence, and a
confirmation dialog on it would punish the correct flow. The bug is that
the app moves someone's work without saying so.

## What changes

Two sentences.

**Afterwards** — a notice naming how much came along and where: *"Your 2
uncommitted changes came along into #62281, and will go into its
patch."*

**Beforehand** — a line under the Link ticket field: *"Anything you have
edited so far will come along into a ticket this site has not worked on
yet."* Scoped on purpose: it is false for the tickets listed directly
above it, which are refused instead.

The decision a reviewer would otherwise have to reconstruct: **the count
runs after the answer, on its own channel.** Linking a ticket is a HEAD
move and is instant; counting is a walk of the whole worktree. Holding
the reply for it would make the most common Contributor Day sequence —
install a site, link a ticket, *then* start editing — pay a scan for an
answer that is always zero. It is counted against the new branch's own
HEAD, so the number says what this ticket's patch will contain rather
than what trunk happened to have.

And it is **not** a stage of the switch progress (#173), which was my
first attempt: on trunk with no ticket to name, a `scan` stage renders
as *"Saving your work…"* — about the one branch this whole feature
refuses to write to.

**Deliberately not in this PR:** the count is `statusMatrix`'s idea of
changed, so it includes untracked files a contributor may not think of
as edits. Naming the files instead of counting them would be truer; it
is also a different, larger design.

## How to test this

**Platforms:** any. Stacked on #205.

**Starting state:** a site with **no** ticket branches, sitting on
trunk.

1. Edit a file and create another. Do not link anything yet.
2. Look under the **Link ticket** field. → It says what will happen to
those edits.
3. Link a ticket number this site has never used. → The link is
**instant** — no pause while it thinks — and a moment later a notice
appears: *"Your 2 uncommitted changes came along into #N, and will go
into its patch."*
4. **Submit patch.** → Both files are in it. The notice told the truth.
5. **Unlink**, edit something on trunk again, and now click **Continue
working on #N** for the ticket you just created. → The other path:
refused, with *save as a patch* / *discard and continue*.
6. Link a ticket on a **clean** trunk. → No notice at all.
7. With two sites open, link a ticket on the first. → Nothing appears on
the second.

**What must not have happened:**

- **No pause on step 3.** If linking a ticket now takes a second on a
real `wordpress-develop`, the count is back on the reply path.
- **No claim about trunk.** Nothing in the switch progress line may say
"Saving your work…" while you are on trunk — trunk is never written to,
and saying so contradicts the refusal in step 5.
- **No notice when nothing moved** (step 6), and none on a site that did
not move anything (step 7).
- **The count matches the patch.** If it says 2 and the patch has 3
files, the sentence is worse than silence.

**What could not be tested by hand:** a worktree that cannot be walked
(a permission, a volume that went away). Covered by a wiring test
asserting the link still succeeds, no notice is sent, and the failure
reaches the log.

## Risks and limitations

**Review outcome: 6 `[fix here]` · 2 `[follow-up]` — all 6 fixed.** Two
are worth naming here because both were mine, introduced in the first
commit: the count sat in front of the reply, and the progress frames I
wrapped it in rendered as *"Saving your work…"* on trunk followed by
*"Ready to work on #N"* before the branch existed.

- **The count includes untracked files** the contributor may not think
of as theirs (above).
- **A failed count costs the notice, silently as far as the panel goes**
— deliberately: the link already happened and is not in doubt. It is
logged, so a bug report carries it.
- **The up-front line is static**, not a reading of the worktree. It
costs nothing and holds for the case it describes, but it appears
whether or not anything is actually edited.

## Related

Part of #108. Stacked on #205#198#185#168.

---

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

**Why not ask before carrying?** Because the carrying case is the good
one. "I started editing, then realised which ticket this is" is the
sequence the behaviour was designed for, and a confirmation on every new
ticket would tax the correct flow to fix what is an information problem,
not an intent problem.

**Why a separate channel instead of the switch progress stream?** The
progress stream describes an operation while it runs, and its vocabulary
is stages. This is one fact after the fact. Forcing it into a stage
produced literally false sentences (above) — which is the argument,
rather than tidiness.

**Why count against the branch's HEAD rather than trunk?** They are the
same commit at that moment, since `git.branch({checkout:true})` only
moves HEAD. But counting against the branch says "what this ticket's
patch will contain", which is what the sentence claims; counting against
trunk would be describing where the work came from.

</details>

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

**6 [fix here] · 2 [follow-up] — all 6 fixed.** Judgement pass in a
fresh context per the skill, on the first commit here.

| # | Dimension | What was wrong |
|---|---|---|
| 1 🟡 | Performance | A full worktree walk in front of the reply, on a
path that was a HEAD move — and always zero in the most common flow.
Moved behind the answer, onto its own channel. |
| 2 🟡 | Architecture | The progress frames rendered as "Saving your
work…" on trunk, and "Ready to work on #N" before the branch existed.
Removed; a fact after the fact is not a stage. |
| 3 🟡 | Architecture | `.catch(() => 0)` was a silent catch against the
repo's explicit invariant — and `.catch` does not cover a synchronous
throw, which is exactly what a partial test stub produced. Now logged. |
| 4 🔵 | Tests | Nothing asserted where the scan does **not** run, so a
refactor could have moved it onto every switch with the suite green.
Added. |
| 5 🔵 | Architecture | The second render site was unreachable — it lived
in the no-ticket branch, and the notice only has content once a ticket
is linked. |
| 6 🔵 | Correctness | The up-front line promised work would come along,
directly under a list of tickets for which it is refused. Scoped. |

`[follow-up]`: the count including untracked files, and the
index-vs-worktree edge where a staged-then-reverted file is not counted.
Both in Risks.

Worth flagging rather than burying: **every one of the six was in code
this PR added.** The change is small; the first attempt at it still
managed to put a lie on screen and a scan on the critical path.

</details>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
## Why

The card's unsubmitted-changes note never appears for the work its own
sentence describes. Link a
ticket, make a change, switch to another ticket, come back — the card
says nothing, while the patch
modal shows the change exactly as it was left. For anyone using more
than one ticket, that is all of
their work.

Observed on a real site, and the two readings disagree on disk:

```
git status                  ->  empty
git diff <baseOid>..HEAD    ->  src/wp-login.php, +3 lines
```

Both answers are correct. They answer different questions, and only one
of them is the question a
contributor is asking. `#239` has the reasoning.

## What changes

**Root cause.** The note measured the worktree against `HEAD`. Under the
ticket-as-branch model
(#108) a ticket parks its work in a WIP commit, so `HEAD`-relative is
*correctly* "clean" for every
change that has survived a ticket switch — which is the work the note
exists to speak about.
`changes-note.cjs` was written before tickets were branches, when "not
written down" and "not
submitted" described the same edits. Splitting them is what opened the
gap.

**The fix is to make the note ask the patch's question**, measured from
the ticket's branch point
rather than from the last time anything was written down. That is the
measurement
`createMinimalPatchForDir` already makes, so this is the two agreeing
rather than a third answer
being invented.

**Which reader wants which question** — the part worth reviewing:

| Reader | Question | Why |
| --- | --- | --- |
| The card's note | **wide** — `git:unsubmitted-work`, from the branch
point | Parked work is unsubmitted work. It is the whole subject of the
sentence. |
| `startTrunkUpdate`'s dirty dialog | narrow — `git:worktree-dirty` | It
protects what a force checkout would overwrite. Parked work survives
one; uncommitted edits do not. |
| The `dirty-trunk` switch refusal (`sites:set-ticket`) | narrow —
`countChangesAgainst` | Same thing: it is about edits on trunk that have
nowhere to go. Trunk is never committed to, so there is no parked work
there to see. |
| `git:preview-patch`'s collision scan | narrow — `collectDirtyFiles` |
It lists the files an incoming patch would collide with in the *working
tree*. |

So the narrow signal is untouched and keeps its channel; the wide one is
a new channel beside it,
and only the note reads it. Nothing was widened underneath a caller that
did not ask for it.

Two consequences that are not obvious from that summary:

- **A discard now reports what it left behind.** On a ticket branch a
discard rewinds to the last
park — the WIP commit is not its to take, since destroying a ticket is
what deleting its branch is
for. The old code asserted "clean" locally after a discard, which would
now hide the note over work
that is still there. `git:discard-changes` replies with a recount and
the card renders that.
- **A branch change re-takes the measurement.** A branch-point count is
*about a particular branch*,
so a stale one is not an old version of the new answer — it is about
different work. Linking,
unlinking, resuming and deleting a ticket now invalidate the note and
walk again. This was a
self-review finding, not part of the original design; see the review
block.

**Deliberately not in this PR:** the note's wording is unchanged (it was
already right — the
reassurance about unlinking is about work that outlives a link, which is
exactly the parked work it
could not see), and #236's applied-patch record is untouched.

## How to test this

**Platforms: any.** The change is git-graph logic through
`isomorphic-git`, with no path, spawn or
line-ending surface. Buildkite has signed artifacts for this branch —
check the build matches the
head commit.

**Starting state:** a site set up and built, with no ticket linked and a
clean tree.

1. Link ticket **12345**. Edit `src/wp-login.php` — add a comment line.
Return focus to the app.
→ The ticket card reads **"You have 1 unsubmitted change for ticket
#12345."**
2. Link ticket **54321** (a ticket this site has not worked on). The
switch parks #12345's work.
→ The card names #54321 and shows **no** changes note. It must **not**
say "1 unsubmitted change
for ticket #54321" — that is the stale-count bug the second commit
fixes, and it is the one thing
   here you have to be looking for to see.
3. Link **12345** again.
→ **This is the bug.** Before this PR the card is silent. Now it reads
**"You have 1 unsubmitted
change for ticket #12345"**, with *review and submit* and *discard your
changes*.
4. Click **review and submit**. → The modal's diff shows the same single
change to `wp-login.php`.
   The note and the modal now agree, which is the whole point.
5. Close the modal and click **discard your changes**, confirm.
   → The note goes away and the tree is clean.
6. Make two edits on #12345. Switch to #54321 and back, then click
**discard your changes**.
   → The note goes away. Nothing is left claiming changes that are gone.

**Then the narrow readers, which must be unchanged:**

7. On **trunk**, with an uncommitted edit, click **Update to latest
trunk**.
   → The dirty-tree dialog still appears and lists the file.
8. On **trunk** with an uncommitted edit, link a ticket.
→ The "decide what happens to it" panel from #238 still appears, with
the same three options.
(This one shares the area with #238 — worth a look for that reason as
much as this one.)
9. Delete a *different* ticket's work from the switcher while on #12345.
→ The note for #12345 stays on screen and does not blink out and back.
Deleting the ticket you are
   **on** does clear it.

**What must not have happened:**

- **No work discarded.** Step 3 is the load-bearing one: the parked
change must still be there and
must still be the only thing in the patch. If the note appears but the
patch is empty, or the patch
gained files from another ticket, the base being diffed against is
wrong.
- **The note must not outlive its branch.** A count from the ticket you
just left, rendered under the
number of the ticket you just arrived at, is the failure mode of this
design — with a live
  *discard your changes* link under it. Steps 2 and 9 are what catch it.
- **`node_modules` must not be rebuilt** by any switch here.
- **The trunk-update and ticket-switch guards must not have loosened.**
If step 7 or 8 stops asking,
the narrow signal was widened and a force checkout can now land on
uncommitted edits.

**Which tests cover it, and the check that they fail without the fix:**
the 10 new tests in
`test/ipc-wiring.test.cjs` and `test/changes-note.test.cjs`. Verified by
resetting `src/` to the
stack base with the tests held at their new state — all 10 fail, 124
pass. The end-to-end one is
`git:unsubmitted-work sees the parked ticket work a clean status hides
(#239)`, which builds a real
repository in the parked state and asserts both readings on the same
tree: `git:worktree-dirty`
clean, `git:unsubmitted-work` dirty.

**What could not be tested by hand:** nothing in this change; every path
above is reachable in the
app. What is *not* covered by the suite is the wiring in `index.jsx` —
`reprobeAfterBranchChange`
being called from the right places is a call, not a branch, so it is
verified by steps 2 and 9 rather
than by a test (see Risks).

## Risks and limitations

**Review outcome: 5 [fix here] · 1 [follow-up] — all 5 fixed, 1
deferred.** Two passes; the first
found a bug this PR introduced. Detail in the collapsed block.

- **The focus probe costs more than it did.** `collectChangedFiles`
reads both sides of every changed
row into buffers and decodes them, where `collectDirtyFiles` read a blob
only for CRLF candidates —
and the note probes on every window focus. `statusMatrix` still
dominates, and the changed-row count
is small for a normal ticket, so no user-visible path was identified.
Deferred rather than fixed:
the cheap version needs the buffers only up to the line-ending
comparison, which is a change to the
shared classifier and belongs on its own. **Follow-up issue not yet
filed** — happy to file it.
- **`index.jsx` wiring is not test-covered.** The re-probe calls live in
the component, which the
suite cannot load. The *decisions* are all in `changes-note.cjs` with
tests, per the repo
invariant; what is untested is that they are called from the right four
places. Steps 2 and 9 of
  the manual test are what stand in for that.
- **`branches:delete` gained a reply field** (`movedToTrunk`). Additive,
and no stored shape changed,
  so there is no migration.
- **This PR sits on #238, which touches the same handlers.** Rebased
onto its current tip (`0c9902c`)
  rather than merged, so the stack keeps one shape.

## Related

Closes #239.
Sits on #238 (base branch), the tip of #168#185#198#205#218#238.
Related: #234 (the other decision that hangs on this reading of
"changes"), #236 (the same shape
again — one thing moved to a branch, another stayed behind), #108 (the
ticket-as-branch model that
split the two questions apart).

---

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

**Rejected: widen `git:worktree-dirty` so everyone gets the branch-point
answer.** The smallest diff
and the wrong one. It would have made the trunk-update dialog offer to
save and discard *parked*
work, and made the `dirty-trunk` refusal fire for a ticket that has
nothing loose in its tree at all
— a switch refused over work that is already safely committed. The issue
calls this out directly, and
it is the finding I most expected to have to defend, so: two questions,
two channels, and a table in
"What changes" saying which caller asks which.

**Rejected: derive the note's count in the renderer from the patch
text.** The modal already fetches
a diff; counting its `+++` lines would have needed no new channel. But
it means generating and
transferring the full patch on every window focus to answer a yes/no,
and it makes the note's truth
depend on the patch *renderer* rather than on the walk. The count and
the diff should be two readings
of one walk, not one derived from the other's output.

**Why the classifier was extracted rather than the count re-derived.**
`collectUnsubmittedFiles`
could have filtered on its own rules — "skip binaries, skip
line-ending-only churn". It would have
drifted from the patch's rules within one change to either.
`classifyChangedFile` now holds that
decision once and both read it, which is what makes "the note and the
modal never disagree" a
property of the code rather than a promise. It also decides the *count*:
a binary change counts,
because the patch names it above the diff and the contributor has to
hear about it, while a
CRLF-only difference does not, because nothing would be in the patch.

**Why a discard reports a recount instead of the renderer re-probing.**
It could have called
`refreshDirty()` after the discard. The reply is better: it is one walk
instead of two, it cannot
race the probe already in flight, and it makes "what survived this" a
fact the main process states
rather than something the renderer infers from a second question. The
recount is non-fatal — if it
throws, the discard still succeeded and the reply just says less.

**`branches:delete` gained `movedToTrunk` rather than the renderer
inferring it.** `current` reports
`trunk` both when the delete took the checkout with it and when the
delete was simply *made* from
trunk. The renderer cannot tell those apart, and it now re-walks the
tree on this answer, so main
states the fact instead. Pinned by a test that asserts `current` is
`trunk` on a path where
`movedToTrunk` is false — the exact ambiguity that made the field
necessary.

</details>

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

**5 [fix here] · 1 [follow-up] — all 5 fixed, the follow-up deferred
with a reason.**

Run per `.claude/skills/self-review`, dispatching the judgement pass to
a subagent with the diff and
`.github/instructions/code-review.instructions.md` and nothing else from
the authoring session. Run
twice: the first pass found a bug the fix itself introduced, so the
second pass reviewed the
corrections. Deterministic layer clean throughout — `npm run lint`
passes, and the suite is green on
`.nvmrc`'s Node (24.18.0) and on Electron's bundled Node.

**First pass — 2 [fix here] · 1 [follow-up]:**

1. **🟡 architecture, `index.jsx` — the note was never re-taken when the
branch changed.** Introduced
by this PR: with a `HEAD`-relative probe the post-switch answer was
always "clean" and a stale
value was harmless; a branch-point measurement changes with the branch.
So resuming a ticket
showed the *previous* ticket's count next to the *new* ticket's number,
with a working discard
link, until the window lost focus. **Fixed** —
`reprobeAfterBranchChange` on every path that moves
the checkout, clearing the note while the new walk runs so no wrong
sentence is ever rendered. The
probe's `inFlight` guard also silently dropped a request made mid-walk,
which was safe only while
the answer could not change without the window losing focus; it now
queues and re-runs.
2. **🔵 architecture, `index.jsx` — an inline second answer to a question
the module already
answered.** `noteAfterDiscard` decided "does this reply carry a recount"
inline, slightly
differently from `discardOutcome`, and its three call sites disagreed
about whether they passed a
raw reply or a normalised one. Exactly the shape of #180, and against
the §1 invariant. **Fixed** —
the decision is `noteAfterDiscard` in `changes-note.cjs` with its own
tests; the component holds
   a state assignment and a call.
3. **🔵 performance, `main.js` — the focus probe reads and decodes more
than it used to.**
   **Deferred**, see Risks.

**Second pass — findings 1 and 2 confirmed resolved; 2 new [fix here],
both 🔵, both fixed:**

4. **architecture — `deleteTicketWork` re-walked unconditionally.**
Deleting a ticket you are not on
does not move the checkout, so the note blanked and rebuilt the
identical sentence, paying a full
walk for a tree that never changed; and the walk was started before
`loadStatus()`, so a fast
answer could render trunk's count under the ticket number the delete had
just cleared. **Fixed** —
`movedToTrunk` (new, tested) gates it, and it runs after the status
reload.
5. **architecture — the discard behind "discard them and start clean"
dropped its recount.** Harmless
on the happy path, since the switch that follows re-walks; but a switch
that *fails* returns
without re-probing, leaving the note offering to discard trunk work that
was already gone —
   finding 1's shape on a narrower path. **Fixed.**

The second pass also verified the parts most likely to be wrong and
found them sound: the `do/while`
probe queue cannot spin or leak `inFlight` (the only `await` is inside
the inner try, and no `await`
separates the loop test from the `finally`), the `useCallback`
dependency arrays are complete,
`setWorktreeDirty(null)` is safe for its single reader, and
`classifyChangedFile` preserves the
original precedence exactly so patch output is byte-identical.

</details>

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

- **`src/main.js`** — `classifyChangedFile` extracted from
`createMinimalPatchForDir`, preserving its
precedence (unreadable workdir → unreadable base → binary →
line-ending-equal → text) so the patch
is unchanged. `collectUnsubmittedFiles` runs `patchBaseOid` +
`collectChangedFiles` and keeps
everything the patch would speak about. New `git:unsubmitted-work`
handler beside
`git:worktree-dirty`, same reply shape. `git:discard-changes` appends
the recount.
- **`src/preload.js`** — `hasUnsubmittedWork`, a named bridge function;
the surface is not widened.
- **`src/renderer/changes-note.cjs`** — `discardOutcome` passes a
recount through when there is one;
new `noteAfterDiscard` decides the post-discard card state, falling back
to clean when a reply
carries no recount (what the old `markTreeClean` asserted
unconditionally).
- **`src/renderer/index.jsx`** — the probe reads the new channel;
`markTreeClean` becomes
`applyDiscardToNote`; `reprobeAfterBranchChange` added and called from
`saveTicket` (which every
link, unlink, resume and carry funnels through) and `deleteTicketWork`.
- **On trunk** `patchBaseOid` returns `null` and the walk falls back to
`HEAD`, so a site that never
  linked a ticket gets exactly the answer it always got. Pinned by
  `git:unsubmitted-work matches git:worktree-dirty on trunk (#239)`.

</details>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
…es that predate it (#245)

> [!WARNING]
> **Do not merge this before the ticket-branches stack (#168 #185 #198
#205 #218 #238 #241 #242 #243).** It documents features that are not on
trunk yet. Merged first, the published guide describes an app nobody can
download.

## Why

The user guide describes a site as one working tree and one implicit
patch, because that is what the app was when the guide was written. The
ticket-branches stack makes a ticket a branch on the site, which leaves
several pages not merely incomplete but wrong: what a patch contains,
what **Unlink** costs you, what a trunk update does to work in progress,
whether the applied-patch banner describes the ticket you are looking
at.

The stack also adds surface nothing documents at all — the tickets card,
the question asked about edits made before a ticket was picked, the note
about unsubmitted work, the switch progress line, coloured pull request
states.

## What changes

Docs only. The diff is `docs/` — guide pages,
`docs/.vitepress/config.mjs` for the sidebar, and screenshots. No
`src/`, no `scripts/`.

**New page: `guide/ticket-branches.md`, "Working on several tickets".**
The model (a site is the expensive substrate, a ticket is a cheap
branch), starting a second ticket, the tickets card in both its states,
what a switch says while it runs and what to do when one fails part-way,
deleting one ticket's work, the four-choice question about loose trunk
edits, the unsubmitted-work note, and what a ticket's patch does and
does not contain. Added to the sidebar under "Contributing changes",
after "Working on a Trac ticket".

**Corrections to pages that describe the old model:**

| Page | What was wrong |
| --- | --- |
| `trac-tickets.md` | Unlink described as only forgetting an
association; no mention that a site holds many tickets, that linking
asks about loose edits, or that PR states are now coloured pills with a
third value |
| `submitting-changes.md` | "everything this site has that its copy of
trunk does not" — the diff is now the ticket's own work, measured from
its branch point. Also gained what a patch can and cannot carry:
deletions now travel, binaries and unreadable files are named above the
diff rather than dropped |
| `trunk-updates.md` | Said nothing about being on a ticket — that the
update parks and returns, and that the branch point deliberately does
not move |
| `applying-patches.md` | Applied-patch state is per ticket now; and
"use Update to latest trunk to reset the checkout" does not hold on a
ticket branch, where the update puts you back exactly where you were |
| `creating-a-site.md` | Implied a site per piece of work |
| `managing-sites.md` | **Delete this site** takes every ticket's work
with it; and its dirty-tree dialog is about loose edits only, never
parked ticket work |
| `troubleshooting.md` | New entry for the refused-switch message, which
is what someone will search for |
| `getting-started.md`, `setup-wizard.md`, `submit-*.md`,
`running-the-site.md` | The button is **Review & submit changes**; the
guide still quoted **Submit changes** everywhere |

**Deliberately not in this PR:** new entries in
`scripts/screenshots/shots.cjs` for the panels the stack adds. A shot
definition belongs with the code that draws the panel, and one added
here would fail on trunk where the panel does not exist — see Risks.

## How to test this

**Platforms:** any. This is a static site; nothing here touches the app.

**Starting state:** this branch checked out.

1. `npm run docs:build` → completes. This is the real check:
`ignoreDeadLinks: false`, so a broken link or a missing image fails it.
2. `npm run docs:dev` and open `/guide/ticket-branches` → the page
renders, the six screenshots load, and "Working on several tickets" is
in the sidebar under **Contributing changes**, between "Working on a
Trac ticket" and "Applying patches and PRs".
3. Follow the internal links out of that page —
`#what-a-patch-contains`, `#edits-you-made-before-picking-a-ticket`,
`#deleting-a-ticket-s-work`,
`trunk-updates#updating-while-you-are-on-a-ticket`,
`submitting-changes#what-a-patch-can-and-cannot-carry` → each lands on
the heading it names, not at the top of the page.
4. `npm run lint` → clean.

**And the part a build cannot check** — drive the stack (the Buildkite
artifact for `juanmaguitar/pr-state-colours` exercises all nine PRs)
with the new page open, and check the prose against the app:

5. On a site with two tickets, read "Starting a second ticket" and
follow it literally. → Unlink, then type the second number. The page
should not describe a control that is not on screen.
6. Read the switch progress sentences in the page against the ones the
panel actually shows.
7. Compare the six screenshots against the panels as they render for
you.

**What must not have happened:**

- **No `src/` or `scripts/` file in the diff.** `git diff --stat
origin/trunk...HEAD` must list only `docs/`. If a screenshot-harness
change slipped in, it will fail on trunk.
- **No invented UI string.** Every label quoted in these pages was read
out of the stack's source or seen in the app. A plausible-sounding
button that does not exist is worse than no documentation.
- **No page left describing the old model.** The table above is the
list; if another page still says a site carries one ticket, it was
missed.

## Risks and limitations

**This PR must merge after the stack.** It is the only real risk here
and it is not defended by anything automated: nothing in CI knows the
difference. The Pages deploy runs on trunk, so merging early publishes
it immediately.

- **`scripts/screenshots/shots.cjs` has no entries for the new panels**,
so the six new images are not reproducible with `npm run shots` as
things stand. They were taken by driving the stack through the same
Playwright harness in a throwaway worktree, with fixture and shot
definitions that were not committed anywhere. Follow-up, and it belongs
on a branch that has the panels: the fixtures need real repositories
with ticket branches, where the current ones are empty directories.
**Follow-up issue not yet filed** — happy to file it.
- **`site-view.png` is re-shot** and is the only existing image
replaced. Its button still read **Submit changes**, which was already
stale on trunk after #235 renamed it — so that image and the text
corrections around it are true of trunk today, stack or no stack.
- **The `MERGED` pill is documented but not screenshotted.** A
repository-wide search returns no merged pull requests on
`wordpress-develop`; the state is real in the API and unit-tested, and
#243 says the same.
- **The screenshots name a real ticket and its real pull requests**
(#29798), pulled live from GitHub when the images were taken. They will
age the way any screenshot of live data ages.
- Review outcome: **5 [fix here] · 1 [follow-up] — all 6 addressed.**
Every one was a factual claim about the app, not prose. Details in the
collapsed block.

## Related

Documents #168, #185, #198, #205, #218, #238, #241, #242, #243 — the
ticket-branches stack, tip branch `juanmaguitar/pr-state-colours`. Part
of the contribution-flow tracker #110. Follows the docs site (#230), its
screenshot harness (#231) and the guide itself (#232).

---

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

**A new page rather than growing `trac-tickets.md`.** That page is about
one ticket: linking it, and reading the work that already exists on it.
Several tickets is a different subject with its own vocabulary —
parking, switching, deleting a branch — and folding it in would have
doubled the length of a page a first-time contributor reads on their
first ticket, to describe something they do not do yet.
`trac-tickets.md` gains pointers instead.

**Corrections before additions.** A page that is wrong costs more than a
page that is missing: someone acts on it. So the pass over the existing
guide came first, and the new page was written to fill what was left.

**Screenshots taken by hand rather than by adding shot definitions.**
The harness lives on trunk and the panels live on the stack, so a shot
definition committed here would fail every `npm run shots` on trunk
until the stack lands. The images were taken by assembling the two in a
scratch worktree — stack tip, plus trunk's `scripts/screenshots/` and
the `TOOLKIT_USER_DATA_DIR` hook, plus fixtures that build real
repositories with ticket branches — and only the resulting PNGs were
copied here. That worktree is gone; nothing of it is in this diff.

**Quoting the app rather than paraphrasing it.** Where a sentence in the
app is the thing being explained — the question about loose edits, the
switch stages, the confirm before a delete — the page quotes it
verbatim, so someone reading with the app open can match what they see.
Every quote was read out of the stack's source.

</details>

<details>
<summary>Review outcome (5 [fix here] · 1 [follow-up] — all
addressed)</summary>

Run per `.github/instructions/code-review.instructions.md`.
Deterministic layer inline and clean — `npm run lint` clean, `npm test`
638/638, `npm run docs:build` clean. The judgement pass went to a
subagent with the diff, the instructions file and the stack's source,
and nothing from the session that wrote the pages; it was asked to check
every quoted string and every behavioural claim against the code rather
than to read the prose.

Every finding was an accuracy finding — the guide asserting something
the app does not do. All five were verified against the source before
being fixed.

| # | What was wrong |
| --- | --- |
| 1 🔴 | The new page's central how-to. "Type the number into the **Trac
ticket** panel" — the panel holds no input while a ticket is linked; the
field is in the unlinked branch only. The route is **Unlink**, then link
the new number. Unfollowable as written. |
| 2 🟡 | The mid-switch advice described a recovery that does not exist:
the marker is written only when a checkout throws inside a running app,
so a force-quit leaves nothing behind, and `midSwitchBlock` refuses the
retry along with everything else. The one action it allows is
**Unlink**, which is now what the page and `troubleshooting.md` say. |
| 3 🟡 | `applying-patches.md`'s "use **Update to latest trunk** to reset
the checkout" escape hatch. On a ticket the update parks, resets trunk
and checks the branch back out — applied patch included — so it leaves
you where you started. |
| 4 🟡 | "Nothing is rebuilt" was true and misleading: a switch does not
rebuild, so a running dev server keeps serving the previous ticket's
assets. The page now says to run `npm run build` after a switch, which
is what the site view itself says. |
| 5 🔵 | "If the note says two changes, the patch has two files." The
note counts everything the patch speaks about, binaries included, and
those are named above the diff rather than carried — so one text file
and one image is two in the note and one in the diff. It also
contradicted this PR's own `submitting-changes.md`. |
| 6 🔵 | The by-hand replay recipe skipped the trunk update that gives
the new branch a newer base, and offered **Delete this ticket's work**
for the ticket you are on, which the card never lists. |

**`[follow-up]`, both taken here rather than deferred** because they
were two sentences each: `managing-sites.md` and `trunk-updates.md`
described the same dirty-tree dialog differently, and
`troubleshooting.md` had no entry for the refused-switch message.

Style notes, also applied: the tickets card is additionally gated on the
setup checklist being finished or skipped, and the loose-edits question
can be raised from the **Attach to Trac** card's link field too.

The pass also confirmed what was most likely to be wrong and was not:
every other quoted label and message is verbatim, all six screenshots
match their alt text, `#deleting-a-ticket-s-work` resolves under
VitePress's slugifier, and the new page is in the sidebar.

</details>

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

**How the screenshots were taken.** `git worktree add` from the stack
tip, then `git checkout <trunk> -- scripts/screenshots docs/…`,
`playwright-core` installed, and the `TOOLKIT_USER_DATA_DIR` hook from
`src/main.js` re-applied — the stack predates all of it, so `npm run
shots` does not exist on that branch. Two throwaway fixture variants
were added, both building real repositories through the app's own
`src/ticket-branches.js` (`startTicketBranch`, `parkCurrentWork`,
`switchToBranch`) so the state is the state the app makes: one site on
`ticket/29798` with parked work and a second branch, one parked back on
trunk with a loose edit. Same 1200×800 window and
`--force-device-scale-factor=1` as `capture.cjs`, so the new images sit
beside the existing ones without a size jump. The worktree has been
removed.

**The new slugs**, none of which `shots.cjs` knows about:
`ticket-list-card`, `ticket-list-unlinked`, `trunk-work-question`,
`carried-work-notice`, `linked-pull-requests`, `site-with-tickets`.

**`linked-pull-requests.png` is one image doing three jobs** — the
coloured state pills (#243), the unsubmitted-changes note (#241), and
the reorganised ticket card with the tickets list no longer inside it
(#242).

**`docs/` is its own npm package**, so `npm run docs:build` needs `npm
--prefix docs ci` first on a fresh checkout.

</details>

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

Six new images, and one replaced.

**The tickets card, linked** — `ticket-list-card.png`:

![Other tickets on this
site](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/docs-ticket-branches/docs/public/screenshots/ticket-list-card.png)

**The tickets card, unlinked** — `ticket-list-unlinked.png`:

![Your tickets on this
site](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/docs-ticket-branches/docs/public/screenshots/ticket-list-unlinked.png)

**The question about loose trunk edits** — `trunk-work-question.png`:

![You have 1 uncommitted change on this site, not on any ticket yet.
What should happen to
them?](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/docs-ticket-branches/docs/public/screenshots/trunk-work-question.png)

**After choosing to carry them** — `carried-work-notice.png`:

![Your 1 uncommitted change came along into #62281, and will go into its
patch.](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/docs-ticket-branches/docs/public/screenshots/carried-work-notice.png)

**The ticket card with coloured pull request states and the
unsubmitted-work note** — `linked-pull-requests.png`:

![A red CLOSED pill and a green OPEN pill on the two linked pull
requests](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/docs-ticket-branches/docs/public/screenshots/linked-pull-requests.png)

**The three cards in their new order** — `site-with-tickets.png`:

![Trac ticket, then Other tickets on this site, then Apply a patch or
PR](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/docs-ticket-branches/docs/public/screenshots/site-with-tickets.png)

**Replaced:** `site-view.png`, whose action bar read **Submit changes**.
No stack behaviour is visible in it — it is a straight re-take of a
stale image.

</details>

---------

Co-authored-by: Claude Opus 5 (1M context) <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.

Generated patches silently omit file deletions, and patch generation mutates the user's git index

1 participant