Skip to content

Write the user guide and point the README at it - #232

Merged
juanmaguitar merged 1 commit into
trunkfrom
docs/guide
Aug 10, 2026
Merged

Write the user guide and point the README at it#232
juanmaguitar merged 1 commit into
trunkfrom
docs/guide

Conversation

@juanmaguitar

@juanmaguitar juanmaguitar commented Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Why

With the site (#230) and the harness (#231) in place, this is the documentation itself: eighteen
pages covering every screen the app has, and a README that hands the user-facing material over
instead of duplicating it.

Stacked on #231; review the two below it first.

What changes

  • Eighteen guide pages: creating a site, the setup wizard, running the site, trunk updates,
    editors, Trac tickets, applying patches and pull requests, the three submit destinations, the
    terminal, logs and debug.log, the mail catcher, Adminer, managing sites, troubleshooting.
  • Eleven screenshots from npm run shots, ~550 KB total.
  • README shrinks to what a contributor to this repository needs. The platform-by-platform
    install, the Gatekeeper notes, the ten-step walkthrough and the whole "Keeping a site up to date
    with trunk" section move into the guide — they are not copied. AGENTS.md asks for one home
    per fact, and a README restating the guide is exactly the drift that rule exists to prevent.
Two things the code corrected while writing

Each page was written against src/renderer/index.jsx rather than from memory, so it names
controls by the words actually on screen. Two claims did not survive that check:

  • The Logs panel has two tabs, not three. logTabs defines Server and debug.log; npm output
    goes to the Terminal. The page says so.
  • The Trac destination never posts anything. It saves the plain diff and opens the ticket's
    attach page; you upload it yourself. The page says that plainly rather than implying the app
    files it for you.

How to test this

Starting state: this branch checked out, npm ci done.

  1. npm run docs:build — expect build complete and no dead links. This is the real check on
    eighteen pages of cross-references.
  2. npm run docs:preview and open the printed URL. Walk the sidebar top to bottom: every entry
    resolves, and each page's screenshots load.
  3. Open the app beside the guide and pick three pages at random — every control named in bold must
    exist on screen with exactly that wording.
  4. Read the README end to end: it must not restate anything the guide covers, and its
    "Documentation" pointer must resolve to https://wordpress.github.io/contributor-toolkit/.

Must not have happened: no user-facing instruction may exist in both README and the guide.
That duplication is the specific failure this PR is shaped to avoid.

Not testable by hand here: the live Pages URL — it only exists once this reaches trunk.

Risks

The guide describes UI that changes. Nothing automated catches a page that has drifted from the
app; the convention added in #231 (re-run npm run shots, re-read the page) is what holds it.

Screens needing a running server are not illustrated yet — the live tier exists for them and
those images can land later without touching the prose.

The README's badge URLs still use the old repository name. They redirect, and renaming them is
unrelated churn; deliberately left for a separate change.

Related

Stack: #230 → #231 → this.

Self-review — 8 findings, all fixed; three of them were false statements about the app

Ran .github/instructions/code-review.instructions.md with the judgement pass in a fresh context,
pointed specifically at verifying every claim against src/. That was the right call: the review
found three places where the documentation simply said something untrue.

8 [fix here] · 1 [follow-up]. All fixed:

Severity Was Now
🔴 The ten-step walkthrough said Install dependencies and Build WordPress. The renderer says Install npm dependencies and Run full build, and "Build WordPress" exists nowhere in the app — while the screenshot directly below showed the real labels both corrected
🟡 terminal.md recommended npm run watch. dev-server-command.cjs documents why the app refuses it: the Gruntfile's watch wrapper runs a full production build first, "30+ minutes on a Windows VM" — the exact failure this project exists to remove replaced with a warning callout explaining the trap, and a pointer to the watcher the dev server already starts
🟡 creating-a-site.md sent a failing contributor to look in "the dialog". The modal closes before setupWordPress is awaited, so createSiteError is set into state nothing renders says the row disappears and the reason is in Help → Open App Log
🟡 Show in File Explorer — the Windows label is Show in Explorer, and editors.md already had it right, so the guide contradicted itself corrected
🟡 The README still duplicated the guide's "what you get" bullets and the "No Git, no Node" paragraph — and the two copies had already drifted, which is the exact failure AGENTS.md's single-source rule exists to prevent README keeps one positioning sentence; the expanded version lives only in the guide
🔵 The update step was called Fetch latest trunk (a label that is never rendered; the screen says Fetch and reset to trunk), and said uncommitted changes "are saved" — unconditionally, when Discard them sits right beside that button and loses the work both corrected, and the destructive choice is now named
🔵 Said a hint stays clickable while a command runs; TerminalCommandLink returns plain text when disabled corrected
🔵 Copy patch — the button says Copy corrected. Also noted that the ☰ Update to latest trunk entry, unlike the notice button, is not disabled while the server runs and silently does nothing

Deferred [follow-up]: none outstanding — the two stale "Ideas and future work" entries the guide
now contradicts (the SMTP catcher and the Trac/GitHub integration, both shipped) were removed here
rather than deferred, since leaving them is the same drift the rest of this PR is fixing.

Verified correct, and worth listing because they were the claims most likely to be wrong: the
Trac destination does not post to Trac (saveForTrac saves, then opens the attach page); Forget vs
Delete match site-registry.js; the terminal's allowed set matches TERMINAL_ALLOWED_SCRIPTS; the
Logs tabs are exactly Server and debug.log; the debug-constants table matches
wp-debug-constants.js line for line including the REST/AJAX caveat; 512 KB matches
MAX_LOG_CHARACTERS; the mail catcher binds 127.0.0.1 on an OS-assigned port; the 14-day amber
threshold, the red incomplete state and the local-only staleness judgement match update-plan.cjs;
the provenance header and filename match patch-provenance.cjs; the 90 s Trac human-check and 120 s
server-start timeouts match their sources.

Content-loss check: every section removed from the README has a covering page in the guide —
nothing was deleted rather than moved.

This was referenced Aug 10, 2026
Merged
Merged
@juanmaguitar
juanmaguitar force-pushed the docs/screenshots branch 2 times, most recently from aec819b to 17c75c8 Compare August 10, 2026 12:52
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
@juanmaguitar
Add the VitePress documentation site and its Pages deploy workflow (#230
b5d3d7f 
)

## Why

The app's user documentation is the README, and it has outgrown it:
install steps, a ten-step
walkthrough and the trunk-update rules all compete for the same page,
and there is nowhere to put
a screenshot. This PR gives the guide a home and a deploy pipeline. It
carries no guide content
beyond the landing page — that arrives in #232, and the harness that
photographs the app in #231.

## What changes

- `docs/` becomes a VitePress site with **its own npm package**, so a
docs-only CI job never runs
the root `postinstall` (electron-builder + esbuild) and the app's
dependency tree stays free of
  a static-site generator.
- `.github/workflows/docs.yml` builds and deploys to GitHub Pages on
pushes to `trunk` that touch
`docs/`. Pull requests get a **build-only** job: a dead link fails
before merge, and the job
  holding the OIDC token never runs on PR code.
- The site's `base` is derived from the repository name at run time. A
project Pages site is served
under `/<repo-name>/`, and this repo has already been renamed once —
deriving it means another
  rename cannot break every asset URL.
- `srcExclude` keeps `docs/testing.md` (from #70) out of the published
site: it documents how to
run the suites, which is contributor material, and this site is for
users of the app.

## How to test this

**Starting state:** this branch checked out, `npm ci` done.

1. `npm run docs:build` — expect `build complete`. Now add a link to a
page that does not exist in
`docs/index.md` and run it again: it must **fail** with `dead link(s)
found`. Undo.
2. `npm run docs:preview`, open the printed URL — the home page renders
with the WordPress
Contributor Toolkit hero and the sidebar. Clean URLs work; this is
exactly what Pages serves.
3. `npm run docs:dev` — pages are served at their `.html` paths in dev
(`/index.html`). With the
dev server open, run `npm run docs:build` in another terminal: the dev
server must **not** spew
   reload lines (it ignores its own output directory).
4. `npm run lint` and `npm test` — both green, unchanged from `trunk`.

**Must not have happened:** no `deploy` job may run on this pull request
— check the Actions tab
and confirm only `build site` ran.

**Not testable by hand here:** the deploy itself. It needs the workflow
on `trunk`, and Pages is
already set to "GitHub Actions" as its source.

## Risks

Merging this alone deploys a site whose sidebar names the full guide, so
**those links 404 until
the content PR lands**. Merge the stack in order rather than leaving
this on `trunk` by itself.

The deploy job holds `pages: write` and `id-token: write`. Its actions
are pinned to commit SHAs
rather than tags, following the reasoning already written down in
`download-stats.yml`.

## Related

Stack: this → #231 (screenshot harness) → #232 (guide content).
Touches #70 only through `srcExclude`; that PR needs no change.

---

<details>
<summary><b>Self-review</b> — 5 findings, all fixed</summary>

Ran `.github/instructions/code-review.instructions.md` with the
judgement pass in a fresh context.
Deterministic layer was clean (ESLint and the unit suite on macOS and
Windows).

**5 [fix here] · 1 [follow-up]. All five fixed in this branch:**

| Dimension | Was | Now |
|---|---|---|
| security 🟡 | `checkout` kept the job's `GITHUB_TOKEN` in `.git/config`
while the job runs PR-authored code (`npm ci` with lifecycle scripts,
and a VitePress build that evaluates `config.mjs`) |
`persist-credentials: false`, matching `lint.yml` |
| security 🔵 | `if: github.event_name != 'pull_request'` let a
`workflow_dispatch` on any ref publish to the live site, which is not
what the comment claimed | `if: github.ref == 'refs/heads/trunk'` |
| cross-platform 🔵 | `node-version: 22` hardcoded while `.nvmrc` says
24.18.0 — reintroducing the Node drift of #37/#46 for the one command
CONTRIBUTING calls "what CI runs" | `node-version-file: .nvmrc` |
| architecture 🔵 | one concurrency group spanning build and deploy, with
`cancel-in-progress: true`, so a second push could cancel an in-flight
`deploy-pages` | split: builds cancel, deploys do not |
| architecture 🔵 | CONTRIBUTING documented `docs:*` without saying the
nested package needs its own install, so the commands fail on a clean
clone | `npm ci --prefix docs` documented |

**Deferred [follow-up]:** `electron-builder` has no `files` filter, so
tracked `docs/` sources ship
inside `app.asar` — and #232 adds screenshots on top. Real,
pre-existing, and overlaps #23; an
`!docs{,/**/*}` entry closes it. Not done here because it changes what
every release artifact
contains, which deserves its own PR and its own testing.

**Checked and clean:** all four action pins resolve to the tags their
comments claim (verified
against the GitHub API); `pull_request_target` is correctly not used and
no fork PR can reach
`pages: write` / `id-token: write`; the lockfile is 175 packages, all
from registry.npmjs.org, with
install scripts only on esbuild and fsevents; the `import/no-unresolved`
exemption is a genuine
false positive and hides nothing; no `src/` code, IPC surface or spawn
path is touched.
</details>
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
@juanmaguitar
Add a screenshot harness that photographs the app for the docs site (#…
e2cd912 
…231)

## Why

A user guide needs pictures of the screens it describes, and they have
to be **retakeable** by
whoever changes the UI — a screenshot nobody can regenerate is a
screenshot that goes stale and
starts lying. This adds the harness. The images themselves land with the
guide in #232.

Stacked on #230; review that one first.

## What changes

- `npm run shots` launches the repo's own Electron binary through
**playwright-core**'s Electron
driver, drives each documented screen by the words on it, and writes a
fixed-size PNG into
  `docs/public/screenshots/`. Eleven screens are declared today.
- A new **`TOOLKIT_USER_DATA_DIR`** hook in `src/main.js` (10 lines)
points `userData` at a
throwaway directory holding a seeded `settings.json`, so the harness
never reads or writes the
  contributor's real site registry.
- `test/user-data-override.test.cjs` pins **both sides** of that hook's
guard.
- Screens a seeded registry cannot reach — a running dev server, a real
diff, the GitHub
device-code screen — are declared as a **live tier** rather than faked:
  `npm run shots -- --tier=live` prompts for each and waits.

<details>
<summary>Why these specific choices</summary>

**`playwright-core`, not `playwright`** — same Electron driver, no
postinstall script, no browser
download, so no new entry in `allowScripts`. If #70 lands
`@playwright/test`, this direct
dependency can go and the harness can require the driver from there; the
note is in the file.

**Under `scripts/`, not `e2e/`** — it is a CLI, not a test. #70 makes
`e2e/` the Playwright
`testDir`, and nothing here is run by `npm run test:e2e`. It also means
no new ESLint exemption:
`scripts/**/*.cjs` already allows `console`.

**Username-free fixture paths** (`/tmp/wpct-docs-fixture/...`) —
`safe-log.js` redacts logs, but
nothing redacts a screenshot, and every path the app renders ends up in
a published image.

**Captured locally, not in CI** — capturing needs the app running
against a seeded site, which is
minutes of flaky work on a runner for images that change rarely.
</details>

## How to test this

**Starting state:** a machine that has used the app before, so you have
**real sites registered**.
`npm ci` done on this branch.

1. Launch the app normally (`npm start`) and note your real sites in the
sidebar. Quit.
2. `npm run shots` — the app opens and closes a few times; expect eleven
`✓ <name>.png` lines and
   the files under `docs/public/screenshots/`.
3. Open each PNG. Every path visible must start
`/tmp/wpct-docs-fixture/` — **your home directory
   must appear nowhere**.
4. `npm start` again — your real sites are still there, exactly as in
step 1.
5. `npm run shots -- --only=terminal` captures just that one;
`--only=nope` fails and lists the
   valid slugs.
6. `npm test` — green, including the three new guard tests.

**Must not have happened:** your real `settings.json` must be untouched
(step 4 is the check), and
no packaged build may honour the variable — the third guard test is what
pins that.

**Platforms:** driven on macOS. The fixture root falls back to
`os.tmpdir()` on Windows, which is
the part most worth a second pair of eyes; the harness is a developer
tool, so a Windows failure
costs a retake, not a shipped bug.

## Risks

`src/main.js` gains an environment-driven `app.setPath` call. It is
guarded by `!app.isPackaged`,
so an installed build ignores it — that guard is the whole security
story here and the reason the
test asserts the packaged case explicitly.

Nothing automated catches a **stale** screenshot. CONTRIBUTING now says
a UI-changing PR re-runs
`npm run shots`; that is a convention, not a gate.

## Related

Stack: #230 → this → #232 (guide content).
Related to #70: it defines `e2e/` as the Playwright test directory,
which is why this lives in
`scripts/`. No change needed there.

---

<details>
<summary><b>Self-review</b> — 2 findings fixed, 1 follow-up taken
anyway, security guard verified</summary>

Ran `.github/instructions/code-review.instructions.md` with the
judgement pass in a fresh context.
Deterministic layer was clean.

**2 [fix here] · 1 [follow-up]. All three fixed:**

| Dimension | Was | Now |
|---|---|---|
| architecture 🔵 | `app.setPath` throws when the directory does not
exist (Electron typings, `electron.d.ts:1781`). It sat at module scope,
before `initLogging()` and before any window, so a stale
`TOOLKIT_USER_DATA_DIR` in a shell profile — or a temp dir the OS reaped
— killed `npm start` with a stack on stdout and nothing in the log file
| wrapped; a bad value is reported on stderr and the real userData is
kept. A fourth test covers it, and it **fails without the fix**
(verified by reverting the guard) |
| cross-platform 🔵 | `spawnSync('npm', …, { shell: true on Windows })` —
a bare host `npm` and a Windows shell, both shapes the standard calls
out | deleted. `"shots": "npm run build:once && node …"` does the same
job with no spawn, no shell, no `existsSync` |
| architecture 🔵 | the live tier called `launchApp({})`, and `launchApp`
spreads `process.env` — so an exported `TOOLKIT_USER_DATA_DIR` (easiest
to acquire while debugging the fixture tier) would silently point the
"your real sites" tier at fixture state, producing committed screenshots
that are wrong in a way only a careful look at the sidebar reveals |
passes `TOOLKIT_USER_DATA_DIR: undefined` explicitly. Taken despite
being `[follow-up]`: one line, and the failure mode is a wrong published
image |

**The security guard was checked hard and stands.** Nothing required
above the hook resolves
`userData` at require time — `settings-store.js` defers its
`import('electron-store')` to the first
`getStore()`, and `logging.js` only reads `app.getPath('logs')` inside
`initLogging()`. `sessionData`
follows the override too, since it resolves lazily from `userData`
pre-`ready`. A packaged build is
genuinely dead to the variable.

**And the test was checked against the "green while proving nothing"
list.** It requires the real
`src/main.js` under a stubbed `electron` and asserts on calls the real
guard makes; deleting either
condition fails a case. `Module._load`, `process.env` and the
require-cache entry all restore in a
`finally`, and `node --test` gives each file its own process, so there
is no cross-file leak.

**Also verified:** `playwright-core` has no `scripts` field and no
`hasInstallScript` in the
lockfile, so it owes no `allowScripts` entry and downloads no browsers.
`cleanFixtureSites` can only
remove the one module-level fixture root; userData dirs come from
`mkdtempSync`; an unexpected
`variant` still writes only under those roots.
</details>
@juanmaguitar
Write the user guide and point the README at it
3867e49 
Eighteen pages covering every screen the app has: creating a site, the setup
wizard, running the site, trunk updates, editors, Trac tickets, applying
patches and pull requests, the three submit destinations, the terminal, logs
and debug.log, the mail catcher, Adminer, managing sites, and troubleshooting.

Each page was written against the renderer rather than from memory, so it names
controls by the words actually on screen. Two things the code corrected along
the way: the Logs panel has two tabs, not three (npm output goes to the
Terminal), and the Trac destination never posts anything — it saves the diff
and opens the ticket's attach page, and you upload it yourself.

The README keeps what a contributor to this repository needs and hands the
rest over: the platform-by-platform install, the Gatekeeper notes, the
ten-step walkthrough and the trunk-update section move into the guide rather
than being copied there. AGENTS.md asks for one home per fact, and a README
that restates the guide is the drift this rule exists to prevent.

The eleven fixture-tier screenshots come from `npm run shots`. Screens needing
a live server are not illustrated yet; the live tier exists for them.
@juanmaguitar
juanmaguitar changed the base branch from docs/screenshots to trunk August 10, 2026 13:21
@juanmaguitar
juanmaguitar merged commit e0be741 into trunk Aug 10, 2026
5 checks passed
@juanmaguitar juanmaguitar mentioned this pull request Aug 10, 2026
Merged
juanmaguitar added a commit that referenced this pull request Aug 10, 2026
@juanmaguitar @claude
Document the site-holds-many-tickets model, and correct the guide pag…
ba9e7eb 
…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
juanmaguitar deleted the docs/guide branch August 11, 2026 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@juanmaguitar