Put every test under one tests/ directory - #377

## Why

TESTING.md's five-layer section names a path per layer and never shows
the tree. The shape of the suite is therefore something you assemble by
reading five separate bullets and holding them in your head — and a
reader arriving at that section is usually there for one reason, to find
out where their new test goes. They want the map before the taxonomy.

This was worth doing anyway, but #377 is what made it obvious: that PR's
whole argument was that the tree should say what the document says. It
moved the tree. This finishes the sentence.

## What changes

One block at the top of **The five layers**, before the ordering rule
and the numbered entries:

```
tests/
  unit/            layers 1-3 — npm test — under three seconds
    fixtures/      package.json trees the integration tests copy; not tests themselves
  e2e/
    journeys/      layer 4 — npm run test:e2e
    packaged/      layer 5 — npm run test:e2e:packaged, and it needs a build first
    helpers/       the app session fixture and the Git site builder; not tests either
```

Plus two sentences stating what the tree makes obvious once it is in
front of you and was never written down: the two directories split on
**what a test costs to run**, not on what it covers — everything in
`unit/` is a function call, everything in `e2e/` launches the app. The
five layers subdivide that, and the *file name* says which layer a file
is (`*.integration.test.cjs` for layer 2, `*.spec.js` for anything
end-to-end).

`fixtures/` and `helpers/` are labelled as not-tests deliberately. Both
are directories full of `.cjs` and `.json` that a reader scanning for
test files will otherwise count as tests, and `helpers/` in particular
is where someone looking for "the e2e tests" lands first.

No path changed and no claim changed. This is the layout the parent PR
produced, written down.

## How to test this

**Platforms: any.** Documentation only — no code, no config, nothing the
suite can reach.

**Starting state:** this branch checked out.

1. `git diff juanmaguitar/tests-folder -- TESTING.md` → 13 added lines,
1 changed, all inside the **The five layers** section.
2. Read the tree against the repository: `find tests -maxdepth 2 -type
d` → `tests/unit`, `tests/unit/fixtures`, `tests/e2e`,
`tests/e2e/helpers`, `tests/e2e/journeys`, `tests/e2e/packaged`. Six
directories, six lines in the block, no directory omitted and none
invented.
3. Check the three commands named in the tree against `package.json`
scripts — `npm test`, `npm run test:e2e`, `npm run test:e2e:packaged`
all exist and map to the layer claimed.
4. `npm run lint` → exit 0 (nothing lints Markdown here; run for the
sake of the branch being clean).

**What must not have happened:**

- **A test count.** TESTING.md's own Conventions section forbids numbers
in this document — "a number here is wrong the moment somebody adds a
test, and nothing makes it go red". The block gives commands and
timings, never a count. Worth checking, because a directory tree is
exactly the place the temptation shows up.
- **A second source of truth.** The tree must not restate what the
numbered layer entries below it already say, or the two drift. It
carries the *locations and costs*; the entries keep the "what each layer
is blind to" reasoning, which is the part that actually decides where a
test goes.
- **A claim about `fixtures/` or `helpers/` that is wrong.** Both are
described by what they hold. `tests/unit/fixtures/` is `package.json`
trees plus `.npmrc` files copied into a temp directory by
`copyFixture()`; `tests/e2e/helpers/` is `app.cjs` (the session fixture)
and `git-site.cjs` (the Git site builder). Neither contains an
assertion.

## Risks and limitations

A hand-written tree in a document is a thing that can go stale, and
nothing checks it. That is the honest cost of this PR. It is bounded —
six directories that have been stable since the suite existed, and a
wrong one is visible to anyone who runs `ls` — but it is a new claim in
a file that previously made none.

Not done: no equivalent block in AGENTS.md or CONTRIBUTING.md. Both
point at TESTING.md for exactly this, and a copy in either would be the
drift this repository already avoids on the review standard.

## Related

Stacked on #377, which produced this layout. Base retargets to `trunk`
automatically when that merges. Follow-up to #376.

---

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

**Why in "The five layers" and not at the top of the file?** The opening
section is **What to run** — commands, for someone who wants to run
something now. A tree there answers a question that reader is not
asking. The layer section is where the "where does this go" reader
arrives, and the tree is the answer to their actual question.

**Why not one line per layer, matching the five entries below?** Because
there are five layers and six directories, and they do not line up:
layers 1-3 share `tests/unit/`, and `fixtures/` and `helpers/` belong to
no layer at all. Forcing the tree into the taxonomy would have hidden
precisely the two directories a newcomer most needs labelled.

**Why not annotate every file?** The suite is large and the file list
changes weekly. Directories are stable; files are not. The Conventions
section already rules out anything count-shaped for the same reason.

</details>

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

**No findings across the five dimensions.**

Deterministic layer: `npm run lint` exit 0. The test suites were not
re-run for this PR and are unchanged by it — the diff is 14 lines of
Markdown in one file, touching no code, no config and no path. They ran
green on the parent commit in #377 (1027 unit on both runtimes, 14
journeys, 4 packaged).

The dimensions do not have much to bite on here: no code, so nothing to
say about architecture, security, performance or cross-platform. Under
**tests**, the standard's rule is that a change without a test is a
finding — this is documentation with no user-visible surface and nothing
`node --test` can assert about, which the standard explicitly allows
for, and the manual verification above is what stands in its place.

The one thing worth flagging is in **Risks** rather than as a finding,
because it is a property of the change rather than a defect: a
hand-maintained tree can go stale and nothing in CI will notice.

**Process disclosure:** the judgement pass ran in the authoring session
rather than a fresh subagent, because this session is configured not to
dispatch them. Same disclosure as #376 and #377.

</details>

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

Trunk moved every test into `tests/unit/` in WordPress#377, and this branch predates
that, so the merge raises a `CONFLICT (file location)` for the new
`test/window-links.test.cjs`.

Git relocates the file on its own; it does not rewrite what is inside it.
Accepting the move alone leaves `require('../src/window-links.js')` pointing
one directory too high, and the test throws `MODULE_NOT_FOUND` the first time
it runs.
This resolution accepts the relocation and takes the require to
`../../src/window-links.js`, the depth every other file in `tests/unit/`
already uses.

---

Generated with the help of Claude Code, https://code.claude.com

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

These two tests were written when unit tests lived one level deep in
test/. #377 moved them to tests/unit/, so their relative requires reach
one directory short of src/. Nothing about what they assert changes.

These two tests were written when unit tests lived one level deep in
test/. #377 moved them to tests/unit/, so their relative requires reach
one directory short of src/. Nothing about what they assert changes.

These two tests were written when unit tests lived one level deep in
test/. #377 moved them to tests/unit/, so their relative requires reach
one directory short of src/. Nothing about what they assert changes.

…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>

…st, Fixes #399, Fixes #387 (#450)

## Why

`build.files` in package.json was an exclusion list, so every top-level
entry of the checkout shipped inside `app.asar` unless someone had named
it. That shape is what packaged the App Store Connect key in #390, and
it is why a developer's checkout ships different contents from a CI
checkout (#387): a baseline `npm run pack:dir` on trunk `4110349` with
seeded leftovers packed `test-results/`, the VitePress build under
`docs/.vitepress/dist/`, and even gitignored personal files
(`CLAUDE.local.md`, `AGENTS.override.md`) next to `tests/`, `docs/`,
`scripts/`, `.buildkite/`, `fastlane/` and every root Markdown file.
This is not a size change and the PR does not claim one: #399 is about
which way the default points.

## What changes

- **`build.files` is now a positive list**: `package.json`, `src/**/*`
(minus `src/renderer/index.jsx`, which the esbuild bundle replaces), and
`local-playground-web/**/*` (optional, served by `src/main.js` when
present). The dugite trims stay as `!` rules; electron-builder applies
only the negative patterns of `files` to `node_modules`, so production
dependencies ship exactly as before (same 28538 entries under
`node_modules`). `!vendor` and `!.codesigning` go, because unlisted is
excluded.
- **Every file under `src/` is checked, not just the archive root.**
`src/**/*` is recursive, so the expected set is derived from Git —
tracked under `src/`, minus the esbuild entry point, plus the two build
outputs that replace it — and compared against a recursive walk inside
the archive. An untracked file anywhere under `src/` fails by name.
Deriving beats enumerating here: a hand-written list of eighty entries
is the exclusion list again.
- **The one native module ships its binary and nothing else, on Windows
too.** The signed Windows artifact carried nineteen MSVC intermediates
beside `fs_ext.node` (`.iobj`, `.ipdb`, `.lib`, `.exp`, the `.vcxproj`
pair, eight `.tlog` logs under `obj/`), about 680 KB that
electron-builder's default excludes miss by an extension. Three `!`
rules trim them. Two assertions hold the line: the build directory in
`app.asar.unpacked` must contain exactly the binary, and the module is
now *loaded* inside the packaged main process rather than only resolved,
so a rule that overreached into the binary, or an ABI mismatch, fails in
CI instead of on a contributor's first site.
- **The bundled npm CLI is now asserted too.** `src/main.js`,
`src/install-runner.js` and `src/script-runner.js` all resolve
`npm/package.json` to run installs and build scripts, and nothing
checked that it survived packaging. It is in `REQUIRED_MODULES` beside
the Playground CLI, the file-lock native module and dugite.
- **The packaged smoke test reads the payload against the same list**,
inside and outside the archive: the root of `app.asar` through
Electron's asar-aware fs from the packaged main process, and the root of
`app.asar.unpacked` on disk. Anything outside `node_modules`,
`package.json`, `src` (and the optional `local-playground-web`) fails,
and the three required entries must be present. The in-asar
`.codesigning` check is subsumed; the plain-fs walk of the whole payload
for `.codesigning` stays, since it covers places neither root listing
reaches.
- **CI seeds the leftovers before packaging**: the packaged job stages a
signing key, `test-results/`, a top-level `dist/intermediate/` and
`docs/.vitepress/dist/` and then proves none ships, on macOS and
Windows.
- **Root cause of the "nested dist" in #387, confirmed before fixing**:
electron-builder always excludes its own output directory
(`directories.output`, so a top-level `dist/` never shipped), but
nothing covered a `dist/` anywhere else. The one in the asar is
`docs/.vitepress/dist/`, the VitePress build. `test-results/` had no
rule at all.

Not in this PR: `build/icon.png`, which `src/main.js` references on
Linux only. electron-builder excludes `buildResources` by default, so
that path has never resolved in a packaged app; the allow-list does not
change it, and Linux is outside the v1.1.0 validation scope (#388).

## How to test this

Platforms: macOS and Windows, because the assertion is about the
packaged payload and the payload is built per platform.

**Automated, any platform with a checkout:**

In the repository root:

```
npm run build:once && CSC_IDENTITY_AUTO_DISCOVERY=false npm run pack:dir
npm run test:e2e:packaged
```

Fifteen tests pass. To watch the new assertion fail on the old rule:
check out trunk's package.json (`git show trunk:package.json >
package.json`), rebuild with the same two commands, and run `npx
--no-install playwright test --project=packaged -g allow-listed`. It
fails listing every root entry that used to ship (verified on macOS: 27
offenders, `test-results` and `docs` among them).

**Automated, run in CI at `010ec0e`:** the packaged job seeds all four
leftovers and then packages, on both platforms. Thirteen tests passed on
**macOS** (job `103260902735`) and thirteen on **Windows** (job
`103260902938`), all seven checks green. **At `7a40ce8`:** fifteen pass
on **macOS** (job `103300828500`) and fifteen on **Windows** (job
`103300828243`). The Windows run is the one that proves the
native-module trim: it is the platform that leaves the MSVC working set
behind, and its build directory now holds exactly `fs_ext.node`. The
module also loads for real on both. Those two runs are the CI half of "a
local build and a CI build contain the same tree".

**Payload verified on the signed Buildkite artifacts, both platforms.**
Buildkite build **#1020** is the build for `7a40ce8`, all three platform
steps green. Its Windows `.exe` was inspected directly, with its SHA-256
(`3c693d13…496c9eb`) matched against the artifact Buildkite lists, so
the file is tied to that exact build and commit. The macOS `.dmg`
inspected is from the branch's previous build (the three earlier commits
are payload-identical to each other; `7a40ce8` changes only the Windows
tree, as the row below shows):

| Check | macOS `.dmg` | Windows `.exe` |
|---|---|---|
| `app.asar` root | `node_modules`, `package.json`, `src` | same |
| `app.asar.unpacked` root | `node_modules` | same |
| `src/` contents vs `git ls-files` | exact match | exact match |
| `tests`, `docs`, `test-results`, `dist`, `scripts`, `fastlane`,
`.buildkite` | absent | absent |
| `.codesigning` anywhere in the payload | absent | absent |
| credential manager and git-lfs in the bundled Git | absent | absent |
| `fs-ext-extra-prebuilt/build/Release` | `fs_ext.node` only |
`fs_ext.node` only (at `7a40ce8`; 19 MSVC intermediates before) |
| Signature | Developer ID Automattic, notarized | Authenticode not
checkable from macOS |

**Same tree, local versus CI (#387's third acceptance): discharged.** A
local `pack:dir` on this branch and the signed macOS artifact list
**28620 identical entries**, the same paths with no diff at all. The
Windows artifact of build #1020 differs from the previous Windows
artifact in exactly the 19 entries `7a40ce8` set out to remove, and in
nothing else. The two platform artifacts now differ only in the bundled
Git (262 entries on macOS, 417 on Windows).

**Manual pass done — JuanMa, Windows 11, Buildkite build #1020 (commit
`7a40ce8`), with Git for Windows installed on the machine, which is the
case that matters.** The installed app booted and painted the site list.
On a site created with the app: two tickets linked (`ticket/60001`,
`ticket/60002`), a switch between them, PR #13456 applied, and an Update
trunk that offered to save the five applied files, then reset the
checkout; afterwards, plain `git status` in the site directory read "On
branch trunk, up to date with origin/trunk, working tree clean" and `git
branch` listed both ticket branches. During a fresh "Create WordPress
Core site", a PowerShell loop over `Get-Process` caught the Git
processes at

```
...\resources\app.asar.unpacked\node_modules\dugite\git\cmd\git.exe
...\resources\app.asar.unpacked\node_modules\dugite\git\mingw64\bin\git.exe
```

and never at the system Git. That is the bundled binary spawning from
the signed installation, with a competing Git on `PATH` and ignored, as
the invariant says.

**Starting state:**

1. Download the macOS `.dmg` (or `.zip`) and the Windows `.exe` from
that build.
2. Install and launch each. The first window paints and the site list
shows "WordPress Core".
3. macOS: in Terminal, from any directory, list the asar root:
`npx @electron/asar list "/Applications/WordPress Contributor
Toolkit.app/Contents/Resources/app.asar" | awk -F/ 'NF>=2{print $2}' |
sort -u`
   Expected: exactly `node_modules`, `package.json`, `src`.
4. Windows: in PowerShell, from any directory (the install folder is
named after the package, not the product):
`npx @electron/asar list
"$env:LOCALAPPDATA\Programs\electron-setup-wordpress-core\resources\app.asar"
| ForEach-Object { ($_ -split '/')[1] } | Sort-Object -Unique`
   Expected: the same three entries.
5. Either platform: create a site or open an existing one and run
**Update trunk** or link a ticket, so the bundled Git spawns from
`app.asar.unpacked`. On **Windows**, also run an `npm install` on a
site, which exercises the bundled npm CLI and the two shims copied out
of `src/`.
6. Same tree, local versus CI (#387's third acceptance). On **macOS**,
in the repository root, build locally and compare the installed
Buildkite artifact against it:

   ```
npm run build:once && CSC_IDENTITY_AUTO_DISCOVERY=false npm run pack:dir
npx @electron/asar list dist/mac-*/*.app/Contents/Resources/app.asar |
sort > /tmp/local.txt
npx @electron/asar list "/Applications/WordPress Contributor
Toolkit.app/Contents/Resources/app.asar" | sort > /tmp/ci.txt
   diff /tmp/local.txt /tmp/ci.txt && echo "same tree"
   ```

Run it from a checkout that has already run the test suites, so the
leftovers are present locally and their absence means something.

**What must not have happened:**

- No `tests`, `docs`, `test-results`, `.codesigning` or any root
Markdown file in the asar listing.
- The bundled Git still works from the installed app (a ticket link or
trunk update completes), which proves the `asarUnpack` rule and the
dugite trims survived the new list.
- The Windows install still runs npm scripts (the `win-spawn-patch.js`
and `electron-node-compat.js` shims are copied out of `src/` at run
time; a dropped `src/` file would surface as a failed `npm install`).

## Risks and limitations

- The two lists (package.json and the spec) are still edited by hand,
deliberately: widening what ships should be a visible edit in two
places. They cannot silently diverge, because a third test derives the
allowed set from `build.files` and asserts it equals the literal list.
- `local-playground-web` is allowed but not asserted present, because it
is not in the repository. A checkout that carries one ships it, as
before.
- Linux artifacts (AppImage, deb, snap) are built with the same list but
not validated here (#388). The Linux window icon path in `src/main.js`
was already unreachable in packaged builds and stays so; recorded here
rather than filed, per the no-new-issues rule for v1.1.0.
- The local-versus-CI tree comparison is a step of the manual pass, not
something CI does on its own: no job has both artifacts. What CI does
prove, on both platforms, is that the four machine-dependent leftovers
are gone and the roots match the list. With a positive list there is
nothing machine-dependent left that could differ, but the diff above is
the evidence rather than the argument.
- **`node_modules` is not governed by the allow-list.** electron-builder
collects production dependencies itself and applies only the `!`
patterns of `files` to them, so a leftover inside a dependency has to be
named. The one such case found, the MSVC working set of
`fs-ext-extra-prebuilt` on Windows, is trimmed in `7a40ce8` and asserted
on both platforms; the packaged smoke test would not have noticed it
before, because the module was resolved but never loaded. Only Windows
can fail the new assertion, so its packaged CI job is the proof, and the
signed Windows artifact of build #1020 was re-inspected: the build
directory holds exactly `fs_ext.node`.
- Code-signing and notarization are unaffected by `files`. The macOS
artifact is signed with Automattic's Developer ID and notarized,
confirmed by `codesign` and `spctl`. The Windows Authenticode signature
could not be checked from macOS.
- electron-builder strips the `build` key from the packaged
`package.json`, so an artifact cannot be tied to a commit from the
inside. The Windows artifact is tied to `7a40ce8` from the outside, by
its SHA-256 against Buildkite build #1020. The macOS `.dmg` inspected
predates `7a40ce8`; that commit changes only the Windows tree, and the
macOS packaged CI job at `7a40ce8` (15/15) covers the rest.

## Related

Fixes #399
Fixes #387
Follow-up to #390 and #396 (the key incident), #377 (the `test/` to
`tests/` move an exclusion list would have had to track).

---

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

- **Exact set, not "no offenders only".** The issue asked that the asar
contain no top-level entry outside the list. Requiring the three runtime
entries to be present as well means a broken glob (say `src/**` stops
matching after a rename) fails here rather than at first launch.
- **Adding `!test-results` and `!**/dist` instead** (what #387 first
proposed) was rejected: it keeps the exclusion shape #399 identifies as
the problem, and a `dist/` glob would need care not to strip `dist/`
directories inside published npm packages, which many have.
- **Reading the asar from outside with `@electron/asar`** was rejected
in favour of `readdirSync(app.getAppPath())` inside the packaged
process: it uses the same asar-aware fs the app runs on, needs no extra
dependency in the test, and is the pattern the other in-asar assertions
already use.
- **Seeding in the workflow, not in the test.** The test runs after
packaging, so it cannot seed. The seeds live in the packaged job's
existing canary step. Locally a checkout that has run the suites or the
docs build carries two of the four already, which is what TESTING.md now
says; `.codesigning` exists only on a signing machine, so the complete
proof is the CI run.
- **`build/icon.png` left out.** Adding it would put a new `build` entry
in every asar for a Linux-only path that has never resolved in a
packaged app. Out of scope for v1.1.0.

</details>

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

**0 [fix here] · 3 [follow-up] — 2 of the 3 follow-ups fixed in the
branch, 1 recorded as a limitation.**

- **Review:** completed — `/self-review`, judgement pass in a fresh
subagent context against
`.github/instructions/code-review.instructions.md`; reviewed head
`b1579be` (reviewed as the working tree at that content) / base `trunk`
`4110349`; evidence: `npm run lint` clean, `npm test` 1284 pass 2 skip 0
fail, `npm run test:e2e:packaged` 11/11 on macOS against a build made
with the new list; outcome below.
- **Since review:** `4110349` → `b1579be` → `c7f430d` → `010ec0e` →
`23b41f9` (trunk merged in from GitHub, #449 and #452, no overlap with
this diff) → `7a40ce8`. `7a40ce8` trims the native module's Windows
build leftovers found while inspecting the signed artifact, and turns
the native-module check from resolve into load; lint, the unit suite
(1285 pass) and the packaged suite (15/15 on macOS) re-run on it. The
self-review ran against the working tree that became `b1579be`, plus the
two fixes listed below. `c7f430d` adds the npm CLI assertion, written
while checking the runtime paths the allow-list could drop. `010ec0e`
answers CodeRabbit's one finding. Each step was re-run through lint, the
unit suite and the packaged suite (13/13 on macOS at `010ec0e`).
- **Review:** completed — delta pass on `c7f430d`, `010ec0e` and
`7a40ce8` (the commits no bot reviewed), judgement in a fresh subagent
context against `.github/instructions/code-review.instructions.md`, at
head `64d9bd0` (trunk merged in, no overlap). 0 [fix here] · 1
[follow-up] · 1 style. Follow-up: `tests/e2e/packaged/smoke.spec.js:87`
runs the host `git` for `ls-files` instead of the bundled binary through
`tests/unit/helpers/git.cjs`; a machine without Git on PATH fails the
packaged suite for a reason unrelated to packaging. Style: the
bundled-Git test's doc block sits above the native-module block, apart
from its test. Both left for a follow-up. CI on `64d9bd0`: all seven
checks green on both platforms.
- **Review:** completed — CodeRabbit, on `c7f430d` only. Its incremental
review of `010ec0e` did not run: "Review rate limited", and it replies
that it does not re-review already-reviewed commits. So the last commit
carries the self-review and the CI evidence, not a bot pass. Full detail
on `c7f430d`: Its first two runs did not happen: the check read `pass`
with the message "Review rate limited". The third ran. One Major
finding, fixed in `010ec0e` and answered inline: `src/**/*` is recursive
and nothing asserted the nested contents. It proposed enumerating the
runtime files by hand; the fix derives the expected set from Git
instead, which closes the same gap without a list that has to be
maintained. Verified by dropping a file into `src/`, packaging, and
watching the test name it.

Fixed here:

1. *The drift guard was one-directional*
(`tests/e2e/packaged/smoke.spec.js`). Removing a pattern from
`build.files` failed nothing. Added a test that derives the allowed set
from `package.json` and asserts it equals the literal list in the spec.
Verified by deleting `local-playground-web/**/*` from package.json: the
test fails naming that entry.
2. *The one negative pattern was unverified* (same file). Both payload
assertions read only tree roots, so `!src/renderer/index.jsx` could stop
matching silently. The asar test now also lists `src/renderer` and
asserts `index.jsx` is absent while `index.html`, `index.js` and
`index.css` are present — which doubles as a check that the renderer
bundle was built before packaging.

Deferred, with reason:

3. *The Linux window icon* (`src/main.js:490`) resolves to
`<app.asar>/build/icon.png`, which has never shipped: electron-builder
excludes `directories.buildResources` unconditionally, so the old
exclusion list did not carry it either. Pre-existing and Linux-only, and
Linux formats are explicitly outside the v1.1.0 validation scope (#388).
Recorded in Risks rather than filed as an issue.

Style notes from the same pass, all applied: the stale `vendor` sentence
in `.gitignore:33` now points at the allow-list; the two failure
messages named the whole listing instead of the offenders; the
local-equivalence claim in `TESTING.md` was unconditional and is now
hedged (`.codesigning` exists only on a signing machine, so the full
proof is the CI run). The `dist/intermediate` seed in the workflow was
noted as redundant — electron-builder already excludes its own output
directory — and kept deliberately: it is the seed that documents *why* a
top-level `dist/` is safe, and it guards a future re-introduction of a
permissive pattern.

</details>

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

- electron-builder 26.15.3,
`node_modules/app-builder-lib/out/fileMatcher.js`: `getMainFileMatchers`
adds `package.json` when the list is positive, prepends
`!**/node_modules/**`, `!<buildResources>{,/**/*}` and
`!<output>{,/**/*}`, and appends the default name and extension
excludes. `getNodeModuleFileMatcher` takes only the `!` patterns from
`files`, which is why the dugite trims keep working.
- Baseline asar root on trunk `4110349` (macOS, local, seeded):
`node_modules`, `tests`, `src`, `docs`, `scripts`, `.buildkite`,
`.claude`, `.agents`, `test-results`, `fastlane`, `.bundle`,
`TESTING.md`, `STATS.md`, `README.md`, `playwright.config.js`,
`package.json`, `LICENSE`, `Gemfile.lock`, `Gemfile`,
`eslint.config.mjs`, `CONTRIBUTING.md`, `CLAUDE.md`, `CLAUDE.local.md`,
`AGENTS.override.md`, `AGENTS.md`, `.ruby-version`, `.nvmrc`,
`.coderabbit.yaml`. Absent: top-level `dist`, `.codesigning`, `build`,
`vendor`.
- With the allow-list: `node_modules` (28538 entries, unchanged), `src`
(81), `package.json`. `app.asar.unpacked` holds only `node_modules`
(`@php-wasm`, `dugite`, `fs-ext-extra-prebuilt`).
- Local-versus-CI tree comparison: recorded in a comment once the CI
artifact for this head exists.

</details>

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

Nothing on screen changed: this PR touches packaging and a test.

</details>

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

https://claude.ai/code/session_01XJUaihwanFHxEps959WLhs

---------

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