Say "Ready" for the next setup step until its action runs - #258

## Why

The next-action cue (#252) points at the block a contributor should act
on next. A block that is *doing* work — a trunk update, a patch apply or
revert — is where attention belongs too: it should scroll into view and
glow like any other cue target, so a contributor who looked away can see
where the work is and that it isn't stuck. The trunk-update tracker was
already highlighted; this completes the idea so **any block displaying
work in progress** gets the same treatment.

## What changes

- `next-action.cjs`: a new high-priority rule — when `isApplying` is
true, point at the `applying-patch` block (the patch apply/revert step
tracker). It sits above the stale-state warnings: a live operation
outranks a warning. One flag covers both operations — a revert runs
through the same apply machinery (`runApply({ reverse: true })`), so
there is no separate "reverting" id.
- Reordered the ladder so the two in-flight, tree-owning operations
(`isApplying`, `isUpdating`) are checked before `updateIncomplete`, and
dropped the now-redundant `!isUpdating` guard. Behaviour-preserving for
the pre-existing cases.
- `index.jsx`: passes `isApplying` to the resolver and spreads
`cueProps('applying-patch')` onto the tracker (the block that only
renders while the operation runs).

After this, every in-progress block in the site view is a cue target:
the trunk-update tracker (wired earlier in the stack) and the patch
apply/revert tracker (here). Each glows and — with the assertive scroll
from the base PR — is centered in view when it starts, and the cue
clears when it ends.

## How to test this

**Platforms:** any (renderer-only). Stacked on #254 + #256 + #258 — test
this branch's Buildkite artifact to exercise the whole stack.

**Starting state:** an initialized site, wizard skipped, with the *Apply
a patch or PR* panel visible.

1. Apply a PR or a `.diff`/`.patch` file (paste a PR URL → **Apply PR**
→ **Apply and rebuild**). → While it runs, the **step tracker block
glows** (soft amber) and the view scrolls to center it.
2. Let it finish. → The glow clears from the tracker (the block
unmounts); the cue moves on to the next pending action.
3. With a patch applied, click **Revert this patch**. → The same tracker
block glows while the revert runs.
4. Trigger **Update to latest trunk** (on a stale site). → Its progress
tracker glows and scrolls into view the same way — confirming the
treatment is general, not apply-only.

**What must not have happened:**

- The glow must not linger after an operation ends (the tracker unmounts
when `isApplying` is false).
- While applying/reverting, the cue must be on the tracker — not on
start-dev / review-changes / link-ticket.
- The screen-reader live region announces "Next step: A patch is being
applied or reverted."

## Risks and limitations

- `isApplying` and `isUpdating` own the working tree and never run at
once (each disables the other's controls), so the ordering between them
is not observable in practice; `isApplying` is placed first regardless.
- Renderer-only. Not hand-verified by the author (no local app run — the
EBADENGINE install constraint); `eslint .` clean, `npm test` **796** (2
new). Buildkite artifact is the hand-test path.

## Related

Part of #252. Stacked on #258 (base branch
`juanmaguitar/setup-step-ready-label`) → #256 → #254. Merge bottom-up.

---

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

**No findings across the five dimensions** (0 [fix here] · 0
[follow-up]). Judgement pass ran in a fresh `Explore` subagent given
only the diff and the review standard, per `/self-review`.

Verified: `isApplying` is true for both apply and revert (`runApply`
sets `applyState` unconditionally, revert calls
`runApply({reverse:true})`); the ladder reorder is behaviour-preserving
across all `updateIncomplete`/`isUpdating` combinations, and dropping
the `!isUpdating` guard is safe now that `isUpdating` is checked first;
the two tree-owning operations are mutually exclusive (both bail on the
shared `running` flag), so `isApplying` winning is moot in practice and
reasonable otherwise; and the `applying-patch` id and its DOM block
appear/disappear together (`cueProps` in scope).

**One change applied from the review** (a style note, not a finding):
removed a second test that fed the resolver the same input as the first
— the apply/revert distinction lives in `runApply`, not the resolver, so
a resolver test can't prove it. Kept one test documenting the
shared-flag behaviour.

Deterministic layer: `eslint .` clean; `npm test` **796 pass** (2 new).

_Note: reviewed as the apply/revert-specific change; the reframing to
"any block showing work in progress" is a title/description change only
— no code changed since review._

</details>

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

## Why

When a contributor pastes a PR or picks a `.diff`/`.patch` file, the app
shows a **staged preview** — "<label> changes N files" with **Apply and
rebuild** / **Cancel**. That block is a clear next action: they set it
up and the decision to apply is theirs. But it was not a cue target, so
nothing pointed at it. This highlights it like the other next-action
blocks.

## What changes

- `next-action.cjs`: a new rule — when `applyPreview` is staged, point
at the `apply-preview` block. Placed **after** the in-flight operations
(`isApplying`, `isUpdating`) and **before** the stale-state warnings and
routine steps: an explicit, waiting choice beats a standing suggestion,
but a live operation still wins. The ordering matters — the preview
block renders only under `applyPreview && !isApplying`, so once applying
starts the cue must move to the progress tracker; checking `isApplying`
first guarantees the id follows the visible block.
- `index.jsx`: passes `applyPreview` to the resolver and spreads
`cueProps('apply-preview')` onto the preview block.

## How to test this

**Platforms:** any (renderer-only). Stacked on #254 + #256 + #258 + #260
— test this branch's Buildkite artifact to exercise the whole stack.

**Starting state:** an initialized site, wizard skipped, *Apply a patch
or PR* panel visible.

1. Paste a PR URL (or number) and click **Apply PR** — or choose a
`.diff`/`.patch` file. → The **preview block** (with *Apply and rebuild*
/ *Cancel*) glows and the view scrolls to center it.
2. Click **Apply and rebuild**. → The preview unmounts and the glow
follows to the **progress tracker** (from #260).
3. Instead, click **Cancel**. → The preview clears and the cue moves on
to the next pending action.

**What must not have happened:**

- The glow must not stay on the preview once applying begins (the
preview block is gone by then).
- No glow on the preview when there is nothing staged.
- Screen reader announces "Next step: A patch is staged — apply and
rebuild, or cancel."

## Risks and limitations

- Renderer-only. Not hand-verified by the author (no local app run — the
EBADENGINE install constraint); `eslint .` clean, `npm test` **799** (3
new). Buildkite artifact is the hand-test path.

## Related

Part of #252. Stacked on #260 (base branch
`juanmaguitar/highlight-patch-progress`) → #258 → #256 → #254. Merge
bottom-up.

---

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

**No findings across the five dimensions** (0 [fix here] · 0
[follow-up]). Judgement pass ran in a fresh `Explore` subagent given
only the diff and the review standard, per `/self-review`.

Verified: the resolver checks `isApplying` before `apply-preview`, so
when applying starts the preview block unmounts and the cue follows to
the progress tracker in the same render (locked by a new test);
`Boolean(applyPreview)` is the right null-vs-object coercion, and the id
is returned only when the preview block is actually rendered (no
id-without-block window); `cueProps` in scope; full suite green. The
reviewer also confirmed a staged preview + in-flight trunk update can
coexist and `updating` correctly wins — placing `apply-preview` below
`isUpdating` is intentional, and the cue still lands on a rendered
block.

Deterministic layer: `eslint .` clean; `npm test` **799 pass** (3 new).

</details>

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

After the clone finished the checklist stopped and waited for two clicks —
Install npm dependencies, then Run full build — with no decision to make
between them. A contributor who walked away came back to a checklist waiting
on them, not to a built environment.

The app already runs this shape twice: planUpdateSteps (fetch → install →
build) for a trunk update, planApplySteps (apply → install → build) for a
patch, both sharing updateStepStatuses. planSetupSteps joins them, and the
chain runs on its own the moment the clone finishes.

What makes running unattended reasonable rather than presumptuous:

- It is visible. A banner names the running step and counts it, and every line
  of npm output goes to the Terminal as it always did (#41).
- It stops. Stop setup — and Ctrl+C in the terminal — ends the child, install
  included now that the kill path can reach one.
- It stops at the first failure. A build on a half-installed tree cannot work
  and its failure would bury the one that mattered (#42).
- It does not start the dev server. That step marks the wizard finished and
  hands the contributor to a browser wizard, so it stays a deliberate click.

The chain fires on the clone-finished edge, once per mount, so reopening the
app on a half-finished site lands on the manual checklist rather than
launching a half-hour build nobody asked for.

Closes the remaining half of #44 along the way: the checklist ladder moves out
of the renderer into setupStepStatuses and gains a `failed` state, so a step
whose last attempt lost says so and keeps its retry, instead of reading as
merely "Ready". Its label was already fixed in #258; the failed/retryable
state was the acceptance criterion still open.

Closes #246

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

After the clone finished the checklist stopped and waited for two clicks —
Install npm dependencies, then Run full build — with no decision to make
between them. A contributor who walked away came back to a checklist waiting
on them, not to a built environment.

The app already runs this shape twice: planUpdateSteps (fetch → install →
build) for a trunk update, planApplySteps (apply → install → build) for a
patch, both sharing updateStepStatuses. planSetupSteps joins them, and the
chain runs on its own the moment the clone finishes.

What makes running unattended reasonable rather than presumptuous:

- It is visible. A banner names the running step and counts it, and every line
  of npm output goes to the Terminal as it always did (#41).
- It stops. Stop setup — and Ctrl+C in the terminal — ends the child, install
  included now that the kill path can reach one.
- It stops at the first failure. A build on a half-installed tree cannot work
  and its failure would bury the one that mattered (#42).
- It does not start the dev server. That step marks the wizard finished and
  hands the contributor to a browser wizard, so it stays a deliberate click.

The chain fires on the clone-finished edge, once per mount, so reopening the
app on a half-finished site lands on the manual checklist rather than
launching a half-hour build nobody asked for.

Closes the remaining half of #44 along the way: the checklist ladder moves out
of the renderer into setupStepStatuses and gains a `failed` state, so a step
whose last attempt lost says so and keeps its retry, instead of reading as
merely "Ready". Its label was already fixed in #258; the failed/retryable
state was the acceptance criterion still open.

Closes #246

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

> **Stacked on #274.** Review that one first — it is the kill path this
PR's Stop button depends on. The diff below is this branch against #274,
not against trunk.

## Why

After the clone finishes, the setup checklist stops and waits. **Install
npm dependencies**, then **Run full build** — two clicks, with no
decision to make between them. The contributor's only job is to notice a
step ended and press the next button.

That is the wrong shape for a wizard. Someone who walks away during the
clone — which is what you do during a ten-minute clone at a Contributor
Day — comes back to a checklist waiting on them rather than to an
environment they can work in. #246 makes the case at length.

## What changes

Setup becomes the third chain of the shape the app already runs twice:
`planUpdateSteps` (fetch → install → build) for a trunk update,
`planApplySteps` (apply → install → build) for a patch, both sharing
`updateStepStatuses` and `updateOutcome`. `planSetupSteps` joins them,
and **the chain starts on its own when the clone finishes** — no button.

What makes running unattended reasonable rather than presumptuous, and
each of these is the reason the issue gives:

- **It is visible.** A banner names the running step and counts it
(`step 2 of 3`), and every line of npm output still goes to the Terminal
(#41).
- **It stops.** **Stop setup**, and Ctrl+C in the Terminal, end the
child — install included, which is what #274 is for.
- **It stops at the first failure.** A build on a half-installed tree
cannot work, and its failure would bury the one that mattered (#42).
- **It starts once, on an edge.** The clone going from running to
finished, once per row. A site already cloned when its row appears never
triggers, so reopening the app on a half-finished site lands on the
manual checklist rather than launching a half-hour build nobody asked
for.

**Deliberately not in it:** starting the dev server. #246 lists it as
the fourth link, and I left it out — that step also calls
`markSkipWizard()` and hands the contributor to a WordPress setup wizard
in a browser, so running it unattended would end the checklist on their
behalf and leave a server listening that nobody asked for. Also not in
it: resuming the chain after a stop. Retry is a plain button on the
failed step; a cancel followed by a retry must not silently restart a
half-hour build.

**It also closes the open half of #44.** The checklist ladder moves out
of `index.jsx` into `setupStepStatuses` and gains a `failed` state, so a
step whose last attempt lost says **Failed**, keeps its retry, and does
not hand `current` to the next step. #258 fixed the label half ("Ready"
until an action runs); the explicit failed/retryable state was the
acceptance criterion still open, and a chain that can stop is what made
it necessary rather than nice.

## How to test this

Platforms: **any** for 1 and 4–6; do **2 and 3 on both macOS and
Windows** — process-tree killing is where they differ.

**Starting state:** the app, with no site for the WordPress repo you are
about to create. This is a real clone plus a real install plus a real
build, so budget for it — the point of steps 2 and 3 is that you do not
have to sit through the build.

1. **Create a WordPress Core site.** Watch the *Download WordPress
development version* step. It should say the install and build start on
their own when it finishes.
2. **When the clone finishes, click nothing.** The blue banner appears,
*Install npm dependencies* turns **In progress**, and npm output streams
into the Terminal. When install ends, the build follows on its own and
the banner reads *step 3 of 3*.
- *Start dev server & finish wizard* must still read **Ready** and must
**not** have started.
3. **Press Stop setup** while npm install is running. The install
actually dies — check no `node`/`npm` child of the app survives. The
build never starts, the banner is replaced by *Setup stopped*, and the
install step offers **Retry npm install**.
4. **Press Stop setup during the build** instead (create a second site,
let install finish). The whole Grunt tree dies, not just the runner
(#83, #146).
5. **A failing install.** Easiest is to pull the network mid-install.
The step reads **Failed** in red with its retry live, the build stays
**Locked**, and the chain does not advance.
6. **Reopen mid-setup.** Quit during the build, relaunch, open the site.
The checklist shows the real state from disk and the chain does **not**
restart by itself.
7. **Skip initialization wizard** during a running chain. The checklist
goes away, the chain keeps running, the Terminal still shows it.

**What must not have happened:**

- **The dev server must not have started, and the wizard must not be
marked skipped.** The chain ends at the build. If you land on a running
server or a post-init view without clicking, that is the regression.
- **No orphaned `npm`/`node` process after a Stop**, on either platform.
- **A stop must not be reported as a failure.** A killed npm exits
non-zero — on Windows without even a signal — so the step must read
*Setup stopped*, not *npm install failed*.
- **A stop must not respawn itself.** Watch for a second install
starting a moment after you stopped the first (that is #274's
`cancelled` flag doing its job).
- **Reopening the app must not start anything.** Step 6 is the one that
would be easy not to notice: it only misbehaves on a site you left
half-finished, and the symptom is your laptop quietly building for half
an hour.

## Risks and limitations

**The end-to-end chain has not been driven by hand.** I could not: this
Electron build does not expose its accessibility tree to the automation
available here, and creating a site needs a native folder picker. What I
did verify is that the renderer bundles and mounts with these changes,
lint is clean, and both Node runtimes are green (840 tests). **Steps 1–7
above are unverified and want a human**, and step 3 in particular is the
one that justifies the whole feature.

`buildFailed` is session-local. Only the install outcome is persisted
(`src/main.js` records it on the site's meta), so after a restart a
failed build reads **Ready** again rather than **Failed**. That is the
honest fallback — the app still knows there is no build on disk, just
not that the last attempt lost — and persisting it is a separate change.

Auto-start is once per row **mount**, not once per site. Switching away
from a site and back during a clone remounts the row; if the clone
finishes after that remount the chain still fires, which is the wanted
behaviour, but it does mean the guard is memory, not state on disk. The
`hasNodeModules` check is what stops that mattering.

The banner says the build can take up to half an hour on Windows (#72).
It runs a production build where a dev build would do — that is #92,
untouched here, and auto-starting makes it more worth fixing, not less.

## Related

Closes #246. Closes the remaining acceptance criterion of #44. Depends
on #274. Makes #57 (the redundant nested `npm install` between the two
steps) easier to fix now the two are one run.

---

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

**The checklist rows are not driven by `updateStepStatuses`.** #246
suggests they could be — "it already returns the
complete/current/pending/skipped states the checklist needs". They
cannot, and this is the one place I departed from the issue. Those
states are derived from disk (`hasNodeModules`, `hasBuilt`,
`installFailed`) precisely so a site reopened days later, with nothing
running, still shows the truth; driving them from chain position would
show every step `pending` on a reopened half-finished site. So the reuse
is real but narrower: `planSetupSteps` + `updateStepStatuses` +
`setupOutcome` drive the **banner** — progress counter, Stop, how it
ended — exactly as they drive the update panel, and the rows keep their
own ladder.

**`planSetupSteps` lives in `update-plan.cjs`** rather than in
`setup-steps.cjs`, following the `planApplySteps` precedent: the module
is where the chains and the machinery they share live, and splitting the
third one off would have meant importing `updateStepStatuses` across
modules to save a rename. Its docblock now says it owns three chains.

**`setupOutcome` separates `stopped` from
`failed-install`/`failed-build`** even though the exit codes are
identical. Telling a contributor their install "failed" when they
pressed Stop is how a tool loses their trust, and the exit code
genuinely cannot tell the two apart — so `stopped` comes from the fact
that we asked for the kill, not from what the process did.

**Auto-start is a tri-state decision, not a chain of `if`s in an
effect.** `setupAutoStartDecision` returns `skip` / `probe` / `start`,
because the decision is taken in two halves: is this the clone-finished
edge (so reading status off disk is worth it), and then does that status
say this is a fresh clone. Being able to test that is worth more than
the shape being slightly unusual — it is the riskiest behaviour in the
PR.

</details>

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

2 [fix here] · 0 [follow-up] — both fixed.

**architecture 🟡 [fix here]** — the auto-start decision lived inline in
a `useEffect` in `index.jsx`, which the suite cannot reach. Per §1, the
finding is the missing module, not the missing test — and this was the
riskiest decision in the change (too eager and it launches a half-hour
build unasked, too shy and the wizard never finishes itself). Extracted
to `setupAutoStartDecision` with nine tests covering the edge, the
arming, the two refusals and a failed status probe.

**architecture 🟡 [fix here]** — four branchy user-facing strings
(install and build labels and descriptions) were derived inline in
`index.jsx`. §1 names "a string the user reads" explicitly. Extracted to
`setupStepCopy`, which takes the same flags as `computeSetupStepState`
so the words and the button state cannot disagree; four tests, including
one pinning that pairing for the #42 case.

The judgement pass was run in this session rather than dispatched to a
subagent — the tooling in use blocked spawning one. Flagging it because
the standard prefers fresh context for that pass, and self-review is the
weaker version. Worth a second pair of eyes on the effect in `index.jsx`
in particular.

</details>

<details>
<summary>Implementation notes</summary>

The chain reuses the wizard's own `runInstall` / `runScript` wrappers,
the same way `runUpdateInstallAndBuild` does, so exit codes, retries,
terminal streaming and the post-run status reload all behave identically
to every other path. Nothing new crosses IPC.

`runScript` gained one line: it clears `buildFailed` when a build starts
and records it on exit. Clearing on start rather than on the next exit
is what stops a step reading **Failed** while its own retry is streaming
output — there is a test for that shape on the install side.

`deriveNextAction` is unchanged. The call site now passes a failed step
as the current one, since retrying it is what the contributor should do
next and a `failed` row consumes no `current` — without that, a stopped
chain would leave the view with no cue at all.

</details>

<details>
<summary>Screenshots or recording</summary>

None — see the limitation above: I could not drive the app's UI from
this environment, so there is no shot of the banner or the failed row.
This is the part of the PR that most wants a human with the Buildkite
artifact.

</details>

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