[zaerl]

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 (Decouple the build watcher from the dev server, with its own control (#247) #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.

---

Design decisions and alternatives considered

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.

Review outcome (required — see AGENTS.md)

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.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 129ee9b1-09d1-4bc2-9b01-2cec2daab404

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}