Fix a journey run failing on worker teardown after a launch that gave no window - #607

Brings in the journey engine's forced kill (#607): an app still running
five seconds after it was asked to quit is ended, and a launch that gives
no window says what its main process knew. The launch here asks for the
profile and runs a journey's beforeWindow before it waits for the window,
so the evidence is taken around that wait, in this branch's order.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…kens (#608)

## Why

Item 9 of #557 is to replace every hardcoded colour with the design
system's tokens, with as little hand-written CSS as possible. Parts 1
and 2 did the site page and its panels. What was left was inside the
three dialogs still built on the older library: "Review & submit
changes" with its diff and its three destinations, the "Update to latest
trunk?" question, and the mail dialog. Between them they held 95 inline
`style` attributes and 87 colour values, including a dark diff pane and
a dark raw-mail pane that no longer matched the light terminal and logs.

## What changes

The three dialogs keep their frames, buttons and inputs from
`@wordpress/components`. Everything drawn by hand inside them is now the
design system's.

- **The diff** is on the strong surface inside a hairline, and each kind
of line (added, removed, hunk, file name, the app's own header) is a
class painted with tokens, as the prototype has them. **The raw mail**
is on the surface the logs and terminal are on.
- **Boxes that warned, failed or informed** are `Notice`s: the old-trunk
warning, the unreadable diff, nothing to send, someone else's work in
the checkout, the branch based on today's trunk, the update question's
failure, and test mode.
- **Lines that say how something went** are `Text` in one of three
shared classes, `success-text`, `warning-text` and `problem-text`. These
also replace the apply card's and the details sidebar's own copies, and
`.error-text` from part 1 now uses the same red (its token gave a
near-black `#470000`).
- **Destinations** are stacks and text inside a card per group. The
update question's two answers, its file list and the mail's panes are
classes in `shell.css`.
- The rule that gave every button in an older dialog a right margin is
gone; the layouts use gaps.

After this, no inline `style` and no colour value is left in any
component under `src/renderer`.

**Deliberately not in this PR:**

- Turning these dialogs into the design system's `Dialog`, or the review
into the prototype's three-column drawer with a file list. That is a
redesign of the screen, not colour work, and is not listed in #557.
- The three discards that still ask through `window.confirm`.
- The lint rule that forbids a hardcoded colour in the renderer. That is
part 4.
- Mail HTML rendering is unchanged and tracked separately.

## How to test this

Platforms: any. The journeys below drive each dialog on macOS and
Windows and assert the colours against the tokens, read from the window
itself. From the repository root, after `npm ci` and `npm run
build:once`:

```
npx playwright test --project=journeys review-changes.spec trunk-update.spec mail.spec open-pull-request.spec
```

Each new assertion is red without the style it claims; I broke the rules
one at a time to check (10 mutations, 10 red):

- `review-changes.spec.js`: an added and a removed line are in the
success and error colours on their washes; a failed save is in the error
colour and a saved file's line in the success colour.
- `trunk-update.spec.js`: the chosen answer is ringed in the brand
colour and the other is not, the ring moves with the choice, the chosen
answer's detail is in the full text colour, and the answer that loses
work stays red, chosen or not.
- `mail.spec.js`: the raw mail is on the weak surface.

**Worth a look by hand, since no journey judges taste** (any platform,
the current head):

1. On a site with an edit, press **Review & submit changes**. The diff
is light, with green added and red removed lines on a tint; the
destinations are two cards.
2. From the site's menu choose **Update to latest trunk** with an edit
in the tree. The chosen answer is ringed in blue; choosing **Discard
them** moves the ring and keeps that answer red.
3. Start the dev server on a site that has sent mail, open a mail, and
press **Raw**. The pane is light.

**What must not have happened:** no wording, role or accessible name
changed (the existing journeys find everything by role and text and pass
unchanged, all 99), and nothing outside these dialogs moved except the
two colour classes named above.

**What cannot be tested by hand here:** the GitHub device code and a
real pull request result need a GitHub sign-in; the journeys stand in
for GitHub and cover both states.

## Risks and limitations

- Review: 2 passes, 6 findings fixed here, 2 accepted as they are.
Details below.
- **Contrast is lower than the dark pane's, though every pair passes
4.5:1.** Added lines are 4.86:1 and hunk lines 4.50:1, where the dark
pane had about 9.7:1. These are the prototype's tokens for a diff.
- **Visible changes beyond colour:** destination cards have the design
system's 24px padding (was 14/16px); titles are the design system's
medium heading; the GitHub device code is 20px (was 24px); test mode is
a neutral notice and not a dashed box; the mail dialog's two tabs sit
flush, which is the library's default; a stand-alone link in a
destination is clickable on its words only, not across the column.
- The error under the site page's changes note turns from near-black to
red, with `.error-text`.
- Accepted, not fixed: no journey checks the 12px size of the four
stand-alone links.
- The strings in these components are still not translatable. That was
so before, and is not changed here.

## Related

Part of #557, after
#605 and
#606.

---

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

- **Frames and controls stay the older library's.** A dialog half in
each library's buttons would look worse than either, and the frame is
what a later redesign of the review replaces.
- **`Notice` with nothing spoken.** The design system's notice speaks
its text when it mounts. Each one here either carries `role="alert"`, as
the box it replaces did, or was silent before and stays so; none is
announced twice.
- **`Text` renders a span.** Every one is a flex or grid item, or is
given a block element, so no line runs into the next.
- **Shared classes for text colour** and not one per card: three classes
replace six rules that said the same thing.
- **The pressed answer's ring is drawn inside its border**, so choosing
does not move the other answer by a pixel, as the old 2px border did.

</details>

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

**Pass 1: 4 [fix here] · 2 [follow-up]. Pass 2 (on the fix-up): 2 [fix
here], both low. 6 fixed, 2 accepted.**

- **Review:** completed — a separate agent context, read-only; head
`50129cd` (rebased since, as `369cf6e`) / base `9570450`; compared each
component with its old version, read `Notice`'s implementation, computed
contrast for every new pair; 4 [fix here], 2 [follow-up].
- Fixed: the chosen answer's detail text was 4.34:1 on its tint (now the
full text colour, 14.6:1); four stand-alone links had grown from 12px to
13px; the mail journey's text-colour check passed without the rule it
was for.
- Fixed though marked follow-up: the test helper `tokenColour` could not
notice a misspelled token (it now refuses one the window does not have);
`.error-text` painted the same error in a different red.
- Accepted: the mail dialog's tabs sitting flush, the library's default.
- **Review:** completed — a second separate agent context, read-only;
head `86555dd` / base `23d46e7`; 2 [fix here], both low.
- Fixed: nothing pinned the losing answer's detail staying red once
chosen, which holds on source order alone; a journey now asserts it.
  - Accepted: no journey checks the links' size.
