[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. Five have left. Three remain, and they are coming in two PRs: this one is the trunk update, and the ticket and the apply chain, which call each other in both directions, follow together.

What changes

src/renderer/hooks/use-trunk-update.jsx is new. It holds the update's chain: the fetch and reset, the install when the lockfile moved, the rebuild, the hand-off to a resumed watch on projects whose watcher rebuilds everything, the question the update asks first when the tree has edits in it, and the retry after a run that was cut short.

• It is a chain, not a process, so most of what it does comes in as arguments: the script runner, the terminal's lock and writer, the build watch's pause and resume, and the functions that re-read the site once the tree has changed. They are the calls the inline code made, in the same order.
• It returns what the screen shows: whether an update is under way and at which step, the summary of the last one, the dirty-tree question's values, and the four handlers the buttons call.
• The date of the site's trunk and the "update incomplete" marker stay in SiteRow. They are read from the site's status with everything else the status says; the hook only asks for the status to be read again.
• The hook is called earlier than the code it replaces was written, straight after the helpers it needs, because the ticket card's gate reads the update's stage a few lines further down.

The journey owed from #582 is here. Pausing the build watch and bringing it back had no journey with a watch running. A new trunk-update-watch.spec.js has two: the update with the watch running on Core, and on a project where the resumed watch does the update's build (#507). Both 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/trunk-update.spec.js tests/e2e/journeys/trunk-update-watch.spec.js.

• trunk-update.spec.js is the update with no watch running: a clean tree, a linked ticket, and the question about edits in the tree. It gains one assertion: after saving edits and updating, the notice names the file they went to.
• The first new journey, on Core with the watch running, asserts that the update stops the watcher before it touches the tree (the file about to be replaced still held the old trunk when the watcher was stopped); that the tree is reset and the update runs the build itself; that the watch is not brought back while the build runs; that a failed build leaves the update incomplete and the watch is brought back all the same; that a retry stops the watch again, installs and builds; that a good build completes the update on the screen and in the store; and that a watcher the update stopped, exiting late, is not taken for the one running now.
• The second, on a built Gutenberg checkout, asserts that the update runs no build of its own and brings the watch back at once; that it is not complete while the watch is still rebuilding, with the watch's stop enabled while the dev server's button is not; that the watch's ready line is what completes it; and that a watch stopped before it is ready leaves the update incomplete, on the screen and in the store, with the retry offered.

In both new journeys the fetch and the reset are real, against an origin on disk; the install and the scripts are answered by stubs.

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

Starting state: a Core site that has finished setup, whose trunk is behind wordpress-develop, with the build watch running and one file edited.

1. Click "More", then "Update to latest trunk". A dialog asks what to do with the edit. Choose to save it and pick a location.
2. The card "Updating to latest trunk" appears and steps through fetching, installing or skipping the install, and building. The "Build watcher" tab reads "Build watcher (paused)".
3. When it ends, a green notice says the site is up to date and names the file your edit was saved to. The tab reads "Build watcher (watching)" again.

What must not have happened:

• The edit is not lost: it is in the file you chose, and the notice names that file.
• node_modules was not reinstalled if the lockfile did not change: the card names the install step as skipped.

Risks and limitations

• The hook takes twenty arguments. The update touches the runner, the terminal, the watch and the site's status, and takes each as it was used. Every function in it is a plain closure, as before, so none of those arguments is a dependency of anything.
• Not covered by a journey, as before this PR: an update that finds nothing to fetch, an install that fails, and a hand-off where the watch was stopped during the reset.
• Pausing the watch still has no journey for the other three things that do it: checking out or leaving a pull request, applying a patch, and switching tickets. Those belong with the PR that moves the apply chain and the ticket.
• The manual steps above were not run.
• The new journeys cost two more app launches on every pull request, on both platforms: about four seconds locally on macOS.
• Checked on macOS only. Windows is covered by CI.
• Review, two passes: 5 findings to fix, all in the journeys or their comments, all fixed here; 3 follow-ups, one of them applied.

Related

Part of #554: the trunk update, after the dev server in #583. It carries the pause-and-resume journey that #582 left for it. Part of #542.

---

Design decisions and alternatives considered

The trunk's date and the incomplete marker are not the update's. loadStatus sets them with the rest of what the main process says about the site, and the staleness notice and the sidebar read them whether or not an update ever runs. Moving them would mean the status loader writing into the update's hook.

Plain functions, not useCallback. beginTrunkUpdate, finishUpdate and the rest were closures recreated on every render, called from handlers and from each other. Memoising them here would add twenty-item dependency lists and change when they see fresh state, which is the kind of change a move should not make.

The dirty-tree question's state moved with the update. The dialog that asks it is a component of its own (#570) and holds nothing; what it shows was the update's all along, and the two answers it offers both end by starting the update.

A new journey file, not more tests in trunk-update.spec.js. The existing three run the real build script of the fixture. The new two need the runner answered by a stub, so that a watch can be "running" and a build can be made to fail or to wait.

Review outcome (required — see AGENTS.md)

First pass: 5 [fix here] · 2 [follow-up]. It diffed the old blocks against the hook with indentation removed and found one blank line of difference.

• Tests, low, [fix here], fixed in 54620e5: build-watch.spec.js still said no journey pauses the watch.
• Tests, low, [fix here], fixed in 54620e5: in the second journey, "the incomplete marker is gone from the screen" could not fail where it stood, since nothing had re-read the site by then. It is removed; the store is the check there.
• Tests, low, [fix here], fixed in 54620e5: two comments claimed more than their assertions. "Before it touches the tree" is now checked: the stand-in keeps what the file about to be replaced held when the watcher was stopped. "The one thing that can be pressed" says what is checked.
• Tests, low, [fix here], fixed in 54620e5: the watcher's first line was said before the main process was known to have answered the start, so on a slow runner it could be said to nobody. The journey waits for the run to have been asked for.
• Tests, low, [fix here], fixed in 54620e5: the script the watcher is on Gutenberg was marked an invariant. It is a characterisation.
• Tests, low, [follow-up]: paths with no journey, in Risks above. One of them, the notice naming the saved patch, is one assertion in the existing journey and was added in 54620e5, since the ref that carries it moved here.
• Architecture, low, [follow-up]: which step runs the build, and the "already up to date" branch, are decisions in a hook where no unit test reaches them, as they were in index.jsx.

Second pass, on 54620e5 alone: 0 [fix here] · 1 [follow-up], applied in 07f5ddc: the header of build-watch.spec.js listed what pauses the watch and left out checking out a pull request and leaving one.

Both passes confirmed that no moved name is read before the hook is called, that each of the twenty arguments is the value the old closures read, and that the callbacks which fire long after they were made (a fetch ending, a build ending, the watch being ready) see the same render's values as before.

• Review: completed — fresh agent context, local, before the PR existed, read-only; head 309d129 / base 62ec328; committed work only, working tree clean; then a second fresh context on 54620e5 alone; outcomes above. CodeRabbit: not run yet.
• Since review: 54620e5 → 07f5ddc checked. The difference is one comment. It was not reviewed again by a fresh context.

Checks, macOS, local:

Check	On	Result
eslint . --max-warnings=0	07f5ddc	clean
unit suite on Node	309d129	1697 pass, 2 skipped, 0 fail
unit suite on Electron's Node	309d129	1699 pass, 0 fail
npm run test:e2e	54620e5	53 of 53 pass
npm run test:e2e:packaged	309d129	15 of 15 pass

After 309d129 every change is in the journeys.

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 update leaves the watch running	the stop asked for, in both new journeys
The watch is not brought back when the update ends	the watcher being asked for again
The site is not re-read when the update ends	the incomplete marker being gone from the screen
A retry leaves the watch running	the second stop asked for
A retry does not install	the install asked for
The update ending does not give the terminal back	the retry, which is then refused
A completed update is not recorded	the marker gone from the screen and the store, and the old journey's summary
A completed update is not announced	the announcement, in three journeys
The update builds even where the watch will	the scripts asked for
The update is complete as soon as the watch is brought back	the card still showing
Nothing can be pressed while the update waits on the watch	the watch's stop being enabled
The watch being ready does not complete the update	the announcement
A watch stopped before it is ready leaves the update hanging	the incomplete marker
Other buttons can be pressed during an update	the card showing
An update does not ask about edits in the tree	the question, in the old journey
Saving the edits first saves nothing	the save dialog, in the old journey
The question about edits cannot be closed	the question being gone, in the old journey
The watcher is stopped only after the tree was reset	what the file held when the watcher was stopped
The notice does not say where the edit was saved	the notice, in the old journey

One break was not caught, in a journey this PR does not change: with "discard my edits, then update" made to discard nothing, trunk-update.spec.js still passes, because the update's own reset removes the edits a moment later. The screen and the tree end the same either way.

🤖 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: c6073a34-9851-466b-930c-38891b8bf67d

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