🔴 Hot News: WordPress/contributor toolkit/pull/

zaerl · 2026-10-06T09:09:54Z

Why

First part of #559. The app has had two app-wide settings for a while, the contributor's WordPress.org username and the event they are at, asked once in the mentor handoff and kept in the store's preferences; there was no place to see or change them, and nowhere to put the settings the prototype has (#542). This adds the dialog and its first two tabs, with one new setting, so that the next ones (language, the PHP version and debug flags, what starts when a site opens and what happens when you quit, the theme of #560) each arrive as one small change to it.

What changes

A Settings dialog, opened from a cog in the footer beside "Give feedback", and from the menu: Settings… in the application menu on macOS (⌘,) and under File on Windows and Linux (Ctrl+,). The menu brings the main window forward first, as a deep link does; with no window open it opens one and nothing more, since the page is not yet listening.

General: New sites go here. The folder new sites are made in. With one set, the create-site dialog starts on it, and another can still be chosen for one site without changing the setting. A folder that is not there is refused when it is chosen, and Forget this folder clears it.

Account: How you contribute. The WordPress.org username and the event, through the same answers the mentor handoff uses, so the two cannot disagree; a pasted profile link is kept as the username it names, as it is there. And GitHub: which account the app opens pull requests with, and a way to sign out of it. Signing in stays where it is needed, in the review dialog.

The model. src/settings.cjs says what each setting is and what it accepts, pure, with the disk's answer handed to it. Main writes one setting at a time and answers with all of them as they then stand, so the window shows what was kept and not what it sent; the disk is asked only about a full path for a setting there is, and off the main process.

Beside it:

The folder field the create-site dialog asked with is a component of its own, since the settings ask with it too.
The refusals of the username and event parsers are translated. The dialog is the first screen on which they are seen in the pseudo-locale, so the journey found them.
A guide page, Settings, under Reference, and a sentence on the create-site page.

Not in this pull request:

Every other setting of the prototype's dialog: the language (next), the PHP version and the two debug flags, starting the server or the build watch when a site opens, what happens to running servers on quit, the theme (Add dark mode #560), a default editor and terminal, the admin login. Nothing here promises any of them.
Signing in to GitHub from the dialog. The device flow lives in the review dialog and is used there; the tab shows the account and signs out.
Dark mode: the dialog is painted with the design system's tokens, like the rest of the window, and will follow Add dark mode #560.

How to test this

Platforms: any. The journeys drive this on macOS and Windows. From the repository root, after npm ci and npm run build:once:

npx playwright test --project=journeys settings.spec i18n.spec create-site.spec

settings.spec.js: the cog opens the dialog on General; a folder chosen is shown, kept in the store and offered to be forgotten; a folder that is not there is refused with main's words and the kept one stays; the create-site dialog starts on the folder and still lets another be chosen for one site without changing the setting; the menu's Settings… opens the same dialog; forgetting the folder puts both dialogs back. On Account: a username that is not one is refused with the handoff's words; a profile link is kept as the username, the event with it, and Enter saves as the button does; an emptied event is forgotten; the GitHub account is named and signing out asks main and says so; reopened, the dialog is on General and shows what was kept.
i18n.spec.js: both tabs and a refusal in the pseudo-locale, and the menu item's label.
create-site.spec.js: the dialog with no folder set, as before.

I broke four of the settings journey's claims one at a time (the create-site dialog not given the folder; main not asking the disk; the menu's request not reaching the dialog; the username not shown as main kept it) and each turned it red. tests/unit/settings.test.cjs, settings-view.test.cjs, menu.test.cjs and the settings:* and menu tests in ipc-wiring.test.cjs hold the decisions, and the menu's window test is red on the first version of the handler.

Worth a look by hand (any platform, the current head): press the cog, or ⌘, / Ctrl+,, with the window minimised or behind a patch window: the window comes forward with the dialog open. Choose a folder, press Create new site: the Location is that folder. On Account, type your username and press Save; open Review & submit changes on a site with an edit and look at Hand it to a mentor: it offers to save the patch as that username.

What must not have happened: a folder chosen for one site changing the setting; the handoff and the dialog disagreeing about the username; the setting written when main refused the folder; the dialog opening in a patch window.

What I could not test: Windows and Linux, where the item is under File with Ctrl+,; the journeys run on Windows in CI and click the item by its id, which does not say where it is. And the small-window and slowed-status variants of the journeys: the dialog has no layout assertion to be sensitive to them.

Risks and limitations

Review: 2 passes, 11 findings fixed here, 3 left as follow-ups. Details below.
A remembered folder deleted after it was chosen is recreated silently by the setup, which ensureDirs the folder it is given. That was so before; the setting makes it easier to reach. Follow-up: have the setup refuse a folder that is not there.
Signing out here while a review dialog is open leaves that dialog saying "Signed in as …" until it is reopened, when it re-reads the account.
An unsaved Account draft is lost on a tab switch, since the design system's tabs unmount the panel that is not shown.
The window never learns of a setting changed in the store by something other than itself; nothing else writes one.

preferences in the one store, not the renderer's localStorage the prototype uses. Half of the settings to come are consumed in the main process (the PHP version at server start, the quit behaviour, the locale before a window exists), and the two that exist already live there.
One pure module for what a setting is, and one handler for one setting at a time. A value is checked at the IPC boundary as the username is, because the window is not the only thing that writes the store; the answer is the whole, so a canonical form chosen by main is what the window shows.
Global, not per-site. The prototype's "New sites" tab suggests values copied into a site at creation; here a setting is read when it is used. A per-site override can go in siteMeta later if one is wanted.
The username and the event through the handoff's own answers, not a second pair of fields writing the same keys: the handoff asks for them in the moment they are needed, the dialog is where they are changed afterwards, and both hold the one state.
GitHub shown, not signed in to, here. The device flow and the wait for its outcome live in the review dialog, where the account is used. A second copy for the settings would be a second flow to keep right.
The menu item written out beside the role's defaults. Giving a role a submenu replaces its items, so the app menu and the File menu are written item by item as Electron has them, with the one addition; the menu test pins the list.
The theme row left to Add dark mode #560. The design system makes dark cheap at the token level, but the older components still in the window ship light styles only, and a theme row that half works is what color-scheme.test.cjs exists to prevent.

Review outcome (required — see AGENTS.md)

Pass 1: 9 [fix here] · 2 [follow-up]. Pass 2 (on the fix-up): 2 [fix here] · 1 [follow-up]. All 11 fixed, 3 left as follow-ups.