- **Since review:** `86555dd → 1460d81` adds that one assertion;
`trunk-update.spec` re-run, and red with the two rules swapped.

</details>

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

- The branch was rebased once, before it was pushed, onto
`redesign/wpds` after trunk's journey-engine fix
(#607) was merged
into it.
- `ui.tokenColour` and `ui.paintOf` are new shared helpers in
`tests/e2e/helpers/ui.cjs`; `logs.spec.js` and `terminal.spec.js` use
the first in place of their own copies.
- Local checks on macOS: lint clean, `npm test` and `npm run
test:electron` pass (1863 tests), `npm run test:e2e` 99 pass, the
affected journeys pass in a 1024×728 window and with the status read
slowed by a second, `npm run test:e2e:packaged` 15 pass.

</details>

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

Not attached: I have no way to upload images from the command line. The
three steps under "Worth a look by hand" show every changed surface, and
Buildkite's build for this branch can be used for them.

</details>

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

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…r's components (#609)

## Why

Parts 1 to 3 of item 9 in #557 left no colour value and no inline
`style` in any component under `src/renderer`: the window is painted
from classes in `src/renderer/shell.css` with the design system's
tokens. Nothing stopped one coming back. A colour typed into a component
is one the theme cannot reach, and `TESTING.md` rules out the only other
check, a test that reads a component's source.

## What changes

Two lint rules of this repository's own, switched on for
`src/renderer/**/*.{js,jsx,cjs}`:

- **`toolkit/no-hardcoded-colors`** reports a colour written out in a
string or in the fixed part of a template: a hex value of 3, 4, 6 or 8
digits, or a function that takes a colour's channels (`rgb()`, `hsl()`,
`oklch()`, `color()` and the like). A token wrapped in `var()` passes,
and so does `color-mix()` of tokens.
- **`toolkit/no-inline-styles`** reports a `style` attribute on a JSX
element.

They live in `eslint-rules/`, registered as a local plugin in
`eslint.config.mjs`. No new dependency: the rules are plain modules,
tested with ESLint's own `RuleTester`.

**Not in this PR:**

- **The stylesheet.** `shell.css` is not checked by these rules, and it
is where all the styling now is. That is the next pull request:
Stylelint with the design system's own rules.
- A colour's name (`red`, `white`), a colour put together from pieces, a
`style` passed in a spread or set from an effect. Each rule's header
says what it does not read. None of these exists in the renderer today.
- `src/main.js`, whose patch window still carries a few inline colours.
It is not the renderer.

## How to test this

Platforms: any. No user-visible surface; this is lint and its tests.
From the repository root, after `npm ci`:

```
npm run lint
node --test tests/unit/eslint-rules.test.cjs
```

Both pass. To see the rules bite, add `style={{ borderColor: "#dcdcde"
}}` to any element in `src/renderer/components/destination.jsx` and run
`npx eslint src/renderer`: it reports the inline style and the colour. I
did that, and the same with an `rgb()` in
`src/renderer/terminal-theme.cjs`.

`tests/unit/eslint-rules.test.cjs` runs each rule over code that must
pass and code that must not. I removed each guard in the two patterns in
turn (the lookbehinds, the lookahead, the case flag, the string-only
check, reading a template as the program gets it, `color()` in the list)
and the suite went red every time.

**What must not have happened:** the rules must not fire on anything in
the tree as it is (`npm run lint` is clean with no disable comment
added), and `eslint-rules/` must not be packaged into the app
(`build.files` in `package.json` does not include it).

## Risks and limitations

- Review: 2 passes, 9 findings fixed here, 1 kept as it is. Details
below.
- **A reference of three or four digits written out in a string reads as
a colour**: `'Fixed in #607'` is reported, because `#607` is a grey and
`#000` must not pass. Nothing in the tree does this; the renderer builds
such strings from the number (`#%d`, `#${ n }`), and the message says
so. A fragment made only of hex letters, `'#add'`, is reported too
unless it follows a word or a slash.
- Kept: these could have been three selectors given to the core rule
`no-restricted-syntax`, with no rule files. They are rules of their own
because the colour rule's message names what it found and the patterns
are tested by themselves.

## Related

Part of #557, after
#605,
#606 and
#608.

---

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

**Pass 1: 5 [fix here] · 1 [follow-up]. Pass 2 (on the fix-up): 4 [fix
here]. 9 fixed, 1 kept.**

- **Review:** completed — a separate agent context, read-only; head
`e8903a2` / base `5b7e1fb`; ran the test file on Node and on Electron's
Node, eslint over the renderer, and probed both rules with throwaway
code; 5 [fix here], 1 [follow-up].
- Fixed: a test that passed without the guard it named; the header
claiming a written-out reference could not be flagged; a fragment after
a slash being flagged; four wordings that claimed more than the code
does (the `TESTING.md` sentence, both messages, the rule's description).
- Fixed though marked follow-up: tests for uppercase hex, hex after `:`
and `(`, a template that cannot be read, a namespaced attribute.
- Kept: custom rules over `no-restricted-syntax`, with the reason now in
each header.
- **Review:** completed — a second separate agent context, read-only;
head `022aebf` / base `5b7e1fb`; removed each guard in memory and re-ran
the suite; 4 [fix here], all fixed.
- Three guards could be removed with the suite still green; two now have
a test, and the third decided nothing and is gone.
  - Two comments named the wrong case as pinning the string-only check.
- `color()` and `device-cmyk()` were missing from the functions the rule
reads.
  - One of the two reasons given for a custom style rule did not hold.
- **Since review:** `022aebf → b9bc2e2` is those four fixes only; the
test file, `npm test`, `npm run test:electron` and `npm run lint` re-run
on it.

</details>

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

- The preset from `@wordpress/eslint-plugin` does not configure
`no-restricted-syntax`, so nothing is overridden either way.
- `RuleTester` is given `node:test`'s `describe` and `it`, so a failing
case fails `npm test`; checked with a deliberately wrong case.
- The journeys and the packaged smoke were not run: nothing under `src/`
or `tests/e2e/` changes.

</details>

Nothing on screen changes, so there are no screenshots.

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

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>