Skip to content

[Add] Serve a Gutenberg site in a real WordPress via Playground (#251) - #261

Closed
juanmaguitar wants to merge 3 commits into
juanmaguitar/support-gutenberg-as-a-contribution-target-not-ofrom
juanmaguitar/gutenberg-playground-serve
Closed

juanmaguitar wants to merge 3 commits into
juanmaguitar/support-gutenberg-as-a-contribution-target-not-ofrom
juanmaguitar/gutenberg-playground-serve

Conversation

@juanmaguitar

Copy link
Copy Markdown
Collaborator

Why

Part of #251, and the piece that makes a Gutenberg site actually usable: seeing a Gutenberg
change running in a real WordPress
. A Core site is served by mounting its build/ as the
WordPress docroot, because that build is a WordPress install. A Gutenberg checkout is a plugin,
not a WordPress — mount it as a docroot and you get an empty site. So the serve strategy has to
follow the site's project type.

Stacked on #255 (project type driving clone/status/build). Part of the Gutenberg feature that
merges atomically — see #255. Do not merge alone.

What changes

  • src/playground-plan.cjs (new, pure) — planPlaygroundLaunch(strategy) returns the
    @wp-playground/cli mount/install options:
    • docroot (Core, unchanged): mount build/ as /wordpress, wordpressInstallMode: install-from-existing-files-if-needed (skip the download; the build already is WordPress).
    • plugin-mount (Gutenberg): leave the install mode at the default so Playground downloads and
      installs a stock WordPress
      , then mount the checkout under
      /wordpress/wp-content/plugins/<slug> and add an activatePlugin step. This mirrors
      @wp-playground/cli's own --auto-mount plugin handling (verified against the CLI source).
  • server-runner.js now takes a JSON serve config (argv[2]) and builds its runCLI options from
    the plan, still adding the debug + SMTP constants from its own environment. The loopback / hide-
    window patches still run before the CLI is required.
  • playground:start resolves the strategy from the site's project type and hands the runner the
    checkout to mount (Gutenberg) or the build dir to serve (Core).

How to test this

Platforms: any. Requires a Gutenberg site (from #255) that has been installed and built.

Automated / mechanism check (no full Gutenberg build needed):

  1. Create a throwaway plugin dir with a single .php plugin header.
  2. Run node src/server-runner.js '{"strategy":"plugin-mount","pluginDir":"<that dir>","pluginSlug":"smoke"}'.
    → It logs Mount … → /wordpress/wp-content/plugins/smoke, installs a stock WordPress, and prints
    SERVER_URL:http://127.0.0.1:9400/; opening it serves WordPress. (This is exactly what I ran to
    verify the plugin-mount path end to end.)

Full flow (a real Gutenberg site):

  1. Create a Gutenberg site; install deps; Run full build (npm run build).
  2. Start the dev server. → It boots a stock WordPress with the Gutenberg checkout active; open
    wp-admin → the block editor reflects the built Gutenberg, not core's bundled version.

What must not have happened:

  • Core is byte-identical. A Core site still mounts its build/ as /wordpress with
    install-from-existing-files-if-needed; the emitted options match the pre-change ones (empty
    mount/additional-blueprint-steps are no-ops). test/ipc-wiring.test.cjs pins the docroot
    config for a no-type site.
  • The loopback bind still precedes the Playground CLI load (test/runner-wiring.test.cjs), so the
    dev site is never briefly served to the LAN.
  • A bad serve config must fail loudly, not hang — the runner exit(1)s and the start request settles
    with an error rather than waiting out the 120s timeout.

What cannot be tested in the suite: the live WASM-PHP boot (network download of stock WP,
minutes on a slow machine). The suite unit-tests the pure planner instead; the boot was verified by
hand as above. Automated: npm run lint clean, 804 tests pass.

Risks and limitations

  • Debug.log panel is empty for a Gutenberg site. The tail reads a host-side
    build/wp-content/debug.log; a plugin-mounted (VFS) WordPress does not write there. It degrades
    quietly — the watcher no-ops on the missing file/dir, no crash — rather than failing. A
    Gutenberg-aware debug path is a follow-up.
  • Stock WordPress version is Playground's default (latest stable); Gutenberg trunk supports it, but
    pinning a WP version is a future option if a mismatch ever surfaces.

Related

Part of #251. Stacked on #255.


Review outcome (required — see AGENTS.md)

0 [fix here] · 1 [follow-up]. Ran the review in
.github/instructions/code-review.instructions.md; judgement pass in a fresh subagent. Lint clean,
804 tests pass.

  • 🔵 Cross-platform/perf · [follow-up] — the debug.log tail path (build/wp-content/debug.log) is
    Core-shaped; a plugin-mounted Gutenberg WordPress doesn't write there. Verified it degrades
    gracefully (watcher no-ops on the missing file/dir), so the panel is inert rather than broken.
    Documented above; a Gutenberg-aware debug path rides with the later copy/UX work.
  • Verified clean: the loopback-before-CLI security invariant still holds after the refactor; the
    Core docroot option set is behaviorally identical (empty mount/additional-blueprint-steps are
    no-ops, path.resolve drop is harmless since the path is already absolute); failure paths are
    loud (runner exit(1) settles the start request, no silent hang); untrusted-input handling is
    fine (JSON built from a registered path, shell:false); VFS guest paths use POSIX templates while
    host paths use path.join; tests pin both forks and would fail on a regression.
Implementation notes

The plugin-mount recipe is not invented — it's the exact shape @wp-playground/cli builds for a
plugin passed to --auto-mount: a mount of the host dir to /wordpress/wp-content/plugins/<slug>
plus an { step: 'activatePlugin', pluginPath } in additional-blueprint-steps, with the default
(download-and-install) WordPress. Keeping the planner pure (src/playground-plan.cjs) is what lets
the fork be unit-tested without booting WASM PHP; server-runner.js merges the plan with the
env-derived debug/SMTP constants and spawns.

A Core site is served by mounting its build/ as the WordPress docroot, because
that build IS a WordPress install. A Gutenberg checkout is a plugin, not a
WordPress, so the same path serves an empty site. Make the serve strategy follow
the site's project type.

- src/playground-plan.cjs: a pure planPlaygroundLaunch(strategy) that returns the
  @wp-playground/cli mount/install options — 'docroot' mounts build/ as
  /wordpress and skips the download (Core, unchanged), 'plugin-mount' lets
  Playground install a stock WordPress and mounts the checkout as an active
  plugin under wp-content/plugins/<slug> (Gutenberg). This mirrors the CLI's own
  --auto-mount handling (a `mount` plus an `activatePlugin` step).
- server-runner.js now takes a JSON serve config and builds its runCLI options
  from the plan, keeping the debug + SMTP constants it adds from its own env.
- playground:start resolves the strategy from the site's type and hands the
  runner the checkout to mount (Gutenberg) or the build dir to serve (Core).

Verified the plugin-mount path end to end against a throwaway plugin: Playground
downloads a stock WordPress, mounts the plugin, and reports a URL that serves
WordPress. Core's docroot path is byte-identical.

Known gap: the debug.log tail reads a host-side build/wp-content/debug.log, which
a plugin-mounted (VFS) WordPress does not write — so the debug-log panel is empty
for a Gutenberg site. It degrades quietly (the tail no-ops on a missing file)
rather than failing; a Gutenberg-aware debug path is a follow-up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@juanmaguitar
juanmaguitar force-pushed the juanmaguitar/gutenberg-playground-serve branch from 3e81245 to a9d5fc5 Compare August 12, 2026 05:18
Three findings from the review of the Playground serve path:

- playground:start resolved the project type between the in-flight guard and
  the assignment that arms it, so two concurrent starts for one site both
  passed the guard and the second orphaned the first — a live PHP-WASM server
  unreachable to playground:stop and the before-quit sweep, surviving the app
  quitting and holding its port. The read moves above the guard, restoring the
  invariant that nothing awaits between the guard and the assignment.

- plugin-mount hands Playground a read-write mount of the source checkout, not
  a regenerable build/, so WordPress could write through to the contributor's
  working tree: Plugins -> Delete on the mounted plugin unlinks it, .git and
  uncommitted work included. DISALLOW_FILE_MODS and DISALLOW_FILE_EDIT are now
  set for that strategy only. Core keeps WordPress's defaults, since it exposes
  only build/ and a Core contributor may legitimately install a plugin.

- The join between the plan and the CLI was unasserted: both strategies were
  pinned in isolation, so deleting `...launch` from server-runner.js left every
  test green while Core served a freshly downloaded WordPress with the
  contributor's build/ nowhere in it. Pinned from both sides, along with the
  new guards and the race.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Aug 12, 2026
## 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>
@juanmaguitar

Copy link
Copy Markdown
Collaborator Author

Closing along with the rest of the stack — see #255 for the reasoning: trunk has rewritten src/main.js and src/renderer/index.jsx out from under this branch since 907cb65, so rebasing would be a semantic rebase, not a textual one.

The branch stays, so the work is retrievable. What ports forward cleanly here: src/playground-plan.cjs and test/playground-plan.test.cjs.

juanmaguitar added a commit that referenced this pull request Sep 11, 2026
…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>
juanmaguitar added a commit that referenced this pull request Sep 16, 2026
…k WordPress (#251)

A Core site's build/ is a whole WordPress, and the runner mounts it as the docroot before install and skips the download. A Gutenberg checkout is a plugin: Playground installs its stock WordPress, the checkout is mounted at wp-content/plugins/gutenberg and activated. playground-plan.cjs (ported from the closed #261) turns a strategy into the runCLI mount, install-mode and blueprint-step options, and names the constants a strategy needs on top of the shared debug and SMTP set: the plugin mount is the contributor's working tree, .git included, so it sets DISALLOW_FILE_MODS and DISALLOW_FILE_EDIT, otherwise Plugins > Delete in the served site removes the checkout.

The runner takes one JSON argument instead of a bare build directory; playground:start reads the site's type and writes the config, and runs the runner from the checkout for a plugin mount, from build/ as before for a docroot. The map that playground:stop and the quit sweep walk is keyed as it was.

Wiring tests pin the Core options byte for byte and the plugin mount separately; nothing on screen changes, since nothing can create a Gutenberg site yet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Sep 17, 2026
…k WordPress (#251)

A Core site's build/ is a whole WordPress, and the runner mounts it as the docroot before install and skips the download. A Gutenberg checkout is a plugin: Playground installs its stock WordPress, the checkout is mounted at wp-content/plugins/gutenberg and activated. playground-plan.cjs (ported from the closed #261) turns a strategy into the runCLI mount, install-mode and blueprint-step options, and names the constants a strategy needs on top of the shared debug and SMTP set: the plugin mount is the contributor's working tree, .git included, so it sets DISALLOW_FILE_MODS and DISALLOW_FILE_EDIT, otherwise Plugins > Delete in the served site removes the checkout.

The runner takes one JSON argument instead of a bare build directory; playground:start reads the site's type and writes the config, and runs the runner from the checkout for a plugin mount, from build/ as before for a docroot. The map that playground:stop and the quit sweep walk is keyed as it was.

Wiring tests pin the Core options byte for byte and the plugin mount separately; nothing on screen changes, since nothing can create a Gutenberg site yet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Sep 17, 2026
…k WordPress (#251) (#471)

## Why

A Core site's `build/` is a whole WordPress, and the Playground runner
mounts it as the docroot before install and skips the download. A
Gutenberg checkout is not a WordPress at all; it is a plugin, and there
is nothing to mount as a docroot. #251 needs the served site to be a
stock WordPress with the checkout mounted and activated as its Gutenberg
plugin.

## What changes

The stock WordPress installation now belongs to the site's lifecycle:
stopping and restarting preserves posts, settings and uploads; deleting
the registered Gutenberg site stops its processes and removes the
persistent installation before removing the checkout. The runner and
cleanup canonicalize the same directory, including Windows casing and
macOS path aliases. Failed cleanup keeps the checkout and registry entry
retryable.

`src/playground-plan.cjs` (ported from the closed #261) turns a serve
strategy into the `runCLI` options: `docroot` is what Core always sent
(mount before install as `/wordpress`,
`install-from-existing-files-if-needed`), `plugin-mount` uses
Playground's persistent `start` command to retain its WordPress
installation across restarts, mounts the checkout at
`wp-content/plugins/gutenberg` and adds an `activatePlugin` step. It
also names the constants a strategy needs on top of the shared debug and
SMTP set: the plugin mount is the contributor's working tree, `.git` and
uncommitted work included, so `plugin-mount` sets `DISALLOW_FILE_MODS`
and `DISALLOW_FILE_EDIT`; without them **Plugins > Delete** in the
served site removes the checkout. The cost is deliberate: the served
site cannot install a second plugin or theme, and has no file editor.
Losing an afternoon of work is worse than restarting a preview.

The runner takes one JSON argument instead of a bare build directory.
`playground:start` reads the site's type, writes the config, and runs
the runner from the checkout for a plugin mount, from `build/` as before
for a docroot. The map `playground:stop` and the quit sweep walk is
keyed as it was.

Deliberately not in this PR: anything a contributor can see. Nothing can
create a Gutenberg site yet; the choice arrives in the next PR, now that
a Gutenberg site can be cloned, built, watched and served.

## How to test this

Persistence regression, **macOS verified; Windows supported by the same
opt-in lane**, current head. From the repository root, run
`TOOLKIT_REAL_SETUP=1 npx --no-install playwright test --config
tests/e2e/real-setup.config.js -g 'keeps posts'` (PowerShell: set
`$env:TOOLKIT_REAL_SETUP = '1'` first, then run the command without the
POSIX assignment). This boots real WordPress, saves a post and upload,
restarts and checks both, deletes the installation and checks that the
same location starts fresh. It fails with the previous `server` command
(empty title, missing upload) and passes with the correction. The IPC
suite additionally covers registered/Core/unregistered deletion and a
cleanup failure. The fixture and its persistent installation are removed
after the test.

Platforms: any for the Core path; **macOS** for the Gutenberg pass below
(run by the author, 15 Sep, on `3c7d13c`; the fixes since do not touch
the plugin-mount path).

Core, no visible change, from the repository root:

```
node --test tests/unit/playground-plan.test.cjs
node --test tests/unit/runner-wiring.test.cjs
node --test tests/unit/ipc-wiring.test.cjs
```

The runner test pins the Core options byte for byte against what the
runner sent before (`mount-before-install` of the build directory, the
install mode, no plugin mount, no steps, no file-mods constants), and
the plugin mount separately.

Gutenberg, by hand, since the app cannot make such a site yet: with a
built `WordPress/gutenberg` checkout on disk, run `src/server-runner.js`
on Electron's Node with the argument
`{"strategy":"plugin-mount","pluginDir":"<checkout>","pluginSlug":"gutenberg"}`
and the checkout as the working directory, then open the `SERVER_URL:`
it prints.

1. The front page loads. Expected: its HTML references
`wp-content/plugins/gutenberg/` assets.
2. Log in (`admin` / `password`) and open **Plugins**. Expected:
Gutenberg is listed with a **Deactivate** link; no plugin has a
**Delete** link; there is no **Add New Plugin**.
3. Open **Posts > Add New**. Expected: the editor loads its scripts from
`wp-content/plugins/gutenberg/build/`.

Observed on the author's run: up in 5 to 14 s, all three as expected.

**What must not have happened:** a Core site's Playground options
changing in any way (the runner test is the guard); the served Gutenberg
site offering a Delete link on its own plugin; anything written into the
checkout by the served site.

## Risks and limitations

The restart/deletion regression passed locally on macOS; its Windows
network run has not been executed in this correction pass. Cleanup
follows the installed Playground CLI's
`~/.wordpress-playground/sites/<sha256(canonical cwd)>` convention,
pinned by the real CLI test. A checkout removed externally cannot be
resolved safely, so deleting its stale registry entry does not guess
which persistent directory to remove. Previously discarded temporary
content cannot be recovered.

- Review outcome: 1 [fix here] fixed, 4 [follow-up] (three taken, the
start race deferred).
- The Gutenberg pass is on macOS only. Windows runs the same runner on
the same JSON, with backslashes escaped by `JSON.stringify` and no shell
in the spawn; the next PR, which exposes the choice, carries the Windows
pass through its Buildkite artifact.
- Adminer's SQLite path (`/wordpress/wp-content/database/.ht.sqlite`) is
Playground's stock install's under `plugin-mount`, the same as under
`docroot`; verified by the front page and admin loading, not by opening
Adminer.
- `DISALLOW_FILE_MODS` also blocks plugin and theme installs and core
updates inside the served site. Deliberate, and documented in the next
PR alongside the choice.

## Related

Part of #251. Stacked on #470.

---

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

- One JSON argument rather than a second positional one: the runner's
contract becomes "what to serve", and a third strategy is a new value,
not a new argument.
- The strategy's constants are spread last into the blueprint's
constants so nothing shared can override the guard.
- Mounting the checkout read-write with WordPress told not to modify
files, rather than a read-only mount: Playground's NODEFS mount has no
read-only mode, and the two constants are WordPress's own switch for the
same thing.

</details>

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

1 [fix here] · 4 [follow-up] — the [fix here] and three of the
follow-ups fixed in `fee96e5`.

- 🔴 Tests / cross-platform [fix here]
`tests/unit/runner-wiring.test.cjs`: the new mount assertions compared
`hostPath` to POSIX literals while the runner resolves them, which on
Windows gives a drive-rooted path; four red CI jobs. Fixed: expected
values resolved the same way.
- 🔵 Security [follow-up, taken] `src/playground-plan.cjs`: `pluginSlug`
went into the VFS path unvalidated; only the registry supplies it today,
but the module is the seam. Fixed: a plain-name guard, with a test.
- 🔵 Tests [follow-up, taken]: "byte for byte" was not literal, the
docroot plan sent two empty arrays the old runner never did (inert in
the installed CLI, verified by the reviewer, but unpinned). Fixed: the
docroot plan sends exactly the old options and the test asserts the keys
are absent.
- 🔵 Performance [follow-up, taken as a comment]: the first Gutenberg
serve needs the network for the WordPress release; Playground caches it
afterwards. Recorded beside the strategy.
- 🔵 Failure paths [follow-up, deferred] `src/main.js`
`playground:start`: two starts for one site can both pass the
running-server guard, since awaits precede it (pre-existing, the SMTP
await was already there; this PR adds a store read). Deferred as its own
fix: the cheap change is a placeholder in the map at the guard.
- Style, taken: `isPluginMount` once instead of three strategy
comparisons; the runner harness comment no longer claims the Playground
packages are not installed.

Verified clean by the reviewer: the serve config is built from app-owned
values and passed as argv with no shell (Windows backslashes survive
`JSON.stringify` / `JSON.parse`, `path.resolve` is a no-op on an
absolute path); `playground-plan.cjs` has no requires or side effects so
the loopback patch still precedes the CLI load; the `DISALLOW_FILE_*`
trade-off is recorded and no remaining WordPress write path reaches an
active plugin's directory with both set; running the runner from the
checkout writes nothing into it; a plan failure reaches the renderer
through the existing log and exit path; the Gutenberg wiring test fails
if the handler stops reading the type.

- **Review:** completed — fresh Explore subagent context (local
`/self-review`); reviewed head `3c7d13c` / base `34d4005`; deterministic
layer (lint, `npm test`) run by the author; Playground option handling
verified by the reviewer against the installed `@wp-playground/cli`
bundle; outcome above
- **Since review:** `3c7d13c → fee96e5` checked: only the fixes above;
lint and `npm test` re-run green (1480 pass, 2 pre-existing skips)

</details>

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

Nothing on screen changes in the app. The served Gutenberg site's
Plugins page, from the manual pass above: Gutenberg active, no Delete
links.

</details>

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

<details>
<summary>Stack correction review — 2026-09-17</summary>

0 remaining [fix here] · 0 new [follow-up] in this correction pass.
Persistence and deletion lifecycle fixed; the original data-loss finding
and the additional deletion/recreation finding are resolved.

- Current PR head: `244ca76c360e183a8da9f2343d7cf05fc05f5924`;
configured base head: `d3bdaff0b805b55ef8c9a8f0c4093ef68064ab24`.
Previous PR head: `1283b18bf0946eca9b5d1810b299eca1177baa69`.
- Reviewer: fresh Codex context `/root/review_fixes`. Completed
five-dimension review of the correction delta from previously reviewed
integrated head `bc8c4fda3fb78d51cd6f3abd5fc1b0f1e89c0095` to
`c8c86677491012f9c9d4e99fe43b4ceff1c9647b`, then re-reviewed through
final stack head `34550eec7a9031d2d5f03116801a60e313fa0559`. The
additional runtime-deletion finding was fixed and verified; the original
three findings remain resolved. Earlier review records above are
historical.
- Validation on the combined stack: lint; Node unit/integration/IPC
suite (1,547 passed, two runtime skips); Electron suite (1,549 passed);
33 macOS journeys; documentation build. The real Playground
restart/deletion regression passed on macOS. Windows CI results must be
read on these new heads; this pass does not claim a fresh Windows
real-setup run.
- CodeRabbit's earlier check is not review coverage for these new heads.
This local correction review is the review of record for the delta; no
new bot review was requested.

</details>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gutenberg-contributions Support Gutenberg as a contribution target (#251)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant