Work a second ticket without rebuilding the world: the git layer (#108) by juanmaguitar · Pull Request #168 · WordPress/contributor-toolkit

juanmaguitar · 2026-08-07T16:48:38Z

Why

A site is one working tree and one implicit patch. There is no branching, stashing or committing anywhere in the app.

So a contributor who finishes ticket #59234 and wants to start #61002 has two options, and both are bad: clone a second site — another ~54 MB fetch, another npm install, another first build, which is exactly the setup cost this app exists to remove — or keep working in the same tree, where the second ticket's changes pile onto the first and the generated patch mixes both with no way to separate them afterwards.

Finishing a first ticket and starting a second is the moment a first-time contributor becomes a repeat one. It is currently the worst-supported transition in the app.

What changes

The site is the expensive substrate (clone + node_modules + build output). A ticket becomes a cheap branch on top of it, so switching costs seconds instead of a rebuild.

Two invariants hold it up:

trunk is never committed to. It stays the pristine snapshot every branch is diffed against. Work started on trunk is carried into a ticket branch — git.branch({checkout: true}) only moves HEAD, so uncommitted edits come along and nothing is thrown away.
A ticket branch holds exactly one WIP commit. Parking passes parent: [baseOid] explicitly instead of committing onto the previous WIP commit, so re-parking rewrites that commit rather than stacking a pile of saves nobody asked for.

The diff base becomes the branch point each branch records, which is what makes a patch mean "only my work on this ticket" once WIP commits exist.

Deliberately not in this PR: the "Working on:" switcher and splitting today's single Unlink into switch and delete this ticket's work — this is the main-process half, and nothing in src/renderer/index.jsx calls the new bridge yet, so a ticket is linked and resumed through the existing Trac ticket panel. Also left out: replaying an old branch onto an updated trunk, and the half of #85 that drops file deletions from patches. Each is named under Risks with its reason.

How to test this

Any platform. Windows is the more interesting one — this touches paths, line endings and checkout behaviour. Buildkite builds signed artifacts for this branch; check the build matches the current head commit.

Starting state: a site that has finished its first install and build, on no ticket.

In the site's Trac ticket panel, link 59234. → The panel shows #59234.
Edit wp-login.php — add a comment line you will recognise.
Click Submit patch. → The patch contains wp-login.php and that line, and nothing else.
Link 61002 in the same panel. → Completes in seconds, with no install or build output.
Open wp-login.php. → Your step-2 edit is gone — this is #61002's tree. Now edit a different file, say wp-comments-post.php.
Link 59234 again. → Your step-2 edit is back exactly as you left it.
Click Submit patch. → It contains only wp-login.php.

Then the case the invariants exist for: with 59234 linked and an uncommitted edit in the tree, click Update to latest trunk. The log says it is parking your work, then that it is returning to your branch. Afterwards the panel still names the ticket and Submit patch still produces your change — not an empty patch.

What must not have happened:

node_modules was never reinstalled and the site was never rebuilt. Steps 4 and 6 are checkouts; if either took minutes or streamed install output, that is the regression this whole change exists to prevent.
No work was silently discarded. Both edits survive the round trip. Deletions too — delete a file in step 2 instead of editing one, and it must still be deleted after step 6.
The patch never contains the other ticket's work, and never contains reversed upstream changes — lines you did not write, appearing as removals.

Risks and limitations

Review outcome: 9 [fix here] · 2 [follow-up] — all 9 fixed. Breakdown in the collapsed section below. The serious one was user-visible: "Update to latest trunk" parked the ticket and never came back, so the site sat on trunk while the panel still named the ticket, every patch came out empty, and the only route back to a morning's work was to unlink and re-link.

Known limitations, all deliberate:

The renderer does not call the new bridge yet, so branches:delete is unreachable in the shipped app until the switcher lands.
A ticket switch has no progress channel. It resolves an invoke(), so the window is quiet during a large checkout. The duplicated worktree scan is gone, but streaming the progress needs the UI change.
Replaying an old branch onto an updated trunk is not here. isomorphic-git has no rebase. A branch born three weeks ago keeps producing a patch that is correct against its own base but no longer applies on current trunk. The route without rebase — generate patch → branch off new trunk → applyPatchToDir — is the flow already designed in Sites have no update path: patches age against a frozen trunk #94's comments, and is its own issue.
File deletions are still dropped from generated patches (Generated patches silently omit file deletions, and patch generation mutates the user's git index #85's other half). Unchanged here: rewriting the patch generator in the same change that introduces branches would make a failure impossible to attribute.
mergeBranchMeta inherits a read-modify-write race from the existing metadata pattern. Narrow and pre-existing, but the value at stake is baseOid. Filed as Overlapping writes to a site's metadata can drop a ticket branch's patch base #172.
A contributor's own git client can move HEAD behind the registry's back, and while the patch base now follows the worktree, the ticket a handoff file is named after — and three other surfaces — still answer from the registry. Found by the post-merge self-review pass; filed as The registry and the working tree can disagree about which ticket a site is on #175 because the fix is one authority applied to four surfaces, not a patch to this handler.

What could not be tested by hand: the mid-switch recovery. It needs git.checkout to fail part-way — realistically a Windows EPERM from an editor or antivirus holding a file — which I could not stage reliably. Its guard is covered by a wiring test instead.

#169 landed on trunk while this branch was open, and gave git:save-patch a provenance header carrying trunkOid and trunkDate read from the site record. Taking either side of the conflict verbatim would have been wrong: keeping this branch's version drops the header entirely, and keeping trunk's version diffs against the implicit HEAD again — a ticket branch's WIP commit — producing an empty patch under a header that looks authoritative.

The resolution diffs against the branch's recorded baseOid and puts that commit in the header. The date comes off the commit itself rather than the site record, because on a ticket branch the two describe different commits as soon as anyone updates trunk: the oid would say "born at the July snapshot" and the date would say "August 8th". A mentor applying the patch would go to the wrong tree. When there is no branch base — a site on trunk, which is every site today — the site record is used exactly as #169 wrote it, so nothing on that path changes.

Covered by a new wiring test that builds a repository whose trunk has moved past the branch point; it fails on the naive resolution.

Parking amends rather than accumulates

Passing parent: [baseOid] on every park keeps exactly one WIP commit per branch. Accumulating would leave a trail of saves the user never asked for, which every later operation — patch, delete, switch — would then have to stay correct in the presence of. That is the risk #108 itself flags at the end.

WIP commits use a synthetic author

isomorphic-git requires author.name/email and there is no host git config to read — a contributor having no git installed is the premise of the project. These commits never leave the disk; the deliverable is still the patch, and its real author is the name on the Trac ticket.

Rejected: `git.stash`

It would avoid inventing commits, but it is a single stack per repository rather than per branch, and reading a stash back requires the same worktree state it was taken from. Branches are what the model actually wants.

Review outcome (9 [fix here] · 2 [follow-up] — all 9 fixed)

Run per AGENTS.md before opening, with the judgement pass in a fresh context so the session that wrote the code was not the one grading it.

#	Dimension	What was wrong
1 🔴	Architecture	The update parked the ticket and never returned, stranding the site on trunk while the panel still named the ticket. Now it returns; on failure it clears the stored ticket and says in the log where the work is parked.
2 🟡	Architecture	A checkout that failed part-way left HEAD on the source branch over a half-swapped tree; parking again would commit the mixture over the good WIP commit. Errors are now tagged `stage: 'checkout'` and the site is marked mid-switch until reconciled.
3 🟡	Architecture	`appliedPatch` / `updateIncomplete` were declared per-branch but no handler was moved, so ticket A's "patch applied · Revert" banner showed while on ticket B. One reader and one writer now.
4 🟡	Architecture	`branches:delete` reset `currentBranch`/`tracTicket` unconditionally, silently unlinking the ticket you were working on when deleting a different one.
5 🟡	Architecture	The migration performed git writes from read-shaped entry points, and persisted an empty `branches` map on failure — permanently, via its own "already migrated" guard.
6 🔵	Architecture	`withRegisteredSite` swallowed failures without `logError`.
7 🟡	Performance	Every switch scanned the whole worktree twice. One scan now, shared. (The missing progress channel remains — see Risks.)
8 🟡	Tests	The claim that removing the `git.add` loop was safe was only asserted by re-implementing the filter inline, never through the real handler with an untracked file present.
9 🟡	Tests	The `git:update-trunk` test's own comment said it exercised the park path; it did not — the block could be deleted and the suite stayed green.

[follow-up], both in Risks above: the renderer not calling the new bridge, and the mergeBranchMeta race.

One thing worth flagging rather than burying: the test written for finding 3 passed against the broken code on the first attempt, because it seeded no site-level value that could leak. It was hardened until it failed for the right reason. That is the "green while proving nothing" pattern the review standard names, produced while fixing a finding about it.

Second pass, after merging trunk (#169): 0 [fix here] · 1 [follow-up]. A fresh-context review of the merge resolution confirmed baseProvenance on both paths, the completeness of the merge from both parents, and that nothing renderer-controlled reaches the handoff filename or header. The one finding: the handoff header's base now follows the worktree while its ticket and filename still follow the registry, so an out-of-band checkout can produce a correctly-diffed file attributed to the wrong ticket. Filed as #175 rather than fixed here — three other surfaces read the registry the same way, and one authority applied to all four is a separate change.

Implementation notes

New: src/ticket-branches.js — park / start / switch / delete / list, with no Electron dependency so node --test drives it against real repositories, reusing ensureAutocrlf from trunk-update.js.

Changed behaviour this dragged in:

readTrunkInfo resolved HEAD to date the trunk snapshot. On a ticket branch that is a WIP commit made minutes ago, so the staleness dot (Sites have no update path: patches age against a frozen trunk #94) would never light up. It reads refs/heads/trunk, falling back to HEAD for an adopted repository with no trunk branch.
discardChanges checked out trunk by name, which under this model would silently move the contributor to another ticket. It checks out whatever is current; parked work survives, since destroying a ticket is what deleting its branch is for.

Patch generation no longer touches the index. The git.add loop that staged every untracked file and never unstaged it (#85) is redundant — statusMatrix reports untracked files as [path, 0, 2, 0] unaided, and the head !== workdir filter keeps them. Verified empirically before removal, and now through the handler with an assertion that the index is byte-identical afterwards. staleStagedPaths stays: parking stages the worktree, and indexes dirtied by earlier app versions are still out there.

Migration. siteMeta[sitePath] gains branches and currentBranch; tracTicket, appliedPatch and updateIncomplete move under the branch. A site with a linked ticket gets that branch created at the current trunk tip with its work carried onto it; a site without one gains an empty map. It runs only from handlers already about to move HEAD — a list call can land while an install or the Playground server is running against that directory.

Tests: 405 pass on .nvmrc's Node and on Electron's bundled Node; npm run lint clean. The behaviour changes to readTrunkInfo, discardChanges, per-branch applied state and returning to the branch after an update were each confirmed to fail with the previous code by reverting the source and re-running.

Screenshots or recording

Nothing on screen changed. This PR is the main-process layer; the visible surface — the "Working on:" switcher, and splitting today's single Unlink into switch and delete this ticket's work — is the follow-up.

Part of #108. This is the main-process half — branches, parking, and the diff base. The "Working on: #59234" switcher is a follow-up; nothing in `src/renderer/index.jsx` calls the new bridge yet, so the observable change today is that linking a ticket creates a branch and switching back to one restores its files. A site was one working tree and one implicit patch. Starting a second ticket meant either a second clone (~54 MB, another `npm install`, another first build) or piling both tickets into one tree and getting a patch that mixes them. The site is the expensive substrate; a ticket is a cheap branch on top of it. ## Two invariants **`trunk` is never committed to.** It stays the pristine snapshot every branch is diffed against. Work started on trunk is *carried* into a ticket branch — `git.branch({checkout: true})` only moves HEAD, so uncommitted edits come along and nothing is thrown away. **A ticket branch holds exactly one WIP commit.** Parking passes `parent: [baseOid]` explicitly rather than committing onto the previous WIP commit, so re-parking rewrites that commit instead of stacking a pile of saves nobody asked for. ## The diff base is the branch point, not `origin/trunk` The issue asks for `origin/trunk`. That is wrong, and `src/main.js` already said so: diffing against a trunk that has moved embeds reversed upstream changes and the patch applies nowhere. It cannot be the live `refs/heads/trunk` either, because `updateToLatestTrunk` moves that ref while existing branches stay where they were born. So each branch records the trunk oid it forked from, and `statusMatrix({ref: baseOid})` diffs against it — which also sidesteps `findMergeBase` on a `depth: 1` clone's truncated graph. The `origin/trunk` fetch that was performed and never used is deleted rather than put to work, saving a network round trip per "view patch". ## What this drags in - `readTrunkInfo` read `HEAD` to date the snapshot. On a ticket branch that is a WIP commit made minutes ago, so the staleness dot (#94) would never light up. It reads `refs/heads/trunk` now. - `discardChanges` checked out `trunk` by name, which would silently move the contributor to another ticket. It checks out whatever is current; parked work survives, since destroying a ticket is what deleting its branch is for. - `appliedPatch` and `updateIncomplete` describe the work, not the site, so they moved under the branch. Left at site level, ticket A's "patch applied · Revert" banner showed while on ticket B, offering to reverse A's hunks against B's tree. - The update runs from trunk, so it parks the ticket first — and returns to it afterwards. When it fails it clears the stored ticket and says in the log where the work is parked, rather than leaving the panel naming a ticket the worktree is no longer on while every patch comes out empty. ## Two things that cannot be allowed to lose work Leaving a **dirty trunk** cannot be parked (invariant 1) and `checkout({force})` would eat it, so switching refuses with `code: 'dirty-trunk'` for the caller to offer the honest options. `git.checkout` writes HEAD only after every file operation succeeds, so a failure part-way — an `EPERM` on Windows from an editor or an antivirus holding a file — leaves HEAD on the source branch over a half-swapped worktree. Parking then would commit the mixture over the good WIP commit, and parking rewrites. The error is tagged `stage: 'checkout'` (the contract `updateToLatestTrunk` already uses) and the site is marked mid-switch until it is reconciled. ## Patch generation no longer touches the index The `git.add` loop that staged every untracked file to get it into the diff, and never unstaged it (#85), is redundant: `statusMatrix` reports untracked files as `[path, 0, 2, 0]` on its own and the `head !== workdir` filter keeps them. Verified through the handler, with an assertion that the index is byte-identical afterwards. `staleStagedPaths` stays — parking stages the worktree, and indexes dirtied by earlier versions are still out there. File deletions are still dropped from generated patches. That is #85's other half, unchanged here and left to its own change: rewriting the patch generator in the commit that introduces branches would make a failure impossible to attribute. ## Migration `siteMeta[sitePath]` gains `branches` and `currentBranch`. A site with a linked ticket gets that branch created at the current trunk tip and its work carried onto it; a site without one gains an empty map. It runs only from handlers already about to move HEAD, never from a read — a list call can land while an install or the Playground server is running against that directory. Nothing is persisted when the git half fails, so a site on an unmounted volume retries instead of being stranded on the old shape forever. ## Tests 405 pass on `.nvmrc`'s Node and on Electron's. The new integration suite drives real repositories in `tmpdir`; the wiring tests cover the three new channels, the registry gate in front of them, and the mid-switch refusal. The behaviour changes to `readTrunkInfo`, `discardChanges`, per-branch applied state, and returning to the branch after an update were each confirmed to fail with the previous code. Reviewed with `/self-review` before this commit: 9 `[fix here]` findings, all fixed. Two `[follow-up]` remain — the renderer does not call the new bridge yet, and `mergeBranchMeta` inherits a read-modify-write race from the existing metadata pattern. One partial: a ticket switch still resolves an `invoke()` with no progress channel, so the window is quiet during a large checkout; streaming that needs the UI change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…-on-a-second-ticket-means-rebuilding-the # Conflicts: # src/main.js

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

… five minutes (#170) ## Why Two things keep going wrong on pull requests here, and neither is anybody's fault — nothing in the repo says otherwise. **Nobody says how to test the change by hand.** This app fails in the places `node --test` cannot reach: a real clone of `wordpress-develop`, a `node_modules` that takes minutes to install, an OS file dialog, a Windows path with a space in it. A green suite is not evidence the feature works, and a reviewer who has to guess how to drive the change usually does not drive it at all. **The descriptions are hard to read.** Detail arrives in the order it was discovered rather than the order a reviewer needs it, so the first screen is rarely the part that explains the change. The cost is not that the detail exists — it is that it sits in front of the summary. ## What changes A pull request template, plus the two rules it encodes, written down where agents and humans both read them. `.github/pull_request_template.md` — GitHub loads it into every new PR, including ones opened with `gh pr create` (as long as no `--body` replaces it). The organising rule is **a reviewer understands the change in five minutes**: Why, What changes, How to test this, Risks, Related stay visible, and everything deeper goes into a `<details>` block collapsed by default. Depth is not the enemy of a readable PR; depth *in the way* is — so detail moves rather than disappears. `AGENTS.md` gains two subsections under "Before opening a pull request": the shape of the description, and the manual-testing requirement spelled out (starting state, numbered steps in the words on screen, expected results stated so they can come out false, **what must not have happened**, platforms, and what could not be tested by hand). `CONTRIBUTING.md` gets the human-facing pointer and a fourth step in the pre-PR checklist. **Deliberately not in this PR:** any enforcement. No workflow fails a PR with a missing section — the same reasoning that keeps AI review off CI here (a public repo, no credentials) applies, and a bot that checks for a heading measures headings, not testability. ## How to test this **Platforms:** any — nothing here executes. **Starting state:** a clone of this branch, and a browser signed in to GitHub. 1. Open <trunk...juanmaguitar/pr-description-template> and click **Create pull request**. → The description box is pre-filled with the template: Why, What changes, How to test this, Risks and limitations, Related, then four collapsed blocks. **Close the tab without opening the PR.** 2. Expand one of the `<details>` blocks in the preview of [`.github/pull_request_template.md`](.github/pull_request_template.md). → The guidance inside is an HTML comment, so it never appears in the rendered description a reviewer reads. 3. Read this PR's own description against the template. → It follows it — this is the first PR written to the new shape, and the "How to test this" you are reading now is the section the rule requires. 4. From a checkout of this branch, run `gh pr create` on a throwaway branch. → The body opens pre-filled with the same template, confirming it is not a web-UI-only feature. **What must not have happened:** - No second template file. GitHub shows **no picker** when a PR is opened — several templates in `.github/PULL_REQUEST_TEMPLATE/` are only reachable by appending `?template=name.md` to the URL, so the default loads anyway and the rest are never seen. `ls .github/` should show one template and no `PULL_REQUEST_TEMPLATE/` directory. - The review standard did not move or gain a copy. `.github/instructions/code-review.instructions.md` is untouched; if it had been restated in the template it would drift, which is the failure mode this repo has already designed against. - No behaviour change. `git diff --stat trunk...HEAD` touches three Markdown files and nothing under `src/`. ## Risks and limitations Nothing enforces any of this, so it holds only as long as people follow it — the honest ceiling of a convention in a repo that deliberately runs no credentialed bots. A template is a guess about what reviewers need. If a section turns out to be dead weight in practice, deleting it is a one-line PR and preferable to leaving a heading everybody skips. The "five minutes" is a target, not a measurement. A 1,500-line PR will not hit it whatever the description says — which is why the template ends by asking whether a diff over ~800 lines should have been two. ## Related Follow-up to #168, where these commits were originally written. They affect every PR rather than that one, so they are split out to review and merge independently — which is also the advice the template itself gives. --- <details> <summary>Design decisions and alternatives considered</summary> **One template, not one per kind of change.** The first draft was written for bugfixes — `## Problem` / `## Solution` — and a feature PR does not have a problem in that sense. The obvious fix is separate `bug.md` and `feature.md` templates under `.github/PULL_REQUEST_TEMPLATE/`. GitHub's documentation rules this out: there is no picker UI at PR-creation time, and selecting a non-default template means appending `?template=name.md` to the URL by hand. In practice the default would load every time and the second file would be dead. So the headings became `## Why` and `## What changes`, which fit a fix, a feature and a process change, and the template calls out the three places where those genuinely want different things: a fix names its root cause and the test that fails without it, a feature names what it deliberately leaves out and shows its surface, and the testing steps follow the path a contributor actually takes. **Guidance inside the sections, not a separate style guide.** The hints live in HTML comments in the template itself, at the moment they are needed. A style document nobody opens while writing a PR description does not change any PR description. **The rule lives in `AGENTS.md`, not only in the template.** An agent drafting a PR body may never open the template file; it does read `AGENTS.md`. The template is the shape, `AGENTS.md` is the requirement, `CONTRIBUTING.md` points at both without restating either. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> No `/self-review` pass. The standard's five dimensions — architecture, security, performance, cross-platform, tests — have no purchase on three Markdown files with no executable content; running it here would produce a paragraph saying so. What was checked instead: that GitHub actually auto-loads a root `pull_request_template.md` (it does, including via `gh pr create`), that multiple templates require an explicit `?template=` parameter (they do — the reason there is one file), and that nothing here duplicates the review standard. </details> <details> <summary>Implementation notes</summary> Three commits, in the order the thinking went: 1. **Require a "How to test this" section on every pull request** — the rule, in `AGENTS.md` and `CONTRIBUTING.md`, before any template existed. 2. **Add a pull request template built for a five-minute review** — the template, and the visible-versus-collapsed split. 3. **Make the template fit features, not just fixes** — the `Problem`/`Solution` → `Why`/`What changes` rename, after the second commit's own template proved bug-shaped when used on a feature PR (#168). The collapsed blocks are Design decisions and alternatives considered, Review outcome, Implementation notes, and Screenshots or recording. Review outcome is collapsed rather than dropped because `AGENTS.md` requires it while a reviewer rarely needs it in the first minute — the headline count surfaces in **Risks and limitations** when it changes how the PR should be read. The **Screenshots or recording** block is deleted from this description rather than left empty: nothing in the app changed, and the only visible surface is GitHub's own PR form, which step 1 above walks you to directly. </details> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…-on-a-second-ticket-means-rebuilding-the # Conflicts: # src/main.js

… turns The Windows leg failed on "git:apply-patch never reported done" while macOS passed. Nothing about the handler is platform-specific: the helper polled a fixed 50 event-loop turns, and per-branch state (#108) made these handlers read the worktree to decide where that state lives. How many turns that takes is a property of the filesystem underneath, and the same failing path lookup is slower on Windows. Waiting on a budget of time instead still returns the instant the message lands, so nothing gets slower in the normal case — and it cannot start failing again because a handler grew one more await. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…-on-a-second-ticket-means-rebuilding-the # Conflicts: # src/main.js

…roke first Windows failed again, in three tests that arrived with #189 and #184 and carry their own inline "poll 50 event-loop turns" loop — the same shape already fixed for git:apply-patch, in code that had not been written yet when that fix landed. The cause is unchanged: per-branch state (#108) makes these handlers read the worktree to decide where that state lives, so how many turns a result takes belongs to the filesystem rather than to the code, and the same failing path lookup is slower on Windows. There is now one helper and no inline loops, with a line saying that a new one is this bug waiting to happen again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ns-rebuilding-the

…een them (#185) ## Why #168 made a second ticket cheap, but invisible: the branches exist, and nothing on screen shows them. A contributor coming back to a ticket has to remember its number and type it into the link field again — and "delete this ticket's work", the destructive half of #108's model, has no UI at all, so `branches:delete` ships unreachable. ## What changes The **Trac ticket** panel lists the site's ticket branches, in both of its states: - **No ticket linked** — a "Your tickets on this site" list above the input: *Continue working on #59234* as a link, with *edited 2 days ago* under it. The input stays for tickets that are genuinely new. - **A ticket linked** — "Other tickets on this site": *You also have work on #61002 — switch*. Without this, seeing your other tickets would require unlinking first — the exact friction #108 removes. When a switch is refused because trunk has loose edits — they cannot be parked, since trunk is never committed to — the message now carries its two ways out: **save them as a patch**, or **discard them and continue**. Separate steps on purpose: saving a patch does not empty the working tree, so the panel says so before offering the destructive one. Carrying the edits along into an existing ticket in one action is #196. Each row carries a confirmed, destructive **Delete this ticket's work** — the only UI for `branches:delete`, and deliberately a different gesture from switching. The decision a reviewer would otherwise reverse-engineer: **resuming does not use the new `switchBranch` bridge call.** `sites:set-ticket` already resumes a branch it recognises, with all the parking rules, and the renderer's `saveTicket` already owns the busy state, the error alert and the sidebar sync — so "Continue working on #N" is that same write, and the new bridge surface this PR consumes is just `listBranches` and `deleteBranch`. Row selection, ordering and the "edited N days ago" label live in a pure module, `src/renderer/ticket-branch-list.cjs`, per the `trac-ticket.cjs` convention — the JSX renders what the module returns. **Deliberately not in this PR:** tickets nested under the site in the sidebar, any change to Unlink's semantics, and a per-row busy spinner distinct from the shared saving state. The first two are the rest of #108's UI; the last is cosmetic. ## How to test this **Platforms:** any. **Stacked on #168** — its branch is the base, so this diff is only the UI. The Buildkite artifact for this branch exercises the whole stack; check it matches the head commit. **Starting state:** a site that has finished its first install and build, with no ticket linked and no ticket branches yet. 1. Open the site's **Trac ticket** panel. → No list renders in either state — just the input, as today. 2. Link `62281`, then **Unlink**. Link `61002`, then **Unlink**. → The panel now shows **Your tickets on this site** with *Continue working on #62281* and *#61002*, most recently used first, each with an "edited…" note. 3. Click **Continue working on #62281**. → In seconds the panel flips to linked `#62281`, and the list becomes **Other tickets on this site** — *You also have work on #61002 — switch*. 4. Edit `wp-login.php`, then click **switch** on #61002. → Seconds again. Click **switch** back on #62281 → your edit is back, and the "patch applied" banner (if any) always describes the ticket you are on. 5. On a row for a ticket you are **not** on, click **Delete this ticket's work**. → A confirm dialog names the ticket and says it cannot be undone. Cancel → nothing changes. Confirm → the row disappears; `git -C <site> branch` no longer shows `ticket/N`. 6. From trunk with no ticket linked, edit any file, then click **Continue working on #N** for a ticket that already exists. → The switch is refused, and under the message: *Save these edits as a patch…* and *Discard them and continue*. Save one → the panel confirms the path and says the edits are still in the tree. Then discard and continue → the switch completes. 7. Start **Update to latest trunk** (or an install/build). → Every row action and the link/unlink controls are disabled until it finishes. **What must not have happened:** - **No install, no rebuild** during steps 3–5 — these are checkouts. Minutes or streaming install output is the regression #108 exists to prevent. - **No work lost on the surviving tickets** after a delete — #62281's edit from step 4 must still be there. - **The refused switch changed nothing** — after step 6's refusal, the edits are still on trunk and no ticket got linked. - **No stale banner**: after a switch, the applied-patch banner and the update-incomplete notice always describe the current ticket — never the one you just left. **What could not be tested by hand:** the mid-switch (`switch-incomplete`) refusal — it needs a checkout to die part-way. Its message surfaces through the panel's existing error alert, which is exercised by the wiring tests on the base branch. ## Risks and limitations - **The delete depends on the confirm dialog** — the app's established `window.confirm` pattern. It destroys a branch and its parked WIP commit; there is no undo. - **`branches:delete` has no mid-switch guard on the base branch** (`sites:set-ticket` and `branches:switch` both have one). This PR is what first exposes the handler to users. Found by the review pass; it is base-branch code, so it is a follow-up there rather than a change here. - **The list loads once per mounted `SiteRow`**, matching the existing `loadStatus` precedent — work that scales with the site registry. An `isActive` gate is the follow-up shape if it ever shows up in practice. ## Related Part of #108, on top of #168 (stacked; this PR's base is that branch). Follow-up: #196 (carrying loose trunk edits into an existing ticket in one action). The sidebar half of the UI remains, tracked in #108. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> **2 [fix here] · 2 [follow-up] — both fixed**, judgement pass run in a fresh context per the skill. Plus one found by driving the app (below). 1. 🟡 Switching refreshed the branch list but not the site status, so the previous ticket's per-branch "patch applied · Revert" banner survived onto a branch it does not describe — and Revert then reported nothing to revert. Fixed: the status reload rides the same await as the list reload. 2. 🟡 The new row actions blocked each other but not Unlink/Link, and nothing blocked any ticket control during an install, build or trunk update — operations that share the working directory a switch checks out. Fixed: one `ticketActionsBlocked` guard, both directions, covering the long-operation trio the panel's other destructive controls already guard on. 3. 🟡 Found in manual testing, not by the review pass: right after a switch the on-screen list can still be the one loaded before it, so its `current` ref excluded nothing — and the panel offered "You also have work on #59234" while linked to #59234. The linked ticket is now excluded by its number as well, independently of the list's `current`, with a regression test reproducing the stale-list state. The same pass turned the unlinked state's row buttons into links, matching the linked state. Deferred, with reasons: the missing mid-switch guard on `branches:delete` (base-branch code — see Risks) and the per-`SiteRow` list load (matches the existing pattern; `isActive` gate if it ever matters). Style notes from the pass (comment wording, the `'trunk'` literal) were applied. </details> <details> <summary>Implementation notes</summary> - `src/renderer/ticket-branch-list.cjs` — `ticketBranchRows({branches, current, now})` filters out foreign branches (`ticketId: null`) and the checked-out ref (which also excludes the linked ticket, since `sites:set-ticket` always leaves you on it), sorts `lastUsedAt` desc with nulls last, and attaches `relativeTimeLabel`. Buckets are hand-rolled rather than `Intl.RelativeTimeFormat` — with an injected `now` they are testable to the millisecond, and past a week the label switches to the absolute date. - `test/ticket-branch-list.test.cjs` — 14 cases; the ≥7-day assertions check only the `"edited on "` prefix so no CI locale can break them. - `loadBranches` is deliberately not part of `loadStatus`: that one runs after every long operation, and the branch list only changes on link, resume and delete — the three paths that refresh it themselves. - The duplicate link form in the "Attach to Trac" destination card does **not** get the list — one home. </details> <details> <summary>Screenshots or recording</summary> Pending a manual pass — the walkthrough above is the script. A short recording of steps 2–5 shows the thing a still frame cannot: that a switch takes seconds, not a rebuild. </details> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## 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 (#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 dropped** — #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. --- <details> <summary>Design decisions and alternatives considered</summary> **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. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **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. </details> <details> <summary>Implementation notes</summary> 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. </details> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## 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>

## 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>

## 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>

…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>

juanmaguitar mentioned this pull request Aug 8, 2026

Give every pull request a testable description a reviewer can read in five minutes #170

Merged

juanmaguitar force-pushed the juanmaguitar/working-on-a-second-ticket-means-rebuilding-the branch from 56fc7df to 4760128 Compare August 8, 2026 08:39

Merge remote-tracking branch 'origin/trunk' into juanmaguitar/working…

dc367d9

…-on-a-second-ticket-means-rebuilding-the # Conflicts: # src/main.js

juanmaguitar requested a balanced review from Copilot August 8, 2026 08:43

Copilot AI reviewed Aug 8, 2026

This was referenced Aug 8, 2026

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

Closed

Working on a second ticket means rebuilding the world: tickets should be branches, not sites #108

Closed

This was referenced Aug 8, 2026

Show the tickets with work on a site, and let a contributor jump between them #185

Merged

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

Merged

Say what a ticket switch is doing while it does it #205

Merged

juanmaguitar and others added 2 commits August 9, 2026 15:09

Merge remote-tracking branch 'origin/trunk' into juanmaguitar/working…

9fdda33

…-on-a-second-ticket-means-rebuilding-the # Conflicts: # src/main.js

juanmaguitar mentioned this pull request Aug 10, 2026

Say when loose work came along into a new ticket #218

Merged

juanmaguitar and others added 4 commits August 10, 2026 08:16

Merge remote-tracking branch 'origin/trunk' into juanmaguitar/working…

31dcad8

…-on-a-second-ticket-means-rebuilding-the # Conflicts: # src/main.js

Merge branch 'trunk' into juanmaguitar/working-on-a-second-ticket-mea…

fae7241

…ns-rebuilding-the

Merge branch 'trunk' into juanmaguitar/working-on-a-second-ticket-mea…

952c93c

…ns-rebuilding-the

juanmaguitar mentioned this pull request Aug 10, 2026

Show the unsubmitted-changes note for a ticket's own parked work #241

Merged

Merge branch 'trunk' into juanmaguitar/working-on-a-second-ticket-mea…

7340320

…ns-rebuilding-the

juanmaguitar merged commit f025add into trunk Aug 10, 2026
3 checks passed

juanmaguitar mentioned this pull request Aug 10, 2026

Document the site-holds-many-tickets model, and correct the guide pages that predate it #245

Merged

juanmaguitar deleted the juanmaguitar/working-on-a-second-ticket-means-rebuilding-the branch August 11, 2026 11:29

Navigation Menu

Uh oh!

Work a second ticket without rebuilding the world: the git layer (#108) - #168

Work a second ticket without rebuilding the world: the git layer (#108)#168
juanmaguitar merged 9 commits into
trunkfrom
juanmaguitar/working-on-a-second-ticket-means-rebuilding-the

juanmaguitar commented Aug 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

juanmaguitar commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changes

How to test this

Risks and limitations

Related

The diff base is the branch point, not origin/trunk — against what #108 asks for

Merging #169: a handoff header must name the base the patch was diffed against

Parking amends rather than accumulates

WIP commits use a synthetic author

Rejected: git.stash

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

juanmaguitar commented Aug 7, 2026 •

edited

Loading

The diff base is the branch point, not `origin/trunk` — against what #108 asks for

Rejected: `git.stash`