Skip to content

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

Merged
juanmaguitar merged 4 commits into
juanmaguitar/ask-before-carrying-trunk-workfrom
juanmaguitar/changes-note-sees-ticket-work
Aug 10, 2026
Merged

Show the unsubmitted-changes note for a ticket's own parked work#241
juanmaguitar merged 4 commits into
juanmaguitar/ask-before-carrying-trunk-workfrom
juanmaguitar/changes-note-sees-ticket-work

Conversation

@juanmaguitar

Copy link
Copy Markdown
Collaborator

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 widegit: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:

  1. On trunk, with an uncommitted edit, click Update to latest trunk.
    → The dirty-tree dialog still appears and lists the file.
  2. On trunk with an uncommitted edit, link a ticket.
    → The "decide what happens to it" panel from Ask what should happen to loose trunk work before a ticket takes or refuses it #238 still appears, with the same three options.
    (This one shares the area with Ask what should happen to loose trunk work before a ticket takes or refuses it #238 — worth a look for that reason as much as this one.)
  3. 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.jsxreprobeAfterBranchChange
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 Ask what should happen to loose trunk work before a ticket takes or refuses it #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).


Design decisions and alternatives considered

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.

Review outcome (required — see AGENTS.md)

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. FixedreprobeAfterBranchChange 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 "Show in Explorer" and "Open in editor" fail with "unknown error" while a new site is still being set up #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:

  1. 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.
  2. 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.

Implementation notes
  • src/main.jsclassifyChangedFile 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.jshasUnsubmittedWork, a named bridge function; the surface is not widened.
  • src/renderer/changes-note.cjsdiscardOutcome 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).

juanmaguitar and others added 3 commits August 10, 2026 15:02
…anch point

The note read the worktree against HEAD, which under the ticket-as-branch
model is correctly clean for work parked in the WIP commit — so it never
fired for any change that survived a ticket switch. It now asks the same
question the patch answers, over a new git:unsubmitted-work channel, while
the checkout guards keep the narrow uncommitted-edits reading. The discard
reply carries a recount of what survived, so the note cannot mark the tree
clean over parked work a discard does not take.

Closes #239

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Self-review findings. The note's count is now measured from the ticket's
branch point, so it is about a particular branch — but nothing re-probed
when the branch changed, leaving the outgoing ticket's count next to the
incoming ticket's number until the window lost focus. Linking, unlinking,
resuming and deleting a ticket's work now invalidate the note and walk
again, and a probe asked for mid-walk is queued rather than dropped.

The card's rule for what a discard leaves behind moves into changes-note.cjs
beside discardOutcome, rather than being answered a second time inline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ter a trunk discard

Second-pass self-review findings. branches:delete now says whether it took
the checkout with it: 'current' reports trunk whether or not anything was
checked out, so deleting a stale ticket from trunk was blanking the note and
rebuilding the identical sentence. And the discard behind 'discard them and
start clean' now feeds its recount to the note, so a switch that fails after
it leaves no offer to discard work that is already gone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juanmaguitar
juanmaguitar merged commit 147a094 into trunk Aug 10, 2026
3 checks passed
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
… panels (#242)

## Why

The Trac ticket card carries four different things once a ticket is
linked: which ticket you are
on, the other tickets you have work on in this site, the pull requests
that cite the ticket, and its
Trac attachments. Three of those describe the ticket in front of you.
The fourth is a list of
everywhere else you could be instead — and switching between tickets is
one of the main things a
contributor does on a site, which is the whole point of tickets being
cheap. It read as a
sub-section of something else, set in a smaller heading than the card it
sat inside.

Closes #240.

## What changes

The list of a site's tickets gets a card of its own, at the same level
as the Trac ticket card and
the "Apply a patch or PR" card, positioned between them. The reading
order now matches what someone
is actually deciding: which ticket am I on, which of my tickets do I
want, bring in work from
elsewhere.

This is placement and heading level, not a rewrite of the list. The row
markup, the switch links,
"Delete this ticket's work" and its confirm, the `ticketActionsBlocked`
disabling and the ordering
are byte-identical — `renderBranchRows` moved, it did not change. What
is new is one exported
helper, `ticketListCard` in `src/renderer/ticket-branch-list.cjs`,
holding the two decisions the
card makes: which heading the state calls for, and that no rows means no
card rather than an empty
one.

**It moves in both states**, deliberately. Linked ("Other tickets on
this site") and unlinked
("Your tickets on this site") both leave the ticket card. Making it
conditional would mean the same
list jumps between two places depending on where you are, and unlinking
would move it. The
consequence is accepted: on a site with no ticket linked, the Trac
ticket card is left holding
little more than the field to type one into — which is honest, because
on that site typing a ticket
number is the only thing there is to do there.

**What is deliberately not in this PR:** the switch feedback
(`ticketError`, the switch progress
line, the carried/saved notices, and the "what should happen to your
loose edits" panel from #234)
still renders in the Trac ticket card, not next to the rows that can now
trigger it. See Risks.

## How to test this

**Platforms:** any. Nothing here touches paths, spawning, line endings
or signing.

**Starting state:** a site that is initialized and past the wizard, with
work on at least two
tickets. Two runs, one per state.

### With a ticket linked

1. Open a site that has a ticket linked and work on at least one *other*
ticket.
→ Three cards in this order: **Trac ticket**, **Other tickets on this
site**, **Apply a patch or
PR**. The tickets card is a card in its own right — same white panel,
same border, same
heading size as the two around it — not a section inside the ticket
card.
→ The **Trac ticket** card now holds only the ticket pill with Open in
Trac / Unlink, Linked
     pull requests, and Trac attachments.
2. Click **switch** on a row.
→ The switch runs exactly as before. Nothing about the row behaviour
changed.
3. Start a trunk update (or an install), and while it runs look at the
tickets card.
→ Every **switch** link and every **Delete this ticket's work** button
is disabled, and comes
     back when the operation ends.
4. Click **Delete this ticket's work** on a row and cancel the confirm.
   → Same confirm wording as before, nothing deleted.

### With no ticket linked

5. Open a site with no ticket linked but work on at least one ticket.
→ **Trac ticket** (heading, one sentence, the input field and Link
ticket, the "you will be
asked what should happen to those edits" line, and the
Browse-good-first-bugs link), then
**Your tickets on this site** as its own card, then **Apply a patch or
PR**.
6. Click **Continue working on #NNNNN**.
→ Switches as before. If trunk has uncommitted edits you get the #234
question panel — note it
renders at the bottom of the **Trac ticket** card, immediately above the
tickets card, not
     inside the card you clicked in.

### The empty case

7. Open a site whose only ticket is the one currently linked (or a site
with no ticket branches at
   all).
→ **No tickets card at all** — not a card with a heading and nothing
under it. The Trac ticket
     card sits directly above the patch card.

**What must not have happened:**

- The list must not appear **twice** — once in the ticket card and once
in its own. If you can see
the same ticket offered in two places, that is the bug this change could
most easily have
  introduced.
- The list must not **vanish** in either state. Unlinking a ticket must
keep it on screen, in the
same card, with only the heading changing from "Other tickets" to "Your
tickets".
- An **empty card** must not be left behind after you delete the last
other ticket's work — the
card should disappear along with the last row, not linger as a heading.
- The switch and delete controls must not become **clickable during a
long operation**. That guard
moved with them and is easy to lose silently: a switch during an install
is a checkout of a
  working directory something else is writing to.

**What could not be tested by hand:** the #234 question panel appearing
above the rows when a
switch is blocked from the new card. I could drive the app and confirm
both card layouts, but the
synthetic clicks I used would not fire that link's handler, so the
proximity claim in Risks below
is argued from the rendered geometry (measured on the captured
screenshots) rather than observed.
Worth a click by a human reviewer.

## Risks and limitations

Self-review: **2 [fix here] · 2 [follow-up]** — both [fix here]
addressed, see the collapsed block.

The one real consequence is that the switch feedback did not move with
the controls. `ticketError`,
the switch progress line, the carried/saved notices and the #234 "what
should happen to your loose
edits" panel all still render at the bottom of the Trac ticket card,
while the switch and delete
links that set them now live in the card below. Nothing is swallowed —
the ticket card is the
immediately preceding sibling in a `gap: 24` column, so the panel lands
directly above the row you
clicked, about 30px away in the unlinked state. In the linked state
proximity actually improves:
the panel used to sit at the bottom of a card that also held Linked pull
requests and Trac
attachments, a long scroll from rows near the top; now it is adjacent.
Moving the feedback into the
tickets card was rejected because the same panel is also raised by the
Link ticket field in the
ticket card above, so it would have to render in two places or be
threaded through a shared slot —
a behaviour change, and this issue is explicitly placement only.

Nothing pins where that feedback renders relative to the rows, so if it
is later moved next to
them, no test will notice. That is the assertion to add if it moves.

## Related

Closes #240. Follow-up to #185, where this list came from — a layout
change on top of it, not a
correction to it.

---

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

**Moving it only in the linked state.** Rejected in the issue and not
revisited. With a ticket
linked the list is a secondary way to move; with none linked it is the
primary way to start, sitting
directly above the field for typing a ticket number. Tempting to leave
the unlinked one where it is
for that reason — but then the same list jumps between two cards
depending on state, and unlinking
makes it move. A list that stays put is worth more than a list that is
optimally placed in each
state separately.

**Where the heading choice lives.** In `ticket-branch-list.cjs`, not
inline in the JSX. Two
literals in a component of this size drift; the module already owns
which rows exist and in what
order, so which heading they sit under belongs with them. The test
asserts index.jsx does *not*
contain either heading string, which is what keeps the two copies from
reappearing.

**Why `ticketListCard` returns null rather than the JSX deciding.** The
"no rows, no card" rule is
the kind of thing that gets lost in a conditional chain. It is a
decision, it has a reason (an
empty card is worse than no card — and unlike the card it left, this one
has no input field to
justify the space), and decisions with reasons get a test. The JSX guard
is then just
`skipInit && ticketsCard`.

**Testing layout with a source scan.** There is no DOM harness in this
repo, and the whole point of
#240 is *where* the list renders — untestable through the module's
return value alone. So the third
test reads `index.jsx` as text and asserts the ordering of the three
card headings and that
`renderBranchRows(` is called exactly once. It is not a substitute for
looking at the app, and it
is written to fail loudly with a specific message rather than subtly.

</details>

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

**2 [fix here] · 2 [follow-up] — both [fix here] resolved.**

Run per `.github/instructions/code-review.instructions.md`, with the
judgement pass dispatched to a
subagent that had the diff and the instructions file and nothing from
the session that wrote the
code.

**Fixed:**

1. *tests* — the layout test asserted the inline style literal
`fontSize: 16` to pin the heading
level. `fontSize: 16` is repeated verbatim on all three card headings,
so extracting them into a
shared constant — an obvious, harmless cleanup — would have failed the
test with the layout
unchanged. Assertion dropped; the ordering assertion above it already
covers the finding it was
   aimed at.
2. *tests* — the "renders once" assertion counted the bare string
`renderBranchRows`, which
includes occurrences in comments, so a documentation edit naming the
helper would have turned the
suite red. Now counts `renderBranchRows(` calls and expects exactly one.

**Deferred, with reasons:**

3. *architecture* — the switch feedback stayed in the Trac ticket card
while its controls moved.
Written up in **Risks and limitations** rather than fixed: the fix is a
behaviour change (the
same panel is raised from two places), and #240 is explicitly placement
only. The reviewer asked
for a by-hand confirmation that the panel is still visible when a switch
is blocked; I could not
fire that handler under synthetic clicks, so it is called out as
untested above.
4. *tests* — nothing pins where that feedback renders relative to the
rows. Deferred with 3: the
assertion to add is the one that would fail when the feedback moves, and
it should be written
   then, not now against the state it is not yet in.

**Checked and clean:** no double or zero render (one `renderBranchRows`
definition, one call site,
`Boolean(tracTicket)` reproducing the old `linked` argument in both
states); `ticketActionsBlocked`
still closed over from the same component; the empty-list guard;
`skipInit` gating matching the two
neighbouring cards exactly. Security, performance and cross-platform are
out of scope — no IPC, no
spawn, no path handling, no dependency, no `electron-store` shape
change.

</details>

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

Three files.

- `src/renderer/ticket-branch-list.cjs` — adds `ticketListCard({
rowCount, linked })`, returning
`{ heading }` or `null`. Pure and dependency-free like the rest of the
module, so `node --test`
  requires it directly.
- `src/renderer/index.jsx` — deletes the two mutually exclusive sections
(the linked one at the top
of the ticket card, the unlinked one above the Link ticket field) and
adds one card between the
ticket card and the patch card, gated on `skipInit && ticketsCard`.
`renderBranchRows` itself is
  untouched.
- `test/ticket-branch-list.test.cjs` — three tests, all of which fail
against the pre-change source.
Proved by checking out the branch point into a detached worktree,
copying the new test file in,
  and running it there: 3 failed, 15 passed. On the branch: 18 passed.

`npm test` 754/754 on Node 24.18.0 (.nvmrc) and 754/754 on Electron's
bundled Node via
`npm run test:electron`, both re-run after the rebase onto #241. `npm
run lint` clean.

</details>

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

**With a ticket linked** — "Other tickets on this site" as its own card
between the Trac ticket
card and the patch card:

_(screenshot: ticket-linked)_

**With no ticket linked** — "Your tickets on this site" as its own card;
the Trac ticket card is
left with the field to type one into:

_(screenshot: no-ticket-linked)_

</details>

---------

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

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

## Why

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

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

## What changes

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

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

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

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

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

## How to test this

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

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

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

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

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

**What must not have happened:**

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

## Risks and limitations

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

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

## Related

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

---

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

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

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

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

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

</details>

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

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

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

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

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

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

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

</details>

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

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

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

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

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

</details>

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

Six new images, and one replaced.

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

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

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

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

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

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

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

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

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

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

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

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

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

</details>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The unsubmitted-changes note never sees a ticket's own work

1 participant