[Fix] Stop a Gutenberg build from spawning processes without bound (#275)

[juanmaguitar]

Do not merge alone. Part of the Gutenberg stack (#251): #255 → #261 → #264 → #269 → this. The whole stack merges together.

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 --requires 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 site whose contribution target is Gutenberg, cloned and with dependencies installed, not yet built.

1. Start the build from the wizard's Run build step.
2. While it runs, watch the process count: ps -A | grep -c concurrently on macOS/Linux.
3. The build completes, and build/modules/block-library exists in the checkout.

What must not have happened: the process count must stay flat — on the broken code it climbs without stopping and never recovers. The build must also finish: a run that merely stops spawning but hangs is the earlier, subtler half of this bug.

To watch the old behaviour fail to reproduce, the trigger needs no Gutenberg at all: a throwaway package whose only script is concurrently "npm run a" "npm run b", with concurrently@9, run through the app, reached ~50 processes in three seconds before this change and finishes in one after it.

Which test covers it, and yes, I checked it fails on the old code: test/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, test/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.
• The preload reaches forks, not every descendant. child_process.fork inherits execArgv, so worker pools are covered. 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.

---

Design decisions and alternatives considered

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.

Review outcome (required — see AGENTS.md)

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.

Implementation notes

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.