Skip to content

Add the VitePress documentation site and its Pages deploy workflow - #230

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

Add the VitePress documentation site and its Pages deploy workflow#230
juanmaguitar merged 1 commit into
trunkfrom
docs/site

Conversation

@juanmaguitar

@juanmaguitar juanmaguitar commented Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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 e2e: packaged-app smoke test on macOS and Windows #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.

Self-review — 5 findings, all fixed

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.

This was referenced Aug 10, 2026
Merged
Merged
@juanmaguitar
Add the VitePress documentation site and its Pages deploy workflow
fd9810e 
The user guide gets a real home. `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 it and deploys to
GitHub Pages on every push to trunk that touches it; pull requests get a
build-only job, so a dead link fails before merge and the job holding the
OIDC token never runs on PR code.

The site's base path is derived from the repository name at run time rather
than hardcoded, because a project Pages site is served under /<repo-name>/ and
this repository was renamed. Deriving it means a rename cannot break every
asset URL.

`srcExclude` keeps `docs/testing.md` (#70) out of the built site: it documents
how to run the suites, which is contributor material, and the published guide
is for users of the app.

This PR carries the pipeline and the home page only. The sidebar already names
the full guide, so its links 404 until the pages land — merge the stack in
order rather than leaving this one on trunk on its own.
@juanmaguitar
juanmaguitar merged commit b5d3d7f into trunk Aug 10, 2026
5 checks passed
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 added a commit that referenced this pull request Aug 10, 2026
@juanmaguitar
Write the user guide and point the README at it (#232)
e0be741 
## 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.

<details>
<summary>Two things the code corrected while writing</summary>

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.
</details>

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

---

<details>
<summary><b>Self-review</b> — 8 findings, all fixed; three of them were
false statements about the app</summary>

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.
</details>
@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/site 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