Review: completed — a separate agent context, read-only; head a58fb4e / base 0cf005f; the store discipline of the two handlers read against Overlapping writes to a site's metadata can drop a ticket branch's patch base #172, the validation at the boundary, the menu against Electron's default roles, every string checked for render-time wrapping, the pot generation run, the journey read against the standard's rules; ESLint and the unit files run, Playwright not; 9 [fix here], 2 [follow-up].
- Fixed: the menu opened the dialog in whichever window Electron listed first and never brought the main window forward; the folder was stat'd synchronously on the main process; the i18n test's wait could pass before the Account tab had rendered; the folder field said "Not set" before the settings had been read; the folder was trimmed before it was stored; the guide promised settings that are not there; the General tab's heading pointed at settings that are not there; two claims in the model's header; the create-site page did not say where its folder now starts.
- Follow-ups left: a remembered folder deleted since is recreated by the setup; signing out while a review dialog is open.
Review: completed — a second separate agent context, read-only; head fc9b8b3 / base 0cf005f; the fix-up's yield checked against the store rule, the window choice, the untrimmed path through the create flow, the .or() wait; ESLint and the unit files run; 2 [fix here], 1 [follow-up].
- Fixed: the handler stat'd any string for any key before the pure check could refuse it; the corrected window choice had no test.
- Follow-up left: the menu's send is outside a try, where the deep link's is inside one; no realistic way to reach a throw was found.
Since review: fc9b8b3 → 0bee55b is those two fixes, not reviewed again. On 0bee55b: npm run lint, npm test (1975), the three journeys above and the packaged smoke pass. On fc9b8b3: npm run test:electron (1973) and the docs build pass; on a58fb4e: all 117 journeys.

Implementation notes

New: src/settings.cjs, src/renderer/settings-view.cjs, src/renderer/hooks/use-settings.jsx, src/renderer/components/settings-dialog.jsx, src/renderer/components/folder-field.jsx, docs/guide/settings.md, tests/unit/settings.test.cjs, tests/unit/settings-view.test.cjs, tests/e2e/journeys/settings.spec.js.
src/main.js: settings:get, settings:set, isDirectory, openSettingsFromMenu. src/preload.js: getSettings, setSetting, subscribeSettingsOpen. src/menu.js: the item and the two menus written out.
src/wporg-handle.cjs, src/patch-provenance.cjs: refusals through __() at the moment they are given.
tests/unit/ipc-wiring.test.cjs: settings:* wired, and the menu's window choice through the harness's recorded menu and windows.

Screenshots or recording

Not attached: I have no way to upload images from the command line. The journey above drives everything that changed, and the guide page says what each tab holds.

🤖 Generated with Claude Code

