[Fix] Stop a Gutenberg build from spawning processes without bound (#275) by juanmaguitar · Pull Request #283 · WordPress/contributor-toolkit

juanmaguitar · 2026-08-11T15:14:55Z

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.

Start the build from the wizard's Run build step.
While it runs, watch the process count: ps -A | grep -c concurrently on macOS/Linux.
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.

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:

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.
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.
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.
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:

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.
Instrumenting the task runner's spawn showed the original process launching three children for two commands: its own path, then the two real ones.
That pointed at argument parsing rather than at process management, and from there to hideBin's Electron branch.
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.

## Why Nothing in the app can stop a running `npm install`. `npm:kill` resolves a child out of the script registry only — by `runId`, or by the directory index `npm:run-script` maintains — and a running install lives in a different map, `runningInstalls`, keyed by an `installId` the preload never hands back. So Stop answers `{ok: false, error: 'No running script'}` and the install runs to completion. Nobody has hit it because nothing had asked to stop one: install is reached from the checklist button, and the terminal's Ctrl+C is only wired up during chains that do not include it. #246 changes that — it starts install unattended after the clone, and being able to stop it is the condition that makes running unattended reasonable at all. ## What changes `installIdByDirectory`, beside the existing `runIdByDirectory`, and a fallback in `npm:kill` that consults it once the two script lookups miss. A directory is all a caller can offer for an install, and the two never run for the same directory at once. Everything downstream is already generic: `cancelledChildren`, `killChildTree`, the 3-second `SIGKILL` backstop. No `src/preload.js` change — `npmKill` already forwards `{ runId, directoryPath }`, and during an install `currentRunIdRef.current` is null, so the directory branch is the one that runs. Marking the child cancelled matters more for an install than for a script. A stopped install exits non-zero, and on Windows a kill surfaces as a plain exit code rather than a signal, so without it `runNpmWithEngineRetry` would read the stop as an engine mismatch and respawn the install the contributor just stopped. `test/npm-runner.test.cjs` already pins that `cancelled` short-circuits the retry; this makes the install path reach it. **Not in this PR:** any caller. The Stop control that uses it ships in the stacked PR for #246. On its own this is reachable only through the terminal's Ctrl+C during a chain that runs an install. ## How to test this Platforms: **macOS and Windows both** — process-tree killing is the one thing that differs, and this is a kill path. This has no button of its own until the stacked PR lands, so it is driven from the terminal. **Starting state:** a site whose clone has finished and whose dependencies are **not** installed. 1. Click **Install npm dependencies**. Wait for npm to start producing output in the Terminal. 2. Press **Ctrl+C** in the Terminal. 3. The install stops. Check no `node`/`npm` child of the app survives — Activity Monitor on macOS, Task Manager on Windows. 4. The **Install npm dependencies** step now offers **Retry npm install**, and the build step stays locked. 5. Click the retry. It starts a fresh install rather than resuming a dead one. Before this change, step 2 does nothing at all and the install runs to completion. **What must not have happened:** - No orphaned `npm`/`node` process left behind after the stop. That is the failure mode `killChildTree` exists for (#83, #146), and an install is a tree too. - The stop must **not** be retried as an engine mismatch — watch for a second `npm install` starting itself a moment after you stopped the first. On Windows this is the likely shape of a regression, since the kill there produces a plain non-zero code with no signal. - The site must **not** read as having completed the install step. A cancelled install leaves a partial `node_modules`, and `installFailed` is recorded so it does not read as done (#42). Covered by `npm:kill ends a running install, which only its directory can name` in `test/ipc-wiring.test.cjs`, which fails on the old code (the handler returns `{ok: false}` and never reaches kill-tree). ## Risks and limitations Small and additive: one map, one fallback branch, no change to any existing lookup. The blast radius is a `npm:kill` call that used to fail and now succeeds. The lifetime of `installIdByDirectory` mirrors `runIdByDirectory` exactly, including the identity-guarded delete, so a second install for the same directory cannot be made unkillable by the first one's exit. Not tested by hand on Windows by me — the artifact from this branch is the way to do that. ## Related Groundwork for #246. Does not close anything on its own. --- <details> <summary>Design decisions and alternatives considered</summary> **A separate `npm:install-kill` channel** was the obvious alternative. Rejected: the renderer already calls `npmKill({runId, directoryPath})` from one place (`killCurrent`), and it does not know whether the thing it is stopping is an install or a script — that is precisely the knowledge the main process has. Two channels would have pushed that question back into the renderer for no gain. **Returning the `installId` to the renderer** so an install could be killed by id like a script. Rejected: the preload deliberately keeps `installId` private, correlating log and done events internally so callers get a plain `(onLog, onDone)` pair. Widening that to hand out the id would change the shape of the API for every caller to serve one. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 1 [fix here] · 0 [follow-up] — fixed. **tests 🟡 [fix here]** — the new test passed under Electron's Node but failed under `npm test` on `.nvmrc`'s Node: emitting `close` reaches `npm:install`'s `onDone`, which calls `getStore()`, which pulled in the real `electron-store` and through it the real `electron` package, tripping the harness's own guard. Fixed by adding `fakeSettingsStore()` to the stubs, the same way the quit-sweep test does for the same reason. This is exactly the "green on one of the two runtimes" shape the review standard names; both runtimes are green now (840 tests each). The judgement pass was run in this session rather than dispatched to a subagent — the tooling in use blocked spawning one. Flagging it because the standard prefers fresh context for that pass, and a self-review is the weaker version. </details> <details> <summary>Screenshots or recording</summary> Nothing on screen changes. The behaviour is in the main process; the control that exposes it ships in the stacked PR. </details> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

The pill was decided by GitHub's `updated_at`, which a comment, a label change or an upstream force-push bumps just as hard as a push. On Trac #62064 a force-push of trunk restamped ~2,580 open pull requests inside one window; the two on that ticket landed 19 seconds apart in the sweep, and the app crowned the November 2024 patch that no longer applies over the April 2026 one that does. Pull requests are now dated by their newest commit. The extra requests are kept small by a bound — `updated_at` is never earlier than the last commit, so on the list already sorted by it, it bounds every row below — plus a cap of four requests and reuse of cached dates. When the walk cannot finish, or the top two are within an hour, there is no pill: an undated row competes as an upper bound on the runner-up rather than as an answer. Verified against live GitHub on #62064: the pill moves to #11517 (last commit 10/04/2026), which applies cleanly, away from #7871 (22/11/2024), which fails with "patch does not apply". One search plus two lookups; a Refresh spends none. Closes #281 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

## What & why In **Review & submit changes**, clicking **Discard all changes** appeared to do nothing on a ticket-linked site (issue #270). The modal shows the working tree diffed against the ticket branch's **base** (its point off trunk), so "your changes" includes the **parked WIP commit** — but `discardChanges` only reset the tree to the branch **HEAD**, keeping that commit. When the shown changes were the parked commit, discard removed nothing the modal measures, so it reloaded identically. ## Approach - **`discardToBase(dir, baseOid)`** (`src/trunk-update.js`) — rewinds the branch ref to the same base the diff is taken against (`git.writeRef` → `git.checkout({ force })`) before materialising the tree, so the *whole* diff — parked WIP included — is discarded. The branch survives and the ticket stays linked; only its work is rewound. Deleting the branch remains what "Delete this ticket's work" is for. - **`git:discard-to-base`** IPC (`src/main.js`) picks the baseline via `patchBaseOid` (on trunk there's no base past HEAD, so it falls back to the uncommitted-only `discardChanges`). `discardChanges` itself is **untouched**, so the trunk-update dirty flow and switch-off-trunk flow still preserve parked work. - The submit modal's discard points at the new channel; the update-flow discard stays HEAD-relative. - Header now names the ticket: **"Your changes for ticket #N"**. - A completed discard is confirmed through the accessible toast (#253) from both the modal and the update flow. ## Decision "Discard all changes" now discards *everything the modal shows* while keeping the ticket linked (empties it to base) — chosen over deleting the branch outright. ## Tests - `discardToBase` integration test (parked WIP + edits + untracked → tree back at base, HEAD at base, still on the ticket branch). - `git:discard-to-base` IPC wiring test + coverage-guard entry. - Full suite: 813 passing; lint clean; renderer bundles. Closes #270 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

> **Stacked on #274.** Review that one first — it is the kill path this PR's Stop button depends on. The diff below is this branch against #274, not against trunk. ## Why After the clone finishes, the setup checklist stops and waits. **Install npm dependencies**, then **Run full build** — two clicks, with no decision to make between them. The contributor's only job is to notice a step ended and press the next button. That is the wrong shape for a wizard. Someone who walks away during the clone — which is what you do during a ten-minute clone at a Contributor Day — comes back to a checklist waiting on them rather than to an environment they can work in. #246 makes the case at length. ## What changes Setup becomes the third chain of the shape the app already runs twice: `planUpdateSteps` (fetch → install → build) for a trunk update, `planApplySteps` (apply → install → build) for a patch, both sharing `updateStepStatuses` and `updateOutcome`. `planSetupSteps` joins them, and **the chain starts on its own when the clone finishes** — no button. What makes running unattended reasonable rather than presumptuous, and each of these is the reason the issue gives: - **It is visible.** A banner names the running step and counts it (`step 2 of 3`), and every line of npm output still goes to the Terminal (#41). - **It stops.** **Stop setup**, and Ctrl+C in the Terminal, end the child — install included, which is what #274 is for. - **It stops at the first failure.** A build on a half-installed tree cannot work, and its failure would bury the one that mattered (#42). - **It starts once, on an edge.** The clone going from running to finished, once per row. A site already cloned when its row appears never triggers, so reopening the app on a half-finished site lands on the manual checklist rather than launching a half-hour build nobody asked for. **Deliberately not in it:** starting the dev server. #246 lists it as the fourth link, and I left it out — that step also calls `markSkipWizard()` and hands the contributor to a WordPress setup wizard in a browser, so running it unattended would end the checklist on their behalf and leave a server listening that nobody asked for. Also not in it: resuming the chain after a stop. Retry is a plain button on the failed step; a cancel followed by a retry must not silently restart a half-hour build. **It also closes the open half of #44.** The checklist ladder moves out of `index.jsx` into `setupStepStatuses` and gains a `failed` state, so a step whose last attempt lost says **Failed**, keeps its retry, and does not hand `current` to the next step. #258 fixed the label half ("Ready" until an action runs); the explicit failed/retryable state was the acceptance criterion still open, and a chain that can stop is what made it necessary rather than nice. ## How to test this Platforms: **any** for 1 and 4–6; do **2 and 3 on both macOS and Windows** — process-tree killing is where they differ. **Starting state:** the app, with no site for the WordPress repo you are about to create. This is a real clone plus a real install plus a real build, so budget for it — the point of steps 2 and 3 is that you do not have to sit through the build. 1. **Create a WordPress Core site.** Watch the *Download WordPress development version* step. It should say the install and build start on their own when it finishes. 2. **When the clone finishes, click nothing.** The blue banner appears, *Install npm dependencies* turns **In progress**, and npm output streams into the Terminal. When install ends, the build follows on its own and the banner reads *step 3 of 3*. - *Start dev server & finish wizard* must still read **Ready** and must **not** have started. 3. **Press Stop setup** while npm install is running. The install actually dies — check no `node`/`npm` child of the app survives. The build never starts, the banner is replaced by *Setup stopped*, and the install step offers **Retry npm install**. 4. **Press Stop setup during the build** instead (create a second site, let install finish). The whole Grunt tree dies, not just the runner (#83, #146). 5. **A failing install.** Easiest is to pull the network mid-install. The step reads **Failed** in red with its retry live, the build stays **Locked**, and the chain does not advance. 6. **Reopen mid-setup.** Quit during the build, relaunch, open the site. The checklist shows the real state from disk and the chain does **not** restart by itself. 7. **Skip initialization wizard** during a running chain. The checklist goes away, the chain keeps running, the Terminal still shows it. **What must not have happened:** - **The dev server must not have started, and the wizard must not be marked skipped.** The chain ends at the build. If you land on a running server or a post-init view without clicking, that is the regression. - **No orphaned `npm`/`node` process after a Stop**, on either platform. - **A stop must not be reported as a failure.** A killed npm exits non-zero — on Windows without even a signal — so the step must read *Setup stopped*, not *npm install failed*. - **A stop must not respawn itself.** Watch for a second install starting a moment after you stopped the first (that is #274's `cancelled` flag doing its job). - **Reopening the app must not start anything.** Step 6 is the one that would be easy not to notice: it only misbehaves on a site you left half-finished, and the symptom is your laptop quietly building for half an hour. ## Risks and limitations **The end-to-end chain has not been driven by hand.** I could not: this Electron build does not expose its accessibility tree to the automation available here, and creating a site needs a native folder picker. What I did verify is that the renderer bundles and mounts with these changes, lint is clean, and both Node runtimes are green (840 tests). **Steps 1–7 above are unverified and want a human**, and step 3 in particular is the one that justifies the whole feature. `buildFailed` is session-local. Only the install outcome is persisted (`src/main.js` records it on the site's meta), so after a restart a failed build reads **Ready** again rather than **Failed**. That is the honest fallback — the app still knows there is no build on disk, just not that the last attempt lost — and persisting it is a separate change. Auto-start is once per row **mount**, not once per site. Switching away from a site and back during a clone remounts the row; if the clone finishes after that remount the chain still fires, which is the wanted behaviour, but it does mean the guard is memory, not state on disk. The `hasNodeModules` check is what stops that mattering. The banner says the build can take up to half an hour on Windows (#72). It runs a production build where a dev build would do — that is #92, untouched here, and auto-starting makes it more worth fixing, not less. ## Related Closes #246. Closes the remaining acceptance criterion of #44. Depends on #274. Makes #57 (the redundant nested `npm install` between the two steps) easier to fix now the two are one run. --- <details> <summary>Design decisions and alternatives considered</summary> **The checklist rows are not driven by `updateStepStatuses`.** #246 suggests they could be — "it already returns the complete/current/pending/skipped states the checklist needs". They cannot, and this is the one place I departed from the issue. Those states are derived from disk (`hasNodeModules`, `hasBuilt`, `installFailed`) precisely so a site reopened days later, with nothing running, still shows the truth; driving them from chain position would show every step `pending` on a reopened half-finished site. So the reuse is real but narrower: `planSetupSteps` + `updateStepStatuses` + `setupOutcome` drive the **banner** — progress counter, Stop, how it ended — exactly as they drive the update panel, and the rows keep their own ladder. **`planSetupSteps` lives in `update-plan.cjs`** rather than in `setup-steps.cjs`, following the `planApplySteps` precedent: the module is where the chains and the machinery they share live, and splitting the third one off would have meant importing `updateStepStatuses` across modules to save a rename. Its docblock now says it owns three chains. **`setupOutcome` separates `stopped` from `failed-install`/`failed-build`** even though the exit codes are identical. Telling a contributor their install "failed" when they pressed Stop is how a tool loses their trust, and the exit code genuinely cannot tell the two apart — so `stopped` comes from the fact that we asked for the kill, not from what the process did. **Auto-start is a tri-state decision, not a chain of `if`s in an effect.** `setupAutoStartDecision` returns `skip` / `probe` / `start`, because the decision is taken in two halves: is this the clone-finished edge (so reading status off disk is worth it), and then does that status say this is a fresh clone. Being able to test that is worth more than the shape being slightly unusual — it is the riskiest behaviour in the PR. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 2 [fix here] · 0 [follow-up] — both fixed. **architecture 🟡 [fix here]** — the auto-start decision lived inline in a `useEffect` in `index.jsx`, which the suite cannot reach. Per §1, the finding is the missing module, not the missing test — and this was the riskiest decision in the change (too eager and it launches a half-hour build unasked, too shy and the wizard never finishes itself). Extracted to `setupAutoStartDecision` with nine tests covering the edge, the arming, the two refusals and a failed status probe. **architecture 🟡 [fix here]** — four branchy user-facing strings (install and build labels and descriptions) were derived inline in `index.jsx`. §1 names "a string the user reads" explicitly. Extracted to `setupStepCopy`, which takes the same flags as `computeSetupStepState` so the words and the button state cannot disagree; four tests, including one pinning that pairing for the #42 case. The judgement pass was run in this session rather than dispatched to a subagent — the tooling in use blocked spawning one. Flagging it because the standard prefers fresh context for that pass, and self-review is the weaker version. Worth a second pair of eyes on the effect in `index.jsx` in particular. </details> <details> <summary>Implementation notes</summary> The chain reuses the wizard's own `runInstall` / `runScript` wrappers, the same way `runUpdateInstallAndBuild` does, so exit codes, retries, terminal streaming and the post-run status reload all behave identically to every other path. Nothing new crosses IPC. `runScript` gained one line: it clears `buildFailed` when a build starts and records it on exit. Clearing on start rather than on the next exit is what stops a step reading **Failed** while its own retry is streaming output — there is a test for that shape on the install side. `deriveNextAction` is unchanged. The call site now passes a failed step as the current one, since retrying it is what the contributor should do next and a `failed` row consumes no `current` — without that, a stopped chain would leave the view with no cue at all. </details> <details> <summary>Screenshots or recording</summary> None — see the limitation above: I could not drive the app's UI from this environment, so there is no shot of the banner or the failed row. This is the part of the PR that most wants a human with the Buildkite artifact. </details> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

… not apply (#291) ## Why When a patch no longer applies, the app names one file, says the checkout was not changed, and stops. That sentence reads identically whether one region of twenty missed or all twenty did — opposite decisions for the contributor, who either spends ten minutes rescuing the patch or throws it away. The failures beyond the first file never reach the card at all, and the most useful thing on screen at that moment — the ticket's other pull request, which applies cleanly — is never pointed at. #282 makes the full case, from applying a pull request on Trac #62064. ## What changes The refusal itself is untouched: all-or-nothing stays, and nothing here writes to the checkout. What changes is what the refusal says. `resolveFile` now asks its existing question once per region instead of once per file, only on the failure path. Each failing region reports **why** — its surroundings changed, or its change looks already present, told apart by whether the region's reverse fits (the same reasoning `patchIsAbsent` already uses; this is #226's first half) — and carries the `-`/`+` lines it wanted to make plus a **searchable anchor line verified to exist in the contributor's file** (the hunk's own line numbers are coordinates in the file as the patch author had it, and on an old patch they miss by the very drift the patch failed on). A new pure module, `src/renderer/apply-conflict.cjs`, turns the failure payload into the notice (the `open-failure.cjs` shape), with **two framings chosen by where the patch came from**: - **A pull request** has an author, and its conflicts are theirs to fix — showing the contributor line-level detail invites them into work that is not theirs. The notice names the stale side (*"written against an older trunk"*, not "your checkout has moved on"), sizes the problem (places and files), says whose work the fix is, and frames the contributor's one real act: leaving a comment asking for a rebase. Buttons: **Ask its author for a rebase** (opens the PR) and **Try another patch on this ticket**. - **A loose patch** (Trac attachment, file from disk) has nobody to send the contributor to, so it keeps the full per-region breakdown — there, the contributor with the failing lines in hand is the only way out. Every failing file reaches the card in both framings, rather than only the first. Deliberately not in this PR: loosening the matching or applying the parts that fit (ruled out in #226 and #282), applying from the pull request's recorded base (three-way — #226's second half, with verified groundwork in its comments), and naming which side is stale (needs the PR-date work shared with #281). ## How to test this Platforms: any. **Starting state:** 1. A Core site on current trunk, linked to Trac ticket **62064**. 2. In *Apply a patch or PR*, paste `7871` and preview it, then **Apply and rebuild**. **Expected:** the apply refuses; the banner says the pull request was written against an older trunk, sizes it ("7 of its 24 changes, in 1 file, would need rework"), says updating it is its author's work and suggests a comment on the PR, and offers **Ask its author for a rebase** (the PR shows GitHub's own "conflicts that must be resolved" banner, corroborating) and **Try another patch on this ticket** (which dismisses the failed attempt entirely — banner and preview — and scrolls to the list, whose Apply buttons are enabled again). Applying PR **11517** from the same list then succeeds. For the detailed framing, download a failing patch as a `.diff` and apply it via "choose a .diff / .patch file": the same failure now shows the per-region breakdown — `near …` anchor lines you can Cmd+F in the checkout and find, "the code around it has changed" labels, and the `-`/`+` lines for the first three regions. **What must not have happened:** - No file in the checkout changed on the refusal (`git status` clean, or exactly as it was before). - A patch that fails for a non-conflict reason (a `.diff` whose file is not in the checkout) still shows its plain sentence — no empty breakdown, no headline claiming counts. - Reverting an applied patch that conflicts shows the plain message, not the breakdown with buttons that make no sense for a revert. The old behaviour — one file named, the rest of the failures only in the terminal — is pinned by the new tests: `test/apply-conflict.test.cjs` (module) and the `#282`/`#226` blocks in `test/patch-apply.integration.test.cjs` (per-region diagnosis on a real repo, anchors asserted to exist in the file). ## Risks and limitations - The per-region labels are **evidence, not proof**: matching searches for a fit and can find one in the wrong place, which is why the wording hedges ("looks like") and why the diagnosis never decides a write. Stated in the module comment. - The wording throughout is a draft for review — headline, region labels, button copy. - Verified by hand on macOS against the real case (PR 7871 / Trac 62064); Windows through the suite only. - Self-review outcome: 4 [fix here] · 1 [follow-up] — all five addressed before opening, details in the collapsed block. ## Related Closes #282. First half of #226 (its second half — apply from the recorded base — stays open there, with the measured groundwork in its comments). Related: #286 (next-step guidance from a different angle), #290 (trying the PR as written), #281 (which side is stale). --- <details> <summary>Design decisions and alternatives considered</summary> **Anchors instead of line numbers.** The hunk's `oldStart` is in the patch-base file's coordinates: on the motivating case it says "line 132" for code sitting at line 149 of the contributor's file, and the drift grows toward the end of the file. Candidates from the hunk are checked against the file and the longest one actually present wins (first line of a hunk is often a bare brace, and "near `}`" locates nothing); preference order follows the failure kind — a moved region usually still contains its own `-` line, an already-applied one its `+` result. Falling back to "line N of the patch" only when nothing from the hunk survives. **Diagnosis stays in `patch-apply.js`, presentation in a `.cjs` module.** Same split as `open-failure.cjs`, for the same reason: `index.jsx` cannot be loaded by the suite, so every branch that decides what the contributor reads lives where a test reaches it. **No three-way in this PR.** The apply is two-way by construction (no base is known). Three-way with the PR's recorded base collapses most of these false failures — measured on this very case: 41 of 49 regions merge cleanly, and the `diff3` algorithm reproduces git's six conflicts exactly — but it is a different change with its own surface (GitHub API, new dependency), tracked in #226 with the measurements posted there. **Reverse skips the diagnosis.** A failed revert's answer is never "try the ticket's other patches"; the panel discards the breakdown, so the resolver does not compute it. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Run per `.github/instructions/code-review.instructions.md`, judgement pass in a fresh-context subagent. **4 [fix here] · 1 [follow-up] — all five addressed, plus the three style notes.** - 🟡 "Try another patch" scrolled to a list whose Apply buttons were disabled by the still-open preview — the exact dead end the module's own comment forbids. → the button clears the preview first. - 🟡 `otherTicketPatchCount` was a derivation inside `index.jsx`, unreachable by the suite. → moved to `apply-conflict.cjs` as `otherPatchCount`, with tests including the picked-from-disk label case. - 🟡 A patch entirely already-in-tree got the headline "None of this patch's changes still fit" above regions saying the opposite. → dedicated headline when every failing region is `already-applied`. - 🔵 One path still set `applyError` without clearing the breakdown, leaving a stale breakdown hiding a parse error. → routed through `clearApplyError()`. - 🔵 [follow-up, done here] `diagnoseHunks` exported with no direct test — the caps and fallbacks were unasserted. → direct tests for `REGION_LINE_LIMIT`, `REGION_DETAIL_LIMIT`, the anchor's last-resort branch, and the `null` on overlapping hunks. - Style notes applied: reverse no longer pays for a discarded diagnosis; region React keys use the hunk index (`oldStart` can collide in concatenated patches, which also double-counted via the by-sentence map — replaced with consume-on-match); anchor picks the longest present candidate. Clean and verified by the reviewer, not reported as findings: EOL/CRLF handling end-to-end, IPC surface (nothing added to the bridge; the payload rides the existing done channel), performance (worst plausible diagnosis measured at 38 ms, failure path only). </details> <details> <summary>Implementation notes</summary> - The apply result now carries `failures` (all sentences — previously computed and forwarded but read only as `error`) and `conflicts` (`{ path, error, total, regions[] }`). `main.js` and `preload.js` needed no changes: the handler already spreads the whole result into the done payload. - `diagnoseHunks` applies each hunk in isolation against the unshifted file — the only question askable when nothing is applied. Two hunks that overlap once applied can each pass alone while the file fails; the resolver returns `null` for that and the original sentence stands, rather than claiming zero conflicts. - Payload is bounded: 3 regions per file carry lines, 10 lines per region, anchors capped at 120 chars; the rest are counted and located. - End-to-end verification script (throwaway, scratchpad only) drove the packaged renderer against a fixture checkout with the real PR 7871 diff and asserted every anchor exists in the current `forms.css`. </details> <details> <summary>Screenshots or recording</summary> Banner on the real case (PR 7871 against current trunk, Trac 62064) — will attach the capture in a comment: ``` This pull request was written against an older trunk and no longer fits it: 7 of its 24 changes, in 1 file, would need rework. The checkout was not changed. src/wp-admin/css/forms.css — 7 of 24 changes Bringing it up to date is its author's work — a rebase, or merging trunk in. Leaving a comment on the pull request to let them know is a real contribution in itself. [Try another patch on this ticket] [Ask its author for a rebase] ``` The same failure from a `.diff` file shows the detailed framing instead: ``` 7 of this patch's 24 changes no longer fit — the other 17 do. The checkout was not changed. src/wp-admin/css/forms.css — 7 of 24 changes near line-height: 1.42857143; /* 20px */ · the code around it has changed - line-height: 1.42857143; /* 20px */ + line-height: 1.42857143; + /* 20px */ … (7 regions, all anchors present in the contributor's file) ``` </details>

…pens (#293) ## Why A linked ticket shows its number and the work attached to it, but nothing about itself. A ticket closed `wontfix` years ago reads the same as one filed last week — and the contributor finds out on Trac, after the time is spent. #292 makes the case; the gap surfaced while testing #291, where a ticket whose only pull request is closed had its explanation (the ticket's own status) one unread page away. ## What changes The ticket's own facts — summary, status, resolution, type, milestone, component, keywords, opened date — ride the scrape the attachments already do: same embedded Trac window, same one-time human-check, one extra read of the `#ticket` block (with the description cut before it crosses out of the page). No new mechanism, no second challenge, and `main.js`/`preload.js` are untouched — the handler already spreads the scrape result. A new pure module, `src/trac-ticket-info.cjs`, does the parsing — regex over the HTML like `trac-attachments.cjs`, proven against fixtures taken from **archived copies of real ticket pages** (one open with milestone/component/keywords, one closed `wontfix`). It also owns the one display decision: the status pill folds the resolution in, because "closed (fixed)" and "closed (wontfix)" are opposite instructions to a contributor. The card shows the summary under the ticket number, the status pill, type, age ("opened 18 months ago", absolute date in the tooltip), milestone, and the component and keywords as links — **Trac's own query URLs lifted from the page**, not rebuilt, opened in the browser like any other link. A "Read details from Trac" link sits next to "Open in Trac" and triggers the same scrape the attachments button uses. The details are read **automatically at the moment a ticket is linked** — that is when the facts matter most (a `wontfix` ticket should say so before work starts), and it is the one moment a human-check window has context, since the contributor just acted on this ticket. On mount or site re-activation nothing opens: there the #109 on-demand rule stands, and the "Read details from Trac" link is the way in. Deliberately not in this PR: any writing to Trac, and any freshness machinery — the facts can go stale exactly like the attachment list, and the same re-read answers both. ## How to test this Platforms: any. **Starting state:** 1. A Core site with no ticket linked. 2. Link a ticket with real properties — **62881** (the case from #292) or any `good-first-bug`. The details load on their own; pass the human-check if it appears. (On a site whose ticket was linked before this build, use **Read details from Trac** next to "Open in Trac" — nothing auto-opens on merely selecting a site.) **Expected:** under the ticket number: the ticket's title; a status pill (red for closed, with the resolution folded in; green otherwise); the type; "opened … ago" with the absolute date on hover; the milestone when there is one; component and keywords as links that open Trac queries in the browser. **What must not have happened:** - No second Trac window / second challenge beyond the one visit. - A ticket page that fails to parse shows nothing — no empty shell of "component:" labels; attachments still work. - Nothing was written anywhere; this is read-only. Tests: `test/trac-ticket-info.test.cjs`, table-driven over the two real-markup fixtures. ## Risks and limitations - The parse is regex over markup that Trac could change — same exposure as the attachments parse, contained in one module with fixtures, failing soft (no info shown, everything else intact). - Wording and layout are a draft for review. - Verified against archived real pages and the suite; not yet driven by hand against live Trac (needs a human to pass the check). ## Related Closes #292. Stacked on #291 (merge that first; this PR's base is its branch). Related: #109 (the embedded read this extends), #286. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> Deterministic layer: `npm test` 933/933, `npm run lint` clean, renderer bundle builds. The judgement pass for the stack will run once on the combined branch before merge — flagged here so it is a decision, not an omission. </details>

…#295) ## Why The docs' install step handed visitors a link to the Releases page and a bulleted list explaining which of four similarly-named files was theirs — `.dmg` "whose name contains `arm64`", `.exe`, `.AppImage`. Every newcomer had to read release-asset naming before they could download anything, and at a Contributor Day that is the first thing standing between someone and the app. Suggested by Adam Zieliński: replace all of it with a Download button that just starts downloading the right file. No issue — it came from Slack. ## What changes A `<DownloadButton />` component in a new VitePress custom theme, used in the guide's **Install the app** section (replacing the per-OS list) and as the landing page's primary hero action. Every affordance labelled "Download" is now the smart one. That took three edits beyond the component: the hero's static Download action is gone and the button is injected in its place via the `home-hero-actions-after` slot (hero actions come from frontmatter and cannot host a component); "Get started" drops to the secondary style so the page has one primary action; and the nav item is renamed **Releases**, which is where it goes and what it is for. Two links called "Download" that behave differently is worse than none. The asset URL cannot be hardcoded. `artifactName` in `package.json` bakes the version into every filename, so there is no stable "latest .dmg" URL — the button resolves the asset at runtime from the GitHub releases API and matches on the OS/extension portion of the name. Two decisions worth surfacing: - **One request per visitor, not per page view.** The unauthenticated API allows 60 requests/hour *per IP*, and a Contributor Day room shares one address. A single response resolves all three platforms into `sessionStorage`, so a room of 30 people costs 30 requests instead of exhausting the budget and silently degrading for everyone who arrives after. - **The user-agent decision lives in `match-platform.mjs`, not in the component.** The docs package has no test harness, so a branch inside the `.vue` file is unreachable by anything — same reasoning as the renderer-module invariant in AGENTS.md §1. The module is imported by the root `node --test` suite. Deliberately not in this PR: nothing about how releases are built or named. The button reads the existing assets; `artifactName` is untouched. ## How to test this **Platforms:** any — it is a docs-site change, driven entirely in a browser. Nothing in the Electron app is touched. **Starting state:** the branch checked out, docs dev server running. 1. `cd docs && npm install && npm run dev`, then open `http://localhost:5173/contributor-toolkit/`. The hero shows a **Download for macOS (Apple Silicon) v0.1.2** button as its primary action (label matches whatever machine you are on), with "Get started" above it in the secondary style. 2. Hover or copy the button's link. It points at `.../releases/download/v0.1.2/wordpress-contributor-toolkit-0.1.2-mac-arm64.dmg` — the asset itself, not the Releases page. 3. Click it. The download starts directly; no intermediate page. Then click the nav's **Releases** and confirm that one still goes to the releases list — it is the route to `.deb`/`.snap` and older versions. 4. Open `/guide/getting-started`. The **Install the app** section shows the same button in place of the old numbered list, with "Intel Macs are not currently supported." beneath it on macOS. 5. DevTools → three-dot menu → **Network conditions** → uncheck "Use browser default" and paste a Windows user agent, then reload. The button reads **Download for Windows** and links to the `.exe`. Repeat with a Linux UA for the `.AppImage`. 6. Same again with an iPhone UA (`Mozilla/5.0 (iPhone; CPU iPhone OS 17_5 like Mac OS X) ...`). The button reads **Download from the Releases page** and links there — an iPhone must not be offered a `.dmg`. 7. DevTools → Network → **Block request URL** on `api.github.com`, then reload with any UA. The button falls back to the Releases-page link. Same with the network offline. 8. With DevTools' Network tab open and `sessionStorage` empty, load the landing page and then navigate to the guide. Exactly **one** request to `api.github.com` across both, and the second page's button still carries the direct asset URL. 9. Narrow the window below 960px and reload. The button stays aligned with the hero text and the "Get started" action above it, rather than centring on its own. **What must not have happened:** - No link labelled "Download" may point at the Releases page. That was the state this PR first landed in — the button sat below the feature cards while the hero action and nav item, both called "Download", still sent people to the file list — and it is the thing to re-check if the hero is ever touched. The only remaining Releases links are the nav's **Releases** and the "All platforms and previous versions" line under the button, both of which say so. - The button must never be a dead link. Every failure path — undetectable platform, API down, rate limited, JavaScript disabled — has to land on the Releases page, which is what the docs did before. A button that silently does nothing is worse than the list it replaced. - The Releases-page link under the button must survive on every platform: it is the only remaining route to `.deb`/`.snap` and to older versions, and the old Linux bullet was the only place `.deb`/`.snap` were mentioned. - Reloading must not re-fetch. If step 8 shows two API calls, the session cache is not working and the room-shares-one-IP problem is back. - `npm run docs:build` must still pass — the component reads `navigator` and calls `fetch`, and doing either outside `onMounted` breaks SSR at build time rather than in the browser. ## Risks and limitations Review outcome: **3 [fix here] · 3 [follow-up] — all 6 fixed.** Details in the collapsed block below. - **iPadOS in desktop mode is indistinguishable from a Mac** and will still be offered a `.dmg`. Safari on iPad reports a `Macintosh` user agent by design; there is no reliable signal short of touch-point heuristics, which misfire on touchscreen laptops. An iPhone is handled correctly. - **The API dependency is real.** GitHub rate-limiting or an outage degrades the button to the old behaviour rather than breaking it, but the smart path does depend on a third-party call from the visitor's browser. The alternative — generating a static URL at release time — is noted below. - **Asset-name coupling.** The button matches release filenames. It is now loose on the arch token so an `x64` → `x86_64` rename cannot break it, but a change to the OS or extension portion of `artifactName` would silently drop that platform to the fallback with no error anywhere. `test/download-button-platform.test.cjs` pins the current names, so that test is the tripwire. - **Not tested by hand:** an actual rate-limit 403 from GitHub. Blocking `api.github.com` in DevTools exercises the same fallback branch (`response.ok` is false either way), but I could not stage 60 real requests to see the genuine response. ## Related Suggested by Adam Zieliński in Slack. No issue. --- <details> <summary>Design decisions and alternatives considered</summary> **Generating a static redirect at release time instead of calling the API.** The obvious alternative: have the release workflow write a small JSON file (or a set of stable redirect URLs) into the docs site, so the page needs no third-party call. It is strictly better on rate limits and privacy, and it is what I would build if this were load-bearing. Not done here because it couples the docs deploy to the release workflow — the docs would have to be rebuilt on every release, or the file fetched anyway — and the runtime call is a self-contained change to one component. Worth revisiting if the API dependency ever bites. **Leaving the hero action alone.** The first version of this PR did exactly that, on the reasoning that the static Download was a harmless always-works fallback and a layout override was disproportionate. It was wrong, and it was wrong in the way that shows up immediately: asked to try it, the first thing JuanMa clicked was the hero's "Download", then the nav's — the two most prominent Downloads on the page, both of which went to the file list this PR exists to remove. A fallback that outranks the real thing is not a fallback. Hence the `home-hero-actions-after` slot, which turned out to be a documented default-theme slot rather than the full layout override I had assumed. **Renaming the nav item rather than deleting it.** The releases list is still the only route to `.deb`/`.snap` and to older versions, so it earns a place; it just cannot be called "Download" while a smarter Download sits on the same page. **Keeping the per-OS list as a collapsed section.** Rejected: the button's own fallback link goes to the Releases page, which shows every asset with its name. Restating the naming scheme in prose is a second copy that drifts the moment a target is added. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **3 [fix here] · 3 [follow-up] — all 6 fixed.** The judgement pass ran in a fresh context per `.claude/skills/self-review`. Deterministic layer: `npm run lint` clean, `npm test` 898/898 (894 before this PR, +4 new). [fix here]: 1. 🟡 **Cross-platform** — `/Mac/` matched iPhones. Every iOS user agent contains `like Mac OS X`, so an iPhone visitor was told "Download for macOS (Apple Silicon)" and handed a `.dmg`. Now matches `Macintosh`, with `iPhone|iPad|iPod` added to the mobile guard alongside Android. This was a real bug, not a hypothetical — the new test fails on the pre-review code. 2. 🟡 **Performance** — the API call fired on every page view with no caching, against a 60/hour/IP limit shared by a whole Contributor Day room. Now one response resolves all three platforms into `sessionStorage`. 3. 🔵 **Architecture** — the Intel-Mac caveat was gated on comparing against the user-visible label string, so a copy edit would have silently dropped the warning. Now gated on a non-display `id`. [follow-up] — all fixed here rather than deferred, since each was a one-liner in code the PR was already touching: 4. 🔵 **Cross-platform** — `-linux-x64.AppImage` was an exact match, and electron-builder maps `x64` → `x86_64` for AppImage when expanding `${arch}`. A rebuild could have renamed the asset out from under it with no error surfacing. Patterns are now loose on the arch token. 5. 🔵 **Security** — `asset.browser_download_url` went into `:href` with no scheme check. Now only assets under `https://github.com/` are accepted; anything else is skipped and the visitor gets the fallback. The threat is remote (an attacker who controls that field controls the binaries), but the repo's standard is that a URL crossing a trust boundary gets validated before something acts on it. 6. 🔵 **Tests** — the only branchy logic sat in a component in a package with no test harness. Extracted to `match-platform.mjs`; finding 1 is exactly what a table test over real user-agent strings catches. </details> <details> <summary>Implementation notes</summary> Eight files, all docs-side except the test: - `docs/.vitepress/theme/index.js` — new. Extends the default theme to register the component globally and to install the Layout wrapper; there was no custom theme before this PR. - `docs/.vitepress/theme/Layout.vue` — new. Wraps the default layout for the single purpose of filling `home-hero-actions-after`. The button sets no alignment of its own so it inherits the hero's, which switches between left and centred by viewport and by whether a hero image is set — a hard-coded media query on the button got this wrong at 700px. - `docs/.vitepress/config.mjs` — nav "Download" → "Releases". - `docs/.vitepress/theme/DownloadButton.vue` — new. All `navigator`/`fetch` access is inside `onMounted`, so SSR renders the fallback state and `npm run docs:build` is the regression check for that. - `docs/.vitepress/theme/match-platform.mjs` — new. `matchPlatform( ua )` plus the `PLATFORMS` table. ESM because the browser loads it through Vite as-is; the CJS test imports it dynamically. - `docs/guide/getting-started.md`, `docs/index.md` — the two usages. - `test/download-button-platform.test.cjs` — new, 4 tests. Real user-agent strings rather than synthetic ones, since the iOS case only exists because a real iPhone UA says `like Mac OS X`. Also asserts the asset patterns against both the current Linux filename and the `x86_64` form from finding 4. Verified in Chromium via Playwright across spoofed macOS / Windows / Linux / Android / iPhone user agents, with `api.github.com` blocked, and with a request counter across two navigations to confirm the session cache holds at exactly one call. </details> <details> <summary>Screenshots or recording</summary> Not attached — `gh` cannot upload images, and I would rather not paste an external host into a WordPress repo. Both surfaces are one dev-server command away (steps 1 and 4 above), and the visual change is small enough to describe: **Landing page hero** — under the tagline, "Get started" in the grey secondary style, and below it the brand-coloured "Download for macOS (Apple Silicon)" pill with the version in lighter text beside it, then two small grey lines: the Intel-Macs note and "All platforms and previous versions on the Releases page". The nav reads "Releases" where it read "Download". **Guide → Install the app** — the same button in place of the numbered list and its four per-OS bullets, followed by one sentence: "Download the latest build for your platform with the button above, then open the app." The "If macOS blocks the app" subsection below is untouched. Both were checked in light and dark theme; the button uses VitePress's own `--vp-button-brand-*` tokens, so it matches the hero's "Get started" button in both. </details> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## Why Twenty PRs landed on `trunk` between `v1.0.0-beta.1` and this release candidate, and several moved behaviour the guide describes. Four of the guide's claims had gone from incomplete to simply false: - **The build watcher is no longer tied to the dev server** (#247/#272) — `running-the-site.md` said it ran "alongside the server… while the server is up". - **A trunk update no longer needs the server stopped** (#262/#273) — the guide said the opposite. - **The setup checklist is no longer four clicks** (#246/#276) — install and build now start themselves once the clone finishes. - **There is a wp-admin link now** (#250) — the guide told you to type `/wp-admin/` onto the URL by hand. ## What changes Correcting those four, and documenting what came with them: the build watch's own control, dot and log tab; what a failed apply says now and why the framing differs for a pull request versus a loose patch file (#282); the ticket's own facts and the "Latest" pill's real ranking (#292, #281); **Discard all changes** rewinding to the ticket's base (#270); severity colour in the log panes (#280); and one short passage on the "Start here" cue and the completion toasts (#252, #253) — cross-cutting enough that repeating them per page would read worse than naming them once. ## Review Ran `/self-review` per `AGENTS.md`, dispatched to a fresh `Explore` subagent so the session that wrote the docs was not the one grading them. `npm run lint` / `npm test` don't apply — docs-only, no JS touched — so the review focused on the one dimension that does: **every behavioural claim verified against the actual source**, not assumed. It caught ten inaccuracies, five worth fixing here — including two pages I hadn't touched (`editors.md`, `creating-a-site.md`) that my changes to `running-the-site.md`/`setup-wizard.md` had left contradicting. All five are fixed in the second commit; the other five were softened as follow-up-scale wording tightenings, folded into the same commit since they were one-line each. ## Not in this PR Screenshots. `site-view.png`, `setup-wizard.png` and `trac-ticket-panel.png` now show a UI that has moved on (Build watch button, wp-admin link, ticket-facts card). Retaking them needs the shots harness against a running app; filing a follow-up issue rather than blocking this PR on it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

Closes #298. ## Why #297 fixed the guide's text for everything that shipped since `v1.0.0-beta.1`, but three pictures still showed the pre-1.0 app: no build-watch button, the old four-click checklist, a Trac panel with no ticket facts. Retaking them turned out to be blocked on the harness, not on the camera. Each of these states is one no seeded site can reach: - the **wp-admin** link renders only while a dev server is serving, and fixture sites are empty directories; - the **self-setup chain** arms on the clone-finished edge, so a site that was already in the registry when the app started never runs it; - a **ticket's own facts** come from a live visit to its Trac page and are held in memory, never written to the site's metadata. ## What changes `shots.cjs` moves `setup-wizard`, `site-view` and `trac-ticket-panel` to the live tier, next to `dev-server-running`, ordered so one site walks all four in sequence. The header comment records why each cannot be a fixture, so the next person does not move them back. `capture.cjs` grows `--user-data=<dir>` for the live tier, so a capture session runs against a documentation profile instead of the maintainer's own registry. The existing guard stays — it exists so a stray *exported* `TOOLKIT_USER_DATA_DIR` cannot silently redirect a run, and a path typed on the command line is a decision, not a leftover. The flag is refused on the fixture tier, which builds its own throwaway profile. The four images come from one real site the app created at `/private/tmp/wpct-docs/my-first-patch` — a real clone, install, build, dev server and Trac read. No username in any pixel. `dev-server-running.png` is new, and answers the half of #298 a single picture could not: `site-view.png` stays in the stopped state, so the guide's "shows a **Start dev server** button" prose still matches, and the wp-admin link gets its own shot in *Running the site*. Re-running the fixture tier also refreshed `site-menu`, `stale-site-notice` and `debug-log`: same pre-1.0 button row, same fix. Alt text follows the new pictures in `getting-started.md`, `running-the-site.md`, `setup-wizard.md` and `trac-tickets.md`, and the `docs.yml` comment now says four shots need a maintainer. ## Review The ticket in the panel is [#65856](https://core.trac.wordpress.org/ticket/65856), which has no keywords set, so that row of the facts card is absent — the card renders what the ticket has. The images were driven programmatically rather than clicked by hand, using the same launch config the harness uses (1200×800, DPR 1) with the folder picker answered through `dialog.showOpenDialog`. The live tier itself is unchanged as the documented, maintainer-at-the-keyboard path. ## Testing - `npm run lint`, `npm test` (937 pass), `cd docs && npm run build` — green. - `npm run shots` (fixture tier) still captures its eight shots; `--only=site-view` on the fixture tier now fails loudly with the known-slug list, and `--user-data` on the fixture tier is refused. - Every image is 1200×800 at DPR 1, and no path in any picture contains a username. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

## Why Start/stop the dev server and Review & submit are things you *do* to a site. Adminer is a *place* in the running site, like the front end and wp-admin. Keeping it in the button row split three destinations across two parts of the page. ## What changes Adminer moves down beside the other two links, so the running site's destinations read as one row: > `http://127.0.0.1:9400/` · **wp-admin** · **Adminer** The credentials line stays underneath, where it now covers all three. The action row keeps only actions. Two parts are not just a move: - **Adminer keeps its own `running` guard.** The URL row renders on `serverUrl`; Adminer rendered on `running && serverUrl`. Rather than let it inherit the row's weaker condition, the link and its separator sit in a `running ?` fragment together. - **As an anchor it needs `preventDefault`.** A `Button` could not navigate the window; a bare `href` can, and nothing binds `will-navigate` on the main window — so a click would load Adminer *into the app* and replace the UI. The label is `Adminer`, not `Open Adminer` — it is a destination among destinations now, not a button. **Deliberately not in this PR:** the front-end link still shows the full URL rather than the word "Site" from the issue's sketch, since the port is worth being able to read and copy; and the middle-click gap under **Risks** is left for its own change. ## How to test this Platforms: any — this is renderer markup, with no path, spawn or line-ending behaviour in it. **Starting state:** an initialized site, dev server stopped. 1. Open the site and click **Start dev server**. Wait for the URL to appear. 2. Under the site name, one row reads `<url> · wp-admin · Adminer`, with `Log in with admin / password.` below it. 3. The button row above holds only **Stop dev server** and **Review & submit changes** — no Adminer button anywhere. 4. Click **Adminer** → the database browser opens in your **browser**, already logged into the site's SQLite database. 5. Click **wp-admin**, then the URL itself → both still open in the browser. 6. Click **Stop dev server** → all three links disappear together, and no `·` is left behind. **What must not have happened:** - **Adminer must not open inside the app window.** It was a `Button` before, which could not navigate; as a link it can, and that would replace the app UI with a web page and no way back. This is the one regression the new shape introduces. - **Adminer must not appear while the server is stopped**, and must not appear without wp-admin beside it. - `Open Adminer` must not still exist in the action row — a duplicate would be easy to miss if you only look at the new row. ## Risks and limitations Review found `2 [fix here] · 1 [follow-up]`; both `[fix here]` are fixed. Details in the collapsed block below. **Middle-clicking any of these three links opens it inside the app.** `preventDefault()` runs on `click`, and middle-click dispatches `auxclick`, so nothing intercepts it — and `createWindow()` binds neither `setWindowOpenHandler` nor `will-navigate`. Such a window inherits the parent's `webPreferences`, `preload.js` included, and the URL never reaches `external-url.js`. Not introduced here: ~20 links in this file share the pattern and #250 added three of them. The fix is one guard on the main window, not three `onAuxClick` handlers, so it belongs in its own change. **The `running` guard is not covered by a test.** Review established the state it guards against is unreachable — `setServerUrl` and `setRunning` only ever change together — so it is redundant rather than load-bearing. Kept because it costs nothing and states the intent. **The rendered row is not automated.** No DOM test infrastructure exists here, which is what #216 chose; the tests read `index.jsx` as source instead. Step 2 above is what actually proves the row. ## Related Fixes #249. Follow-up to #250, which built the row. --- <details> <summary>Design decisions and alternatives considered</summary> **Keeping `running` rather than inheriting `serverUrl`.** Simply dropping the Adminer link into the row would have widened its condition, since the row renders on `serverUrl` alone. Review then showed the two flags always change together, so the guard is provably redundant — but it is one word, it documents that Adminer needs a live server in a way `serverUrl` does not, and removing it would mean re-deriving that argument the next time someone reads the row. Left in. **A link, not a small button in the row.** A button among links would keep the widget and lose the point of the issue: these three are destinations and should look alike. It also drops "Open" from the label, which only made sense while it was a button. **Not renaming the URL link to "Site".** The issue sketches the row as **Site · wp-admin · Adminer**. Showing the literal URL keeps the port visible and copyable, which is worth more than the symmetry — contributors paste it into other tools. **Pinning placement by position, not by parsing.** The test asserts the Adminer link falls between the site page's wp-admin link and the credentials line. Parsing the JSX would be sturdier but needs a parser the suite does not have; a position check is what the existing source-assertion tests in this repo already do. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `2 [fix here] · 1 [follow-up]` — both `[fix here]` fixed, follow-up deferred with reason. Ran per `.github/instructions/code-review.instructions.md`, with the judgement pass in a fresh context rather than the session that wrote the change. Lint clean; docs site builds; 781 tests pass on both system Node and Electron's Node. **Fixed — 🟡 docs: three guide pages described a UI that no longer exists.** `running-the-site.md` and `database.md` both stated an **Open Adminer** button appears in the action row / next to the URL, and `setup-wizard.md` listed it as the action bar's third item. The widget and its name are both gone. Screenshots were checked and are unaffected — the row only renders while the server runs, `site-view.png` shows it stopped, and the `dev-server-running` live shot has never been captured — but prose is not covered by that. Also dropped `running-the-site.md`'s "Append `/wp-admin/` to the site URL", stale since #250. **Fixed — 🔵 tests: placement was unpinned.** The four Adminer assertions covered the widget change and the URL derivation, but all four still passed with the anchor rendered back inside the action-button row — the exact state #249 exists to move away from. Verified by mutation, then fixed by anchoring the link between the site page's wp-admin link and the credentials line. **Deferred — 🟡 security: middle-click is not intercepted.** Described under **Risks and limitations**. Pre-existing pattern shared by ~20 links in this file; the fix belongs on the main window, not on these three anchors. Verified and not findings: the `running` gate cannot produce a dangling separator or a dead link, since `setServerUrl`/`setRunning` only change together and the separator shares a fragment with the link; nothing of value was lost with the button (it carried no `disabled`, `title` or `isBusy`, and unlike its neighbours was not gated on `isUpdating`); no new decision landed inline in `index.jsx` that §1 wants in a module. The source assertions were mutation-tested rather than assumed — 8 mutations, each assertion isolated: | Mutation | Result | | --- | --- | | Adminer link loses `preventDefault` | caught | | label reverts to `Open Adminer` | caught | | Adminer href hardcoded | caught | | Adminer link deleted | caught | | Adminer link duplicated | caught | | button re-added while keeping the link | caught | | anchor moved back into the action-button row | **passed before this review; now caught** | | label kept but href points at `adminUrl` | caught, but by the wp-admin assertion rather than an Adminer one — fails safe, though the message points at the wrong link | </details> <details> <summary>Implementation notes</summary> Four commits: the move, its tests, the guide update, then the placement assertion the review asked for. `604bb31` does not pass on its own — it changes `index.jsx` while the assertion inherited from #250 still expects one `adminerUrl(` call. Splitting these two files cannot avoid that in either order, since each half contradicts the other's expectations. Head is green; kept split for readability rather than squashed. Both URLs still come from `src/renderer/site-urls.cjs` (added in #250) — this change moves a link and swaps a widget, and adds no URL logic. That module's docstring still refers to the "Open Adminer button" in the present tense; left alone to keep this diff to the move, but worth a sweep later. </details> <details> <summary>Screenshots or recording</summary> Starting the dev server, the row it produces, each of the three links opening in the browser, and all three disappearing together when the server stops. https://github.com/user-attachments/assets/c9bef140-727d-4dfd-abe8-b6f8d0227449 </details>

…#300) ## Summary - Two `useEffect`s in `src/renderer/index.jsx` were both keyed off `tracTicket`: one auto-triggered a Trac scrape right after a ticket was freshly linked, the other bumped `scrapeGenRef` to mark later scrapes as stale. React runs same-component passive effects in declaration order, so the auto-scrape fired *before* the generation bump — `loadTracAttachments` captured the stale generation, and its `finally` guard never matched, leaving the "Reading ticket…" spinner stuck indefinitely. - Merges the two effects and bumps `scrapeGenRef.current` synchronously at the top, before the auto-scrape can fire, so the generation is always current. ## Test plan - [x] Added `test/trac-ticket-scrape-ordering.test.cjs` — a source-scan test (this repo's existing pattern for `index.jsx`, which has no render harness) asserting the generation bump appears in source before the auto-triggered scrape call, and that there's exactly one bump site. Verified it fails against the pre-fix code and passes after. - [x] `npm test` — 673/690 pass, same pre-existing failure count as on `trunk` (17 failures from a missing `fs-extra` dependency in `node_modules`, unrelated to this change). - [ ] Manual: link a Trac ticket to a task, switch away and back, confirm the panel stops showing "Reading ticket…" once the scrape resolves. Fixes #299 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

… last save (#302) ## Why Before a patch or PR is applied, the app warns which incoming files collide with work the contributor already did. That warning was measured against HEAD, and under the ticket-as-branch model (#108) HEAD is the ticket's *last saved state* — the parked WIP commit — not the ticket's base. Switching tickets parks the current work automatically, so the two baselines separate the moment a ticket has been left and resumed. From then on the warning is wrong in both directions, and the silent one is the reason to fix it: a resumed ticket has a worktree matching HEAD exactly, which reads as a clean tree, so a PR applied at that point lands on top of the contributor's own work with no warning at all. That is not an edge case — it is the ordinary shape of a ticket someone has come back to. Fixes #301. ## What changes Root cause: `git:preview-patch` fed `planApply` with `collectDirtyFiles(sitePath)`, a `git.statusMatrix` walk with no `ref` — i.e. "what differs from the last commit". It now asks `collectUnsubmittedFiles(sitePath)`, the base-relative measurement everything else in the patch flow already uses (`patchBaseOid` → `collectChangedFiles` → drop the rows the patch would not speak about). No new measurement code: the two questions and their two channels already exist from #239, and this check was simply on the wrong one. `collectDirtyFiles` is unchanged and keeps its callers: `git:worktree-dirty` and the trunk-update guards still need the narrow "what a force checkout would overwrite" answer, where parked work correctly does not count. On trunk `patchBaseOid` answers `null` and the walk falls back to HEAD, so a site that never linked a ticket keeps exactly the answer it had. ## How to test this Platforms: any — no paths, spawning or line endings involved. **Starting state:** a site with a linked ticket, and a second ticket to switch to. 1. On ticket A, edit a file the incoming patch also touches (e.g. `src/wp-login.php`). 2. Switch to ticket B, then back to ticket A. The work is parked; the tree now looks clean. 3. Apply a patch or a PR that touches that same file — Trac attachment, PR diff, or a `.patch` file, all three go through the same preview. **Expected:** the amber block appears — "You have your own edits to `src/wp-login.php`…". Before this change it did not appear at all. 4. Apply a patch touching a file the ticket never touched. **Expected:** no such block. 5. Optional, the other direction: edit a file back to its original content, then preview a patch touching it. **Expected:** no warning — that file holds none of your work, whatever `git status` thinks. **What must not have happened:** - The trunk-update dirty dialog must not start firing on a clean-but-parked tree — it deliberately keeps the narrow reading. Run "Update to latest trunk" on a resumed ticket and confirm it does not now claim uncommitted changes. - No preview should fail with "Could not check your work for conflicts" on a healthy site; that path is the fail-closed branch, not a normal outcome. - The preview must still list every file the patch touches, warning or not. Covered by three new tests in `test/ipc-wiring.test.cjs`, built on the existing real-repo `parkedTicketRepo` harness. Verified the first two fail on the old code — one with `conflicts: []` where the warning should fire, the other with a spurious `['src/wp-login.php']` — and pass after. ## Risks and limitations Self-review: **0 [fix here] · 1 [follow-up]**, and the follow-up predates this PR. - **More warnings than before, by design.** A resumed ticket now warns where it used to stay quiet. The existing copy already reads correctly for parked work. - **A slightly wider scan per preview.** `collectChangedFiles` reads both sides of each changed file, where `collectDirtyFiles` only walked status. It is the same cost the card's unsubmitted-work note already pays on the same repo, on an action that is already a file read. - **Not tested by hand yet** — I have not driven the app through the steps above; the coverage here is the suite plus the reasoning about the two baselines. ## Related Fixes #301. Builds on #300 — this PR targets its branch, so review that one first. Follow-up: #303 — the post-failure narration blames the pull request's author for a collision with the contributor's own work, using the same fact this PR puts at hand. --- <details> <summary>Design decisions and alternatives considered</summary> **Why not a union of both readings** (dirty-vs-HEAD ∪ changed-vs-base)? The base-relative walk already contains everything the HEAD-relative one reports on a ticket branch — uncommitted edits differ from the base too. A union would only re-import the false alarm: a file edited back to the base still differs from HEAD, and that is exactly the row #301 says should not be announced. **Why not compute the overlap inside `patch-plan.cjs`?** `planApply` is deliberately baseline-agnostic — it takes `dirtyPaths` and intersects. The bug was in what the handler passed it, so that is where the fix belongs; `test/patch-plan.test.cjs` needed no change at all. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `/self-review` per AGENTS.md, judgement pass dispatched to a fresh-context subagent: **0 [fix here] · 1 [follow-up]**. `npm run lint` clean; `npm test` 941/941 pass. The follow-up (🔵, architecture): `patchBaseOid` has two silent fallbacks that this warning now becomes a second consumer of — a ticket branch with no recorded `baseOid` falls back to live `refs/heads/trunk`, which an "Update to latest trunk" can have moved ahead of the branch point (over-warning); and any throw inside it returns `null`, routing the walk back to HEAD with no signal to the renderer. Both predate this PR — `git:unsubmitted-work` and the card's note have ridden on them since #239 — so fixing them is a change to `patchBaseOid`'s contract, not to this handler. Deferred rather than widened into this diff; happy to file it as its own issue. </details> <details> <summary>Implementation notes</summary> - `src/main.js` — `git:preview-patch` swaps its source and says which baseline it reads and why. The fail-closed `catch` stays (failing open would promise "no collisions" precisely when the app could not look); its message moves from "your working tree" to "your work", which is what it now failed to read. - `src/main.js` — the comment above `git:worktree-dirty` listed the patch-apply collision scan as a caller of the narrow reading. It was documenting the bug; it now names the checkout guards only. - `test/ipc-wiring.test.cjs` — `parkedTicketRepo` takes an optional `workFile` so the same harness can build the `src/` layout a patch's paths are mapped into by `mapToSrcLayout`. The default is unchanged, so the #239 tests are untouched. </details> <details> <summary>Screenshots or recording</summary> Nothing on screen changed: the amber "You have your own edits to …" block at `src/renderer/index.jsx:4722` renders exactly as before. What changed is when it appears. </details> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

## Why A contributor adds a new empty file to their ticket — a placeholder, a fixture, a file they are about to fill in — and the patch modal says the ticket has no changes at all. Deleting a file that was already empty does the same. It is not merely missing from the generated `.diff`: the card's unsubmitted-work note does not count it either, so nothing on screen suggests anything was dropped. Silent today, destructive soon. This is the same classification the planned "bring a ticket up to date" flow (#305) uses to carry a ticket's work onto newer trunk. A file the classification does not mention is a file that flow would not carry — and the force checkout it performs would delete it. A cosmetic omission now is lost work then. ## What changes **Root cause.** `classifyChangedFile` in `src/main.js` compared the two sides' normalized text before consulting the existence flags it already held. For an added empty file `file.base` is null and `file.work` is a zero-length buffer, so both sides render `''`, compare equal, and the row is classified `unchanged` — skipped by `createMinimalPatchForDir` and filtered out of `collectUnsubmittedFiles`. Deleting an already-empty file is the mirror image. Whether a file was added or deleted is not a question about its contents. `collectChangedFiles` already carries the answer: `inHead` and `inWorkdir` come from `statusMatrix` status codes, which is the rule #85 established for whole-file reads — the codes, not the buffers, are what say a file is gone. So `unchanged` now requires the file to exist on both sides. Nothing else about the condition moves: a file that differs only in line endings is still nothing. Three consequences had to be handled, and they are the reason this is not a one-line diff: 1. **The patch has to be emittable.** An empty file has no line to diff, so jsdiff produces a section with no hunk at all — and `git apply` rejects that as `No valid patches in input`, taking the *whole* patch and every unrelated file in it, the same all-or-nothing failure mode the phantom `\ No newline` work was about. Git's own extended header (`diff --git` plus `new file mode` / `deleted file mode`) is how an empty file is carried, so `emptyFileSection` in `src/main.js` emits exactly that, keeping the `---`/`+++` pair because `/dev/null` is what this app's own reader classifies from. Verified against real `git apply`, mixed into a patch alongside an ordinary edit. 2. **The patch has to be readable back.** `parsePatchFiles` in `src/patch-plan.cjs` treated every zero-hunk section as a rename, a binary, or garbage. It now classifies from `/dev/null` on either parsed filename — the same rule `classify()` applies to a hunked section — before falling through to the raw-section checks. It reads the parsed filenames rather than `scanSections`' output on purpose, so it does not depend on the two arrays lining up in a patch that mixes git-style and bare sections. 3. **The applier must not become destructive.** A deletion with no hunk gives `src/patch-apply.js` no pre-image to match, so its existing "did the file move on since the preview" check could not run and the file would be removed whatever was in it. A hunkless deletion *claims* the file was empty, so `planFile` now refuses one whose target has content, routed through the same `conflict()` path as every other mismatch. `git apply` refuses the identical case as `removal patch leaves file contents`. Deliberately not in scope: binary and unreadable handling, file modes (this generator has never recorded them, for empty files or otherwise), and reading empty add/delete sections out of patches written by *real git*, which omits the `---`/`+++` pair entirely — see Risks. ## How to test this **Platforms: any.** Nothing here touches paths, spawning or signing. The generated section is built with `\n` like every other, and `statusMatrix` paths are forward-slash on every OS. **Starting state:** a site with a linked Trac ticket, on its ticket branch, with no other uncommitted work. (Link a ticket from the card if the site is still on trunk.) 1. In the site's folder, create an empty file — `touch src/placeholder.php`, or save a new empty file from your editor. Do not put anything in it. - **Expected:** the site card's note names unsubmitted work and counts **1 file**. 2. Open **Review and submit** (the patch modal). - **Expected:** the diff area is not "No changes." It shows a section for `src/placeholder.php` reading `new file mode 100644`, `--- /dev/null`, `+++ b/src/placeholder.php`. 3. Save the patch to disk with **Save patch**, then in a real `wordpress-develop` clone at the same base run `git apply --check the-file.diff`. - **Expected:** exits 0 and prints nothing. Running `git apply the-file.diff` creates an empty `src/placeholder.php`. 4. Back in the app, edit an ordinary file too (add a line to `src/wp-login.php`) and reopen the modal. - **Expected:** both files are in the one patch, and step 3 still passes on it — the empty section must not break the file next to it. 5. Now the deletion side. Delete `src/placeholder.php` from disk *after* it has been carried into the ticket's parked work — or, more simply on a fresh site: pick any file that is empty in trunk, delete it, and open the modal. - **Expected:** a section reading `deleted file mode 100644`, `--- a/<path>`, `+++ /dev/null`, and the card's note counts it. 6. Apply a patch containing an empty addition into a second site: open the second site's **Apply a patch**, paste the `.diff` from step 3, and confirm. - **Expected:** the preview lists the file as **added**, not modified; applying creates it, empty. 7. The refusal. Take the step-5 deletion patch, but before applying it into the second site, put some text into the file it names. - **Expected:** applying fails with "moved on since the patch was written", and the file — with your text in it — is still there. **What must not have happened:** - No file may be deleted in step 7. A silent deletion there is work destroyed with nothing on screen to say so, and it is a path this PR opens: the applier had no pre-image check that could run on a hunkless deletion. - `git apply` must not fail in steps 3 and 4 with `No valid patches in input` or `corrupt patch at line N`. A malformed empty section takes the whole patch down, including the unrelated file in step 4 — that is the failure this shape was chosen to avoid. - A file whose only change is line endings must still be absent from the patch and from the note's count. Check it: on Windows, or by rewriting a file with CRLF, confirm the modal still says "No changes." Widening the existence test must not have become "stop comparing text". - Nothing may be staged into the contributor's index by generating the patch (#85). **The steps that used to reproduce it:** steps 1 and 2, on `trunk`, produce a card note saying nothing and a modal saying "No changes." **Tests, and which fail on the old code.** Seven of the nine new tests fail without this change, checked by stashing the `src/` edits and re-running: - `test/ipc-wiring.test.cjs` — `an added empty file reaches the patch (#311)`, `a deleted empty file reaches the patch (#311)`, `a generated empty addition parses back as an addition (#311)`, `a generated empty deletion parses back as a deletion (#311)`, `a generated patch carries empty additions and deletions into another checkout (#311)`, `git:unsubmitted-work counts empty files added and deleted (#311, #239)` — all six fail on the old code. - `test/patch-apply.integration.test.cjs` — `an empty-file deletion refuses a file that has content (#311)` fails without the applier guard (checked by stashing only `src/patch-apply.js`). - The remaining two pin the negative and the happy path: `a file that differs only in line endings is still no change (#311, #85)` and `an empty-file deletion removes the empty file (#311)`. `npm test` 950 pass / 0 fail. `npm run lint` clean. ## Risks and limitations Review outcome: **2 [fix here] · 1 [follow-up] — both [fix here] fixed.** One of them found a real regression this PR would have introduced (the applier deleting a file with content), so the collapsed block below is worth reading. - **Empty add/delete sections written by real git are still not read.** `git diff` emits `diff --git` + `new file mode` + `index` and *no* `---`/`+++` pair for an empty file, so jsdiff hands back no filenames and `parsePatchFiles` still rejects — and because it returns a whole-patch error, a Trac attachment or PR `.diff` that happens to add one empty fixture is rejected entirely, unrelated files included. That is the read half of #311 and it predates this change; `scanSections` already captures the paths and would need to record the mode markers. Deferred as a follow-up rather than widened into this PR. - **File modes are not carried.** The emitted section hardcodes `100644`. An empty file with the executable bit set loses it — as every non-empty addition already does, since this generator has never recorded modes. Applying a `deleted file mode 100644` section over a `100755` file makes `git apply` warn and then apply, which is acceptable. - **A hunkless deletion is refused whenever the target is non-empty**, including the case where the contributor emptied the file themselves and would have been happy to see it go. That is the conservative direction, and it matches `git apply`. - **Not tested by hand: step 5's "empty file in trunk" starting state.** `wordpress-develop` has no empty tracked file to delete, so the deletion half was driven through the suite and through real `git apply` against a synthetic repo rather than in the app against a real clone. ## Related Fixes #311. Related to #85 (the codes, not the buffers, say a file is gone), #239 (the unsubmitted-work count reads this same classification) and #305 (the carry-forward flow that would delete anything this classification fails to mention). Stacked on #302 — review that first; this PR targets its branch. --- <details> <summary>Design decisions and alternatives considered</summary> **Why not just return a new `kind` from `classifyChangedFile`?** The two consumers ask different questions of it: the generator switches on the kind, the note only asks whether it is `unchanged`. A new kind would have meant touching the note for a case it already handles correctly once the classification is right. Keeping the row as `text` with `a === b` and special-casing the emission keeps the note's filter untouched. **Why emit `diff --git` at all, when nothing else in this generator does?** Because nothing shorter works. Tested against real `git apply`: `--- /dev/null` + `+++ b/foo` with no hunk is "no valid patches in input"; adding `@@ -0,0 +0,0 @@` or `@@ -1,0 +1,0 @@` is "corrupt patch"; `diff --git` without the mode line is "patch with only garbage". `diff --git` plus `new file mode 100644` applies cleanly, and so does the deletion form. The mode line is load-bearing, not decoration. **Why keep the `---`/`+++` pair, which real git omits here?** Two reasons. It is what this app's own reader classifies an add or a delete from (#85) — without it, jsdiff returns a section with no filenames at all and the round trip breaks. And it makes the section look like every other one in the patch, so the renderer's diff highlighting treats its lines as meta the way it does everywhere else. **Why classify from the parsed filenames in `patch-plan.cjs` rather than from `sections[i]`?** `scanSections` pushes an entry only for a line starting `diff --git` or `Index:`. In a patch that mixes the new git-style sections with jsdiff's bare ones — which is exactly what this generator now produces — `sections` is shorter than `parsed` and the indices do not correspond. Reading `file.oldFileName` sidesteps that entirely. The misalignment itself is pre-existing and harmless here (app-generated patches contain no renames or binaries, which are the only things the section lookup is used for), but the new branch deliberately does not depend on it. **Why refuse a hunkless deletion over a non-empty file instead of applying it?** Symmetry with the hunked case, which exists precisely so that "an edit made after the preview fails all-or-nothing rather than being silently deleted with the contributor's changes in it". Without the guard this PR would have made the app *more* destructive than `git apply`, on a code path that did not exist before it. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **2 [fix here] · 1 [follow-up] — both [fix here] fixed, the [follow-up] deferred.** The judgement pass ran in a fresh context (a subagent given the diff and `.github/instructions/code-review.instructions.md`), not the session that wrote the change. **Fixed — Architecture 🟡 [fix here], `src/patch-apply.js:288`.** A zero-hunk deletion skipped the pre-image check, so an empty-file deletion removed its target whatever it contained — verified: applying the generated section over a file holding `<?php // real content` returned `ok: true` and the file was gone, where real `git apply` refuses with "removal patch leaves file contents". A new code path this PR opened, and the app was briefly more destructive than the tool the patch is destined for. Fixed with an explicit emptiness check routed through the existing `conflict()`, plus `an empty-file deletion refuses a file that has content (#311)` in `test/patch-apply.integration.test.cjs`, which fails without it. **Fixed — Tests 🔵 [fix here], `src/main.js:478`.** The narrowed condition had no test on its negative side: nothing in the suite pinned that a CRLF-only difference must *still* be suppressed, so deleting `file.inHead === file.inWorkdir` would have broken nothing in the direction of over-reporting. Added `a file that differs only in line endings is still no change (#311, #85)` covering both the patch and the note. **Deferred — Architecture 🟡 [follow-up], `src/patch-plan.cjs`.** Empty add/delete sections written by real git (no `---`/`+++` pair) are still unreadable, and the whole-patch error means one such section rejects an entire `.diff`. Genuinely not introduced by this PR — it is the read half of #311, reachable today from any Trac attachment — and fixing it means teaching `scanSections` the mode markers and resolving its index alignment with `parsed`. Written up in **Risks and limitations** rather than folded in, to keep this diff to the generator bug and its immediate consequences. **Confirmed clean by the pass**, having read the surrounding files rather than the diff alone: the CRLF suppression is unaffected (every ordinary modification has `inHead === inWorkdir === true`, and `[path, 0, 0]` rows never reach the classifier); the new section shape is safe across `diff-highlight.cjs`, `patch-provenance.cjs`, `github-pr.cjs`, `pr-files.cjs` and `git:preview-patch`; `JsDiff.applyPatch('', {hunks: []})` returns `''`, so the add path writes an empty file correctly; `reverseFile` inverts both directions; the `scanSections`/`parsed` misalignment breaks no rename or binary detection; and paths containing spaces survive both readers. </details> <details> <summary>Implementation notes</summary> **`src/main.js`** - `classifyChangedFile` — `unchanged` now additionally requires `file.inHead === file.inWorkdir`. One conjunct; the comment block above it carries the reasoning. - `createMinimalPatchForDir` — when `a === b` on a row that reached the `text` branch (only possible for an empty add or delete), emits `emptyFileSection` instead of calling `createTwoFilesPatch`, which would produce a hunkless section. - `emptyFileSection` — new. `===` separator (67, matching jsdiff), `diff --git a/<p> b/<p>`, the mode marker, then the `---`/`+++` pair. **`src/patch-plan.cjs`** — `parsePatchFiles`, inside the existing `hunks.length === 0` branch, ahead of the rename and binary checks. **`src/patch-apply.js`** — `planFile`'s delete branch gains an `else if (previous.length)` arm. **Verification against real git**, since the suite exercises only this app's own reader: a patch combining the new deletion section, the new addition section, and an ordinary jsdiff edit section applies with `git apply` at exit 0, leaves the deleted file gone, creates the added file at 0 bytes, and updates the edited file. Each rejected alternative shape was tested the same way. </details> <details> <summary>Screenshots or recording</summary> No new UI. The visible change is entirely in surfaces that already exist and whose content is text: the patch modal shows a section where it used to say "No changes.", and the site card's note counts a file where it used to count none. Both are stated as expected results in **How to test this**, in the words that appear on screen, which is falsifiable in a way a still frame of a diff pane is not. </details> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## Why A Git-authored patch that adds or deletes an empty file omits the `---` / `+++` pair. The reader cannot classify that section today, so one empty file rejects the whole patch, including unrelated valid edits. ## What changes Recognise only Git's unambiguous, text empty-file form and supply the missing headers before the existing parser runs. Binary sections, renames and ambiguous/quoted paths remain untouched rather than guessed. ## How to test this **Platforms:** any. **Starting state:** a site linked to a ticket, plus a patch generated by Git that edits one file, adds an empty file and deletes an empty file. 1. Open the ticket's patch action and select that patch. Expect the preview to list all three paths. 2. Apply it. Expect the ordinary edit, empty creation and empty deletion all to occur. 3. Repeat with content already present at the addition path. Expect the whole apply to fail and every file to remain unchanged. **What must not have happened:** the empty section must not disappear silently, and a failed apply must not partially edit the tree. The regression tests fail on the parent branch and pass here: `node --test test/patch-plan.test.cjs test/patch-apply.integration.test.cjs`. ## Risks and limitations Git-quoted path names (for example names containing tabs, quotes, backslashes or some non-ASCII characters) are still refused. Decoding Git's C-style quoting is deliberately deferred to keep the 1.0 reader narrow. The full desktop flow has not yet been driven by hand; it will be exercised from the integration artifact. ## Related Fixes #316. Follow-up to #311. --- <details> <summary>Design decisions and alternatives considered</summary> The implementation normalises the narrow Git form into the header shape already supported by the parser. It does not create a second patch parser or weaken the all-or-nothing guards. </details> <details> <summary>Review outcome</summary> 0 [fix here] · 1 [follow-up]. The follow-up is quoted Git paths, documented above. Lint is clean; 957 tests pass; no security, performance or cross-platform findings. </details> <details> <summary>Implementation notes</summary> Only three files change: the parser plus unit and integration coverage. There is no UI, IPC or persistence change. </details>

…ted (#313) ## Why When a pull request fails in a file the ticket has changed, the app used to state that the PR was stale and tell a newcomer to ask its author for a rebase. File overlap does not prove that: the contributor and PR may have changed different regions of the same file. ## What changes The failure copy now preserves what the app actually knows. It names the files containing ticket work, says that work may be part of the failure, and suggests saving a copy or trying from a clean ticket. It only recommends asking the author for an update when failures also occur in files the ticket did not change. No new measurement, IPC or Git operation is introduced. ## How to test this **Platforms:** any. 1. On a linked ticket, edit one region of a file. 2. Apply a PR that fails in a different region of that same file. **Expected:** the notice says your work may be part of the conflict; it does not claim either side is certainly responsible. 3. Apply a PR that fails only in files untouched by the ticket. **Expected:** the existing stale-PR framing and author-update action remain. 4. Try a mixed failure. **Expected:** both facts are named, without assigning the shared file categorically. **What must not have happened:** a failed apply must not alter the checkout, and the link to the PR must remain available. ## Risks and limitations Attribution remains file-level because this two-way apply path does not have the PR’s base. The copy deliberately expresses that uncertainty. The desktop flow remains to be driven from the integration artifact. ## Related Fixes #303. Part of #309. Builds on #302. --- <details> <summary>Review outcome</summary> The review found the original categorical same-file attribution unsound. Fixed with a regression covering edits in different regions of the same file. Focused tests and the full integration suite are green. </details> <details> <summary>Implementation notes</summary> The decision remains in the pure `apply-conflict.cjs` module; `index.jsx` only passes the existing collision list. </details> <details> <summary>Screenshots or recording</summary> No new component or layout; only the existing failure banner’s wording/action changes. </details> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## Why The ticket base is the reference used to decide what belongs in its patch and what can be discarded. A ticket branch with no recorded base was silently measured against current trunk; a failed base read was silently treated as trunk. Both guesses can misdescribe or discard the wrong work. ## What changes For 1.0 the rule is deliberately narrow: - trunk keeps its existing behavior; - a ticket with a recorded base uses it; - a ticket without a recorded base refuses patch generation, preview, discard and PR opening with one honest error; - a failed probe clears the old changed-file count instead of leaving stale state on screen. The former four-state taxonomy and “approximate” fallback have been removed. There is no base adoption UI in this release. ## How to test this **Platforms:** any. 1. On a normal app-created ticket, edit a file, review/save its patch, and discard to base. **Expected:** all behave as before against the recorded base. 2. In a disposable profile, remove that ticket branch’s `baseOid` from the stored metadata and reopen the app. 3. Try previewing, saving, discarding and opening a PR. **Expected:** each refuses because the starting point is unknown; none falls back to HEAD or current trunk. 4. Trigger the unsubmitted-work probe after previously seeing a count. **Expected:** the stale number disappears when the probe fails. **What must not have happened:** no destructive operation runs after the missing-base refusal. ## Risks and limitations Externally created/adopted ticket branches without recorded base are unsupported in 1.0. That is intentional for an experimental newcomer-oriented app and preferable to presenting an approximate measurement as fact. The desktop flow remains to be driven from the integration artifact. ## Related Fixes #308. Part of #309. Builds on #313. --- <details> <summary>Design decisions and alternatives considered</summary> The earlier branch tried to preserve unsupported tickets using current trunk as an approximate base. That added a status taxonomy and hedged copy to every consumer while still being capable of a wrong answer. The simplified branch rejects that state instead. </details> <details> <summary>Review outcome</summary> The review found one stale-count failure and the over-broad approximate path. Both are fixed in the rewritten branch. Full suite and lint pass. </details> <details> <summary>Implementation notes</summary> The PR is about 173 added lines across seven files after removing the approximate-base module and its tests. </details> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## Why An applied PR remains part of a ticket’s work, but the app described every touched file as the contributor’s own and a failed Revert could blame the PR author. That attribution is misleading and makes the safe exit hard to find. ## What changes The applied patch is retained as a named layer: - conflict copy says a file includes changes from the named PR and may also contain the contributor’s edits; - Revert remains available whenever the patch text was retained; - pressing Revert performs the existing reverse-apply check and reports overlapping edits there; - a patch too large to retain offers save-a-copy plus discard-to-base; - the one-patch slot remains occupied until Revert or discard clears it. Routine `site:status` no longer reads and diffs patch files synchronously. The proactive “absorbed” classifier/state and categorical “not by you” copy have been removed. ## How to test this **Platforms:** any. 1. Apply a PR on a linked ticket. **Expected:** its named banner appears with **Revert this patch**. 2. Preview another patch touching the same file. **Expected:** the warning says the file includes changes from the applied PR and may also contain your edits; it never says “not by you”. 3. Edit a line brought by the applied PR and press **Revert this patch**. **Expected:** Revert fails without touching anything and explains the overlap, offering save-a-copy/discard rather than asking the author for a rebase. 4. Undo that edit and press Revert again. **Expected:** it succeeds and clears the named layer. **What must not have happened:** routine refreshes must not synchronously scan patch files; a failed Revert must not partially edit the tree or free the one-patch slot. ## Risks and limitations The banner does not predict whether a retained patch is still reversible; it answers only when the user requests Revert. This keeps routine main-process status reads cheap and keeps error details attached to the action they explain. The desktop flow remains to be driven from the integration artifact. ## Related Fixes #306. Part of #309. Builds on #317 and sharpens #313. --- <details> <summary>Design decisions and alternatives considered</summary> The previous version diagnosed every applied layer during `site:status`, adding synchronous file reads and an “absorbed” state. The simplified version uses the reverse apply operation as the diagnostic only when Revert is pressed. </details> <details> <summary>Review outcome</summary> 2 [fix here] — both fixed: synchronous status-path diagnosis was removed, and same-file ownership now preserves uncertainty. Lint is clean and the branch’s full suite passes 979 tests. </details> <details> <summary>Implementation notes</summary> The simplification removed 310 net lines from the reviewed version. Patch text remains in the main process and never crosses IPC. </details> <details> <summary>Screenshots or recording</summary> No new layout. The existing applied-layer and failure banners render the revised copy/actions. </details> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

## Why A ticket can remain on the trunk snapshot where it started after the site updates trunk. Newer patches may then fail, but 1.0 should warn without attempting to rewrite a newcomer’s worktree. ## What changes `site:status` compares the ticket’s recorded base OID with the current trunk OID. When they differ, the existing Trac ticket card shows one notice with the deliberately manual exit: save a copy, unlink, delete that ticket’s work, and link it again. There is no carry-forward operation, classifier, patch transport, new persistence, or network request. Missing base metadata stays silent rather than guessed. ## How to test this **Platforms:** any. **Starting state:** a site with ticket A linked and an uncommitted edit on it. 1. Use **Update to latest trunk** after trunk has advanced. Return to ticket A. **Expected:** its card says “Trunk has moved since this ticket started” and gives the save → Unlink → delete → re-link sequence. 2. Save the patch, click **Unlink**, delete ticket A from **Your tickets on this site**, then link A again. **Expected:** the new ticket starts on current trunk and the notice is gone. 3. Switch from an older ticket to a ticket created on current trunk. **Expected:** the old notice never appears with the new ticket number. **What must not have happened:** no branch, file or metadata is changed merely by displaying the notice; work is removed only after the existing explicit delete confirmation. The state/copy tests fail on the parent branch and pass here. Lint, Node tests and Electron-bundled-Node tests are green. ## Risks and limitations OID inequality proves that trunk changed, not whether a particular patch will conflict. The wording therefore says patches “may” fail. Tickets without a recorded base get no notice because the app cannot compare them honestly. The desktop flow remains to be driven from the integration artifact. ## Related Part of #305 and #309. The automatic carry-forward PRs #320 and #321 remain post-1.0. --- <details> <summary>Design decisions and alternatives considered</summary> The 1.0 boundary is observation plus an exit made from existing safe operations. Rewriting the ticket onto new trunk introduces conflict classification, recovery state and concurrency hazards that are not needed to ship an honest experimental release. </details> <details> <summary>Review outcome</summary> 2 [fix here] · 0 [follow-up] — both fixed. The stale flag is cleared atomically when a switch names the next ticket, and IPC now pins the equal-base case. Final review found no security, performance, compatibility or proportionality findings. `npm run lint` is clean and `npm run test:electron` passes 984 tests. </details> <details> <summary>Implementation notes</summary> Five files, 117 added lines including tests. The only new value crossing IPC is `ticketBehindTrunk: boolean`. </details> <details> <summary>Screenshots or recording</summary> Not yet captured. The visible notice will be exercised from the signed integration artifact. </details>

## Why PR #324 was marked merged by GitHub's stacked-PR endpoint, but its squash commit was attached to the old stacked base rather than to `trunk`. The tested discard explanation is therefore absent from `trunk`, despite #324 appearing merged. ## What changes This lands the exact #324 patch directly on current `trunk`. It adds the same disabled-link explanation beside both “Discard all changes” entry points; there is no new behavior beyond the version already approved on macOS and Windows. ## How to test this **Platforms:** macOS and Windows. **Starting state:** a linked ticket with one local edit and the dev server running. 1. Open **Review & submit changes** and hover **Discard all changes**. It must explain that the dev server must be stopped first. 2. Close the dialog and hover **discard your changes** in the ticket summary. It must show the same explanation. 3. Stop the dev server. Both discard actions must become available. **What must not have happened:** - Merely hovering or opening the explanation must not discard work. - The explanation must not remain after the dev server stops. These steps were completed against the approved integration artifact at commit `012d7df` on macOS and Windows. ## Risks and limitations This recovery commit has the exact same Git tree as the reviewed #324 head. Self-review: 0 [fix here] · 0 [follow-up]. ## Related Replacement landing for #324. Follow-up to #323. --- <details> <summary>Design decisions and alternatives considered</summary> A standalone PR is necessary because merging the old stacked PR again is impossible and reusing its stale stack base could repeat the topology error. The patch itself is unchanged. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 [fix here] · 0 [follow-up]. The recovery commit and #324 resolve to the same Git tree. Lint, 1008 Node tests, 1008 Electron tests, `build:once`, and `git diff --check` all pass. </details> <details> <summary>Implementation notes</summary> Recovery head before squash: `752a79a`. Tree: `f7048eb8ca9606b998c81ebaaf1a3868695e4d38`. </details> <details> <summary>Screenshots or recording</summary> The visible behavior is unchanged from #324 and was manually validated in the approved integration artifact on both target platforms. </details>

## Why After a contributor chooses **Discard this ticket to its base** from a failed Revert, the discard succeeds but the old red patch error remains on screen. The checkout is clean, yet the interface still claims the operation failed. The original #325 belonged to the stale stack that GitHub closed while #324 was being recovered, so this PR lands the already approved fix directly on `trunk`. ## What changes After a successful discard, clear the stale apply error and related preview state. If discard fails, retain all of that state so the contributor can still understand and recover from the failure. ## How to test this **Platforms:** macOS and Windows. **Starting state:** ticket #65819 linked, PR #13017 applied, with this contributor edit on an applied line in `tests/phpunit/tests/customize/widgets.php`: ```php $this->assertIsCallable( $args['sanitize_callback'], 'sanitize_callback is callable' ); // Mi cambio. ``` 1. Click **Revert this patch**. Revert must be rejected, the checkout must remain unchanged, and the red error must offer **Save a copy of your work** and **Discard this ticket to its base**. 2. Click **Discard this ticket to its base**. 3. The applied PR, contributor edit, applied-layer card, and red error must all disappear. Ticket #65819 must remain linked. **What must not have happened:** - A failed discard must not clear the error or preview state. - The ticket itself must not be unlinked. - No unrelated ticket work may be removed. This flow passed in the approved integration artifact at commit `012d7df` on macOS and Windows. ## Risks and limitations The final Git tree is exactly the tree manually approved for the release candidate. Self-review: 0 [fix here] · 0 [follow-up]. ## Related Replacement landing for #325. Follow-up to #318 and #330. --- <details> <summary>Design decisions and alternatives considered</summary> The cleanup is derived in the existing pure reducer and only committed after `outcome.ok`. Keeping failure state unchanged avoids hiding recovery information after an unsuccessful destructive operation. A standalone PR avoids reusing the closed stack topology that caused #324 not to land on `trunk`. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 [fix here] · 0 [follow-up]. Lint, 1010/1010 tests, and `git diff --check` pass. The branch resolves to approved tree `d3904782d5e434df8591e99d3ccf2a49a52ebcc9`. </details> <details> <summary>Implementation notes</summary> Recovery head before squash: `6a88a26`. It contains one commit directly on #330's squash commit. </details> <details> <summary>Screenshots or recording</summary> The user-visible flow was manually validated on both target platforms in the approved integration artifact. </details>

## Why `v1.0.0-rc.1` shipped on 12 August. Twenty changes have since landed on `trunk`, including safer applied-patch handling, clearer stale-ticket guidance, fixes for missing linked pull requests and stalled Trac loads, and exclusion of local coding-agent files from generated patches. This second release candidate carries those fixes, so the package version must move to `1.0.0-rc.2` before its tag and artifacts are created. ## What changes Only `package.json` and `package-lock.json`: all three package-version fields move together from `1.0.0-rc.1` to `1.0.0-rc.2`. No dependency versions or application behavior change. `electron-builder` derives artifact names from `package.json`, producing: - `wordpress-contributor-toolkit-1.0.0-rc.2-mac-arm64.dmg` - `wordpress-contributor-toolkit-1.0.0-rc.2-win-x64.exe` - `wordpress-contributor-toolkit-1.0.0-rc.2-linux-x86_64.AppImage` ## How to test this Platforms: any; this is package metadata and is platform-independent. **Starting state:** this branch checked out with dependencies installed. 1. Run `node -p "require('./package.json').version"`. - Expected: `1.0.0-rc.2`. 2. Run `node -e "const p=require('./package-lock.json'); console.log(p.version, p.packages[''].version)"`. - Expected: `1.0.0-rc.2 1.0.0-rc.2`. 3. Run `npm run lint` and `npm test`. - Expected: lint succeeds and all 1,017 tests pass. **What must not have happened:** dependency versions and resolved packages must remain unchanged; no source, documentation, workflow, or build configuration file should be in the diff. ## Risks and limitations No user-visible behavior changed, so there is no desktop flow or screenshot to test by hand. The signed artifacts will only exist after this PR lands and the release build runs. ## Related Follow-up to #296. --- <details> <summary>Design decisions and alternatives considered</summary> The version remains SemVer-shaped as `1.0.0-rc.2`, matching `1.0.0-rc.1` and keeping artifact names in one series. The release tag will add the conventional `v` prefix; package metadata does not. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `0 [fix here] · 0 [follow-up]`. No findings across architecture, security, performance, cross-platform, or tests. `npm run lint`, `npm test` (1,017/1,017), and `git diff --check` pass. </details> <details> <summary>Implementation notes</summary> npm's documented versioning behavior keeps `package.json`, the lockfile root version, and `packages[""]` synchronized. The diff was checked to confirm those are the only changed values. </details>

## Why The submission and trunk-update guides describe visual, multi-step interfaces but previously showed no corresponding screenshots. Contributors had to translate terms such as “Your changes,” destination cards, GitHub device flow, and “step N of 3” into an unfamiliar UI on their own. ## What changes Adds four focused screenshots for the existing live-tier capture definitions: the highlighted diff, all three submission destinations, GitHub's device-code state, and trunk-update progress. The capture targets now crop the relevant panel and use the current **Review & submit changes** label, and the guide places each image next to the text it explains. ## How to test this **Starting state:** Any platform, with this branch checked out and dependencies installed. 1. Run `npm run docs:build`. Expect VitePress to render every guide page without missing-image or Markdown errors. 2. Run `npm run docs:dev`, open **Submitting your changes**, and confirm the diff image appears under **Your changes** and shows readable highlighted PHP. 3. Continue to **Where this patch goes**. Confirm the image shows **Open a pull request**, **Attach to Trac**, and **Hand it to a mentor**, including their primary actions. 4. Open **Opening a pull request**. Confirm the device-code screenshot includes **Copy the code**, its waiting state, and **Cancel**. 5. Open **Keeping a site up to date with trunk**. Confirm the progress screenshot shows step 1 of 3 and the three update stages. **What must not have happened:** No screenshot should expose a personal filesystem path or active credential. The GitHub device code was canceled immediately after capture and cannot authorize an account. ## Risks and limitations The images document the current 1.0 UI and will need recapturing if those labels or layouts change. The live tier still requires a prepared site and a maintainer at the keyboard; this PR makes its output focused and reproducible but does not automate the underlying app states. ## Related Follow-up to #310. --- <details> <summary>Design decisions and alternatives considered</summary> The screenshots are cropped to the UI they explain rather than publishing a full Electron window. This keeps text readable at documentation width and prevents local site paths from entering published images. The three destinations remain one screenshot because their comparison is the useful relationship. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 `[fix here]` · 0 `[follow-up]`. No findings across architecture, security, performance, cross-platform, or tests. Deterministic checks: `npm run lint` passed; `npm test` passed 1,017/1,017; `npm run docs:build` passed. </details> <details> <summary>Implementation notes</summary> The screenshots were captured from an isolated local clone and a throwaway Electron profile. The GitHub device flow was canceled immediately after capture without authorizing an account. </details> <details open> <summary>Screenshots or recording</summary> Four screenshots are included directly in the relevant guide pages: submission diff, submission destinations, GitHub sign-in, and trunk-update progress. </details>

## Why Several recovery paths were described only in prose, which made contributors translate an unfamiliar warning or failed apply into the words in the guide. The Mail guide also claimed to show a captured message while its screenshot showed the empty state. ## What changes - Adds screenshots for an incomplete trunk update and a pull request that cannot be applied without changing the checkout. - Replaces the empty Mail screenshot with a deterministic captured-message row. - Extends the screenshot fixtures and catalogue so the Mail and update-warning images can be regenerated. - Pins the screenshot harness locale and timezone, and excludes the ephemeral SMTP port from the Mail crop. Adminer is deliberately not added here: its UI exists only while a real WordPress Playground server is running, so a fixture would not be representative. That remains a live-capture follow-up. ## How to test this Platforms: any. The fixture screenshots should render identically across supported platforms. **Starting state:** this branch checked out with the root and `docs/` dependencies installed. 1. Run `npm run shots -- --only=mail-panel`. Expect `mail-panel.png` to show one row with time, sender, and subject, without the SMTP port. 2. Run `npm run shots -- --only=update-incomplete`. Expect `update-incomplete.png` to show the red notice and **Retry install & build** button. 3. Run `npm run docs:dev`, then open **Mail**, **Applying patches and PRs**, **Keeping a site up to date with trunk**, and **Troubleshooting**. Expect each screenshot to sit next to the state it explains and remain legible at the documentation width. 4. Run `npm run lint`, `npm test`, and `npm run docs:build`. Expect all commands to pass. **What must not have happened:** the fixture must not use the contributor's real site registry, send mail externally, expose a personal path, or include a changing SMTP port. The failed-apply screenshot must clearly say that the checkout was not changed. The conflict image cannot be recreated fully automatically: it needs an isolated real checkout and a deliberately incompatible patch or pull request. Its live-tier instructions record that setup. ## Risks and limitations The conflict capture remains maintainer-driven because fake repository state would not prove the app's all-or-nothing behavior. Adminer still needs a separate live capture against a running Playground site. Self-review: 0 `[fix here]` · 0 `[follow-up]` after fixing two low-severity determinism findings. ## Related Stacked on #336. --- <details> <summary>Design decisions and alternatives considered</summary> Persisted fixture mail is loaded when the dev server starts, so the harness starts the fixture server instead of injecting renderer state. The committed crop is limited to the message list: it teaches sender, subject, and time while omitting the server's intentionally ephemeral port. The update warning is fixture-tier because `updateIncomplete` is persisted site metadata. The failed apply remains live-tier because its credibility depends on a real checkout and patch diagnosis. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 `[fix here]` · 0 `[follow-up]` after fixes. The independent pass initially found that the Mail timestamp inherited the host locale/timezone and that the card included a changing SMTP port. The harness now pins `en-GB`/UTC and crops the deterministic email list. Captures made under the default environment and under `TZ=America/New_York LANG=es_ES.UTF-8` had zero differing pixels. No findings remain across architecture, security, performance, cross-platform, or tests. </details> <details> <summary>Implementation notes</summary> Verification passed: both fixture captures, ESLint, 1,017 Node tests, VitePress production build, and `git diff --check`. </details> <details> <summary>Screenshots or recording</summary> ![A captured email with its time, sender, and subject](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/document-diagnostic-states/docs/public/screenshots/mail-panel.png) ![The Update incomplete notice](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/document-diagnostic-states/docs/public/screenshots/update-incomplete.png) ![A pull request that does not fit the checkout](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/document-diagnostic-states/docs/public/screenshots/apply-patch-conflict.png) </details>

## Why Creating a WordPress core development site downloads the repository, installs its dependencies, and runs a full build. Doing that for every participant at the start of a Contributor Day puts unnecessary pressure on the venue's shared connection and delays contributions. ## What changes The landing page now recommends creating the first site at home before the event and links directly to **Update to latest trunk**. The Getting Started guide explains why the initial setup downloads many files and how arriving with one prepared site lets contributors fetch only recent changes, reusing cached packages where possible. ## How to test this Platforms: any. This changes only the documentation site. **Starting state:** this branch checked out with the `docs/` dependencies installed. 1. Run `npm run docs:dev` and open the landing page. 2. Find **Made for Contributor Days**. Expect it to recommend creating a site before the event. 3. Click **Update to latest trunk**. Expect the trunk-update guide to open. 4. Open **Getting started**. Expect a **Prepare at home before Contributor Day** tip before **Install the app**. 5. Follow the link in that tip. Expect the same trunk-update guide to open. 6. Run `npm run docs:build`. Expect the production build to finish without errors. **What must not have happened:** the landing page must not display Markdown brackets as literal text, and the copy must not claim that a trunk update can never reinstall dependencies. ## Risks and limitations The exact download savings depend on how much trunk and `package-lock.json` changed since setup. The copy says dependencies are reinstalled when needed and packages are generally reused from the cache; it does not promise a fixed download size. Self-review: 0 `[fix here]` · 0 `[follow-up]`. ## Related Stacked on #337, with #336 as the first PR in the stack. --- <details> <summary>Design decisions and alternatives considered</summary> The recommendation appears as a short linked feature on the landing page and a fuller tip in Getting Started. VitePress does not parse Markdown inside a home feature's `details` field, so the feature uses the supported `link` and `linkText` fields instead. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 `[fix here]` · 0 `[follow-up]`. No findings across architecture, security, performance, cross-platform, or tests. The reviewer also verified that the wording matches the app's shallow clone, conditional dependency installation, package cache, and trunk-update behavior. </details> <details> <summary>Implementation notes</summary> Verification passed: ESLint, 1,017 Node tests, VitePress production build, `git diff --check`, and inspection of the generated internal links. </details>

## Why WordPress Contributor Toolkit RC2 writes `core.autocrlf=true` into every managed repository's local Git configuration, including on macOS/Linux and when a contributor explicitly chose `false` or `input`. The compatibility workaround is needed for native-Git CRLF checkouts on Windows, but it must not silently change persistent repository policy. ## What changes `isomorphic-git` now receives a Windows-only, in-memory view of `core.autocrlf=true` when the local value is unset. Explicit local values pass through unchanged, non-Windows platforms use the original filesystem, and config-read failures emit a diagnostic instead of being swallowed. Status, patch generation, ticket parking, discard, and trunk-update operations use that scoped filesystem view. Nothing writes `.git/config`. ## How to test this **Platform: Windows 11.** Use the signed Buildkite artifact for the current PR head commit. The procedure uses only PowerShell and tools bundled with the app; Git, Node, npm, and Docker are not required on the host. **Starting state:** A disposable site created by the toolkit with setup complete. From **Open directory in → File Explorer**, open PowerShell in the site's root. Confirm `Test-Path ".git\config"` and `Test-Path "src\wp-login.php"` both return `True`. 1. Back up `.git/config`, then remove only `autocrlf` from its `[core]` section: ```powershell $configPath = Join-Path $PWD ".git\config" $configBackup = Join-Path $env:TEMP "toolkit-git-config.backup" Copy-Item $configPath $configBackup -Force $lines = [IO.File]::ReadAllLines($configPath) $inCore = $false $filtered = foreach ($line in $lines) { if ($line -match '^\s*\[([^\]]+)\]\s*$') { $inCore = $Matches[1] -ieq 'core' } if ($inCore -and $line -match '^\s*autocrlf\s*=') { continue } $line } $utf8NoBom = New-Object System.Text.UTF8Encoding($false) [IO.File]::WriteAllLines($configPath, $filtered, $utf8NoBom) Select-String -Path $configPath -Pattern '^\s*autocrlf\s*=' ``` Expected: `Select-String` prints nothing. 2. Convert one tracked WordPress file to CRLF without Git and record the configuration hash: ```powershell $target = Join-Path $PWD "src\wp-login.php" $targetBackup = Join-Path $env:TEMP "toolkit-wp-login.php.backup" Copy-Item $target $targetBackup -Force $content = [IO.File]::ReadAllText($target) $content = $content.Replace("`r`n", "`n").Replace("`n", "`r`n") [IO.File]::WriteAllText($target, $content, $utf8NoBom) ([IO.File]::ReadAllText($target)).Contains("`r`n") $configBefore = (Get-FileHash $configPath -Algorithm SHA256).Hash ``` Expected: the line-ending check returns `True`. 3. Return focus to the toolkit and wait for status to refresh. Expected: the site remains clean. No unassigned-changes notice appears and there are no phantom modifications. Back in PowerShell, `(Get-FileHash $configPath -Algorithm SHA256).Hash -eq $configBefore` returns `True`. 4. Create one real file: ```powershell $testFile = Join-Path $PWD "src\autocrlf-toolkit-test.txt" [IO.File]::WriteAllText($testFile, "toolkit autocrlf test`r`n", $utf8NoBom) ``` 5. Return focus to the toolkit and click **create and save a patch** in the one-change notice. Expected: **Review & submit changes** contains only `src/autocrlf-toolkit-test.txt` and `+toolkit autocrlf test`. `src/wp-login.php` and other CRLF-only files do not appear. Close the modal; the `.git/config` hash must still equal `$configBefore`. 6. Delete the real change with `Remove-Item $testFile`, return focus to the toolkit, and confirm the site becomes clean again. Keep `wp-login.php` in CRLF. Choose **More → Update to latest trunk**. Expected: the update starts without a dirty-tree warning and completes successfully. Dependencies remain unchanged; a rebuild may run if trunk changed. 7. Verify the final state: ```powershell $configAfterUpdate = (Get-FileHash $configPath -Algorithm SHA256).Hash $configBefore -eq $configAfterUpdate ([IO.File]::ReadAllText($target)).Contains("`r`n") ``` Expected: both expressions return `True`. Do not restore the old `wp-login.php` backup after updating trunk because it may contain an earlier revision. Delete the disposable site through the app when finished. **What must not have happened:** `.git/config` must not change; the patch must not contain CRLF-only WordPress files; status and patch checks must not remove or rebuild `node_modules`; the trunk update must not reinstall unchanged dependencies. Automated checks: - `npm run lint` - `npm test` — 1,027 passed - `npm run test:electron` — 1,027 passed The automated suite also covers explicit `core.autocrlf` values `true`, `false`, and `input`, non-Windows behavior, config-read failures, and worktree-style `.git` files. ## Risks and limitations Repositories already modified by RC2 retain `core.autocrlf=true`. The app cannot safely distinguish its historical write from a contributor's intentional value, so automatic cleanup is deliberately out of scope. There is no visible UI change. The signed Windows artifact for commit `1957967` was tested manually on Windows 11. Status, patch generation, and trunk update all handled the CRLF fixture correctly; `.git/config` remained byte-for-byte unchanged and the update kept dependencies unchanged. ## Related Fixes #341 --- <details> <summary>Design decisions and alternatives considered</summary> Temporarily writing and restoring `.git/config` was rejected because a crash or concurrent operation could leave the temporary value behind. The filesystem wrapper instead changes only what `isomorphic-git` sees for the duration of an operation. Explicit `false` and `input` values are honored rather than virtually overridden. A checkout inconsistent with its explicit local policy may therefore appear dirty; preserving that policy is preferable to silently second-guessing it. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 `[fix here]` · 1 `[follow-up]`. The follow-up is the known RC2 residue described under Risks and limitations. It is deferred because removing an existing `true` value could erase an intentional contributor setting. No new findings across architecture, security, performance, cross-platform, or tests. </details> <details> <summary>Implementation notes</summary> The wrapper supports both ordinary `.git/config` directories and worktree-style `.git` files. Tests inject Windows and non-Windows platforms from one machine and cover unset, `true`, `false`, `input`, config-read failures, and a worktree file named `config`. </details>

## Why `v1.0.0-rc.2` shipped on 13 August as the second release candidate. Testing it on Windows found one behaviour bug, now fixed on `trunk` (#342: the CRLF workaround wrote `core.autocrlf` into every managed repository's config, on every platform). The other four commits since that tag are documentation and guide screenshots (#336, #337, #339). Nothing else is outstanding, so the next version is the stable `1.0.0`. The package version must move before its tag and artifacts are created, because `electron-builder` derives artifact names from `package.json`. Two metadata corrections, no application behavior change. ## What changes **The version.** All three package-version fields in `package.json` and `package-lock.json` move together from `1.0.0-rc.2` to `1.0.0`. No dependency versions change. **`homepage`.** It still pointed at `https://github.com/WordPress/experimental-wp-dev-env`, the repository's name before the rename, and it pointed at a repository rather than at a place a contributor would want to land. It now points at the documentation site, `https://wordpress.github.io/contributor-toolkit/` — the Pages deployment `docs/.vitepress/config.mjs` builds with `base: '/contributor-toolkit/'`. Shipping a 1.0 whose package metadata names the old repository is the reason to fix it now rather than later. **`repository` and `bugs`, which did not exist.** `homepage` was the only field naming the repository, so pointing it at the documentation site would have left the package metadata with no reference to the repository at all. Both fields are now present and name `contributor-toolkit`, which is where that information belongs. `electron-builder` derives artifact names from `package.json`, producing: - `wordpress-contributor-toolkit-1.0.0-mac-arm64.dmg` - `wordpress-contributor-toolkit-1.0.0-win-x64.exe` - `wordpress-contributor-toolkit-1.0.0-linux-x86_64.AppImage` ## How to test this Platforms: any; this is package metadata and is platform-independent. **Starting state:** this branch checked out with dependencies installed. 1. Run `node -p "require('./package.json').version"`. - Expected: `1.0.0`. 2. Run `node -e "const p=require('./package-lock.json'); console.log(p.version, p.packages[''].version)"`. - Expected: `1.0.0 1.0.0`. 3. Run `node -p "require('./package.json').homepage"`. - Expected: `https://wordpress.github.io/contributor-toolkit/`, and opening it in a browser reaches the toolkit's documentation site rather than a 404. 4. Run `npm run lint` and `npm test`. - Expected: lint succeeds and all 1,027 tests pass. **What must not have happened:** dependency versions and resolved packages must remain unchanged; no source, documentation, workflow, or build configuration file should be in the diff. ## Risks and limitations No user-visible behavior changed, so there is no desktop flow or screenshot to test by hand. The signed artifacts will only exist after this PR lands and the release build runs. Dropping the pre-release suffix means the resulting GitHub release is the first one to take the **Latest** badge from `v0.1.2`. ## Related Follow-up to #335. Ships the fixes in #342, #336, #337 and #339 as the stable 1.0. --- <details> <summary>Design decisions and alternatives considered</summary> The version goes straight to `1.0.0` rather than to a third release candidate: the only behaviour change since rc.2 is #342, which was itself the finding rc.2 existed to surface, and it has been verified on Windows. The release tag will add the conventional `v` prefix; package metadata does not. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `0 [fix here] · 0 [follow-up]`. No findings across architecture, security, performance, cross-platform, or tests. `npm run lint`, `npm test` (1,027/1,027), and `git diff --check` pass. The stale `homepage` was itself the review's one observation, and is fixed in this PR. Nothing reads `homepage` at runtime, and `electron-builder` has no `publish` block that would derive an update feed from it, so the change is metadata only. Verified as part of the review: no source, script, docs, `.buildkite/` or workflow file embeds the app version (`src/logging.js:78` reads it via `app.getVersion()`); the lockfile diff carries no dependency or integrity churn; `electron-builder` is prerelease-agnostic — the artifact template at `package.json:30` is `…-${version}-${os}-${arch}.${ext}` and the repo has no `publish` block, no channel and no auto-updater, so dropping the suffix only shortens the filenames; and nothing in CI keys off a tag or version pattern. One consequence worth stating: `docs/.vitepress/theme/DownloadButton.vue` resolves `releases/latest`, which the GitHub API defines as excluding pre-releases. Since `v0.1.2` the button has been falling back to the Releases page for everyone. Publishing 1.0.0 as a non-prerelease is what makes it resolve a real asset again. </details> <details> <summary>Implementation notes</summary> `npm version 1.0.0 --no-git-tag-version` keeps `package.json`, the lockfile root `version` and `packages[""].version` synchronized. The diff was checked to confirm those are the only changed values. </details>

> **Stacked on #343.** Base branch is `juanmaguitar/Release-v1.0`, not `trunk`. Merge #343 first. ## Why The repository was renamed from `experimental-wp-dev-env` to `contributor-toolkit`. GitHub redirects the old URLs, so nothing is broken — which is exactly why the stale references survived. But they are user-facing, and 1.0 is the release that gets read: the README badges are the first thing on the repository page, and two of them are shields.io images built from the repository name rather than plain links. #343 fixed `package.json`. This is the rest of it. ## What changes Nine occurrences across six files, all of them `WordPress/experimental-wp-dev-env` → `WordPress/contributor-toolkit`: - `README.md` — the unit-tests, latest-release and downloads badges (image source and link target). - `STATS.md` — the two links to the `metrics` orphan branch and to `downloads.csv` on it. - `.github/ISSUE_TEMPLATE/Bug_report.yml` — the "search existing issues" URL shown to a reporter. - `.github/ISSUE_TEMPLATE/installation-bug.yml` — the download-source placeholder. - `src/trac-view.js` and `src/github-http.cjs` — the `+https://…` project URL inside the `WordPress-Contributor-Toolkit` user agent sent to Trac and to the GitHub API. The identifying prefix is unchanged; only the URL a server operator would follow moves. **Deliberately kept:** the old name in the comments at `docs/.vitepress/config.mjs:9` and `.github/workflows/docs.yml:14`. Both describe the rename itself and explain why the Pages base path is derived at run time rather than hardcoded, so the old name is the point of the sentence. The workflow comment did still say the repository "is being renamed"; it now matches the VitePress one and says the rename has already happened, which is also the accurate reason to keep deriving the path — the next rename, not this one. ## How to test this Platforms: any; this is text and metadata. **Starting state:** this branch checked out with dependencies installed. 1. Run `grep -rn "experimental-wp-dev-env" . | grep -v node_modules | grep -v "^./.git"`. - Expected: exactly two hits, the two explanatory comments named above. 2. Open the rendered `README.md` on this branch and confirm all three badges load an image rather than a broken-image icon, and that clicking each lands on the `contributor-toolkit` repository. 3. Follow both links in `STATS.md` and confirm they reach the `metrics` branch and `downloads.csv`. 4. Open **New issue** on this branch's fork and confirm both templates render, with the corrected URL in the bug-report description and the corrected placeholder in the installation-bug form. 5. Run `npm run lint` and `npm test`. - Expected: lint succeeds and all 1,027 tests pass. **What must not have happened:** the `WordPress-Contributor-Toolkit` prefix of either user agent must be unchanged — `test/github-http.test.cjs:56` asserts on it, and Trac's proof-of-work session is identified by it. ## Risks and limitations The user-agent change is the only line that leaves the machine. Its value is not persisted, not used as a cache key and not part of the Trac session partition (`persist:trac`), so an existing site's cleared proof-of-work challenge survives the change. Nothing here can be verified from the diff alone for the badges: shields.io builds them server-side from the repository name, so they can only be confirmed on the rendered page. ## Related Stacked on #343, which fixed the same stale name in `package.json` and added the missing `repository` and `bugs` fields. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> `0 [fix here] · 1 [follow-up, fixed here]`. No findings across architecture, security, performance, cross-platform or tests. `npm run lint` and `npm test` (1,027/1,027) pass. The follow-up was the tense of the `docs.yml` comment, which is in this PR's own subject matter and so was fixed here rather than deferred. Verified as part of the review: no test, fixture or snapshot pins the old user-agent string — the only assertion is `test/github-http.test.cjs:56` on the `WordPress-Contributor-Toolkit` prefix, and `test/trac-view.test.cjs:39` stubs `setUserAgent()` without inspecting its argument. Neither constant is value-sensitive: `src/github-http.cjs:91` only sets a request header, and the Trac partition is the literal `'persist:trac'` (`src/trac-view.js:27`), not derived from the user agent, so no cleared proof-of-work session is orphaned. `download-stats.yml:68` already uses `$GITHUB_REPOSITORY`, so the metrics branch keeps writing under whatever the repository is called. </details>

## Why `v1.0.0` is published, but the repository's front page still describes the app as it was at `v0.1.x`: a thing that sets up a development environment with no prerequisites, full stop. Nothing that 1.0 is actually about appears on it — linking a Trac ticket, reading its facts, applying the pull requests and patches already on it, holding work for several tickets in one site, debugging, or sending a change back as a pull request, a Trac attachment or a mentor handoff. `### Why` had the same problem in argument form: it makes the case against setup friction, which is the wall `v0.1.x` removed, and stops there. Someone arriving from the Make announcement or from the release page reads this file first. It was underselling the release they had just heard about. ## What changes **`README.md`.** The top of the file only; every section about contributing to *this repository* — `Build from source`, `App icon`, `Why Electron?`, `Ideas and future work`, `Download stats`, `Contributing`, `License` — is current and untouched. - The description paragraph now covers the whole loop rather than only setup, including the two guarantees that are easy to miss and hard to re-derive: nothing installed on the host, and no push credential written to disk. - `### Why` keeps the Contributor-Day setup argument as it was and adds the second wall: a newcomer with a running environment still has to find the existing patch, get it into the checkout, understand a failed apply, and work out that WordPress reviews on Trac but takes pull requests on GitHub. - A new **What you can do with it** section: seven capabilities, each linking the guide page that already documents it, so the README stays a map and the guide stays the manual. - Both YouTube blocks are removed. They show the pre-1.0 interface, and the second one demonstrates the patch screen that 1.0 reworked. Their thumbnails, `docs/setup-start.png` and `docs/create-patch.png`, are deleted with them — the README was their only reference. - One current screenshot near the top, `site-view.png`, served from the docs site. **This was not asked for:** removing two images and adding none leaves the page worse, and this is the image the release notes and the announcement both open with. Say so if you would rather the README carried no image. **`package.json`.** `description` was `Electron app to setup WordPress develop structure and run npm install`. electron-builder carries this into the packaged application's metadata, so it is user-visible, and it described a subset of `v0.1.x`. It now describes the app. No other field changes. **Not in this PR, for JuanMa to apply in Settings:** the GitHub repository description is still "An experiment to explore an easy to install core WordPress development environment app". Proposed replacement, same sentence as the new `package.json` description: > Desktop app that sets up a WordPress core development environment with no prerequisites, applies > the work already on a Trac ticket, and submits a contribution back. ## How to test this Platforms: any; this is documentation and package metadata. 1. Run `npm run lint` and `npm test`. - Expected: lint succeeds and all 1,027 tests pass. 2. Run `npm run docs:build`. - Expected: the VitePress build completes, confirming nothing under `docs/` referenced the two deleted images. It does **not** validate the README's links: `README.md` is not part of the docs source tree, and the links are absolute external URLs, which `ignoreDeadLinks` does not reach. Step 3 is what checks those. 3. From the repository root, confirm every guide page the README links actually exists: ```bash for p in $(grep -o 'contributor-toolkit/guide/[a-z-]*' README.md | sed 's|.*/guide/||' | sort -u); do test -f "docs/guide/$p.md" && echo "ok $p" || echo "MISSING $p" done ``` - Expected: 16 lines, all `ok`. 4. View `README.md` rendered on this branch on GitHub. - Expected: the three badges load, the screenshot loads, and every link in **What you can do with it** opens a real guide page rather than a 404. 5. Run `node -p "require('./package.json').description"`. - Expected: the new sentence. **What must not have happened:** no source file, workflow or build configuration in the diff; the version and every dependency unchanged. ## Risks and limitations The main risk in a change like this is a claim that reads well and is not true. Every capability listed was checked against the guide page it links and against the shipped behaviour rather than against memory; the review below covers that specifically. The screenshot is hotlinked from the docs site rather than committed, so it tracks whatever the shots harness last published. That keeps it from going stale silently, at the cost of depending on Pages being up — the same trade the release notes make. The new `description` is 163 characters. It reaches the AppImage and deb `.desktop` `Comment`, the deb control `Description` and the snap `description`; no length ceiling is hit and no build breaks. Snap's 78-character limit applies to `summary`, which electron-builder fills from `productName`. One cosmetic consequence: Debian's `lintian` warns `description-too-long` on a synopsis over 80 characters, so the `.deb` picks up a warning it did not have before. Not a build failure, not user-visible. ## Related Follows `v1.0.0` (#343, #344). --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> `0 [fix here] · 0 [follow-up]`. No findings across architecture, security, performance, cross-platform or tests. `npm run lint`, `npm test` (1,027/1,027) and `npm run docs:build` pass. Because the risk in this change is a claim that reads well and is false, the review was pointed at the claims themselves. Each was traced to code, not to memory: - *no push credential written to disk* — the token exists only as `githubToken` in `src/main.js:775`, cleared at `:783`, passed by value into `src/github-pr.cjs`. No `store.set`, no keychain, no file write. - *says which regions failed and why, and leaves the checkout untouched* — `src/patch-apply.js` validates before writing. The one path that does write is a rollback after a disk error mid-write (`src/patch-apply.js:526-530`), which is a write failure rather than a patch that will not apply; the guide draws the line in the same place. - *each ticket gets its own branch inside the site* — `src/ticket-branches.js:4-23`. - *fatals surfaced instead of hidden behind the recovery screen* — `src/wp-debug-constants.js:47` sets `WP_DISABLE_FATAL_ERROR_HANDLER: true`. - *a terminal using the Node.js runtime the app bundles* — `src/script-runner.js:26,36` spawn `process.execPath`; `src/npm-runner.js:88` sets `ELECTRON_RUN_AS_NODE=1`. - *keeps your name on the work* — `src/patch-provenance.cjs`. - *fetching only what changed* — `src/trunk-update.js:269-273`, `depth: 1`. - *with a rebuild when one is needed* — conditional rather than decorative: `src/renderer/update-plan.cjs:144` skips the build when the watcher is live. - The Trac field list maps 1:1 onto `src/trac-ticket-info.cjs:133-142`. Also verified: no doc page, workflow, issue template or `scripts/screenshots/` entry referenced either deleted PNG or either YouTube URL; every guide target exists; and the extensionless link form is what the file already used on `trunk` and what Pages resolves through its `.html` fallback. The review corrected one claim in this PR's own test plan: `npm run docs:build` does not validate the README's links, because `README.md` is not in the docs source tree and the links are absolute external URLs. Step 2 above has been rewritten to say so, and step 3 is what actually checks them. </details>

…362) ## Why Nothing in the suite has ever launched the app. The Git modules are covered against real repositories and the IPC handlers against a loaded main process, but the flows a contributor actually performs — linking a ticket, applying a patch, moving between branches — are verified only by a person following a **How to test this** section by hand. That matters now because of #350. The git-native refactor moves ticket branches, applied patches, conflicts and trunk updates all at once, and a characterisation suite written afterwards can only record the refactor. Written first, it says which of the old guarantees were quietly dropped — which is the failure mode here, since nearly every regression in this area is silent: work parked and never restored, a patch quietly missing a file, a ticket that forgets its base. This PR is the engine. The journeys themselves are #361. ## What changes Two suites, because there are two different apps under test and they answer different questions. **`npm run test:e2e` — the journeys.** Drives the app built from the source tree and writes real state. Everything a journey needs before it can assert anything lives in one session helper, so no spec has to remember it: - **It cannot reach your own sites.** Every launch goes through the development-only data-directory redirect, and then reads back the path the app actually chose and refuses to continue if it is any other one. The env var is set last and is not overridable by the ambient environment. - **Teardown always terminates.** `close()` is raced against a timeout and the process killed after it — closing is known to hang on Windows in apps that keep child processes alive, which this one does constantly. - **A failure leaves evidence.** The trace is Playwright's; the screen and the state the app had persisted are attached here, because the interesting half of a failure in this app is on disk rather than on screen. - **It can relaunch against the same profile.** This is not a convenience. It is the only way to tell "the app persisted this" from "the app still had it in memory", and it is exactly what a change to the storage layer breaks without any other test noticing. **`npm run test:e2e:packaged` — the packaged smoke test.** Launches an unsigned `--dir` build and asks only whether packaging worked. This is the test from the now-closed #70, **rebuilt rather than rebased** — see below. Both run on macOS and Windows for every non-draft pull request, as two separate jobs so a five-second suite is not waiting behind a fifteen-minute one. Neither downloads a browser: the only thing launched is the Electron already in the tree, so there is no `playwright install` step anywhere. **Deliberately not in this PR:** any journey. `e2e/journeys/engine.spec.js` asserts nothing about ticket branches or patches — it asserts that the four things every journey will depend on actually work, so that when a journey fails it is about the flow and not about the harness. ## How to test this Platforms: **any** for the journeys. The packaged half is macOS or Windows only, and the point of the CI matrix is that Windows is the half nobody can check locally. **Starting state:** a clean checkout of this branch, `npm ci` done. Note the modification time of your real settings file before you start — on macOS, `~/Library/Application Support/electron-setup-wordpress-core/settings.json`. 1. `npm run build:once && npm run test:e2e` - Expected: 4 passed, in a few seconds. A real Electron window opens and closes four times. 2. `npm test` - Expected: **1027 passed** — the same count as on `trunk`. The unit runner only collects `test/`, so it must not pick up anything added here. 3. `npm run lint` - Expected: clean. It is repo-wide. 4. `CSC_IDENTITY_AUTO_DISCOVERY=false npm run pack:dir && npm run test:e2e:packaged` - Expected: 4 passed. Skip this if you would rather read the CI result — it takes several minutes. 5. Break the guard on purpose: in `e2e/helpers/app.cjs`, change `TOOLKIT_USER_DATA_DIR: this.userDataDir` to `TOOLKIT_USER_DATA_DIR: undefined`, then `npm run test:e2e`. - Expected: all four tests fail, each saying the app is using your real profile and that it refuses to run. Put the line back. 6. Break an assertion on purpose: in `e2e/packaged/smoke.spec.js`, delete any one entry from `EXPECTED_API_KEYS`, then `npm run test:e2e:packaged`. - Expected: the bridge test fails and names the missing key. Put it back. **What must not have happened:** - **Your real settings file must not have been touched.** Compare its modification time with what you noted in the starting state. This is the whole reason step 5 exists: the guard is the only thing standing between a future change to the redirect hook and a suite that silently edits the site list you actually work on. - **`npm test` must not have grown.** If the count moved off 1027, the unit runner started collecting end-to-end specs, and `npm test` stops being something you run without thinking. - **No Electron processes left running** after any of the above. `ps aux | grep -i electron` on macOS, Task Manager on Windows. ## Risks and limitations - **Windows is unverified by hand.** Everything above was run on macOS. Two of the four fixes in this branch are specifically about Windows behaviour I could not reproduce locally — path shapes and file handles — so the CI run on this PR is the first real evidence either way. That is the honest state of it. - **CI time.** The packaged job packages on both platforms for every non-draft PR: roughly ten to fifteen minutes per platform. The journeys job takes seconds. If the feedback loop gets annoying, the packaging job is what to trim, not the journeys. - **The trunk update cannot be driven from a journey at all**, on any platform. It reaches for a hardcoded clone URL rather than the checkout's own remote, so exercising it means cloning from the network — unacceptable in CI. Its two interesting cases stay in the integration suite, against a local server. Making the update read the remote is worth doing on its own merits and belongs in #350; I left a note there. - The teardown's last-resort kill is a bare `kill()` rather than `src/kill-tree.js`. It only runs after `close()` has already had its timeout, and it is test code, but if orphaned Electron processes ever show up on the runners, that is where to look. ## Related Part of #359. Closes #360. **Closes #67** — its three assertions ship here as the `packaged` project, on both platforms, documented in `CONTRIBUTING.md`, with Buildkite untouched. Replaces #70, which is closed. Unblocks #361, and #350 after it. --- <details> <summary>Design decisions and alternatives considered</summary> **`@playwright/test` rather than `node --test` with `playwright-core`.** The first draft went the other way, to keep one runner in the repo and add no dependency. Requiring the suite to run in CI on two platforms for every pull request inverted that: retries, per-test timeouts, traces, the HTML report and the reporting artifacts are all things I would otherwise have written and then maintained, badly. Two runners is not a defect here — `npm test` stays the fast offline suite and `npm run test:e2e` is the one that launches an app, and they are meant to be told apart. The dependency cost turned out to be smaller than assumed: `@playwright/test` at this version has no install script and downloads no browser. I checked the published tarball rather than trusting the recollection — an older version of Playwright did download browsers on install, which is why `scripts/screenshots/capture.cjs` depends on `playwright-core` directly and says so in its header. **A session object rather than a `page` fixture.** A journey has to build its Git fixture *before* the app opens, because the site list has to name directories that already exist, and it needs the profile path to write that list. A fixture that launches the app for you takes that away. So the profile directory exists from construction and the spec decides when to start. **Rebuilt #70 rather than rebasing it.** Two of its three assertions had gone stale in ways that would fail on first run: the list of bridged keys had roughly half the entries the app exposes today, and the native module it probes is no longer optional and no longer absent on Windows — it is a hard dependency of the bundled PHP runtime with a Windows-specific file-lock path — so its documented Windows exclusion had turned from a decision into a hole. Its structure, its comments and its reasoning are otherwise carried over almost verbatim; the closing comment on #70 lists what came across. **Not adding `data-testid` anywhere.** The selectors use the text and roles already on screen, which also pins the visible copy as a contract. Where a name appears twice — the sidebar entry and the heading of the open site — roles tell them apart, and the assertion says which half of the app it is about. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `2 [fix here] · 2 [follow-up]` — both `[fix here]` fixed before this was pushed. **🔴 cross-platform · `[fix here]` — fixed.** The launch guard compared `path.resolve(a) !== path.resolve(b)`. That would have aborted every test on Windows, and possibly on macOS, for reasons unrelated to what it guards: macOS returns `/var/folders/…` from `os.tmpdir()` and `/private/var/…` from the app, since the first is a symlink to the second, and Windows returns short `RUNNER~1` names in some environments and long ones in others, on a case-insensitive filesystem. Replaced with a `samePath()` that resolves through `realpath` and lowercases off POSIX. **🔴 cross-platform · `[fix here]` — fixed.** The fake site directories were removed in an `afterEach`, which Playwright runs *before* fixture teardown — with the app still alive and holding handles on them. On Windows that is `EPERM`, and a failure that is not the test's. Cleanup moved onto the session, which removes them after `close()`. **🟡 tests · `[follow-up]`.** The teardown's last-resort `kill()` is not `src/kill-tree.js`. Noted in **Risks** above. **🔵 performance · `[follow-up]`.** The packaging job's cost per pull request. Noted in **Risks** above. No findings in architecture (nothing under `src/` changes), security (the file-dialog stub is installed at runtime from the test, so nothing test-shaped ships in the app), or the new-dependency rule (devDependency, no install script — verified against the published tarball — no native compilation, so no `allowScripts` entry is needed and the zero-prerequisite promise is untouched). **One caveat.** The standard asks for this pass to run in fresh context rather than in the session that wrote the code. It did not: the session that wrote this reviewed it. Worth a second pass by someone who has not already decided it is correct. </details> <details> <summary>Implementation notes</summary> Two things surfaced only by running this against the current tree, neither of which #70 could have seen: **`process.mainModule.filename` is not stable in Electron.** The module-resolution assertions anchored a `createRequire` there. Partway through a run it reads back as the bare string `'electron'`, so the first module checked failed and the same test passed when run alone — a packaging test going red for a reason that has nothing to do with packaging. Now anchored at `join(app.getAppPath(), 'package.json')`, which is deterministic and is inside the asar either way. **Two bridged keys cannot be found by reading `src/preload.js`.** `signInToGithub` and `cancelGithubSignIn` are spread in from an immediately invoked function that closes over a listener, so they exist only once the file has run. Any attempt to derive the expected-keys list by grepping the object literal misses them — which is the same class of gap the assertion itself exists to close, one level up. There is a comment in the file saying so, because the next person to regenerate that list will otherwise reintroduce the same two-key error. The `react-hooks/rules-of-hooks` override for `e2e/` is not a suppression of a real problem: Playwright's fixture API is `async ({ deps }, use) => {}`, and the rule reads a bare `use(...)` as React's `use` hook outside a component. There is no React in that directory; the app under test is a separate process. </details> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

**Stacked on #362.** Review that one first; this diff is only the three files under `e2e/` that this PR adds. ## Why #362 built an engine and proved it works. This is the first thing it was built for: the ticket-branch flow, driven through the app rather than through a module. That flow is what a contributor spends a Contributor Day inside — link a ticket, edit a file, realise it belongs to another ticket, come back later — and it is the flow #350 moves wholesale. Today it is verified by a person following a **How to test this** section. After this, it is verified on macOS and Windows on every pull request. It is also the flow whose failures are silent. Work that is not restored produces no error; the app shows a clean tree and the contributor discovers the loss when a reviewer asks where the change went. ## What changes Four journeys, and one addition to the engine. - **Linking a ticket** creates its branch, checks it out, and leaves the gitignored substrate alone. - **Unlinking parks the work**, and the next ticket starts from trunk with none of the first one's edits. - **Switching back restores the work byte for byte** — the edit *and* the deletion. An edit that returns while a deletion does not is a half-restored tree, which is worse than an obvious failure. - **Deleting a ticket's work** removes that branch and only that branch, after asking. **Every assertion is marked `INVARIANT` or `CHARACTERISATION`,** and that is the point of the file rather than decoration. An invariant must hold under any model of how work is stored; a characterisation is true because of how the app stores things today. During #350, a red invariant is a bug and a red characterisation is a prompt to read it, decide whether the new model is what you meant, and update it on purpose. Without the distinction every failure looks the same, and the suite becomes noise at exactly the moment it is supposed to be useful. **Two things the app does turned out not to match how the flow reads from the outside.** The tests follow the app: - Linking a second ticket means unlinking the first. Once a ticket is linked, its card shows that ticket's pull requests and attachments and the ticket-number field is not on screen at all. Unlinking is not throwing the ticket away — it parks the work on its branch and returns the checkout to trunk — but it is a step, and a test that skipped it would exercise a path no contributor can take. - A ticket cannot be deleted while it is the linked one, because the list of a site's tickets deliberately leaves out the current one. So the `wasActive` branch in the delete handler — the one that returns the checkout to trunk and clears the link — appears to be unreachable from the interface. Left alone here; noting it because it is the kind of thing worth knowing before #350 rewrites around it. **The engine grows one thing: answering `window.confirm`.** Electron implements the JavaScript dialogs natively and blocks the renderer on them, so Playwright's `dialog` event never arrives — a test relying on it clicks "Delete this ticket's work" and watches nothing happen. Replacing `window.confirm` in the page works, and counting the calls lets a test assert that the app *asked* before it deleted anything. **Deliberately not in this PR:** applying and reverting patches, and what survives a restart. Those are the next two in the stack. ## How to test this Platforms: **any**. Windows is covered by CI, which is where the interesting half is. **Starting state:** this branch, `npm ci` done, `npm run build:once` run once. 1. `npm run test:e2e` - Expected: 8 passed in under ten seconds — the four from #362 and the four here. Electron windows open and close as it goes. 2. Watch one of them rather than trusting the count: `npx playwright test --project=journeys -g "switching back" --headed` - Expected: the app opens, a ticket is linked, another, and the panel switches back. Nothing is typed by a human. 3. Break an invariant on purpose. In `e2e/journeys/ticket-branches.spec.js`, change `expect( read( site.dir, 'wp-login.php' ) ).toBe( MY_EDIT )` to any other string, then rerun. - Expected: "switching back to a ticket restores its work byte for byte" goes red on that line. Put it back. 4. Break the substrate on purpose. In `e2e/helpers/git-site.cjs`, delete the line that writes `SUBSTRATE_CONTENT`, then rerun. - Expected: every ticket-branch journey goes red. Put it back. This is the assertion that stands in for "a switch did not silently wipe `node_modules`". 5. `npm test` and `npm run lint`. - Expected: 1027 passed, and clean. **What must not have happened:** - **Your real settings file must not have been touched** — the guard from #362 covers this, and it is worth confirming once more now that the journeys write far more state than the engine test did. - **No leftover directories.** Each journey builds a real repository under the system temp directory and removes it after the app stops. After a run, nothing matching `wpct-e2e-*` should remain. - **No Electron processes still running.** ## Risks and limitations - **Linking a ticket reaches the network, twice.** The app starts a GitHub lookup for the ticket's pull requests, and — because the ticket was linked by hand rather than restored on mount — it also auto-reads the ticket's own facts from Trac (#292). Nothing here asserts on either, and no journey waits for them, so an offline or rate-limited runner does not fail these tests. But both calls happen, and on a machine where Trac's human-check appears a window may open. An earlier version of this section said Trac is only read on request; that is true on mount and re-activation, and not true of a hand-linked ticket, which is what every journey here does. - **Selectors read the visible copy.** Renaming "Link ticket" or "Delete this ticket's work" breaks these tests. That is deliberate — it makes the copy a contract — but it is a maintenance cost, and worth saying out loud rather than discovering in a rename. - **Timing is waited for, never slept through.** Every step waits on something the app renders or on the state of the repository on disk. There is no `waitForTimeout` in the journeys, because a sleep tuned on a laptop is a flake on a Windows runner. - The row for a ticket is addressed by the ticket it names, not by position: the list is ordered by how recently each ticket was used, so `.first()` picks a different branch depending on how far the render has got. That cost a wrong-branch deletion during development, which is why it is called out in a comment in the file. ## Related Part of #359 and #361. Stacked on #362, which closes #360. --- <details> <summary>Design decisions and alternatives considered</summary> **A real repository per test, built from scratch.** These are Git operations; a stub would only prove the app can talk to a stub. The fixture mirrors the one the integration suite already uses one level down — same trunk branch, same three tracked files, same gitignored `node_modules` — so the two levels describe the same site and a failure at one level can be read against the other. **Nothing is shared between tests.** Each builds its own repository and its own profile, so no journey can be affected by the order it runs in. It costs about a second per test, which is the right trade at this size. **Assertions read the repository, not the app.** Whether a branch exists is asked of `isomorphic-git` against the directory on disk, not of the app that just claimed to have made it. An assertion that asks the app to confirm its own work is the "mocking the thing under test" shape from the review standard, one level up. **Marking assertions rather than splitting the files.** An alternative was two files per flow, invariants in one and characterisations in the other. Rejected because the reason a characterisation exists is the invariant next to it — reading "the applied patch is a record in the store" is only useful beside "the contributor's edit comes back". Splitting them would put the two halves of one argument in different files. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `0 [fix here] · 1 [follow-up]`. **🔵 architecture · `[follow-up]` — not this PR's to fix.** The `wasActive` branch of `branches:delete` in `src/main.js` returns the checkout to trunk and clears the link when the deleted ticket is the current one. The panel excludes the current ticket from the list it offers delete controls for, so that branch appears unreachable from the interface. Not touched here; worth confirming before #350 rewrites around it. No findings in architecture (nothing under `src/` changes), security, performance, or cross-platform. The two cross-platform traps in this diff — path comparison and deleting directories the app still holds open — were fixed in #362 and are reused here rather than re-solved. **Tests:** every invariant in this PR was verified by mutating it and confirming the test goes red. Seven mutations, seven reds: | Mutated assertion | Result | | --- | --- | | the store records the linked ticket | red | | the second ticket is the one checked out | red | | the edit comes back on switch | red | | the deletion comes back on switch | red | | delete leaves the other ticket alone | red | | delete asked for confirmation | red | | the substrate survives a switch | red | **Same caveat as #362:** this pass ran in the session that wrote the code, not in fresh context as the standard asks for. </details> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

**Stacked on #363.** Review that one first; this diff is `TESTING.md`, plus a much shorter testing section in `CONTRIBUTING.md`. ## Why CONTRIBUTING.md is the map of this project's guardrails. Testing had grown to half of it — 134 of 271 lines describing five layers, two containers and a constraint in the packaged app — so a contributor arriving to ask "what will fail on my PR" had to read all of that first. The detail is worth having. It just is not what belongs on that page. The need for it surfaced in a plain question there was no good answer to: *shouldn't the unit tests cover the code, and the end-to-end tests cover the artifact built from it?* It is the natural reading, and answering it properly needs the whole map — which lived in nobody's head but the person who had just built the last two layers. ## What changes **New `TESTING.md`** — the canonical description of the suite: - **What to run.** Three commands at the top, then the two you need less often, then how to run a single file or a single journey. - **The five layers**, cheapest first, each leading with **what it is blind to** — the fact that actually sends a test one layer up or down. Unit, integration against real Git repositories, IPC wiring with a stubbed Electron, journeys through the whole app, and the packaged smoke test. - **Where a new test belongs**, by kind of change, with the rule the ordering implies: write it at the highest layer that can still see the failure. - **Why the two end-to-end layers are separate.** They run the same source in two containers, and each container has failures the other cannot have. - **How to read a failure**, what CI runs, and the conventions a new test is expected to follow. **`CONTRIBUTING.md` keeps only what a reader needs at that point** — both platforms, the fast suite run twice per platform and why, the end-to-end suites as two jobs — and links to `TESTING.md` for the rest. It goes back to roughly the length it was before this stack started: 150 lines on `trunk`, 153 now, with the third workflow properly represented. **`AGENTS.md` gets the same pointer.** An agent adding a test is exactly who needs to know which layer it belongs in. This follows the rule the repository already uses for the review standard: one source of truth, everything else pointing at it rather than restating it, so nothing can drift. Two things `TESTING.md` says out loud rather than implying: **The end-to-end split is forced, not preferred.** The packaged app ignores a redirected application-data directory — `!app.isPackaged` in `src/main.js` — so it always writes to the real site registry of whoever runs it. A test that drives a flow therefore cannot run against the artifact without writing to somebody's actual sites. That constraint is why the packaged smoke test writes no state, and reading it as minimalism leads to the wrong conclusion about what belongs where. **The gap that leaves.** A flow that behaves differently *because* of packaging is caught by neither layer. The page names it, says what closing it would cost — a test-only seam in shipped software — and says it is not worth opening until a failure of that shape actually escapes. It also fixes the opening paragraph of the checks section, which said "Neither needs secrets" while listing three workflows. ## How to test this Platforms: **any**. This is documentation; there is no user-visible surface. 1. Read `TESTING.md` top to bottom. - Expected: it answers "what do I run?" in the first ten lines and "where does my new test go?" without needing anything else open. 2. Read the `### Tests` section of `CONTRIBUTING.md`. - Expected: it tells you what CI will run against your PR and nothing more, and points at `TESTING.md` for the rest. 3. Check every number and command, because they date fast: ``` ls test/*.test.cjs | grep -vc integration # 63 unit files ls test/*.integration.test.cjs | wc -l # 6 integration files for f in test/*.integration.test.cjs; do grep -cE '^(test|it)\(' $f; done | paste -sd+ - | bc # 89 grep -cE '^(test|it)\(' test/ipc-wiring.test.cjs # 151 grep -c "ipcMain.handle(" src/main.js # 58 npm test # 1027, ~2.7s ``` 4. Follow the links. Every relative link in `TESTING.md`, `CONTRIBUTING.md` and `AGENTS.md` should resolve to a file that exists, and every `npm run …` it mentions should be a real script in `package.json`. 5. `npm run lint`. - Expected: clean. **What must not have happened:** no code changed. `git diff --stat` against the base should show three Markdown files and nothing else. ## Risks and limitations - **The counts go stale.** Six numbers in this section describe the repository as it is today, and nothing checks them. They are there because "63 files" and "8 tests" say something about proportion that "several" does not, and the proportion is the point of the section. The commands above regenerate all of them in a few seconds. - **The five-layer count is a description, not a rule.** If a sixth kind of test earns its place, the section is where that argument gets made — it should not become a reason to squeeze a test into a layer it does not fit. - This is one long section on a page that is already long. It sits after the CI checks because the first question a contributor has is "what will fail on my PR"; the second is "where does my test go". - **Written as prose, not as a table.** The first draft used one, and it was wrong for this repository: no document here has a single Markdown table, every page is hard-wrapped at around 100 columns, and the table's rows ran to 236 characters — three times the width of anything else in the file. Rewritten as one paragraph per layer, wrapped like its neighbours. The longest line in the section is now 107, which is exactly the longest line already on the page. ## Related Part of #359. Stacked on #363. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

**Stacked on #365.** Review that one first; this diff is one line of code and a section of `TESTING.md`. ## Why The journeys are over in eight seconds. That is what makes them worth running on every pull request, and it is useless when the question is the one that matters for a suite of tests: **is this driving the app, or passing through a shell that happens to satisfy its assertions?** A green run looks identical either way. There was no way to look. This adds one, and corrects something the guide said about the other. ## What changes **`E2E_VIDEO=<dir> npm run test:e2e` records every window**, and each recording is saved under the name of the test it came from: ``` E2E_VIDEO=/tmp/e2e-video npm run test:e2e open /tmp/e2e-video/switching-back-to-a-ticket-restores-its-work-byte-for-byte.webm ``` Inert unless the variable is set — nothing changes for CI, or for anyone not asking to watch. The naming is not a nicety. Playwright names a recording after the page that produced it, an opaque hash, and writes a file for every window the run opened — a full run left twenty of them, most empty or a fraction of a second long, three worth opening. The first person to try it opened a zero-byte one and got an error from their video player, which is the whole feature failing at the last step. Recordings are now saved by test name once the app has closed, and everything else the recorder left is cleared away. A test that closes and reopens the app keeps both launches, as `-1` and `-2`. **The guide no longer claims the trace replays a failing run.** It does not, for an Electron test. Playwright collects screenshots and DOM snapshots through a browser context, and an Electron window is not one, so the trace carries the actions, their timings and their source lines — and no pictures at all. `TESTING.md` now says what is actually in there and points at the recording for the rest. **`slowMo` is recorded as a trap, in the code rather than the guide.** This PR started as an attempt to add it. `_electron.launch` does not accept it, and Playwright drops launch options it does not recognise without a word — so a `slowMo` added there leaves the tests running at full speed while looking like it worked. That belongs beside the launch, where somebody would try to add it, and not in a guide for contributors. `--headed` is not mentioned anywhere: it is simply not applicable, with no silent failure to warn about. ## How to test this Platforms: **any**. **Starting state:** this branch, `npm ci` and `npm run build:once` done. 1. `E2E_VIDEO=/tmp/e2e-video npm run test:e2e` - Expected: 8 passed, and **9** `.webm` files in `/tmp/e2e-video`, every one named after its test. Nine rather than eight because the restart test records both launches. No file is empty, and nothing is left called `page@…`. 2. Open `switching-back-to-a-ticket-restores-its-work-byte-for-byte.webm`. - Expected: the app, being driven. A ticket number typed into the field, **Link ticket** clicked, the panel changing. This is the check the video exists for — if it did not look like the app doing the work, the test would be worth nothing and would still be green. 3. `npm run test:e2e` with no variable set. - Expected: 8 passed, and **no** video directory created anywhere. The option must be inert by default. 4. Read `## Reading a failure` in `TESTING.md`, then confirm it for yourself: ``` npx playwright test --project=journeys -g "switching back" --trace on unzip -l test-results/*/trace.zip ``` - Expected: three files, about 12 KB, and no images — exactly what the page now says. Before this PR it said the trace replays the run. 5. `npm run lint`. - Expected: clean. **What must not have happened:** - **No behaviour change with the variable unset.** That is step 3, and it is the one that matters: a recording option that quietly slowed or altered a CI run would be worse than no option. - **No video files committed.** `test-results/` and `playwright-report/` are already ignored; a directory chosen with `E2E_VIDEO` is wherever you point it, so point it outside the repository. ## Risks and limitations - **The video directory is not emptied between runs**, and a rerun overwrites a recording of the same name. Deliberate — the point is to keep them and watch them — but worth knowing before pointing it somewhere you forget about. - **The cleanup removes every `page@*.webm` in that directory**, not only the ones this run wrote. That is safe because the journeys run on a single worker and each test tidies up after itself, and it is the reason to point `E2E_VIDEO` at a directory kept for the purpose rather than at somewhere with other files in it. - **Some recordings are only a second long and a few kilobytes.** Those are tests that assert on state rather than on the screen, so there is genuinely little to see. They are kept rather than filtered by size, because "too small to matter" is a guess and a missing file is confusing. - **Not wired into CI.** A failing journey already comes back with a screenshot and the persisted state attached, which has been enough so far. Recording every window on every run would upload a few hundred kilobytes per job for something nobody looks at when the run is green. - **`PWDEBUG=1` needs a browser this repository otherwise never installs.** The Inspector is a Chromium window, and nothing else here downloads one — the suites and CI launch only the Electron already in the tree. It worked on the first machine it was tried on purely because that machine had a Playwright browser cached from another project. Pointed at an empty `PLAYWRIGHT_BROWSERS_PATH` it does not error: the run pauses and no window opens, which reads as a hang. `TESTING.md` states the one-time `npx playwright install chromium` and says what skipping it looks like. ## Related Part of #359. Stacked on #365. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

**Stacked on #368.** Review the ones below it first; this diff is one new journey file, plus the seed repository it needed. ## Why The other half of a contributor's day, and the half #350 changes most. Someone else's work arrives as a patch file or a pull request, goes onto the checkout, and has to come off again without taking anything of theirs with it. Today "applied" is a layer the app holds. After #350 it is a commit. So what these journeys pin is the part that has to survive either model — and the part whose failure is silent, because a patch that half-applied or a revert that ate an unrelated edit produces no error at all. ## What changes Three journeys. - **A patch applies.** The file changes on disk, the gitignored substrate and the untouched files are left alone, and the app records what it applied. - **A revert puts the checkout back**, byte for byte — and the contributor's own work in a file the patch never touched survives both directions. - **A patch that does not fit is refused.** This is the one worth having. The app warns *before* writing that the patch is about to land on the contributor's own edits, while they can still stop; then it refuses, says the checkout was not changed, names the file, and says how much of the patch failed. Nothing is written and nothing is offered to undo. **Two things about the app the fixture had wrong**, both found by running it rather than by reading: - **Patches are rewritten onto core's modern layout.** A patch naming `wp-login.php`, which is how Trac patches are written, is applied to `src/wp-login.php` — see `src/patch-plan.cjs`, which exists because core moved everything under `src/` and old patches predate it. A fixture with its files at the repository root sends every patch somewhere the test never looks, and the test then fails for a reason that is not a bug. The seed repository now mirrors a real checkout. - **Applying ends by rebuilding the site.** Without a `build` script the chain fails *after* the patch is already on disk, so the journey would be asserting on a half-finished flow. The seed repository has a `package.json` whose build does nothing and exits. **One assertion is deliberately narrow.** The conflict is asserted against the alert, not the page. When an apply fails the preview stays on screen and already names the file, so `getByText('src/wp-login.php')` passes whether or not the app reported anything — green, and proving nothing. ## How to test this Platforms: **any**. Windows is covered by CI. **Starting state:** this branch, `npm ci` and `npm run build:once` done. 1. `npm run test:e2e` - Expected: **11 passed** — four from #362, four from #363, three here. 2. Watch the interesting one: ``` E2E_VIDEO=/tmp/e2e-video npx playwright test --project=journeys -g "does not fit" open /tmp/e2e-video/a-patch-that-does-not-fit-is-refused-and-writes-nothing.webm ``` - Expected: the app opens, a ticket is linked, a patch file is chosen, the preview lists `src/wp-login.php`, a warning appears about your own edits, **Apply and rebuild** is clicked, and the refusal appears. Nothing in the file changes. 3. Break the all-or-nothing invariant. In `e2e/journeys/patch-apply.spec.js`, change `expect( read( site.dir, LOGIN ) ).toBe( mine )` to any other string, then rerun. - Expected: "a patch that does not fit is refused, and writes nothing" goes red on that line. Put it back. 4. Break the path rewrite. In `e2e/helpers/git-site.cjs`, change `LOGIN` from `src/wp-login.php` to `wp-login.php`, then rerun. - Expected: the patch journeys go red — the patch is applied where the fixture is not looking. This is the trap the fixture layout exists to avoid. Put it back. 5. `npm test` and `npm run lint`. - Expected: 1027 passed, and clean. **What must not have happened:** - **The ticket-branch journeys must still pass.** This PR moves their fixture's files under `src/`, so all four are re-exercised by step 1. If only the patch journeys were run, that move would go unchecked. - **No leftover directories.** Nothing matching `wpct-e2e-*` should remain in the system temp directory after a run. - **Your real settings file must not have been touched.** ## Risks and limitations - **Linking a ticket reads Trac.** Correcting something I wrote on #363: linking a ticket *by hand* auto-reads the ticket's own facts (#292), so these journeys do reach the network at that point, and on a machine where Trac's human-check appears a window may open. Nothing here asserts on it and no journey waits for it, so an offline or blocked runner does not fail these tests — but it happens, and #363 said it did not. - **The patches are hand-written unified diffs**, one whole-line replacement per file. That is deliberate — generating them with the same code the app reads back would prove less than it looks — but it means the shapes core actually produces (renames, binary files, empty additions) are not exercised here. Those are covered a layer down, in `test/patch-apply.integration.test.cjs`. - **The rebuild is a no-op.** The seed repository's `build` script exits immediately, so what these journeys prove about the rebuild is that the chain reaches it and survives it, not that a real build works. - **Applying a pull request is not covered**, only a patch file. Fetching a PR puts GitHub in the path of a test about the checkout. The app treats both identically once the patch text is in hand, which is the point where these journeys join the flow. ## Related Part of #359 and #361. Stacked on #368. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

**Stacked on #369.** Review the ones below it first; this diff is one new journey file. ## Why This is the only test in the repository that exercises the real persistence layer. Everywhere else `electron-store` is a stand-in. The wiring tests load the real `src/main.js` but hand it a fake store; the integration tests never reach it at all. So nothing today can tell **"the app persisted this"** from **"the app still had it in memory"** — and that is precisely the distinction a change to how state is stored breaks while every other test stays green. #350 changes what is stored and where. This is the test that notices. ## What changes Three journeys. Each acts, **closes the app**, opens it again against the same profile untouched, and asks what came back. - **The linked ticket and the work on it.** The app opens where the contributor left it, and the checkout is untouched on the way past — a restart is not a switch. - **An applied patch, still applied and still revertable.** The revert is *performed* after the restart rather than merely offered, because a button that renders and does nothing is the failure worth catching. Forgetting an applied patch leaves a contributor unable to tell their own changes from somebody else's, about to submit both as theirs. - **Every ticket on the site, including the parked one**, each keeping the commit its patch is measured from. Losing that is what #317 was: a ticket whose base is unknown can no longer say what the contributor changed. With this, the baseline battery #361 asked for is complete: ticket branches (#363), applying and reverting a patch (#369), and what survives a restart. ## How to test this Platforms: **any**. **Starting state:** this branch, `npm ci` and `npm run build:once` done. 1. `npm run test:e2e` - Expected: **14 passed**, in under twenty seconds. Windows open and close more often than in the other journeys — each of these launches the app twice. 2. Watch the one that matters most: ``` E2E_VIDEO=/tmp/e2e-video npx playwright test --project=journeys -g "still applied after a restart" open /tmp/e2e-video/ ``` - Expected: two recordings, `-1` and `-2`. In the first the patch is applied; in the second the app has reopened, still offers **Revert this patch**, and the revert is carried out. 3. Break the persistence invariant. In `e2e/journeys/store-persistence.spec.js`, change `expect( read( site.dir, LOGIN ) ).toBe( \`${ PATCHED_LOGIN }\n\` )` to any other string, then rerun. - Expected: "an applied patch is still applied after a restart" goes red. Put it back. 4. Prove the restart is real, not a reload. In `e2e/helpers/app.cjs`, make `restart()` return without closing anything — replace its body with `return { app: this.app, page: this.page }`. - Expected: the persistence journeys still pass, because nothing was ever persisted or re-read. That is the shape of the vacuous test this file exists to avoid, and it is worth seeing once. Put it back. 5. `npm test` and `npm run lint`. - Expected: 1027 passed, and clean. **What must not have happened:** - **Your real settings file must not have been touched.** These journeys write more to the store than any other, so if the guard were going to fail anywhere it would be here. - **No Electron processes left running.** Twice the launches means twice the chances to leak one. ## Risks and limitations - **Step 4 above is a real gap, and it is not automated.** Nothing in the suite proves `restart()` genuinely restarts; a change that turned it into a no-op would leave these three tests passing and meaningless. The check exists as a manual step because asserting on it would mean asserting on the harness rather than on the app, and the honest place for that is a reviewer's eyes. - **Slower than the rest.** Two app launches each, so these three take about as long as the other eleven together. Worth it for the only coverage of the real store, but it is why the layer stays small. - **The store is read as a file, not through the app.** The characterisation assertions parse the profile's `settings.json` directly. That is the point — it is what actually survived — but it means a change to the file's shape shows up here as a failure to interpret rather than as a clean statement of what moved. ## Related Part of #359. **Closes #361.** Stacked on #369. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

## Why Every `.md` file that carries repository instructions was hard-wrapped at roughly 100 columns, while `README.md` and the whole of `docs/` were not. Nothing wrote either convention down, so each new paragraph was a guess, and the guess kept landing on the wrapped side. Markdown imposes no line limit — a wrapped paragraph and a single long line render identically — so this was never a rule, only a habit that half the tree had. The habit has a cost the other half does not pay: text arrives pre-chopped when it is pasted into an issue or a chat, it does not reflow to the reader's width, and a one-word edit reshuffles every line beneath it into a diff that claims they changed. ## What changes Prose is no longer wrapped anywhere: one paragraph is one line, however long. That is the convention `README.md` and `docs/` already followed, now applied to the remaining 18 files, and written down in `AGENTS.md` so it stops being a guess. Line breaks that mean something are untouched — list items, table rows, headings, fenced code, VitePress `:::` containers, YAML frontmatter, and the body of an HTML comment all keep their shape. `.editorconfig` gains `max_line_length = off` for `[*.md]`, so an editor configured from it does not re-wrap on save. No prose was rewritten. The only content change in the whole diff is that a blockquote spanning several wrapped lines now carries one `>` instead of one per line. ## How to test this Platforms: any — nothing here is platform-specific, and no shipped code changed. **Starting state:** trunk checked out, `npm ci` done. 1. `npm run docs:build` — the user guide builds. Compare against a build of trunk: the rendered HTML is byte-identical on all 21 pages once VitePress's content hashes are normalised away. 2. Open any changed file and confirm no paragraph is split across lines, and that tables, lists, code blocks and the `:::` blocks in `docs/guide/getting-started.md` and `docs/guide/terminal.md` are intact. 3. `npm test` and `npm run lint` — both green, and both untouched by this. **What must not have happened:** a word lost, gained or reordered anywhere. That is the whole risk of a mechanical rewrite, and it is silent: nothing fails, the text is just quietly wrong. Verified by comparing every file against its `HEAD` version with all whitespace collapsed — identical, except the blockquote markers noted above — and by counting headings, list markers, table rows, code fences and `:::` lines per file before and after: unchanged in every file. Also: no VitePress container left broken. An earlier pass of this change joined a closing `:::` into the paragraph above it, which silently swallowed the title of four tips and a warning. The rendered-HTML comparison in step 1 is what caught it. ## Risks and limitations Diffs on documentation get coarser: change one word and GitHub marks the paragraph. `git diff --word-diff`, and GitHub's own intra-line highlighting, narrow it back down. This is the trade the convention accepts, and it is stated in `AGENTS.md`. The README's three release badges are now on one line rather than three. They render the same — they were always one paragraph — but adding a fourth badge means editing a long line. Nothing enforces the convention automatically. `.editorconfig` steers editors and `AGENTS.md` states the rule; neither fails CI. A markdownlint rule would, and is not worth a CI job for this. ## Related Follow-up to a review comment rather than an issue: the convention was noticed by its absence, not filed. --- <details> <summary>Design decisions and alternatives considered</summary> **Wrap at a fixed width instead.** The obvious reading of the problem was "TESTING.md exceeds 100 columns in 16 places", and the obvious fix was to re-wrap it to 100. That takes the habit as given. Since Markdown does not care, the question is which convention serves editing better, and the unwrapped half of the tree was already the answer. **Semantic line breaks** (one line per sentence or clause) keeps diffs narrow and lines short. It also asks every author to make a judgement call on every sentence, which is exactly the kind of rule that decays — and it still chops text on paste. **Leave `AGENTS.md` and `CONTRIBUTING.md` wrapped**, as the files most often reviewed line by line. Rejected: two conventions in one repository is what produced this in the first place. **A markdownlint check in CI.** It is the only thing that would actually enforce this, but it means a workflow, a config, and an exception list for long link targets — for a rule that costs nothing when it is broken. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 1 `[fix here]` — found and fixed before this PR existed. The finding: closing `:::` lines of VitePress containers were being joined into the paragraph above, breaking four tips in `docs/guide/getting-started.md` and one warning in `docs/guide/terminal.md`. The containers still rendered, so the page did not error — the title line was simply absorbed into the body text. Fixed by treating `:::` as a block boundary, and confirmed by the rendered-HTML comparison, which now reports zero differing pages. Two smaller ones caught the same way, in the same pass: empty numbered list items (`1.` with nothing after it, in the PR template) were treated as prose and merged onto one line, and HTML comment bodies were being re-flowed, which mangled the templates a contributor reads while filling them in. Both fixed — empty list markers are list items, comment bodies are verbatim. The judgement pass was run in this session rather than dispatched to a subagent. For a mechanical rewrite whose correctness is decided by a byte-for-byte comparison of the rendered output, the check that matters is the comparison, and it is reproducible from step 1 above. </details> <details> <summary>Implementation notes</summary> The rewrite was done by a throwaway script, not by hand, so that the transformation was uniform and could be re-run after each fix. It is not checked in: it has served its purpose, and a one-shot migration tool in the repository invites someone to run it again. What it treats as verbatim, and therefore never joins: YAML frontmatter, fenced code, table rows, headings, thematic breaks, HTML blocks, HTML comment bodies, link reference definitions, `:::` container markers, and any line ending in a hard break (two trailing spaces or a backslash — this tree has none). List items start a new line, including empty ones; a list item's continuation lines join into it. Consecutive blockquote lines join into a single `>` line. Three checks were run over every tracked `.md`, before and after: - whitespace-collapsed content identical (ignoring `>` markers), - per-file counts of headings, list markers, table rows, code fences and `:::` lines identical, - for `docs/`, a full VitePress build compared page by page against a build of `HEAD`, with content hashes normalised — 21 pages, zero differences. </details> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

## Why There is no way to find out how to run a journey in "manual" mode — pausing between actions and advancing each step yourself — from TESTING.md. The instruction was there, as the last paragraph of a collapsed `<details>` block titled "Watching one run", which reads as being about video recording. Somebody looking for it under "What to run" or "Auditing an end-to-end test" does not find it. Worse, what it said was wrong. It gave `PWDEBUG=1`, which is what Playwright's own documentation gives, and which does nothing to an Electron test: the run finishes at full speed, unpaused, and you are left assuming you held it wrong. ## What changes `PWDEBUG=1` alone never pauses a journey. `--debug` does, because it also passes `--headed`, and `--headed` is the part that matters — Playwright only enables the debugger for a headful browser, and `_electron.launch()` is not headful unless asked. Same story for `page.pause()`: a silent no-op on a default run, a working breakpoint under `--headed`. All four combinations were run against Playwright 1.62.1 to establish this; see the collapsed notes below. So the section now gives `--debug` for stepping from the first action, `page.pause()` plus `--headed --timeout=0` for stepping from a chosen one, and says plainly that the `PWDEBUG=1` everyone will reach for first does not work here. It also moves out of the video block into its own section next to the audit instructions — both are "drive this yourself" workflows — with a pointer from "What to run", which is where you look first. Two things that were missing either way: name a single test with `-g`, or you are hand-stepping every journey in sequence; and `--timeout=0` is not optional, because `playwright.config.js` sets a 60 second `timeout` that ends the session while you are still reading it. The video block keeps answering its own question and now points here. ## How to test this Platforms: any. Docs only — nothing in the app changes, so the test is whether the commands in the new section do what it says. **Starting state:** a clean checkout of this branch, dependencies installed, `npx playwright install chromium` done once. 1. Read `## Stepping through a journey by hand` in TESTING.md. It should be findable from the "What to run" section without opening a `<details>` block. 2. Run `npx playwright test --project=journeys -g "the app launches from source" --debug`. The Playwright Inspector window opens, the app opens, and the run stops before the first action and waits. Step it with the Inspector; end it with Ctrl-C. 3. Run the same test without `--debug`, as `PWDEBUG=1 npx playwright test --project=journeys -g "the app launches from source"`. It should pass in about five seconds without ever pausing — this is the behaviour the old text got wrong. 4. Put `await page.pause();` in `e2e/journeys/engine.spec.js` above the `toHaveTitle` assertion and run `npx playwright test --project=journeys -g "the app launches from source" --headed --timeout=0`. It pauses there with the Inspector open. Drop `--headed` and it does not pause at all. Revert the spec afterwards. **What must not have happened:** no spec file is left modified — step 4 edits `e2e/journeys/engine.spec.js` and must be reverted. `git status` clean apart from TESTING.md. And nothing in `e2e/` or `src/` is touched by this branch: the diff is one file. ## Risks and limitations The `--headed` explanation is behavioural, not mechanical. I verified what happens in all four combinations; I did not trace exactly which branch inside Playwright's `BrowserContext.initialize` an Electron context takes to get there. If a future Playwright release changes it, the recipe breaks silently and the file goes stale the same way the `PWDEBUG=1` line did — the version it was checked against is stated in the text for that reason. ## Related Follow-up to #368, which added the video and `PWDEBUG=1` paragraph this corrects. --- <details> <summary>Design decisions and alternatives considered</summary> The obvious smaller change was to leave the paragraph where it was and add the `-g` and `--timeout=0` details to it. Running the command is what ruled that out — there was no point improving an instruction that does not work. Recommending `--debug` rather than `--headed --timeout=0 PWDEBUG=1` spelled out: `--debug` is one flag, it is the documented Playwright entry point, and it sets `--workers=1 --max-failures=1` too, which are both right here. The longer form is given only for the `page.pause()` case, where you do not want to stop at the first action. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> `npm run lint` clean. `npm test` 1027 passing, 0 failing. Neither is affected by the change; both were run because the standard asks for the deterministic layer first. 2 findings, both `[fix here]`, both fixed before this PR was opened: - 🔴 **tests** — the section as first written repeated the file's own `PWDEBUG=1` claim, which is false for Electron tests. Found by running it rather than reading it. Rewritten around `--debug`, with the four verifying runs described above. - 🔵 **process** — the draft said "all fourteen journeys", and this file's own Conventions section forbids test counts in it, for exactly the reason that it goes stale unnoticed. Changed to "every journey in the project". Nothing across architecture, security, performance or cross-platform: the diff is one Markdown file and touches no code. One deviation from the procedure, stated because it changes how much the above is worth: the judgement pass was run in the session that wrote the change rather than dispatched to a fresh context, which the standard asks for and which this session was configured not to do. The two findings above came out of running the commands, not out of re-reading the prose, so a fresh reader may still find something in the wording. </details> <details> <summary>Implementation notes</summary> What was actually run, on macOS, Playwright 1.62.1, against `-g "the app launches from source"`: | command | result | | --- | --- | | `PWDEBUG=1 …` | passed in 5.1s, never paused | | `page.pause()` in the spec, default flags | passed in 8.6s, never paused | | `… --debug` | paused; Inspector window up, still held after 50s | | `page.pause()` + `--headed --timeout=0` | paused; Inspector window up, still held after 36s | Separately, a 75 second hold (`await new Promise( ( r ) => setTimeout( r, 75000 ) )`) under `--timeout=0` passed at 1.3m, confirming the config's 60 second `timeout` is genuinely lifted rather than merely large. `node_modules/playwright/lib/program.js:194` is where `--debug` is defined as `PWDEBUG=1` plus `--timeout=0 --max-failures=1 --headed --workers=1`, which is the source for that sentence in the new section. </details> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

## Why The suite lived in two top-level directories, `test/` and `e2e/`, and nothing about those names said they were two halves of one thing. TESTING.md already describes a single five-layer suite; a newcomer looking at the tree sees two unrelated folders and has to read a document to learn that layers 1–3 are in one and layers 4–5 are in the other. ## What changes ``` test/ -> tests/unit/ e2e/ -> tests/e2e/ (helpers/, journeys/, packaged/ unchanged inside) ``` Moved with `git mv`, so blame survives — `git log --follow tests/unit/azure-sign.test.cjs` still reaches #21. **No test logic changed.** 83 of the 92 files are pure renames; the +167/−156 is relative paths one level deeper (`'../src/…'` → `'../../src/…'`, `__dirname, '..'` → `__dirname, '..', '..'`, and the two Playwright `REPO_ROOT` joins), the two config globs, and comments naming the old locations. Integration tests are still identified by their `*.integration.test.cjs` **filename** rather than by a directory, exactly as before. Splitting all five layers into five folders was considered and rejected — see the collapsed section. `package.json` needed no change: `npm test` is bare `node --test`, which discovers by filename from the repo root. **Three comments asserted something this move makes false, and are rewritten rather than path-substituted.** `node --test` collected every `.cjs` under a directory literally named `test/` — that is why the `ipc-wiring` and `runner-wiring` harnesses could not be extracted into shared helper files, and why the `nested-install` fixture ships as `run-build.js.txt`. From `tests/unit/` that constraint is gone. The fixture keeps its extension anyway (one rename, and it stays inert under any discovery rule) and the harnesses stay put for the reason that still holds, but the comments now say which is which instead of citing a rule that no longer applies. **That shifts a guarantee, and TESTING.md now states it.** The fast suite and the end-to-end suite are kept apart by filename, not by directory. Under a shared parent, naming is the only thing between `npm test` and a seven-second app launch — so the Conventions bullet now names all five shapes Node treats as a test file (`*.test.cjs`, `*-test.cjs`, `*_test.cjs`, `test-*.cjs`, `test.cjs`) and says plainly that end-to-end files are `.spec.js`. ## How to test this **Platforms: any.** This change has no user-visible surface — no renderer, main-process or packaging behaviour is touched. The commands below are the demonstration. **Starting state:** a clean checkout of this branch, `npm ci` done. 1. `npm test` → **1027 passing, 0 failing.** The number is the point: it is identical to `trunk`, and a changed total is the single most likely way a move like this goes wrong silently. 2. `npm run test:electron` → same 1027, on Electron's bundled Node. Proves the bare-`--test` discovery still works on the other runtime. 3. `npm run lint` → exit 0. To check the ESLint globs actually *landed* rather than silently matching nothing: `npx eslint --print-config tests/e2e/journeys/engine.spec.js` should show `react-hooks/rules-of-hooks` off, which only the `tests/e2e/**` block does. 4. `npm run build:once && npm run test:e2e` → 14 passing. This is what proves the `REPO_ROOT` depth change in `tests/e2e/helpers/app.cjs`. 5. `CSC_IDENTITY_AUTO_DISCOVERY=false npm run pack:dir && npm run test:e2e:packaged` → 4 passing. The only thing that exercises `smoke.spec.js`'s own `REPO_ROOT`. 6. `git log --follow tests/unit/azure-sign.test.cjs` → history continues past the move. All six were run on macOS before opening this. CI covers 1–5 on Windows. **What must not have happened:** - **A test quietly dropped from discovery.** Renaming out of a directory called `test/` removes one of Node's default collection patterns (`**/test/**/*.cjs`). Every file here matches `*.test.cjs` independently, and the 1027 total is unchanged — but a lower total, or a *higher* one, is the failure to look for. - **A journey silently not collected.** `npm run test:e2e` must report **14 tests**, not 0. A wrong `testDir` produces "no tests found", which exits 0 in some Playwright configurations and reads exactly like a pass. - **A path-traversal fixture rewritten by the depth fix.** `tests/unit/wporg-handle.test.cjs` and `tests/unit/patch-provenance.test.cjs` assert that `'../../etc/passwd'` is refused, and `tests/unit/patch-apply.integration.test.cjs` does the same for `'../escaped.txt'`. Those strings are the *input under test*, not filesystem paths — adding a `../` to them would turn a security assertion into a test of a different string that still passes. The rewrite was anchored on `__dirname` and on `'../src/'`-style prefixes precisely to avoid them; all three are unchanged in the diff. - **A fixture lost, dotfiles included.** `tests/unit/fixtures/*/.npmrc` are easy to miss in a directory move. 76 files in `test/` and 7 in `e2e/` before; 76 in `tests/unit/` and 7 in `tests/e2e/` after. ## Risks and limitations Review outcome: **1 `[fix here]`, fixed · 1 `[follow-up]`** — detail in the collapsed block. The one thing this makes easier to get wrong is the thing TESTING.md now warns about: someone adding `tests/e2e/something.test.cjs` puts an Electron launch inside `npm test`, and nothing enforces the convention. A lint rule or an explicit `node --test` glob would enforce it; both were rejected for now (below), so the guard is a documented convention and a reviewer. Not done, and deliberately: the flat `tests/unit/` directory is 76 files. If it wants subdividing, that is a separate change with its own argument, and doing it here would bury the mechanical move in judgement calls. ## Related Follow-up to #376, which this stacks on — the base retargets to `trunk` automatically once that merges. --- <details> <summary>Design decisions and alternatives considered</summary> **Why not five folders, one per layer?** `tests/{unit,integration,ipc,journeys,packaged}` would make the tree mirror TESTING.md exactly. Rejected because it changes the *rule*, not just the location: today a layer-2 test is identified by its `*.integration.test.cjs` filename, and moving those files into a directory means two competing signals for the same fact, or a bulk rename on top of a bulk move. This PR is the move; if the layers want folders, that argument stands on its own afterwards. **Why not `tests/` flat, with `e2e/` inside it?** Smaller diff — `test/` → `tests/` is one rename. Rejected because it leaves the asymmetry that prompted this: unit tests at the top level and end-to-end tests one level down still reads as "the tests, plus some other thing". **Why not make `npm test` explicit about its path?** A glob such as `node --test "tests/unit/**/*.test.cjs"` would enforce the separation mechanically instead of by convention. Rejected here because `scripts/run-tests-electron.cjs` documents at length why `--test` takes no positional argument — since Node 22 a positional is a glob matching *files*, and getting it wrong makes Node treat it as an entry module, which Electron 43 fails on where Electron 32 was fine. Changing that in the same PR as an 83-file move would mix a real behavioural decision into a mechanical one, and it needs its own testing across both runtimes. **Why keep `run-build.js.txt` as `.txt`?** The extension is no longer load-bearing — Node will not walk `tests/unit/fixtures/` looking for files to execute. Renaming it to `.js` would be one more content change in a move PR, in exchange for nothing, and the `.txt` keeps the fixture inert whatever a future discovery rule does. The comment now says that, rather than citing the rule that used to force it. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **1 `[fix here]` · 1 `[follow-up]` — the `[fix here]` is fixed.** Deterministic layer: `npm run lint` exit 0; `npm test` 1027/1027 on both the system Node and Electron's; `npm run test:e2e` 14/14; `npm run test:e2e:packaged` 4/4. **Tests · 🔵 · `[fix here]` · `TESTING.md:173`** — the Conventions bullet, as first written for this PR, said the fast suite collects `*.test.cjs`, implying that was the only pattern. It is not. Since the entire point of the rewritten bullet is that filename is now the *only* barrier between the two suites, an incomplete list is the wrong kind of wrong. Probed all six candidate names under `tests/e2e/` against Node 24.14.1: `test-zz.cjs`, `zz.test.cjs`, `zz-test.cjs`, `zz_test.cjs` and `test.cjs` are collected; `zz.spec.js` is not. The bullet now names all five. **Fixed.** **Architecture · 🔵 · `[follow-up]`** — the test suite ships inside the packaged app: 94 entries under `/tests` in `app.asar`. **Not introduced by this PR** — `build.files` in `package.json` contains only `!vendor{,/**/*}` and is unchanged, so `test/` and `e2e/` were being packaged under their old names too. The rename made it visible and makes the fix a one-liner (`!tests{,/**/*}`), but it changes what users download and deserves its own PR and its own packaged smoke run. **Deferred deliberately.** Nothing across security, performance or cross-platform. Three checks worth naming, since a path rename is exactly where they go wrong: no file lost (76 + 7 in, 76 + 7 out, `.npmrc` fixtures included); the path-traversal fixture strings deliberately excluded from the depth rewrite; `git log --follow` traverses the moves. **Process disclosure:** the review standard asks for the judgement pass to run in a fresh context, and `.claude/skills/self-review` dispatches it to a subagent for that reason. It ran in the authoring session here, because that session is configured not to dispatch subagents — the same disclosure as on #376. A reviewer who did not write the change may still see something this pass did not. </details> <details> <summary>Implementation notes</summary> **The depth rewrite, and what it deliberately did not touch.** Two mechanical rules, both narrow on purpose: 1. `'../src/…'`, `'../scripts/…'`, `'../docs/…'` → `'../../…'`. Anchored on the directory name, so no bare `'../…'` string was in range. 2. `__dirname, '..'` → `__dirname, '..', '..'`. Anchored on `__dirname`, which leaves alone every `path.join(dir, '..')` where `dir` is a temp directory — `tests/unit/ipc-wiring.test.cjs:1355`, `tests/unit/patch-apply.integration.test.cjs:252`, and `path.join(SRC_DIR, '..')` at `ipc-wiring.test.cjs:103`, which resolves correctly once `SRC_DIR` itself is fixed. Neither rule can reach `'../../etc/passwd'` or `'../escaped.txt'`, which is the property that made this safe to do in bulk. **`tests/e2e/` needed almost nothing.** Every intra-suite require is sibling-relative (`../helpers/app.cjs`), so moving the tree as a unit left them correct. Only the two `REPO_ROOT` computations — `tests/e2e/helpers/app.cjs:27` and `tests/e2e/packaged/smoke.spec.js:19` — reach outside the suite, and both gained one level. **ESLint was verified by resolved config, not by a green run.** A glob that matches nothing produces exit 0 just as a glob that matches everything does. `npx eslint --print-config tests/e2e/journeys/engine.spec.js` returns `react-hooks/rules-of-hooks: [0]`, which only the `tests/e2e/**` block sets; the `tests/unit/**` block resolves to 137 Node globals with `sourceType: commonjs`. **Files changed outside `tests/`:** `playwright.config.js` (three `testDir`s and its header comment), `eslint.config.mjs` (two globs), `TESTING.md`, `AGENTS.md`, `.github/instructions/code-review.instructions.md`, `.github/workflows/unit-tests.yml`, `scripts/run-tests-electron.cjs`, `scripts/screenshots/capture.cjs`, `src/settings-store.js` — the last five only in comments that name a test file by path. Checked and confirmed to contain no references at all: `docs/`, `README.md`, `CONTRIBUTING.md`, `.buildkite/`, `.gitignore`. </details> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

…d not (#382) A real Git marks its loose object files read-only, and plain removal has a different hole on each platform: on POSIX nothing clears a directory whose write bit is missing, and on Windows nothing survives an entry something still holds open, because fse.remove passes no retry budget. Today the app mostly gets away with it because isomorphic-git sets no attribute; a mentor's own Git writing into a checkout changes that. And sites:delete swallowed the failure after the registry entry was already gone, so the site vanished from the sidebar while its folder quietly survived (#381). - src/remove-tree.js: try the cheap removal, and only on EPERM/EACCES walk what survived, add the owner write bit, and remove once more. The pass is additive (mode | 0o700 for a directory, mode | 0o200 for a file), so a tree that outlives the retry keeps the modes it came with; symlinks are skipped at the top of the recursion, root included, because chmod follows them out of the tree. Anything that still fails propagates. - sites:delete routes through it and stops pretending: the failure is logged with its path and returned as { ok: false, reason: 'remove-failed', path, code }. forget() still runs first, a recorded decision, so the honest thing left is to say what survived. - The renderer turns that into the confirmations queue's first error-tone notice, composed by a tested deleteFailureMessage in confirmations.cjs. - The e2e teardown discards through the same helper, keeping its final swallow and its ENOTEMPTY retry budget. Fixes #381

The node/npm shims the app puts on PATH are Electron running as Node, and Electron keeps process.versions.electron set in that mode. yargs reads exactly that to decide where a command's arguments begin, 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 is a command to run — itself, with no arguments — and the copy it starts does the same, without end. Each shim now preloads a small module that hides the Electron version from the process it starts, as an explicit --require argument rather than through NODE_OPTIONS: we are the ones invoking these processes, and an argument cannot fail to be inherited. Only versions.electron is hidden. Hiding versions.chrome alongside it broke Gutenberg's bundling step, and it answers a different question — what to compile for, not who is running the compiler. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

- Pin the wiring, not just the formatter: blanking the preload path at main.js's six call sites left the whole suite green, so ipc-wiring now reads the shims ensureNodeShimDir actually wrote. - Drop nodeCompatPath from the buildChildEnv call. It accepts no such key, so it was discarded silently and read as though descendants were covered by the environment. - Skip the two Electron-only tests explicitly instead of returning early, so a runtime-specific assertion cannot pass by asserting nothing. - Report a failed preload copy through the app's log rather than stderr, which a packaged app has nobody to read. Co-Authored-By: Claude Opus 5 (1M context) <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.

juanmaguitar · 2026-08-21T08:53:35Z

Closing in favour of a fresh PR against trunk. GitHub refuses to change the base of a stacked PR even after its parents are closed, so the rebased branch had to arrive as a new pull request.

The work is unchanged and now sits on trunk (a0fcbc9) — replayed cleanly, plus one commit pointing the two new tests at the tests/unit/ layout from #377. Lint clean, 1058 tests pass.

juanmaguitar added bug Something isn't working area: build-install npm install, builds, the dev server gutenberg-contributions Support Gutenberg as a contribution target (#251) labels Aug 11, 2026

juanmaguitar force-pushed the juanmaguitar/fix-electron-node-shim-argv branch from eb00606 to b497c6f Compare August 12, 2026 05:20

juanmaguitar and others added 4 commits August 12, 2026 08:15

juanmaguitar force-pushed the juanmaguitar/fix-electron-node-shim-argv branch from b497c6f to 6639cf5 Compare August 12, 2026 06:37

juanmaguitar added 2 commits August 12, 2026 09:39

juanmaguitar mentioned this pull request Aug 12, 2026

Bump version to 1.0.0-rc.1 #296

Merged

juanmaguitar and others added 16 commits August 12, 2026 10:28

Clarify Codex review instructions

ecb929f

juanmaguitar and others added 21 commits August 13, 2026 15:38

Remove test mode from pull request docs

5df2e48

Fix macOS signing key leakage in packaged apps (#390)

a0fcbc9

juanmaguitar mentioned this pull request Aug 21, 2026

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

Closed

juanmaguitar marked this pull request as draft August 21, 2026 08:42

juanmaguitar and others added 3 commits August 21, 2026 10:50

Point the rebased tests at the tests/unit layout

aac6d36

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.

juanmaguitar force-pushed the juanmaguitar/fix-electron-node-shim-argv branch from 83af07d to aac6d36 Compare August 21, 2026 08:51

juanmaguitar closed this Aug 21, 2026

juanmaguitar mentioned this pull request Aug 21, 2026

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

Draft

Navigation Menu

Uh oh!

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

[Fix] Stop a Gutenberg build from spawning processes without bound (#275)#283
juanmaguitar wants to merge 53 commits into
juanmaguitar/gutenberg-issue-and-pr-authoringfrom
juanmaguitar/fix-electron-node-shim-argv

juanmaguitar commented Aug 11, 2026

Uh oh!

juanmaguitar commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

juanmaguitar commented Aug 11, 2026

Why

What changes

How to test this

Risks and limitations

Related

Uh oh!

juanmaguitar commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants