[Add] a per-site project type driving clone, status and build (#251) - #255

## Why

`v1.0.0-beta.1` shipped on 10 August. Twenty changes have landed on
`trunk` since — the setup chain, the decoupled build watch, the
failed-apply explanation, the ticket's own facts, the toasts. That is
the release candidate for 1.0, so the version moves to `1.0.0-rc.1`
before the tag is cut.

## What changes

`package.json` and `package-lock.json`, and nothing else. Written by
`npm version 1.0.0-rc.1 --no-git-tag-version` rather than by hand, so
the lockfile's two copies of the string move with it.

`git grep 1.0.0-beta` outside the lockfile returns nothing, so no doc,
workflow or script carries the version. `electron-builder` derives the
artefact names from `package.json`, so the assets become:

- `wordpress-contributor-toolkit-1.0.0-rc.1-mac-arm64.dmg`
- `wordpress-contributor-toolkit-1.0.0-rc.1-win-x64.exe`
- `wordpress-contributor-toolkit-1.0.0-rc.1-linux-x86_64.AppImage`

The `rc.1` shape (not `rc1`) keeps the same form as `beta.1`, so the
filenames stay in one series.

## Scope

The open Gutenberg stack (#255 → #261 → #264 → #269 → #283) is
deliberately **not** in this release candidate. An RC stabilises what is
there; a feature of that size belongs in the release after it.

## Review

No behaviour changes, so the review standard has nothing to grade beyond
the diff itself: two version strings, produced by npm, verified against
`git grep`. Lint and unit tests run on this branch.

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

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

…395)

> Rebased out of the Gutenberg stack (#251) and retargeted at trunk. The
rest of that stack — #255, #261, #264, #269 — is closed; this change
never depended on it, it only sat on top of it. Continues #283, which
could not be retargeted because GitHub locks the base of a stacked PR.

## Why

Building a Gutenberg site never finished. The wizard sat on "Run build"
forever while the app spawned processes without bound — over 1,300 in a
few minutes, until the machine was unusable and the app had to be
killed. Nothing was ever written to `build/`.

The same checkout builds fine outside the app, so this was never a
Gutenberg problem. Core sites never hit it either: their build is Grunt,
which does not reach the code path below.

Fixes #275.

## What changes

Root cause: the `node`/`npm`/`npx` shims this app puts on `PATH` are
Electron running under `ELECTRON_RUN_AS_NODE`, and Electron keeps
`process.versions.electron` set in that mode. `yargs` reads exactly that
to decide where a command's arguments begin — "electron set,
`defaultApp` unset" reads as a packaged Electron app whose argv carries
no script path — so **every yargs-based tool started through a shim
treats its own executable path as the first argument it was given**.

For a task runner that extra argument is a command to run: itself, with
no arguments. The copy it starts does the same, forever. Each link
spawns exactly one child, which is why the process tree is an unbounded
chain rather than a fan-out.

Argument shifting is the general failure here; the runaway processes are
only its loudest form. Other tools reached through the shim have been
misreading their arguments quietly.

The fix: each shim now `--require`s a small module that hides the
Electron version from the process it starts. Two decisions worth naming,
because both were arrived at by measurement rather than by reasoning:

- **An argument, not `NODE_OPTIONS`.** `win-spawn-patch.js` uses
`NODE_OPTIONS` and is untouched — it is right there, because it must
reach a process several levels down that we never invoke ourselves. Here
we *are* the one invoking the process, and `NODE_OPTIONS` did not
survive every chain reliably in testing. An argument cannot fail to be
inherited, and it confines the patch to processes that actually go
through the shim.
- **Only `versions.electron` is hidden, not `versions.chrome`.** Hiding
both was the plan; it broke Gutenberg's bundling step outright. Build
tooling reads `chrome` to decide what it is compiling *for*, which is a
question about the output, not about who is running the compiler.

Deliberately not in this PR: `versions.v8` still carries its `-electron`
suffix (tools parse it as a version number), and the shim directory is
still a predictable path under `os.tmpdir()`.

## How to test this

Platforms: **any** for the suite. The manual path below was driven on
macOS; Windows is covered by unit tests only — see Risks.

**Starting state:** a throwaway package whose only script is
`concurrently "npm run a" "npm run b"`, with `concurrently@9`, in a site
the app can run a script in. The original Gutenberg path is no longer
reachable from trunk — that support lived in the closed stack — and it
was never needed to see this: the trigger is the task runner, not
Gutenberg.

1. Run the script through the app.
2. While it runs, watch the process count: `ps -A | grep -c
concurrently` on macOS/Linux.
3. It finishes in about a second.

**What must not have happened:** the process count must stay flat — on
the broken code the same package reached ~50 processes in three seconds
and never recovered. The run must also *finish*: a run that merely stops
spawning but hangs is the earlier, subtler half of this bug.

Which test covers it, and yes, I checked it fails on the old code:
`tests/unit/ipc-wiring.test.cjs` → *"npm:run-script"* now asserts the
shims `ensureNodeShimDir` really wrote carry the preload. Blanking the
preload path at all six `main.js` call sites — the exact way this
regresses — left the entire suite green before that assertion existed,
and now fails it. Under `npm run test:electron`,
`tests/unit/electron-node-compat.test.cjs` → *"without the preload the
child still looks like Electron"* pins the runtime condition itself.

## Risks and limitations

Review outcome: **4 `[fix here]` · 3 `[follow-up]` — all 4 fixed.**

- **Windows is unit-tested, not hand-tested.** The generated
`.cmd`/`.bat` content is asserted directly (quoting, `set` ordering,
`%*` last, backslashes kept), and the review checked it line by line,
but no one ran a Gutenberg build on a real Windows machine. Buildkite
has a signed artifact for this branch if someone wants to.
- **Two preload delivery mechanisms now exist** (`NODE_OPTIONS` in
`buildChildEnv` for `win-spawn-patch.js`, `--require` in the shims and
in the patch's redirects for `electron-node-compat.js`), with different
reach and different quoting rules. Nothing ties them together beyond the
comments in `node-shims.cjs` and `win-spawn-patch.js`. Consolidating the
choice into one documented place is a follow-up.
- **The preload reaches forks and the Windows redirects, not every
descendant.** `child_process.fork` inherits `execArgv`, so worker pools
are covered, and since the 2026-09-10 rebase the Windows spawn patch
re-attaches the `--require` to the `node`/`npm`/`npx` spawns it
redirects past the shim. A descendant started with an explicit
`spawn(process.execPath, …)`, or a `worker_threads` worker, inherits
`ELECTRON_RUN_AS_NODE` and sees `versions.electron` again. No such case
is known to be reachable today; `NODE_OPTIONS` would cover them, at the
cost of the reliability problem that ruled it out.
- The shim directory remains world-readable and predictably named under
`os.tmpdir()`. This PR adds one more file to a directory that already
holds executable shims, so it extends an existing exposure rather than
introducing one — but it is worth closing with `mkdtempSync` for all of
them.

## Related

Fixes #275. Part of #251, whose remaining PRs are closed pending a redo
against current trunk.

---

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

**Preferring a real system Node over the shim.** Verified to work — the
same Gutenberg build completes in 30s through the app's own spawn path
once `node` on `PATH` is a real Node. Rejected because it does nothing
for a contributor with no Node installed, which is precisely the case
the shims exist for: the app's promise is zero prerequisites.

**Neutralising only yargs' branch** (setting `process.defaultApp`, the
other half of its condition). Narrower, and it would have fixed the
runaway. Rejected because it leaves every other library that asks "am I
inside Electron?" answering wrongly, which is the general bug.

**`NODE_OPTIONS` for the compat preload.** Implemented first, then
abandoned: measured, it did not survive every chain from the app down to
a task runner's children, while the same preload passed as an argument
did. `win-spawn-patch.js` keeps using it because it has no alternative.

**Where the shim content lives.** Moved out of `main.js` into
`src/node-shims.cjs` as pure string building, so the property that
matters — every shim, on every platform, carries the preload — is a unit
test rather than something only a real Windows machine could show.

</details>

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

**4 `[fix here]` · 3 `[follow-up]` — all 4 `[fix here]` fixed.** Run per
`.github/instructions/code-review.instructions.md`, with the judgement
pass given to a subagent with fresh context. Deterministic layer: lint
clean, 889 tests pass on both Node runtimes.

Fixed:

1. **Nothing tested the wiring that ships the fix.** The reviewer
mutated all six `main.js` call sites to pass no preload path and the
suite stayed green on both runtimes — the bug could be fully
reintroduced without a single red test. The unit tests covered
`node-shims.cjs`'s parameters, not the decision to hand it the path. Now
`ipc-wiring` reads the shims from disk.
2. **`nodeCompatPath` was passed to `buildChildEnv`, which does not
accept it.** Silently dropped, and it read as though descendants were
covered through the environment — the exact misreading that would
justify removing a `--require` from a shim later. Argument removed.
3. **Two Electron-only tests returned early instead of skipping**, so on
the system Node they reported as passing while asserting nothing. Now
`t.skip()`, and the two passes no longer report identical counts.
4. **A failed preload copy was reported with `process.stderr.write`**,
which `electron-log` does not hook, so a packaged app recorded nothing
on the one path that decides whether builds run away — and the write
itself sat outside a `try`. Now goes through the app's logger.

Deferred, with reasons:

- **The test reimplements yargs' `hideBin` heuristic** rather than
importing it, so it pins our model of the dependency rather than the
dependency. Verified faithful against `yargs` as vendored today.
Importing from a transitive dependency in a test is its own trap; left
as is, and the comment says what it models.
- **The preload does not reach `worker_threads` or an explicit
`spawn(process.execPath, …)`.** No reachable case today; noted under
Risks so the next reader does not take "an argument always survives" as
covering more than it does.
- **The shim directory is a predictable path in `os.tmpdir()`.**
Pre-existing for the shims and `win-spawn-patch.js`; fixing it properly
means `mkdtempSync` for all of them, which is a change to code this PR
does not otherwise touch.

</details>

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

How the root cause was isolated, since the trail is not obvious from the
diff:

1. The process tree was a chain of `bash → Electron → bash → Electron`,
every one of them running `concurrently` — 25 copies **with no
arguments** alongside a single correct invocation.
2. Instrumenting the task runner's `spawn` showed the original process
launching *three* children for two commands: its own path, then the two
real ones.
3. That pointed at argument parsing rather than at process management,
and from there to `hideBin`'s Electron branch.
4. A throwaway package reproduced it in three seconds with no Gutenberg
involved — and only with `concurrently@9`, which still uses that yargs
path; `@10` does not, which is why a first attempt to reproduce failed
and briefly looked like the trigger was elsewhere.

`versions.chrome` is the interesting negative result: hiding it removed
no recursion (already gone) and broke the bundling step, and there is
now a test whose only job is to stop someone widening the set back.

</details>



---

**Rebase and re-review, 2026-09-10.** Replayed onto trunk at `74e700a`
(past the bundled-Git engine, #420 and #422) with no conflicts. The
self-review was re-run against the current standard with a fresh-context
subagent: **1 `[fix here]` · 1 `[follow-up]`**.

- Fixed, in `65c1f7e`: cross-platform 🔴. The compat preload travelled
only inside the shims, but on Windows `win-spawn-patch.js` rewrites
`spawn('node', …)` straight to Electron's binary, skipping the shim and
its `--require`. A tool reached that way saw `versions.electron` again.
The route #275 itself takes (`npm run` → `cmd.exe` → `concurrently.cmd`
→ `node` on PATH) does go through the shim, so the reported failure was
covered; the redirect route was not. `buildChildEnv` now exports
`WPTK_NODE_COMPAT_PATH` and the patch prepends the same `--require` to
its three redirects. Tests by injection in `win-spawn-patch.test.cjs`
and `npm-runner.test.cjs`, both red before the change.
- Follow-up: the two preload mechanisms, listed under Risks.

Re-verified on that head: lint clean, `npm test` 1273 pass / 2 skipped,
`npm run test:electron` 1275 pass / 0 skipped.

**Manual pass, macOS, 2026-09-10.** The throwaway package from "How to
test this" (`concurrently@9.2.4`, `build` = `concurrently "npm run a"
"npm run b"`), run through shims generated by this branch's
`node-shims.cjs` against the repo's Electron binary. Old-style shims (no
preload): 86 `concurrently` processes after six seconds, still climbing,
never finished. This branch's shims: both scripts printed, exit 0, no
leftover process.

**Earlier rebase note.** Replayed onto trunk (`a0fcbc9`) from the closed
stack; `src/main.js` and `ipc-wiring` merged without conflict. One extra
commit points the two new tests at the `tests/unit/` layout, since they
were written before #377 moved unit tests a directory deeper — nothing
they assert changed. Re-verified on trunk: lint clean, `npm test` 1058
pass / 0 fail / 2 skipped (the two Electron-runtime tests).



---

**Rebase and validation decision, 2026-09-11.** Replayed onto trunk
`dd8bc22` with no conflicts. Lint clean, `npm test` 1260 pass / 2
skipped, `npm run test:electron` 1262 pass / 0 skipped.

**The Windows manual pass was dropped on purpose, not forgotten.**
Recording the reasoning, because the earlier Risks section says someone
should run it.

There is no reachable trigger from the current product surface. The
app's terminal runs only `build`, `build:dev`, `dev`, `test`, `watch`
and `grunt`, and in `wordpress-develop` every one of those is Grunt,
which parses arguments with nopt rather than yargs. The Gutenberg build
that produced the original report is not reachable from trunk: that
support lived in a stack that is now closed. An attempt to stage the
failure by hand on Windows, with a throwaway `concurrently@9` script
installed into a site, ran into the same wall from the other side: the
terminal will not run a script outside that list.

So what this PR fixes is real but currently latent, and the evidence
available matches that status:

- macOS, by hand, on the branch's own shims: old shims reached 86
`concurrently` processes in six seconds and never finished; these shims
exit 0 and stay flat.
- Windows, by unit test: the generated `.cmd`/`.bat` content is asserted
directly (quoting, `set` ordering, `%*` last, backslashes kept),
`win-spawn-patch` and `npm-runner` cover the redirect route that
re-attaches the preload, and `ipc-wiring` reads the shims
`ensureNodeShimDir` actually wrote so the fix cannot be removed from the
call sites without a red test.

What stays open, plainly: nobody has watched a yargs-based task runner
start, misparse and recover under these shims on a real Windows machine.
If that path becomes reachable again, returning Gutenberg support being
the obvious case, run the pass before trusting this on Windows.

This is also why the change is not treated as a release blocker: it
removes a hazard rather than repairing a failure a contributor can hit
today.


**CodeRabbit round, 2026-09-11: 1 `[fix here]` fixed · 1 `[follow-up]`
filed · 1 declined.**

- Fixed in `50a1216`: a failed copy of the compat preload logged a line
and wrote the shims without `--require`, which is the runaway state
reached silently. Now `ensureNodeShimDir` forgets the directory and
throws, and `runNpmWithEngineRetry` reports it through the same "Failed
to start" surface as a spawn failure. The realistic trigger is the file
missing from the packaged bundle. Wiring test by injection, red before
the change. Note for the two Playground handlers that also call
`spawnRunner`: they now reject instead of starting without the preload,
which is the right outcome for a broken package.
- Follow-up #445: the POSIX shims interpolate paths into bash between
double quotes, so `$` and backticks in a path expand. Pre-existing in
`main.js` before this branch moved the strings; not a security boundary,
since whoever controls the app's environment already has `NODE_OPTIONS`.
- Declined: running the two Electron-only tests on the system Node by
planting a fake `process.versions.electron`. That would assert the
fixture, not the runtime; the skips are explicit by design (fix 3 of the
first review) and CI runs both passes, so both branches execute.

---------

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

Every fact that is WordPress-Core-specific today lives in a constant beside the code that uses it: the clone URL, the "is it built" path, the watch command, the terminal's script list, how Playground serves the checkout, where a work item lives, which repository a pull request goes to. This is the one place those facts live per target, so a site's chosen type can drive each of them. Pure data and pure functions, loaded by main and renderer alike.

The invariant every consumer relies on: an unknown or missing type resolves to Core, so a site created before the field existed keeps Core behaviour with no migration and no store write.

Ported from the closed #255 stack with three changes from the 15 Sep spike: the clone entry no longer describes a shallow clone (git-clone.cjs owns the shape), the Gutenberg built marker follows today's layout (build/scripts/block-library/index.min.js, not build/block-library), and the terminal's Gutenberg scripts leave out the bare `test`, which needs Docker.

No caller yet; the next commit wires site creation and status.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Every fact that is WordPress-Core-specific today lives in a constant beside the code that uses it: the clone URL, the "is it built" path, the watch command, the terminal's script list, how Playground serves the checkout, where a work item lives, which repository a pull request goes to. This is the one place those facts live per target, so a site's chosen type can drive each of them. Pure data and pure functions, loaded by main and renderer alike.

The invariant every consumer relies on: an unknown or missing type resolves to Core, so a site created before the field existed keeps Core behaviour with no migration and no store write.

Ported from the closed #255 stack with three changes from the 15 Sep spike: the clone entry no longer describes a shallow clone (git-clone.cjs owns the shape), the Gutenberg built marker follows today's layout (build/scripts/block-library/index.min.js, not build/block-library), and the terminal's Gutenberg scripts leave out the bare `test`, which needs Docker.

No caller yet; the next commit wires site creation and status.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…uilt check (#251) (#469)

## Why

Every fact that makes a site a WordPress Core site is a constant next to
the code that uses it: the clone URL in `wordpress:setup`,
`build/wp-includes/js/dist` in `site:status`, `grunt -- _watch` in the
dev-server planner, the docroot mount in the Playground runner,
`core.trac.wordpress.org` in the ticket parser,
`WordPress/wordpress-develop` in the pull-request flow. #251 asks for a
second target, Gutenberg, and there is no seam to put it through. This
is the seam.

## What changes

A registry, `src/project-type.cjs`: pure data and pure functions, one
entry per target (`core`, `gutenberg`), each carrying what the app needs
to clone it, know whether it is built, build and watch it, serve it,
read its work items and open its pull requests. The invariant every
consumer relies on: an unknown or missing type resolves to Core, so a
site created before the field existed keeps Core behaviour with no
migration and no store write.

Two handlers consult it. `wordpress:setup` accepts a `projectType`,
normalises it at the write boundary (an unknown id is stored as `core`,
not as whatever the renderer sent), clones the type's repository and
branch, names the folder after the type when the name is left empty, and
records the id on the site. `site:status` reads the built marker from
the type and reports the id.

Deliberately not in this PR: the renderer. The create-site dialog does
not send a type yet, so every site created today is still Core. Offering
Gutenberg before build (next PR) and serve (the one after) work would
hand a first-time contributor a checkout the app then runs Core's grunt
and docroot mount on, a state the app must not produce; the choice
appears once those two land. The registry also carries fields no caller
reads yet (serve strategy, patch layout, work-item provider,
pull-request prefix and body line): they are the same module's data and
the next five PRs consume them in order, listed in #251.

Ported from the closed #255 with three changes from the 15 Sep spike
(recorded in #251): the clone entry no longer describes a shallow clone,
since `git-clone.cjs` owns the clone's shape; Gutenberg's built marker
follows today's build layout
(`build/scripts/block-library/index.min.js`, `build/block-library` no
longer exists); and the Gutenberg terminal scripts leave out the bare
`test`, which needs Docker.

## How to test this

Platforms: any. No user-visible surface: nothing on screen changes, and
no site's behaviour changes, because nothing sends a type.

From the repository root:

```
node --test tests/unit/project-type.test.cjs
node --test tests/unit/ipc-wiring.test.cjs
```

The wiring tests are the demonstration: `wordpress:setup` with
`projectType: 'gutenberg'` clones `WordPress/gutenberg` into
`gutenberg-trunk` and records the type; with `'plugin'` it clones
`wordpress-develop` and records `core`; with nothing it does exactly
what it did before. `site:status` on a tree holding
`build/scripts/block-library/index.min.js` reads as built for a
Gutenberg record and as unbuilt for a record without the field.

**What must not have happened:** an existing site's status changing.
`hasBuilt` for a record without `projectType` still reads
`build/wp-includes/js/dist`, and no record is rewritten to add the
field.

## Risks and limitations

- `sites:add` (adopting a folder) records no type, so an adopted
Gutenberg checkout is a Core site. Right for now, since the app cannot
do anything with a Gutenberg site yet; the PR that exposes the choice
decides whether adoption detects one.
- The registry fields with no caller are unexercised beyond their shape
test until their PRs land. One of them, `build.watch`, restates the
`grunt -- _watch` constants in `dev-server-command.cjs`; the next PR
removes one copy.
- `wordpress:setup` accepts a `gutenberg` type over IPC today. Nothing
in the window sends one, and a site made that way would clone the right
repository and then be built, served and diffed as if it were
`wordpress-develop`. Review outcome: 1 [fix here] fixed, 2 [follow-up].

## Related

Part of #251. Stacked on #468.

---

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

- A registry keyed by id rather than a class per target: the facts are
data, and both processes load the module (the renderer bundle will, from
the PR that shows the picker).
- Storing the normalised id rather than leaving the field absent for
Core: a record that says `core` and a record that says nothing both read
as Core, but only one of them was written by a version that knew the
question.
- Not adding the picker here (see What changes).

</details>

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

1 [fix here] · 2 [follow-up] — the [fix here] fixed in `85f8047`, plus
one style note taken.

- 🔵 Security [fix here] `src/project-type.cjs`: `getProjectType` indexed
the registry directly, so `'__proto__'`, `'constructor'` or `'toString'`
resolved to something truthy that was not a project type and the Core
fallback never fired (verified by running the module). Fixed: it goes
through the own-property check; the fallback test now tries those keys
and fails on the old module.
- 🔵 Architecture [follow-up] `src/project-type.cjs` vs
`src/renderer/dev-server-command.cjs`: `build.watch` restates the `grunt
-- _watch` constants that planner owns. Deferred to the next PR, which
teaches the planner about the type and deletes one copy.
- 🔵 Architecture [follow-up] `src/main.js` `wordpress:setup`: the
handler accepts `gutenberg` over IPC while the rest of the app is
Core-shaped. Unreachable from the window; recorded in the handler's
comment and under Risks. `sites:add` recording no type is the deliberate
answer for now.
- Style, taken: `findAvailableDirName`'s `'wordpress-develop-trunk'`
fallback became unreachable; deleted so the default lives in the
registry only.

Verified clean by the reviewer: store migration (every writer spreads
the existing record; nothing rewrites a record to add the field),
`site:status` keeps Core semantics on every failure path, cross-platform
path handling, and the wiring tests fail when the handler stops reading
the registry.

- **Review:** completed — fresh Explore subagent context (local
`/self-review`); reviewed head `9c28408` / base `458fa34`; deterministic
layer (lint, `npm test`) run by the author; outcome above
- **Since review:** `9c28408 → 85f8047` checked: only the fixes above;
lint and `npm test` re-run green (1467 pass, 2 pre-existing skips)

</details>

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

Nothing on screen changes.

</details>

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

---------

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