…r is The first of the settings (#559): a dialog on tabs, opened from a cog in the footer beside "Give feedback" and from the menu's "Settings…", in the application menu on macOS and under File elsewhere, with each platform's shortcut. The settings are app-wide, under the store's `preferences` beside the contributor's WordPress.org username and event, and for the same reason: a setting is about the person or the machine, not a checkout. What each is and what it accepts is one pure module, `src/settings.cjs`; main writes one at a time and answers with all of them as they then stand, so the window shows what was kept and not what it sent. General holds the folder new sites go in. With one set, the create-site dialog starts on it, and another can still be chosen for one site; a folder that is not there is refused. Account holds the username and the event through the answers the mentor handoff already uses, so the two cannot disagree, and the GitHub account the app acts for, which can be signed out of here. The refusals the username and event parsers give are now translated: the dialog is the first screen on which they are seen in the pseudo-locale. The folder field the create-site dialog asked with is a component of its own, since the settings ask with it too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…og gave it From the review. The menu's "Settings…" opened the dialog in whichever window came first, a patch window included, and never brought the main window forward; it now does what a deep link does. The folder is asked of the disk before the pure check and without holding the main process, and is stored as the system's dialog gave it: trimmed, a name that ends in a space would have been another folder's. Until the settings are read the folder field says so, where it said none was set. The General tab's heading names what is under it, the guide promises no setting that is not there, and the create-site page says where its folder now starts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…the menu reaches From the second review. The handler asked the disk about any string for any key before the pure check could refuse it; now only a full path for a setting there is. The menu's choice of window, which the first review corrected, has the wiring test it lacked: the main window gets the request and is brought forward, a patch window listed first does not, and with the main window closed one is opened and sent nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

coderabbitai · 2026-10-06T09:10:04Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 5ac58801-dd88-419a-8a4c-b5a0e3001e12

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

## Why Second part of #559, on the dialog of #638. The app is translatable (#540) and picks its language from the operating system's; a contributor on a machine set to a language they do not read, or at a shared laptop, had no way to choose. This adds the language to the settings. ## What changes **Language, on the General tab.** Your system's language, which is the default, or English, or any language the build ships a catalog for, each named in its own language ("Deutsch", "Português (Brasil)") so that someone looking for theirs finds it whatever the app is showing. The list is what `src/languages/` holds, so it grows as releases ship catalogs; today that is English. **Where it is read.** Main puts the chosen language in front of the OS's languages and loads the first one with a catalog, as it walked the OS's before. In front, not in their place: a chosen language whose catalog a later release dropped falls back to the OS's language, not to English. The `--lang` switch still replaces the whole list; it is how the journeys pick a locale, and a flag typed at launch is a decision. **A change applies after a relaunch**, since main applies the catalog as it starts, for its own strings and the window's. The dialog says so once what is set is no longer what the window started in, and offers **Relaunch now**: a quit and a start, so the quit sweep ends every child the app started, as it does on any quit. The new instance gets this one's arguments less a `wpct://` address a cold start was given, which is not a second request for its ticket, and a `--lang`, which would outrank the language just chosen; on Linux it is started from the AppImage, since the mounted copy is gone once this one quits. **The first read of the store is now before the window exists.** One that cannot be read is logged and counts as no choice, so the window still opens; every later read then fails as it did before. **Beside it:** the catalogs' README and CONTRIBUTING.md say where the language comes from now; a journey can launch without `--lang`, as the app launches for a contributor. **Not in this pull request:** - Switching the language without a relaunch. Main applies the catalog once, at startup, for its own strings; the window would need a second path, and the menu and the native dialogs a third. - The slugs no operating system reports (`pirate`, `art-xemoji`), which are not offered, as they could not be picked from the OS list either. - The rest of #559: the PHP version and the debug flags, what starts when a site opens and what happens on quit, the theme (#560). ## How to test this Platforms: any. The journeys drive this on macOS and Windows. From the repository root, after `npm ci` and `npm run build:once`: ``` npx playwright test --project=journeys settings.spec i18n.spec ``` - `settings.spec.js`, the language journey: the pseudo-locale set in the store, with no `--lang`, is the language the app starts in; the control shows it though the build has no catalog for it; English chosen is kept at once and the dialog offers the relaunch, which asks main; the system's language chosen is kept as none and the offer stays; the pseudo-locale chosen again is refused, since the build has no catalog for it; started again with English kept, the app is in English and the control says so with no offer. - `i18n.spec.js`: the dialog with the control, in the pseudo-locale. I broke two of the journey's claims one at a time (main not reading the chosen language; the relaunch button asking nothing of main) and each turned it red. `tests/unit/i18n.test.cjs` holds the list, `settings.test.cjs` what is accepted, `settings-view.test.cjs` the control's entries and when the relaunch is offered, and `ipc-wiring.test.cjs` the order the languages are tried in, the store that cannot be read, and what the relaunch is given. **Worth a look by hand** (any platform, the current head): Settings → General → Language: with no catalog shipped the list is your system's language and English. Choose English, press **Relaunch now**: the app quits and comes back, the control says English and offers nothing. Choose your system's language, press the cog's relaunch again: as before. With `npm run i18n:download -- --locales=de` run first (it needs the network), "Deutsch" is in the list, and chosen and relaunched, the app is in German where it is translated. **What must not have happened:** a running server or build left behind by the relaunch (the quit sweep ends them, as on any quit); a `wpct://` ticket opened again after a relaunch of an app that was started from a link (Windows and Linux); the app failing to open a window when `settings.json` is not JSON. **What I could not test:** Linux, where the relaunch is started from the AppImage; the unit test pins what Electron is told, not that the image starts. And a real catalog in the dialog: the repository ships none, and I did not run the download. ## Risks and limitations - Review: 2 passes, 4 findings fixed here, 1 follow-up taken here. Details below. - **The language names are not translated**, by design: each is in its own language. - **A store that cannot be read** lets the window open but every later read fails as it did before this change; the app does not say which file is at fault. - The relaunch is a quit: nothing asks first, as nothing asks on ⌘Q. ## Related Part of #559, which is part of #542. Follows #638. Translations: #540. --- <details> <summary>Design decisions and alternatives considered</summary> - **A relaunch, not a live switch.** Main applies the locale once, at startup, for the menu, the native dialogs and the sentences it sends, and the window reads its catalog before its first render; three paths to re-run for a setting changed a few times in the app's life. The dialog says what the relaunch costs. - **The chosen language in front of the OS's, not in their place.** A release drops a catalog that fell under the cut-off; the contributor who chose it should get their OS's language, as they did before choosing. - **The control's entries with their tags as values**, drawn by the dialog, and the list also handed over for the names. Handed the list alone, the design system's select takes each entry as its own value and tells them apart by identity, which a value kept in the store cannot match. - **Entries made from the language the window started in**, not the one set, so that a choice does not change them: an entry taken away under the control while it is choosing is reported as a second choice, of none. A language the window started in that the build has no catalog for is listed, so the control shows what is set, and refused if chosen again. - **The quit sweep over `app.exit`.** The relaunch goes through `app.quit()` so every child the app started is ended first. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **Pass 1: 3 [fix here] · 0 [follow-up] · 4 style notes. Pass 2 (on the fix-up): 1 [fix here] · 1 [follow-up] · 1 style note. All 4 fixed, the follow-up taken here, every style note applied.** - **Review:** completed — a separate agent context, read-only; head `82aac18` / base `fe02e6b`; the store read traced from `whenReady`, the relaunch against Electron's quit events and the single-instance lock, the language names checked on ICU for lowercase tags and Valencian, the select's value handling read in the design system's source, the journey read against the standard; ESLint, the unit files and the pot extraction run; 3 [fix here]. - Fixed: a store that could not be read would have stopped the window opening, since the locale is resolved before it; a chosen language replaced the OS list, so one whose catalog was dropped gave English rather than the OS's language; the relaunch reused the launch's arguments, a `wpct://` address and a `--lang` included. - Style notes, applied: a stale comment in the store module; a silent catch on the relaunch; the system entry's mapping as a tested function; the catalogs' README counting entries not offered; a journey comment. - **Review:** completed — a second separate agent context, read-only; head `7eab0b6` / base `fe02e6b`; the fallback order followed through `resolveCatalog` for each case, the relaunch arguments against Electron's documented default and a dev launch, the three changed wiring tests confirmed red on the first commit; ESLint and the unit files run; 1 [fix here], 1 [follow-up]. - Fixed: the README still described the chosen language as used instead of the OS's rather than before them. - Follow-up, taken here: on Linux the relaunch would start from the AppImage's mount, which is gone once the app quits; the new instance is started from the image. - Style note, applied: one comment line left unwrapped. - **Since review:** `7eab0b6 → b68f7c2` is that finding, the follow-up and the rewrap, not reviewed again. On the head: `npm run lint`, `npm test`, `npm run test:electron`, the settings, i18n and create-site journeys and the packaged smoke pass; all 118 journeys passed on `82aac18`. </details> <details> <summary>Implementation notes</summary> - `src/i18n.cjs`: `languageChoices(names)`. `src/settings.cjs`: `locale`. `src/main.js`: `localeReply` reads the store, `languages()`, `i18n:languages`, `app:relaunch`, `relaunchArgs`. `src/preload.js`: `listLanguages`, `relaunch`. - `src/renderer/settings-view.cjs`: `languageItems`, `languageValue`, `languageChanged`, `SYSTEM_LANGUAGE`. `src/renderer/components/settings-dialog.jsx`: `LanguageControl`. `src/renderer/hooks/use-settings.jsx`: `loaded`. - `tests/e2e/helpers/app.cjs`: `lang: false` drops the `--lang` switch. </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The language journey drives everything that changed. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…he next server start (#640) ## Why Third part of #559, on the dialog of #638. Every site's development server ran on the PHP the bundled Playground chooses and with WordPress's debug constants fixed on; a contributor whose ticket is about PHP 7.4, or about what a site does with `WP_DEBUG` off, had no way to see it. This adds the PHP version and the two debug constants that can be turned off to the settings, and shows what a server will start with in the open site's details. ## What changes **A Sites tab: Development server.** The PHP version, one of the seven the bundled Playground has (7.4 to 8.5), 8.3 unless another is chosen; and two switches, **Report notices and deprecations (WP_DEBUG)** and **Use unminified scripts (SCRIPT_DEBUG)**. The rest of the constants a server is booted with (the log, the display, the disabled fatal handler and updater) stay as they were and are not anyone's to change. **Applied at the next start.** Main reads the three at every server start and hands them to the runner in its config; a server that is running keeps what it started with until it is stopped and started, and the tab says so. A version a release has taken out of the bundle is passed over for 8.3, with a line in the app log, and the control shows 8.3 as chosen with a note saying why. **The details say what a server will start with**, as the prototype has them: "WordPress Core · PHP 8.3" on the Checkout row, and a Debugging row with the constants that are on, or "Off". **What WP_DEBUG off does, said for what it is.** It takes away notices and deprecations. Warnings and errors still reach `debug.log` and the browser, and `error_log()` calls still reach `debug.log`, because Playground's own PHP keeps logging and display on whatever WP_DEBUG says. The switch, the guide and the constants' module say so. **The guide's pictures are retaken.** The details changed here, and the footer's cog from #638 had never been retaken. The three pictures of a site's page are taken in a window 900 high, so the details' Build watch section is whole. **Not in this pull request:** - The PHP version a *running* server is on, in the Server section, where the details show the version the next start is given. Follow-up. - Per-site values: these are global, like the other settings. A per-site override can go in `siteMeta` if one is ever wanted. - The remaining constants (`WP_DEBUG_LOG`, `WP_DEBUG_DISPLAY`, `WP_DISABLE_FATAL_ERROR_HANDLER`, `AUTOMATIC_UPDATER_DISABLED`), on purpose: `src/wp-debug-constants.js` says why each is on. - The rest of #559: what starts when a site opens and what happens on quit; the theme (#560). ## How to test this Platforms: any for the dialog and the details; macOS for the real server, below. The journeys drive the dialog and the details on macOS and Windows. From the repository root, after `npm ci` and `npm run build:once`: ``` npx playwright test --project=journeys settings.spec site-header.spec i18n.spec ``` - `settings.spec.js`, the Sites journey: the fallbacks (8.3, both constants on); 8.4 chosen is kept, WP_DEBUG off is kept and SCRIPT_DEBUG left as it was; the open site's details then say "WordPress Core · PHP 8.4" and "SCRIPT_DEBUG"; restarted, the tab shows what was kept. - `site-header.spec.js`: the details name the project with the PHP a server starts on and which constants are on. - `i18n.spec.js`: the tab in the pseudo-locale. I broke two claims one at a time (main not reading the settings at a start; the switch not writing) and each was caught, the first by `ipc-wiring.test.cjs`, the second by the journey. `tests/unit/runner-wiring.test.cjs` proves the runner hands Playground the version and turns the two constants off in the blueprint; `wp-debug-constants.test.cjs` that nothing else in the set moves; `settings.test.cjs` and `settings-view.test.cjs` what is accepted and what the control shows. **The real server, run by me on this Mac (Apple Silicon, the current head), as TESTING.md asks for a change to `src/server-runner.js`.** The app was driven through Playwright with a throwaway profile listing a built `wordpress-develop` checkout; **Start development server** was pressed; the served WordPress was logged in to with the app's own credentials and asked, on Site Health's Info tab, which PHP it runs on: - PHP version set to **8.4**: the server came up, Site Health reported **PHP 8.4.25**, wp-admin served, Stop ended it. - PHP version left at the fallback, **8.3**: Site Health reported **PHP 8.3.33**. **Worth a look by hand** (any platform, the current head): Settings → Sites, choose 7.4, close: the details say "PHP 7.4". Start the server: wp-admin → Tools → Site Health → Info → Server says PHP 7.4. Turn WP_DEBUG off, stop and start the server: a page with a deprecated call shows nothing for it, and a page that calls a missing function still shows the fatal. **What must not have happened:** a running server changing PHP or constants under the contributor; a change applied to a site other than the next start; `WP_DEBUG_LOG` or the fatal handler's constant moving with the switches; the details saying a version the server will not start on. **What I could not test:** Windows, and a real server with WP_DEBUG off (I checked what the code sends, not a page's output). ## Risks and limitations - Review: 2 passes, 8 findings fixed here, 1 follow-up taken here (the real boot), 2 left. Details below. - **The details show the PHP of the next start, not of a server already running.** Change the version while a server runs and "PHP 8.4" sits above "Server running" on 8.3. Follow-up: the Server section should name the running server's PHP. - **Main requires `@php-wasm/universal`** for the list of versions. It is not a declared dependency; it is `@wp-playground/cli`'s, at whatever version that pins, and the comment says so. Declaring it pinned is a `package.json` decision I have left to the maintainer. - **The Electron unit suite failed one test once** in a run of 2004 and passed on three runs after; I could not see which test it was the first time. - A `WP_DEBUG` switch that is off still leaves `WP_DEBUG_LOG` on, by design, so the debug.log tab keeps showing warnings and errors. ## Related Part of #559, which is part of #542. Follows #639. --- <details> <summary>Design decisions and alternatives considered</summary> - **Global, applied at the next start, and read by main at each start.** The prototype's "New sites" tab suggests values copied into a site at creation. Here the version and the constants are what the server is booted with, so reading them at each start is simpler and also reaches existing sites. - **An explicit fallback, always passed.** The CLI's own default is 8.3 today; passing it ourselves keeps the behaviour stable across a Playground bump, and a stored version the bump took out falls back to it rather than to whatever the CLI then chooses. - **The list from the bundle, not written down.** `SupportedPHPVersions` from `@php-wasm/universal` is what the runner can serve; a list of our own would drift. - **Only two constants have a switch.** The others exist to make a fatal show as itself, the log exist for the tab and the updater stay out of a checkout; a switch for each would be one more thing to get wrong at a Contributor Day. - **One decision for which version is shown and used**, `phpVersionChoice` in `settings-view.cjs`: the dialog, the details and the start handler all agree on the fallback. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **Pass 1: 3 [fix here] · 3 [follow-up]. Pass 2 (on the fix-up and the details): 5 [fix here] · 1 [follow-up]. All 8 fixed, the real-boot follow-up taken here, 2 left.** - **Review:** completed — a separate agent context, read-only; head `9131ed7` / base `77751a1`; the runner's options checked against the CLI's `runCLI` signature and its persistent `start` command, WordPress's `wp_debug_mode()` and Playground's php.ini and mu-plugin read for what WP_DEBUG off does, the design system's switch and toggle props confirmed; ESLint and the unit files run; 3 [fix here], 3 [follow-up]. - Fixed: WP_DEBUG off was said to empty the log and the browser of errors, where it takes away notices and deprecations; which versions the control offers was decided in the component; the real boot TESTING.md asks for was not recorded. - Follow-ups: the undeclared require (a comment, left to the maintainer to declare); a stored version the bundle no longer has was shown as chosen but not used (taken here); the journey pins 8.4 (left). - **Review:** completed — a second separate agent context, read-only; head `ca84255` / base `77751a1`; the four rewordings checked against WordPress and Playground again, the details' new rows and their journeys, the retaken pictures opened against their alt texts; ESLint and the unit files run; 5 [fix here], 1 [follow-up]. - Fixed: the details showed the version set where the dialog and the start used the fallback; `error_log()` was said to reach the browser; three pictures cut the details' Build watch section off under the footer; the require had no comment on whose dependency it rides; a failed read of the versions left an enabled, empty control. - Follow-up left: the Server section naming the PHP a running server is on. - **Since review:** `ca84255 → ba66103` is those five fixes and two style notes, not reviewed again. The commit after it changes one journey: `site-header.spec.js`'s stickiness test failed on both runners, since the Debugging row left the details too tall to fit over the tray at its smallest on their screens; its seeded site now has no creation date and no trunk date, two rows fewer, and the journey was run at windows 700, 680 and 640 high and is red at 640 with the app's rule broken. On the head: `npm run lint`, `npm test` (2002), `npm run test:electron` (2003), all 119 journeys, the packaged smoke and the docs build pass. The twelve pictures were taken four times before the last change and two of them moved by 12 and 30 pixels on a corner between takes, the known noise; the three retaken after it were taken once. </details> <details> <summary>Implementation notes</summary> - `src/settings.cjs`: `phpVersion`, `wpDebug`, `scriptDebug`, `acceptSwitch`. `src/wp-debug-constants.js`: `debugConstants(debug)`. `src/server-runner.js`: `php` and the constants from the config. - `src/main.js`: `phpVersions()`, `playground:php-versions`, `playground:start` reads the store at each start. `src/preload.js`: `listPhpVersions`. - `src/renderer/settings-view.cjs`: `phpVersionChoice`. `src/renderer/hooks/use-settings.jsx`: `php`. `src/renderer/components/settings-dialog.jsx`: `SitesTab`. `src/renderer/site-details.cjs`: `phpVersion` and `debug` in the rows. - `scripts/screenshots/shots.cjs`: the three site views at 1200×900. </details> <details> <summary>Screenshots or recording</summary> The retaken images are in the diff: `docs/public/screenshots/` shows the details before and after. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…what a quit stopped (#641) ## Why Fourth part of #559, on the dialog of #638. Every site started cold: its development server and its build watch waited for a press, every time the site was opened and every time the app was. A contributor who works on one site all afternoon, and a mentor who opens the app to find the sites they left running, each had a press to make first. ## What changes **Opening and quitting, on the General tab.** - **Start the server when I open a site** and **Start the build watch when I open a site**, both off unless turned on. On, opening a site that is set up starts its server, its watch or both, as a press of Start would; on WordPress Core the server's start brings the watch with it. Nothing starts for a site whose setup, update or deletion is under way, and nothing already running or starting is started again. - **When I quit, running servers and build watches**: **Stop them**, which is what the app has always done, or **Stop them, and start them again next time**. Under the second, main writes down at quit which sites had a server or their project's watch running, and the next launch starts them again, whichever site it opens on, then forgets the list. The prototype's third answer, leaving them running past the quit, is deliberately not offered: the quit sweep exists to end every child the app started (#83). **How a site decides.** Opening a site is an edge, and so is the list a quit left: each is consumed once, when the site is ready for it, in a pure module (`src/renderer/auto-start.cjs`) the row asks. The row keeps the gates and the calls: the server's start first, awaited, then the watch where that start did not bring it up. A watch the server's start was refused, because the terminal is held, is not asked for twice. **How the quit remembers.** `before-quit` runs synchronously and is not awaited, so the write goes through the store's synchronous accessor, which the first read at startup has made; a store that cannot be written is logged and does not stop the sweep. A site with both its server and its watch running is listed under both. **Not in this pull request:** - Leaving servers running past the quit. See above. - Starting a site that is not set up: the setup checklist is what starts things the first time. - The theme (#560), the last of #559's rows. ## How to test this Platforms: any. The journeys drive this on macOS and Windows. From the repository root, after `npm ci` and `npm run build:once`: ``` npx playwright test --project=journeys auto-start.spec settings.spec i18n.spec site-header.spec ``` - `auto-start.spec.js`: with the server set to start, the site the window opens on asks for its server once, and on Core the watch with it; opening another site asks for that site's; coming back to one whose server runs asks for nothing and does not stop it. With the watch set to start, the watch is asked for and no server; turned off, a relaunch starts nothing. With a list a quit left, the served site's server and the watched site's watch are asked for, whichever site the window opens on, the served site's watch once; the list is forgotten as it is read; a relaunch with nothing left starts nothing, and a list left under 'restart' is not followed under 'stop'. The quit setting is chosen and kept. - `settings.spec.js`, `i18n.spec.js`: the dialog, in English and in the pseudo-locale. - `site-header.spec.js`: the details' stickiness journey now sizes its window itself. Nothing is run: the handlers that start the server and a script are stood in, and the journeys read what they were asked. The quit's own write is `ipc-wiring.test.cjs`'s subject: under 'restart' it writes the sites with a server and those running their project's watch, under 'stop' nothing, and a store that cannot be written is logged and the sweep still reaches every child. I broke the open-site gate (every row started) and the quit's write, and each was caught. **Worth a look by hand** (any platform, the current head, a site that is set up): Settings → General, turn on **Start the server when I open a site**, close, open another site and come back: the server starts, and the Logs tray shows its lines. Set **When I quit** to **Stop them, and start them again next time**, leave the server running, quit, open the app: the server starts again on that site, whichever site the window opens on. Quit again with it running and set the quit back to **Stop them** before you open the app: nothing starts. **What must not have happened:** a server started for a site whose setup or update is under way; a server stopped by the setting (it only starts); a second watch started on a site whose server's start started one; a server or watch running past the quit; the list followed twice after a relaunch that was not a quit. **What I could not test:** the real quit path end to end, since the journeys stand in for the server; the wiring test drives the handler with recorded children. ## Risks and limitations - Review: 2 passes, 12 findings fixed here, none left. Details below. - **A start asked for cannot be taken back:** a site deactivated, or an update pressed, in the moment between the plan and the server's first answer still gets its start. The window is one request to main. - **Two servers starting at once** (on a relaunch with two sites listed) can both find Playground's first port free and one fall back to another port; pressing Start on two sites quickly has the same race today. - **On Gutenberg a relaunch with both listed starts the watch**, which removes `build/` under the just-started server for its rebuild, as both settings on do. - The stickiness journey's second half held only on screens that leave the tray at its largest too little room; on a tall screen it did not, so the journey sizes its window to the smallest the suite runs on. ## Related Part of #559, which is part of #542. Follows #640. The quit sweep: #83. --- <details> <summary>Design decisions and alternatives considered</summary> - **Edges, consumed once, in a pure module.** The first version decided inline in the component, which the suite cannot load, and predicted whether the server's start would bring the watch from a copy of "built" that the start re-reads from disk. Now the server's start is awaited and the watch's state read after it. - **The quit writes through a synchronous accessor.** `before-quit` is not awaited by Electron; the store is an ESM import made at startup for the locale, so it is there by the quit, and the sweep must not wait on it. - **Two answers to the quit question, not three.** Leaving children past the quit undoes #83. - **The list is read once and forgotten**, and only while the setting still says 'restart': a launch that ends badly does not start everything twice, and a list left under 'restart' is not followed under 'stop'. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **Pass 1: 5 [fix here] · 2 [follow-up]. Pass 2 (on the fix-up): 3 [fix here] · 2 [follow-up]. All 12 applied here.** - **Review:** completed — a separate agent context, read-only; head `e5a0ba4` / base `c34aa45`; the quit path read against the store rule and the sweep, the effect's edges and gates, Playground's port choice for two servers at once, the journeys against the standard; ESLint, the unit files and the pot extraction run, Playwright not; 5 [fix here], 2 [follow-up]. - Fixed: an error writing the list at quit would have skipped the sweep; a site with both its server and its watch was remembered under the servers only, and on Gutenberg with a build the watch was not started again; the start decision lived in the component, with a prediction from a stale copy of "built"; a #557 comment sat over the wrong code; the journeys slept. - Follow-ups, taken here: the stickiness journey's comment and its wait for the resize; a dead check in the effect. - **Review:** completed — a second separate agent context, read-only; head `c75017f` / base `c34aa45`; the awaited server start traced through the hook's two paths for whether the watch's state is set before it resolves, the throwing-store test checked to be red on the first commit; ESLint and the unit files run; 3 [fix here], 2 [follow-up]. - Fixed: a round trip to main was not a sound sign that nothing was started, since a start asks main only after awaits of its own, so the journeys now read the controls after the settings are in the page and the effects have run; the resume journey did not seed the list a quit now writes; the effect's catch was silent. - Follow-ups, taken here: a watch the server's start was refused was asked for a second time; the throwing-store test did not read the log. - **Since review:** `c75017f → 5117052` is those five, not reviewed again. On the head: `npm run lint`, `npm test` (2018), `npm run test:electron` (2019), all 123 journeys, the packaged smoke and the docs build pass. </details> <details> <summary>Implementation notes</summary> - `src/settings.cjs`: `autoStartServer`, `autoStartWatch`, `quitBehavior`. `src/resume-sites.cjs`: `sitesToResume`, `readResume`. `src/settings-store.js`: `peekStore`. `src/main.js`: `scriptByRunId`, `rememberRunningSites` in `before-quit`, `sites:resume`. `src/preload.js`: `takeResumeList`. - `src/renderer/auto-start.cjs`: `autoStartPlan`. `src/renderer/settings-view.cjs`: `quitItems`, `resumeFor`. `src/renderer/index.jsx`: the row's effect; App reads the list once. `src/renderer/components/settings-dialog.jsx`: `OpeningAndQuitting`. - Tests: `tests/unit/auto-start.test.cjs`, `resume-sites.test.cjs`, `settings.test.cjs`, `settings-view.test.cjs`, `ipc-wiring.test.cjs`; `tests/e2e/journeys/auto-start.spec.js`. </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The journeys drive everything that changed, and the guide page says what each control does. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

## Why 41 commits have reached trunk since `v1.2.1`, and milestone `v2.0.0-beta.1` has 77 closed issues and none open. The main changes are the redesign on the WordPress Design System (#591), a settings dialog with language, theme, PHP version and debug flags (#638 to #651), and translation of the whole interface (#634 to #664). This is a beta: it ships with `--prerelease`, so `releases/latest` stays on `v1.2.1`. `electron-builder` names the artifacts after `package.json`, so the version has to move before the tag and artifacts exist. ## What changes - **The version.** The three root version fields in `package.json` and `package-lock.json` move from `1.2.1` to `2.0.0-beta.1`. No dependency versions change. - **The first shipped translations.** `npm run i18n:download` added a catalog for each of the seven locales at 80% or more on translate.wordpress.org. None was removed, and no right-to-left locale was held back. | Locale | Translated | Strings | | --- | --- | --- | | cs | 94% | 1001 | | en-ca | 100% | 1062 | | en-gb | 100% | 1062 | | gl | 100% | 1062 | | hu | 100% | 1062 | | nl | 100% | 1062 | | pl | 96% | 1028 | ## How to test this **Platforms:** any for steps 1 to 3. The artifact checks need macOS for `Info.plist` and Windows for the `.exe`. **Starting state:** this branch checked out with `node_modules` installed. Run every command from the repository root. 1. `npm run lint` is clean and `npm test` passes. 2. `node -e "console.log(require('./package.json').version)"` prints `2.0.0-beta.1`. 3. `python3 -c "import json;d=json.load(open('package-lock.json'));print(d['version'], d['packages']['']['version'])"` prints `2.0.0-beta.1 2.0.0-beta.1`. 4. On the Buildkite build for the current head, the artifacts are named `wordpress-contributor-toolkit-2.0.0-beta.1-*`. The `app.asar` root holds exactly `node_modules`, `package.json` and `src`, with npm 11 under `node_modules/npm` and no `.codesigning`. On macOS, `CFBundleShortVersionString` reads `2.0.0-beta.1`. 5. In the installed app, set the language to Dutch in the settings and relaunch. The interface is in Dutch. **What must not have happened:** no dependency version may change. `git diff trunk -- . ':!src/languages'` shows exactly three changed lines, all of them the root version. ## Risks and limitations `npm test` run from an interactive terminal fails two integration tests on macOS. That is a test bug, fixed separately in #680. It does not affect CI or the app. ## Related Milestone `v2.0.0-beta.1`. Release notes follow on the release. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 [fix here] · 0 [follow-up]. Automatic checks only, as in #533: this change has no logic to review. - **Review:** completed, deterministic layer, in session; head `9853082` / base `5d5f701` (trunk). `npm run lint` passes, `npm test` passes (2098 tests, 0 failures, 2 skipped), both files are valid JSON, and the diff outside `src/languages/` is the three root version fields. Outcome: nothing to fix. - **Since review:** none. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code)

@zaerl

…n the editor (#677) ## Why After a patch is applied or a pull request is checked out, the app says how many files changed and nothing more. Unless a contributor already knows which files to look at, they can't find the change, and their own edits on top of it aren't listed anywhere. The only way to undo one of their edits was **Discard all changes**, which also took the patch away. This is #669 in one PR. It was a stack of five (#677, #678, #679, #682, #683); they are folded together here because each builds the same section, and a reviewer was already testing the top of the stack. ## What changes - The site's details gain a **Changed files** section, last in the sidebar after **Build watch**, so a long list grows without pushing the server and the watch out of reach. - It has up to two groups, each under its own heading and drawn as a tree of folders: - **From the patch** / **From PR #N**: the change's own files, each with a pencil when the contributor has edited it since. - **Your changes**: every other file changed on the branch, with a plus when the contributor created it. On a ticket with nothing applied, this is the only group. - Each folder starts open, and a chain of folders that hold only one another shares one row (`tests/phpunit/tests/wp`), as GitHub and VS Code draw them. A folder row's button, or the Left and Right arrows, close and open it. Each file shows its name with a page icon and its full path as a tooltip. Drawn with `__experimentalTreeGrid` from `@wordpress/components`, which gives the tree roles a screen reader needs and the arrow keys between rows. - One click on a file opens it in the first editor the app detects, inside the site's folder (`open -a App <site> <file>`, `Code.exe <site> <file>`). With no editor detected, the file is selected in Finder or Explorer instead. "Choose application…" after a file fails to open opens that file in the application picked. - **A revert button** on each file the contributor changed, at the end of its row. It asks first, and leaves the file as **Discard your changes** would: a file of theirs back to the base, a new file deleted, a pull request's file back to the pull request's version, a patch's file back to how the patch left it with the patch still applied. A patch file they did not touch, and either end of a rename, have no button. While an install, build, trunk update or dev server runs, the button is held and says why. - **Discard your changes** (renamed from "Discard all changes") now removes only the contributor's own work. With a patch applied, the tree goes back to the base and the patch is re-applied from its stored text, so the patch and **Revert this patch** stay. If the text was not kept, the patch goes too and the dialog says so first. **Discard this ticket to its base** still removes the patch. The ticket card's discard button from #696 calls the same discard, so every "discard your changes" control keeps the patch. - How the pencil is decided: a pull request is its branch's base, so any of its files with changes on top was edited. A patch is an uncommitted layer, so each file it wrote is fingerprinted when applied (SHA-256, line endings normalised, `src/change-fingerprint.cjs`) and compared with the disk. - The list refreshes when the window regains focus, which is when a contributor comes back from their editor, and whenever the app itself changes the tree. - New IPC `git:revert-file`, which accepts only paths Git reported as changed. `editor:open` and `dir:show` take an optional file inside the site, checked by its real path, so a symbolic link that leads out of the site is refused. Not in this PR: an editor preference, per-file diffs, marking deleted files, live updates while the app keeps focus, undo after a revert, reverting several files at once, and remembering which folders were closed across a relaunch. ## How to test this Platforms: macOS and Windows. The launch arguments, path checks and the revert's writes (`core.autocrlf`) differ per platform. The journeys cover the flow with stubbed editors and fixture repositories. Run from the repository root: ``` npm run test:e2e -- tests/e2e/journeys/patch-apply.spec.js tests/e2e/journeys/pr-checkout.spec.js tests/e2e/journeys/i18n.spec.js ``` They check opening a file in the editor or the file manager, "Choose application…", the groups and their marks, the list during a build, after a discard, after a refused apply and after a refocus, each kind of revert, and that a folder closes and opens and the revert button is reached with the arrow keys. What they can't reach is a real editor, a real `wordpress-develop` clone and a real pull request, so that part is by hand. **Starting state:** a site with a real `wordpress-develop` clone, VS Code installed, the dev server stopped, and a ticket linked whose Trac attachment applies cleanly, for example #23602 with `23602_pr14112_20261008.diff`. 1. Apply the attachment. **Changed files** appears at the bottom of the sidebar, under **From the patch**, as a tree of two folder rows: `src/wp-includes` over `class-wp.php`, and `tests/phpunit/tests/wp` over `queryPosts.php`. No pencil, no revert button. 2. Click the `src/wp-includes` folder. Its file is hidden and its chevron points right. Click it again. The file is back. 3. Click `class-wp.php`. VS Code opens the site folder with that file open. Click `queryPosts.php`. It opens in the same VS Code window. 4. In VS Code, change `class-wp.php` and `src/wp-login.php`, and create `src/new-helper.php`. Switch back to the app. `class-wp.php` has a pencil; under **Your changes**, `wp-login.php` has none and `new-helper.php` has a plus. All three have a revert button. 5. Revert `new-helper.php`. The dialog asks "Delete src/new-helper.php?". Confirm. The file is gone from disk and from the list. 6. Revert `class-wp.php`. The dialog says it goes back to how the patch left it. Confirm. The file has the patch's change and not yours, and the pencil is gone. 7. Change `class-wp.php` again and switch back. Click **Review & submit changes**, then **Discard your changes**. The dialog says the patch stays applied. Confirm. `class-wp.php` has the patch's change only, `wp-login.php` is back to trunk, and **Revert this patch** is still there. 8. Click **Revert this patch**. The section is gone. 9. Link a ticket with a GitHub pull request and check it out. **Changed files** lists its files under **From PR #N**, the same files its preview did. 10. On Windows, repeat steps 1, 3 and 6 with a site whose path has a space in it. **What must not have happened:** - A second VS Code window for the same site. - Anything opened outside the site folder. - A file listed in both groups. - Any patched file with a pencil right after applying, or right after step 6, before you changed anything. On Windows this would point at line endings. - The patch's change missing from `class-wp.php` after step 6 or step 7. - Any file other than the one reverted changed by a revert. - After checking out a different pull request, the list from the previous one. - A focus ring cut off at either edge of the sidebar, on a folder, a file or a revert button, after tabbing in or after cancelling a revert. ## Risks and limitations - Review of the combined branch: 7 [fix here] fixed, 3 [follow-up] deferred (see the review outcome). CodeRabbit's one finding, on patches applied before this PR, is fixed. - Rebased onto trunk after #696 and #697. The ticket card's discard button that #696 added now keeps an applied patch or pull request, like the note's link and the review modal's; before the rebase it would have removed the patch. - `__experimentalTreeGrid` is not a stable export yet; the window already uses another `__experimental` control the same way (`create-site-dialog.jsx`). - In the tree, the pencil and plus are the file button's description rather than focusable stops of their own, because only buttons are in the tree's reach of the arrow keys. A screen reader hears them; a sighted keyboard user sees the icon but can't open its tooltip. Follow-up. - Verified by hand with VS Code on macOS. PhpStorm, Zed, and Windows generally are untested with a file argument, and the revert and discard have not been run by hand on a real clone or on Windows. - A chain of folders shares one row, so deep paths indent less, but a long merged folder name, like a long file name, can wrap in a narrow sidebar. - A move onto trunk that updates the branch and then fails at the checkout keeps the patch's old text and fingerprints. A pencil can then mark a file trunk changed, and a discard or revert re-applies text written for the old trunk. Older than this PR; follow-up. - Neither a discard nor a revert is held while an apply or pull request checkout runs. Older than this PR (the discard link had the same gate); follow-up. - A pull request checked out before this ships has no list until it is checked out again. A patch applied before this ships marks every one of its files as edited, since the app did not record what it wrote: **Discard your changes** and each revert stay offered, reverting a file unmarks it, and a discard records the patch afresh. Covered by unit tests only; staging it by hand needs a site from an older build. A file the patch or the contributor deleted is still listed, and says it is missing when clicked. - The diff is about 3,000 lines across 32 files, well over the size AGENTS.md flags. It is one feature in five steps; the commits keep each step apart for reading one at a time, in this order: patches, pull requests, your changes, discard, per-file revert, then the tree. - In one of three local runs of the full journey suite, four journeys failed while the run took three times its usual length; the two runs either side passed all of them. I read it as load on the machine, not the branch. ## Related Part of #669. Replaces #678, #679, #682 and #683, which are folded in here. A pull request's list including files trunk changed, when the site's trunk is behind the pull request, is older than this PR and tracked in #701. --- <details> <summary>Design decisions and alternatives considered</summary> - **One PR, not a stack of five.** Every step builds one section, and a reviewer was testing the top of the stack while commenting on the bottom. - **Trees, one per group.** A reviewer asked for `TreeGrid` because full paths filled the sidebar's width. One tree per group keeps the headings. - **A chain of single folders on one row** (`tests/phpunit/tests/wp`), as GitHub and VS Code draw them. The first version gave every folder its own row; a reviewer pointed out it took five rows to show one file. - **Last in the sidebar**, not first: the list has no fixed height, and first it pushed the server and the build watch out of view. - **A revert leaves the file as Discard your changes would,** so the two never disagree about what "your changes" means. A patch file therefore goes back to the patch's version, not to trunk. - **Discard keeps the patch.** One button that always means "remove my edits, keep the patch or PR" matches what it already did for a PR. Removing the patch stays **Revert this patch**; re-applying the stored text is the only way back to "as the patch left it". - **Renames refused, not modelled.** One change spread over two paths cannot go back one path at a time. - **One click, no per-file menu.** There is no editor preference yet, so the click takes the first detected editor. A preference belongs in the Settings dialog from #638. - **The real path, not just the text.** A lexical check let a symbolic link inside the site point outside it. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Earlier reviews of each folded PR are recorded on it (#678 at `8ddcb27`, #679 at `eb7be4f`, #682 at `85b47f9`, #683 at `ff23819`, before the rebase); every finding there was fixed or deferred, and the deferrals are in Risks. The reviews below cover this combined branch: reviews 1 to 3 on base `60078fe`, review 4 on trunk `819575a` after the rebase. - **Review 1:** completed. A separate agent context reviewed `60078fe..73b47df` (#677 to #679 plus the tree) against `.github/instructions/code-review.instructions.md`. `npm run lint` clean, `npm test` 2147 passed. 1 [fix here] · 1 [follow-up]. - Fixed in `eb93d07` (🟡): the file list was walked as soon as an apply, revert or pull request switch changed the tree, but the status it is grouped by was read again only after the install and the build, so for those minutes a patch's files were listed as the contributor's. The status is now read with the walk. The journey "while the build after an apply runs, the patch's files are the patch's (#669)" holds the build open; it fails without the fix. - Deferred (🔵): the marks and the rule for whether the section shows are decided in `site-details.jsx` rather than `changedFileGroups`. One branch each. - **Review 2:** completed. Separate agent context, the fix above. No findings; confirmed on every path into the install and build. Its comment note was taken in `52ecde4`. - **Review 3:** completed. Separate agent context, `8400845..52ecde4` (#682, #683 and the tree redrawn over them). 1 [fix here] · 2 [follow-up]. - Fixed in `288b7c8` (🟡): the tree remembers the last button focused in it and gives every other one no tab stop, so a file that left the list on a refocus took the whole tree out of the tab order. The tree is now keyed by its files. The journey "a file that leaves the list does not take its tree out of the tab order (#669)" fails without the fix. - Fixed in `288b7c8` (🔵, raised as a follow-up): the treegrid itself had no accessible name; it now has its group's. - Deferred (🔵): a sighted keyboard user cannot open a mark's tooltip (see Risks). - **Review 4 (rebase onto trunk):** completed. Separate agent context, `819575a...455d0e8`, on the conflict resolution with #696: the ticket card's discard button pointed at `discardYourChanges`, `discardLabel` at its question, and `nothingOfYours` in the shared reason. 1 [fix here] · 0 [follow-up]. Fixed in `a54b394` (🔵): a unit test still tied the label to the full-discard question, passing only because both say "Discard changes". Nothing from #696 or #697 lost; no `discardAllChanges` left. - **CodeRabbit** reviewed `a54b394`: 1 finding (🟡), fixed in `d55ba08`. A kept patch applied before fingerprints marked none of its files as edited, so **Discard your changes** was disabled and no file could be reverted. Its files still changed now count as possibly edited. - **Since then:** `c7f4ad9` merges a chain of single folders into one row, at @zaerl's request; `299bc2d` and `9a28812` give the revert button's focus ring room at the sidebar's edge. - **Review 5:** completed. Separate agent context, `a54b394..d55ba08`. 3 [fix here] · 1 [follow-up], all fixed: - Fixed in `1f93225` (🟡): a folder closed before a file left the list could stop being a row of its own and still hide everything under it, leaving an empty tree. Unit test. - Fixed in `1f93225` (🟡): the window dropped `renamedByChange` from the reply, so both ends of a rename were offered a revert the handler refuses. Now carried by `unsubmittedFromReply`, unit-tested. - Fixed in `0dea62b` (🔵): on a patch from before fingerprints, a reverted file still looked edited. Its first revert now records a full set. Unit test. - Fixed in `4a77c79` (🔵, raised as a follow-up): a first-level folder's focus ring was clipped at the sidebar's other edge. Every cell now has room for the ring; the journey checks a folder, a file and a revert button, and fails without it. - **On `4a77c79`:** `npm run lint` clean, `npm test` 2193 passed, full journey suite 163 passed hidden on macOS. </details> <details> <summary>Implementation notes</summary> - `resolveLaunch(editorPath, sitePath, { platform, file })` appends the file after the folder in the argument vector. Never a shell. - `existingSiteFile(sitePath, relPath, { platform, realPath })` runs realpath on both the site and the file, so a site behind a link (macOS's `/var`) still holds its files. - `src/renderer/affected-files.cjs` holds the decisions, unit-tested: `affectedFiles`, `changedFileGroups` (groups, labels, `canRevert`), `unsubmittedFromReply` (the reply as the list reads it), `affectedFileTree` (paths to tree rows, merging single-folder chains), `visibleAffectedRows` (what a closed folder hides), `revertFileQuestion`, `revertOutcome` and `fileOpenTarget`. - `src/git-write.cjs`: `restoreWorktreePaths(dir, source, paths)`, worktree only, literal pathspecs on stdin. `src/trunk-update.js`: `discardFile`. `src/patch-apply.js`: `applyPatchToDir({ paths })` keeps only those files' sections. - `src/main.js`: `git:revert-file`; `git:discard-to-base(sitePath, { keepChange })` re-applies the kept patch and fingerprints it again; `git:unsubmitted-work` adds `entries`, `editedSinceChange` and `renamedByChange`. - The tree's indent is read from each row's `aria-level` in CSS (`attr(aria-level type(<integer>))`), since the repository's lint bans inline styles. The revert button gets the tree's item props through `Tooltip.Trigger`'s render prop. - `forgetChangedFiles` in `src/renderer/index.jsx` bumps the dirty probe's generation, so an answer still in flight from before a change is dropped, then clears the list and asks again. </details> <details> <summary>Screenshots or recording</summary> To add. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code)

zaerl and others added 3 commits October 6, 2026 10:49

zaerl merged commit fe02e6b into trunk Oct 6, 2026
10 checks passed

zaerl deleted the feat/settings-dialog branch October 6, 2026 09:28

zaerl mentioned this pull request Oct 6, 2026

Add a language setting, applied at the next launch #639

Merged

zaerl mentioned this pull request Oct 6, 2026

Add the PHP version and the debug flags to the settings, applied at the next server start #640

Merged

zaerl mentioned this pull request Oct 6, 2026

Start the server or the watch when a site is opened, and start again what a quit stopped #641

Merged

zaerl added this to the v2.0.0 milestone Oct 7, 2026

This was referenced Oct 9, 2026

List the changed files in the site's details as trees, each opening in the editor #677

Merged

Bump version to 2.0.0-beta.1 #681

Merged

Navigation Menu

Repository navigation

Add a settings dialog, with where new sites go and who the contributor is - #638

Add a settings dialog, with where new sites go and who the contributor is#638
zaerl merged 3 commits into
trunkfrom
feat/settings-dialog

zaerl commented Oct 6, 2026

Uh oh!

coderabbitai Bot commented Oct 6, 2026

Review skipped

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

zaerl commented Oct 6, 2026

Why

What changes

How to test this

Risks and limitations

Related

Uh oh!

coderabbitai Bot commented Oct 6, 2026

Review skipped

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant