Decouple the build watcher from the dev server, with its own control (#247) - #272

Closes #262. Stacked on #272 — review/merge that first; this PR targets
its branch, so its diff is only this change.

## What this does

With the watcher decoupled (#272 / #247), applying a patch or updating
trunk no longer has to stop anything — the watch already recompiles
`src/` on save. A single pure decision (`planWatchImpact`) routes the
apply/revert/update flows:

- **A src-only patch, with a watch running, applies and is left for the
watch to rebuild** — no build of its own, no interruption. The apply
checklist shows the build step skipped and names the watch as doing it.
This is the case from #262 that used to print `A command is already
running` and do nothing.
- **A lockfile-changing patch, a full build, or a trunk reset** needs
the build directory and `node_modules` to itself, so the watch is
**paused for the duration and resumed after** (`finishApply` /
`finishUpdate`). The **PHP server stays up** throughout — the rebuild
regenerates `build/` under it.
- **"Update to latest trunk"** and its retry no longer require stopping
the dev server first.

`planWatchImpact` and the extended `planApplySteps` are pure and
unit-tested, including the src-only-must-not-build case that was the
bug.

## Verification

- `npm run build:once`, `npm run lint`, `npm test` — all green (820
tests).
- **Runtime smoke test still recommended**: with the dev server running,
apply a src-only PR (e.g. #11705) → applies with no build and no
"already running"; apply a lockfile-changing patch → watch tab shows
Paused → install+build → Resumed, server never drops; Update to latest
trunk while the server runs → same pause/resume, site stays up; a failed
apply/update still resumes the watch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

## Why

Twenty PRs landed on `trunk` between `v1.0.0-beta.1` and this release
candidate, and several moved behaviour the guide describes. Four of the
guide's claims had gone from incomplete to simply false:

- **The build watcher is no longer tied to the dev server** (#247/#272)
— `running-the-site.md` said it ran "alongside the server… while the
server is up".
- **A trunk update no longer needs the server stopped** (#262/#273) —
the guide said the opposite.
- **The setup checklist is no longer four clicks** (#246/#276) — install
and build now start themselves once the clone finishes.
- **There is a wp-admin link now** (#250) — the guide told you to type
`/wp-admin/` onto the URL by hand.

## What changes

Correcting those four, and documenting what came with them: the build
watch's own control, dot and log tab; what a failed apply says now and
why the framing differs for a pull request versus a loose patch file
(#282); the ticket's own facts and the "Latest" pill's real ranking
(#292, #281); **Discard all changes** rewinding to the ticket's base
(#270); severity colour in the log panes (#280); and one short passage
on the "Start here" cue and the completion toasts (#252, #253) —
cross-cutting enough that repeating them per page would read worse than
naming them once.

## Review

Ran `/self-review` per `AGENTS.md`, dispatched to a fresh `Explore`
subagent so the session that wrote the docs was not the one grading
them. `npm run lint` / `npm test` don't apply — docs-only, no JS touched
— so the review focused on the one dimension that does: **every
behavioural claim verified against the actual source**, not assumed. It
caught ten inaccuracies, five worth fixing here — including two pages I
hadn't touched (`editors.md`, `creating-a-site.md`) that my changes to
`running-the-site.md`/`setup-wizard.md` had left contradicting. All five
are fixed in the second commit; the other five were softened as
follow-up-scale wording tightenings, folded into the same commit since
they were one-line each.

## Not in this PR

Screenshots. `site-view.png`, `setup-wizard.png` and
`trac-ticket-panel.png` now show a UI that has moved on (Build watch
button, wp-admin link, ticket-facts card). Retaking them needs the shots
harness against a running app; filing a follow-up issue rather than
blocking this PR on it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…#582)

## Why

`SiteRow` holds the state of eight separate things in one function, and
#554 splits it into hooks by domain so the redesign in #542 can rebuild
the screen around pieces it can hold. Mail, the logs and the terminal
have left. The issue's next item is "dev server and watch". This is the
first half of it: the build watch, and the script runner it and
everything else runs through. The dev server follows in a PR of its own.

## What changes

Two hooks, because they are two things.

- **`src/renderer/hooks/use-site-scripts.jsx` is the npm runner.** It
holds `runInstall`, `runScript` and `killCurrent`, whether an install or
a build is running, whether the last build failed, whether the last
thing to write `build/` was cut short, and the run a stop should reach.
The terminal, the setup, the update, the apply, the ticket switch and
the build watch all run through it.
- **`src/renderer/hooks/use-build-watch.jsx` is the build watch.** It
holds the watch's state, its run, who is waiting for it to be ready, and
start, stop, pause, resume and hand-off. It takes the runner and the
terminal's lock as arguments: a site with no build is built once before
it can be watched, and that one build holds the terminal.
- **What the chains read from outside a render is handed out as it
was**: the watch's state ref, the queue of who is waiting for it, and
the ref that says which apply's hand-off is current. `SiteRow` takes
them out under the names it always used, so the update, the apply and
the ticket switch did not change.
- **One dependency list gained a ref**, `stopDevServer`'s, for the
reason the terminal's PR gave: the linter cannot know that a ref from a
hook is stable.

**Not in this PR:** the dev server (`stopDevServer`, `startPhpServer`,
`toggleDevServer` and their state), which is next.

**No journey touched the build watch**, so a new `build-watch.spec.js`
has two, and the terminal's journey now pins the "a build is running"
and "an install is running" flags, which nothing on screen had depended
on alone. All were written against the inline code first and pass on
both.

## How to test this

Platforms: any. Use the Buildkite build for the current head of this
branch.

The journeys carry this, from the repository root: `npx playwright test
--project=journeys tests/e2e/journeys/build-watch.spec.js
tests/e2e/journeys/terminal.spec.js`.

- The first build-watch journey, on a built Core site, asserts that the
watch's button starts the watcher in the site's directory and its tab
says what it ran; that what it prints goes to its tab; that it does not
hold the terminal; that the button stops that run and the tab says so;
that a run started straight after is not told it has exited when the
stopped one finally goes; and that an exit nobody asked for is shown
with its code.
- The second, on a Gutenberg checkout with no build, asserts that the
button runs the build first; that the build holds the terminal; that a
failed build starts no watcher and says why; that a good one is followed
by the watcher with nothing clicked; that the watcher is not "watching"
until it prints its ready line; and that it does not hold the terminal
the build held.
- The terminal's journey now also asserts that a running build and a
running install are known to the rest of the site's view, through a
button that waits for both, and that their ending is too.

What they cannot reach is a real watcher and a real build:

**Starting state:** a Core site that has finished setup, with no dev
server and no build watch running.

1. Click "Start build watch". The dot on the button goes green and the
"Build watcher" tab under "Logs" reads "Build watcher (watching)". Click
that tab: it shows what was run.
2. Edit and save a file under `src/`. The tab shows the watcher
compiling it.
3. In the terminal, type `npm run build` and press Enter. It runs: the
watch does not hold the terminal.
4. Click "Stop build watch". The dot goes grey and the tab reads "Build
watcher".
5. Click "Update to latest trunk" in the "More" menu with the build
watch running, on a site where an update is available. The tab reads
"Build watcher (paused)" while the update runs and the watch is back
afterwards.

**What must not have happened:**

- After step 4, no watcher process is left behind. On macOS, `pgrep -fl
grunt` in a terminal shows none for the site's directory.

## Risks and limitations

- **Found on the way, and not fixed here: "Start build watch" is meant
to open the watch's tab and does not.** The app calls its own "select
this tab" function, but the tab panel cannot be switched from outside
it, so the screen stays where it was while the app believes the watch's
tab is open. That is older than this PR (#272) and has its own
consequences for the panes that follow their last line. The journeys
open the tab themselves and say why; nothing here pins the behaviour
either way.
- **Pausing, resuming and handing a change to the watch have no journey
with the watch running.** Step 5 above is the manual check. The journeys
for the update, the apply and the ticket switch run through
`pauseWatcher` and `resumeWatcher` with no watch running, where both do
nothing. A journey for it belongs with the PR that moves the trunk
update's state, which needs the same stand-ins; it is left for that one
on purpose.
- **One local run of the build-watch journeys failed and I could not
explain it.** In 17 runs of `build-watch.spec.js` on the final test
code, one failed at the first click of the second journey; the 14 after
it passed, and the full suite passed. My script did not keep that run's
output, so the cause is unknown. Test windows take focus on the desktop
they run on, and a click or a key from the person at it is the likeliest
cause, but that is a guess. CI is the clean run.
- **Killing the watcher when the site's view goes has no journey.** It
is the effect that moved furthest, from the top of `SiteRow` into the
hook; the code is the same.
- **The state is created about 700 lines further down `SiteRow` than it
was**, where the hooks are called. Nothing reads it before that point;
the one thing that did, the list of log tabs, moved below the hook.
- **The manual steps above were not run.**
- **The journeys cost two more app launches** on every pull request, on
both platforms: about two and a half seconds locally on macOS.
- **Checked on macOS only.** Windows is covered by CI.
- Review, two passes: 6 findings to fix, all in the tests or in
comments, all fixed here; 3 follow-ups, listed in the review outcome
below.

## Related

Part of #554: the first half of "dev server and watch", after mail in
#579, the logs in #580 and the terminal in #581. Part of #542.

---

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

**The watch and the runner now, the dev server next.** The three
together are about 450 lines of logic that guards against each other's
late callbacks. The dev server starts behind the watch and reads its
state; the watch knows nothing of the server. So the watch can move
first and the server can then depend on a hook.

**"The build was cut short" lives with the runner, not with the watch.**
The watch sets it and a build ending well clears it, and the runner is
where a build ends. With the flag in the watch's hook, the runner would
need the watch and the watch needs the runner.

**Which apply's hand-off is current stays in the watch's hook for now.**
It is the apply's state, and pausing the watch is what invalidates it.
When the apply chain moves, it can take the ref with it and give the
watch a function to call.

**The watch takes eleven arguments.** Eight of them are other hooks'
functions and refs. That is the coupling the code already had, written
down in one place; making it smaller means changing what the watch does,
not where it lives.

</details>

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

**First pass: 4 [fix here] · 2 [follow-up].** It compared the lines
removed from `index.jsx` with the two hooks as a sorted set and in
order, and found them identical apart from the headers, the imports and
the return values.

- Tests, medium, [fix here], fixed in 4cecbb6: the build watch's journey
said pausing and resuming had journeys of their own. They do not. The
header says so.
- Tests, low, [fix here], fixed in 4cecbb6: the hint under the terminal
and a tab of the Logs panel were each located in more than one file.
Both are in `tests/e2e/helpers/ui.cjs`.
- Tests, low, [fix here], fixed in 4cecbb6: the wait on the hint before
the first click is what keeps the click off a site not yet known to be
built, and was labelled as a characterisation. It says what it is for.
- Tests, low, [fix here], fixed in 4cecbb6: the checks on the build and
install flags did not tell the flags from the terminal's lock. The
button they watch is now shown to stay enabled while the lock alone is
held, and with the button rewired to follow the lock the journey goes
red on that line.
- Tests, medium, [follow-up]: watch paths with no journey: pause and
resume, the hand-off and the compiling state, the kill when the view
goes, stopping during the first build, refusing to start while the
terminal is busy, and a dev-server start queued behind Gutenberg's ready
line. In Risks above.
- Architecture, low, [follow-up]: the watch's button does not open its
tab. In Risks above. The reviewer found one more consequence: a
contributor reading debug.log who starts the watch sees an unread count
rise on the tab they are reading.

**Second pass, on 4cecbb6 alone: 2 [fix here] · 1 [follow-up]**, all
three about comments, the first two fixed in e947290.

- The journey's header said an update, an applied patch and a restored
pull request each pause, resume and hand over. Only an applied patch can
do all three, and a ticket switch, which the header did not name, hands
over too.
- The watch hook's header said its button is the only way it is stopped.
Ctrl+C stops it too, while its first build holds the terminal.
- Follow-up: a comment in `SiteRow`'s `stopDevServer` says the watch is
stopped only by its own control, with the same gap. It is in the code
the next PR moves.

Both passes confirmed that no moved name is read before its hook is
called, including in dependency lists, that no callback is less stable
than before, and that nothing real can be started by the journeys.

- **Review:** completed — fresh agent context, local, before the PR
existed, read-only; head 6c1327b / base
210a73d; committed work only, working
tree clean; then a second fresh context on
4cecbb6 alone; outcomes above.
CodeRabbit: not run yet.
- **Since review:** `4cecbb6 → e947290` checked. The difference is
comments in five files. It was not reviewed again by a fresh context.

Checks, macOS, local:

| Check | On | Result |
|---|---|---|
| `eslint . --max-warnings=0` | e947290 | clean |
| unit suite on Node | 6c1327b | 1697 pass, 2 skipped, 0 fail |
| unit suite on Electron's Node | 6c1327b | 1699 pass, 0 fail |
| `npm run test:e2e` | 4cecbb6 | 49 of 49 pass |
| `build-watch.spec.js`, repeated | 4cecbb6 | 16 of 17 runs pass; see
Risks |
| `npm run test:e2e:packaged` | 6c1327b | 15 of 15 pass |

After 6c1327b the only changes outside the tests are comments in the two
hooks.

Audit of the journeys, on the commit each line was written for, one
break at a time, each restored before the next. Every one goes red on
the line that makes the claim:

| Broken | Goes red at |
|---|---|
| The button does not start the watcher | the script asked for, in both
journeys |
| The tab does not say what the watch is doing | the tab's name, in both
|
| The tab does not say what was run | the line |
| What the watcher prints is not shown | the line, in both |
| The watcher holds the terminal | the hints being links; the last
command not running |
| Stop does not stop the run | the stop asked for |
| Stop leaves the tab saying it is watching | the tab's name |
| A stopped run's late exit is taken for the new run's | the tab still
saying "watching" |
| An exit nobody asked for is not shown | the tab's name with the code |
| A site with no build is watched without one | the build being asked
for |
| The view does not tell the watch whether the site is built | the build
being asked for |
| The first build does not hold the terminal | no second script having
been started |
| A failed build is followed by the watcher | the sentence saying it was
not |
| A good build is not followed by the watcher | the watcher being asked
for |
| The watcher is watching before it says it is ready | the tab still
saying "building" |
| The watcher saying it is ready changes nothing | the tab saying
"watching" |
| The terminal is still held once the build has ended | the next build
being asked for |
| A script ending is not passed on | the lines that follow an ending, in
three journeys |
| The run Ctrl+C should stop is not recorded | the stop asked for, in
the terminal's journey |
| An install ending is not passed on | the line that follows it, in the
terminal's journey |
| A running build is not flagged | the button that waits for it |
| A build that has ended is still flagged | the same button |
| A running install is not flagged | the same button |
| An install that has ended is still flagged | the same button |
| The button that waits for a build follows the terminal's lock | the
button being enabled while a script that is not a build runs |

The first audit found one break nothing caught: the "a build is running"
flag. Wherever a build ran in a journey the terminal's lock was held
too, and every assertion was satisfied by the lock. The terminal's
journey now watches a button that depends on the flag and not on the
lock.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>