Repository navigation
Commit f63b559
> **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>
1 parent 8f36065 commit f63b559
5 files changed
Lines changed: 666 additions & 39 deletions
File tree
- src/renderer
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
| 32 | + | |
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| |||
1254 | 1254 | | |
1255 | 1255 | | |
1256 | 1256 | | |
| 1257 | + | |
| 1258 | + | |
| 1259 | + | |
| 1260 | + | |
| 1261 | + | |
1257 | 1262 | | |
1258 | 1263 | | |
1259 | 1264 | | |
| |||
1289 | 1294 | | |
1290 | 1295 | | |
1291 | 1296 | | |
| 1297 | + | |
| 1298 | + | |
| 1299 | + | |
| 1300 | + | |
| 1301 | + | |
1292 | 1302 | | |
1293 | 1303 | | |
1294 | 1304 | | |
| |||
1600 | 1610 | | |
1601 | 1611 | | |
1602 | 1612 | | |
| 1613 | + | |
| 1614 | + | |
| 1615 | + | |
| 1616 | + | |
1603 | 1617 | | |
1604 | 1618 | | |
| 1619 | + | |
1605 | 1620 | | |
1606 | 1621 | | |
1607 | 1622 | | |
| |||
1873 | 1888 | | |
1874 | 1889 | | |
1875 | 1890 | | |
1876 | | - | |
| 1891 | + | |
| 1892 | + | |
| 1893 | + | |
1877 | 1894 | | |
1878 | 1895 | | |
1879 | 1896 | | |
| |||
1882 | 1899 | | |
1883 | 1900 | | |
1884 | 1901 | | |
| 1902 | + | |
1885 | 1903 | | |
1886 | 1904 | | |
1887 | 1905 | | |
| |||
1964 | 1982 | | |
1965 | 1983 | | |
1966 | 1984 | | |
| 1985 | + | |
| 1986 | + | |
1967 | 1987 | | |
| 1988 | + | |
1968 | 1989 | | |
1969 | 1990 | | |
1970 | 1991 | | |
| |||
1975 | 1996 | | |
1976 | 1997 | | |
1977 | 1998 | | |
| 1999 | + | |
1978 | 2000 | | |
1979 | 2001 | | |
1980 | 2002 | | |
| |||
2690 | 2712 | | |
2691 | 2713 | | |
2692 | 2714 | | |
| 2715 | + | |
| 2716 | + | |
| 2717 | + | |
| 2718 | + | |
| 2719 | + | |
| 2720 | + | |
| 2721 | + | |
| 2722 | + | |
| 2723 | + | |
| 2724 | + | |
| 2725 | + | |
| 2726 | + | |
| 2727 | + | |
| 2728 | + | |
| 2729 | + | |
| 2730 | + | |
| 2731 | + | |
| 2732 | + | |
| 2733 | + | |
| 2734 | + | |
| 2735 | + | |
| 2736 | + | |
| 2737 | + | |
| 2738 | + | |
| 2739 | + | |
| 2740 | + | |
| 2741 | + | |
| 2742 | + | |
| 2743 | + | |
| 2744 | + | |
| 2745 | + | |
| 2746 | + | |
| 2747 | + | |
| 2748 | + | |
| 2749 | + | |
| 2750 | + | |
| 2751 | + | |
| 2752 | + | |
| 2753 | + | |
| 2754 | + | |
| 2755 | + | |
| 2756 | + | |
| 2757 | + | |
| 2758 | + | |
| 2759 | + | |
| 2760 | + | |
| 2761 | + | |
| 2762 | + | |
| 2763 | + | |
| 2764 | + | |
| 2765 | + | |
| 2766 | + | |
| 2767 | + | |
| 2768 | + | |
| 2769 | + | |
| 2770 | + | |
| 2771 | + | |
| 2772 | + | |
| 2773 | + | |
| 2774 | + | |
| 2775 | + | |
| 2776 | + | |
| 2777 | + | |
| 2778 | + | |
| 2779 | + | |
| 2780 | + | |
| 2781 | + | |
| 2782 | + | |
| 2783 | + | |
| 2784 | + | |
| 2785 | + | |
| 2786 | + | |
| 2787 | + | |
| 2788 | + | |
| 2789 | + | |
| 2790 | + | |
| 2791 | + | |
| 2792 | + | |
| 2793 | + | |
| 2794 | + | |
| 2795 | + | |
| 2796 | + | |
| 2797 | + | |
| 2798 | + | |
| 2799 | + | |
| 2800 | + | |
| 2801 | + | |
| 2802 | + | |
| 2803 | + | |
| 2804 | + | |
| 2805 | + | |
| 2806 | + | |
| 2807 | + | |
| 2808 | + | |
| 2809 | + | |
| 2810 | + | |
| 2811 | + | |
| 2812 | + | |
| 2813 | + | |
| 2814 | + | |
| 2815 | + | |
| 2816 | + | |
| 2817 | + | |
| 2818 | + | |
| 2819 | + | |
| 2820 | + | |
| 2821 | + | |
| 2822 | + | |
| 2823 | + | |
| 2824 | + | |
| 2825 | + | |
| 2826 | + | |
| 2827 | + | |
| 2828 | + | |
| 2829 | + | |
| 2830 | + | |
| 2831 | + | |
| 2832 | + | |
| 2833 | + | |
2693 | 2834 | | |
2694 | 2835 | | |
2695 | 2836 | | |
| |||
3665 | 3806 | | |
3666 | 3807 | | |
3667 | 3808 | | |
| 3809 | + | |
| 3810 | + | |
| 3811 | + | |
| 3812 | + | |
| 3813 | + | |
| 3814 | + | |
| 3815 | + | |
| 3816 | + | |
| 3817 | + | |
3668 | 3818 | | |
3669 | 3819 | | |
3670 | 3820 | | |
| |||
3685 | 3835 | | |
3686 | 3836 | | |
3687 | 3837 | | |
3688 | | - | |
| 3838 | + | |
3689 | 3839 | | |
3690 | 3840 | | |
3691 | 3841 | | |
| |||
3694 | 3844 | | |
3695 | 3845 | | |
3696 | 3846 | | |
| 3847 | + | |
3697 | 3848 | | |
3698 | | - | |
3699 | | - | |
3700 | | - | |
3701 | | - | |
3702 | | - | |
| 3849 | + | |
| 3850 | + | |
| 3851 | + | |
3703 | 3852 | | |
3704 | 3853 | | |
3705 | 3854 | | |
3706 | 3855 | | |
3707 | 3856 | | |
3708 | 3857 | | |
3709 | | - | |
| 3858 | + | |
| 3859 | + | |
| 3860 | + | |
3710 | 3861 | | |
3711 | 3862 | | |
3712 | 3863 | | |
3713 | 3864 | | |
3714 | 3865 | | |
3715 | 3866 | | |
3716 | 3867 | | |
3717 | | - | |
3718 | | - | |
3719 | | - | |
3720 | | - | |
3721 | | - | |
| 3868 | + | |
3722 | 3869 | | |
3723 | 3870 | | |
3724 | 3871 | | |
| |||
3733 | 3880 | | |
3734 | 3881 | | |
3735 | 3882 | | |
3736 | | - | |
3737 | | - | |
3738 | | - | |
| 3883 | + | |
3739 | 3884 | | |
3740 | 3885 | | |
3741 | 3886 | | |
| |||
3744 | 3889 | | |
3745 | 3890 | | |
3746 | 3891 | | |
3747 | | - | |
| 3892 | + | |
3748 | 3893 | | |
3749 | 3894 | | |
3750 | 3895 | | |
| |||
3781 | 3926 | | |
3782 | 3927 | | |
3783 | 3928 | | |
3784 | | - | |
3785 | | - | |
3786 | | - | |
3787 | | - | |
3788 | | - | |
3789 | | - | |
3790 | | - | |
3791 | | - | |
3792 | | - | |
3793 | | - | |
3794 | | - | |
3795 | | - | |
3796 | | - | |
3797 | | - | |
3798 | | - | |
| 3929 | + | |
3799 | 3930 | | |
3800 | 3931 | | |
3801 | 3932 | | |
3802 | 3933 | | |
3803 | 3934 | | |
3804 | 3935 | | |
3805 | | - | |
| 3936 | + | |
| 3937 | + | |
| 3938 | + | |
| 3939 | + | |
3806 | 3940 | | |
3807 | 3941 | | |
3808 | 3942 | | |
| |||
4048 | 4182 | | |
4049 | 4183 | | |
4050 | 4184 | | |
| 4185 | + | |
| 4186 | + | |
| 4187 | + | |
| 4188 | + | |
| 4189 | + | |
| 4190 | + | |
| 4191 | + | |
| 4192 | + | |
| 4193 | + | |
| 4194 | + | |
| 4195 | + | |
| 4196 | + | |
| 4197 | + | |
| 4198 | + | |
| 4199 | + | |
| 4200 | + | |
| 4201 | + | |
| 4202 | + | |
| 4203 | + | |
| 4204 | + | |
| 4205 | + | |
| 4206 | + | |
| 4207 | + | |
| 4208 | + | |
| 4209 | + | |
| 4210 | + | |
| 4211 | + | |
4051 | 4212 | | |
4052 | 4213 | | |
4053 | 4214 | | |
| |||
0 commit comments