Launch the browser LEAF_BROWSER_EXECUTABLE names, else the Chrome channel by leaf-agent · Pull Request #200 · max-sixty/leaf

leaf-agent · 2026-09-01T20:23:40Z

Problem

Both launches on the user path hardcoded p.chromium.launch(channel="chrome") — render_gate/command.py and exporting.py. That channel resolves a Google Chrome release-channel install at a fixed OS path, so a Chrome for Testing, a distro or Homebrew Chromium, or a self-hosted build is invisible to it, and with no outbound network Playwright cannot fetch its own either. A host in that position could neither render-check nor export, and serving-pages.md names export as the fallback for when no network route reaches the page — so the page is lost twice over. Nothing about the invariants needs Chrome: the suite's own fixture runs every one of them on Playwright's headless shell.

Solution

render_gate/browser.py owns the launch and the hint each gate appends when it fails. LEAF_BROWSER_EXECUTABLE names the executable, in the namespace host.py already uses for LEAF_AGENT and LEAF_SESSION_ID; it carries a path and nothing else. Unset, the launch is exactly today's Chrome channel. Both call sites go through the helper, so they cannot drift. Both failure messages stop naming Chrome and end with the hint, which quotes the variable's value where one was given.

Docs follow the code: the README's Install section, references/internals/validation.md, the root CLAUDE.md policy sentence, and the Within render_gate/ module roster in skills/leaf/scripts/CLAUDE.md.

Testing

tests/test_render_commands.py::test_a_named_browser_that_is_not_one_names_the_variable is the reproduction: it points the variable at a path that is no browser and asserts both commands fail naming the variable and the value. Before the fix both silently launched Chrome and succeeded — version check --render returned 0 with "renders clean in Chrome".

The four subprocess tests that shell out to the real launcher each gained a second arm on the headless shell, and their existing arm now sets LEAF_BROWSER_EXECUTABLE="" explicitly rather than inheriting the ambient environment, so an exported value cannot quietly turn the Chrome-channel arm into a second run of the other one. The new headless_shell fixture in tests/conftest.py finds the shell CI already installs.

uv run pytest tests/test_render_commands.py   # 15 passed
uv run pytest tests                           # 772 passed, 6 skipped

Notes

The fixture asks Playwright for its Chromium path in a subprocess: a second in-process sync_playwright() raises where the session-scoped browser fixture already holds one open, which would have made the fixture's success depend on test order.
The success line still reads "renders clean in Chrome". Acceptance criterion 1 asks that behaviour with the variable unset be unchanged, so I left the string alone rather than making it vary; happy to change it if you'd rather it named the browser it actually used.
Out of scope per the issue: the two channel="chrome" launches under scripts/ (record-demo.py, example-previews.py), which no leaf command reaches.

Closes #197 — automated triage

…nnel Both user-path launches — `version check --render` and `version export` — hardcoded `p.chromium.launch(channel="chrome")`, which finds a Google Chrome release-channel install at a fixed OS path and nothing else. A host with a Chrome for Testing, a distro or Homebrew Chromium, or a self-hosted build could neither render-check nor export, and serving-pages.md names export as the fallback for when no network route reaches the page. One helper in `render_gate/browser.py` now owns the launch and the hint each gate appends on failure, so the two call sites move together. With the variable unset the launch is unchanged. Closes #197

leaf-agent

lint is red on this head. Ruff's ISC004 fires on the fixture's inline script — tests/conftest.py:224, "Unparenthesized implicit string concatenation in collection". Inline suggestion below; the rest of pre-commit and the everyday test job are green.

Three other things.

The fixture's path arithmetic only holds on Linux. root, build = chromium.parents[2], chromium.parents[1].name.rsplit("-", 1)[1] assumes Playwright's executable sits exactly two levels under the registry root, which is the Linux layout (ms-playwright/chromium-<build>/chrome-linux/chrome). On macOS the binary is inside an app bundle — chromium-<build>/chrome-mac/Chromium.app/Contents/MacOS/Chromium — so parents[1].name is Contents, and "Contents".rsplit("-", 1)[1] raises IndexError before the fixture's own AssertionError guard can say anything useful. Five tests request headless_shell, so all five error out. CI is ubuntu-only so this stays green here, but scripts/linux-suite.sh exists precisely because the everyday development host isn't Linux, and tests/CLAUDE.md tells a developer to run the complete browser file before handing over a browser-facing change — this is that file. I haven't run macOS from CI, so the layout is from Playwright's documented one rather than observed; the fix (search the parents for the chromium- directory instead of counting levels) is right either way, since it doesn't depend on how deep the binary sits.

The success line. You asked in the Notes whether to leave renders clean in Chrome alone. I'd change it: the PR's own argument is that a message naming Chrome on a host that never ran Chrome is false, and that argument doesn't stop at the failure path — a Chromium host now gets a clean gate that tells it Chrome drew the page. Criterion 1 is satisfied by varying only where the variable is set, which is exactly what browser_hint() already does; a launched_name() beside it returning named_executable() or "Chrome" would keep the two answers in one place. No test pins the in Chrome suffix — the three assertions are all "renders clean" in stdout — so the change is local to command.py.

The CLI help still names Chrome alone. In skills/leaf/scripts/leaf/cli.py, check's option help reads also check the rendered page in Chrome and its docstring --render also checks the drawn page in the installed Chrome; export_page's docstring in exporting.py says version export supplies installed Chrome. That's the surface a host reads with --help before it ever sees a failure message, and the root CLAUDE.md asks that a feature "Update any public docs or generated outputs the feature affects". Outside the diff, so no inline suggestion.

The launch helper itself reads well — one owner for both call sites, empty-means-unset so a parent handing down its environment can unname one, and the two channel="chrome" launches left under scripts/ are correctly out of scope since no leaf command reaches them.

leaf-agent · 2026-09-01T20:31:47Z

+        check=True,
+    )
+    chromium = Path(read.stdout.strip())
+    root, build = chromium.parents[2], chromium.parents[1].name.rsplit("-", 1)[1]


Suggested change

root, build = chromium.parents[2], chromium.parents[1].name.rsplit("-", 1)[1]

versioned = next(p for p in chromium.parents if p.name.startswith("chromium-"))

root, build = versioned.parent, versioned.name.rsplit("-", 1)[1]

Counting two levels up finds the registry root only on Linux. On macOS the executable is chromium-<build>/chrome-mac/Chromium.app/Contents/MacOS/Chromium, so parents[1].name is Contents and the rsplit raises IndexError — the fixture dies before it can raise its own AssertionError, and every test that asks for it errors. Naming the chromium-<build> directory rather than its depth holds on both.

leaf-agent · 2026-09-01T20:46:08Z

This merged about seven minutes after my review, so three of that review's four findings landed on main unaddressed — the ISC004 lint fix went in as a5acaf2, the others did not. #203 carries them: the headless_shell fixture's IndexError on macOS, the success line still reading renders clean in Chrome on a host that never ran Chrome, and the --help surface naming Chrome alone.

#203 makes the same edit to the same line of `tests/conftest.py`, and it got there first (`47edb74` at 20:44Z against this branch's `77205a3` at 21:38Z). Two open PRs rewriting one line conflict whichever lands first, and put the same change through review twice; #203 is the narrower home, existing only for #200's follow-ups and naming the macOS app-bundle layout as its cause. The four `test_render_commands.py` setup errors this hunk cleared are #203's, which is where the PR body already attributed them.

## Problem [`ci` run 33516407327](https://github.com/max-sixty/leaf/actions/runs/33516407327) came back **10 failed, 1614 passed, 6 skipped**. Eight belong to #189, whose `ci` run was cancelled by #190's push before the nightly leg ever ran on it. #188's `test_mcp_app.py` case arrived on the same collision as the MCP test below and is carried here rather than left to red the next nightly. The everyday suite that gates a pull request does not run `test_render_*` or `test_site.py`, so a PR can land its nightly half unread — which is how these arrived, and how five more have arrived since, from #193, #194 and #200. `main` has moved a long way under this branch; the ones it now owns are taken on merge, and three of the new ones are cleared here. #203 reached `tests/conftest.py`'s `headless_shell` first, so the four `test_render_commands.py` setup errors are its. ## Solution Grouped by cause rather than by test, since several tests share one. **The example the tests still name.** #189 rewrote `examples/pr-walkthrough.html` into the Worktrunk review packet, and the shipped patch is now a collapsed manifest that builds no lines until a reader opens a file, which the scroller gate read as `getComputedStyle(null)`. The gate reads the diffs that have drawn lines, with its own non-vacuity floor kept. **The new package's chrome and surfaces.** `.lf-pr-description` stacked a second translucent accent onto a card that is already a tint of one, and the layer's own `--muted` reads 4.32:1 there; the description now sits on the raised-surface token. CallDiff's disclosure was a bare `<button>` rather than an `offer`, so an exported copy kept a hand over a press nothing can take. Its location anchor carried an `href` on the header row, which names no location at all — and `reachScrollers` reads a candidate for a focusable descendant before granting it a stop, so that hidden anchor answered "there is already a way in here" with a link nobody can reach, for a box whose words run off the side. **A reading that counted clipped text as painted.** `coveredWords` measured each run's whole rect, so an ellipsised name in a narrow column read as covering its neighbour. It now intersects each run with the boxes that clip it, stopping at an out-of-flow ancestor where a hidden overflow further out need not reach. The one test that plants this fault plants it properly now: the runtime's note is parked in a one-pixel box with hidden overflow, so opacity alone never put its characters on the screen. **Three focus rings the corpus stopped painting.** `code-pre-light` and `code-pre-shadow` are the tab stops on a scrolling code block and on a diff's lines, and with the old example gone no example held either. The page gets one rendered hunk and one code line long enough to scroll. Then `.lf-diff-review`: it took the outward ring it shared with `.lf-diff-next`, which stands on its own in the toolbar, while this one rides a file's summary row inside a box that clips at the row's bottom edge — its lower run fell 3px past what the sweep can see. It takes the inset the summary beside it already takes, and splitting the rule made each half declare the ring name neither had. **`--diffs-fg-number` on changed rows.** Surfaced by that rendered hunk: a changed row's number is drawn in the base its own fill is mixed from, green on green at 4.03:1. Pierre's number overrides now take the layer's tinted-surface inks. **A sidebar the page got shorter under.** #190's zero-height ToC anchor shortened the release page 58px, putting a named `scrollTo(0, 900)` past the stretch where the box stands on its own offset. Both edges of that stretch are the page's, so they are read off it, with a floor asserting the stretch exists before a point halfway along it says anything. **Playwright's loop against `asyncio.run` and `anyio.run`.** `sync_playwright()` keeps an asyncio loop running in its thread for its whole lifetime, and the `browser` fixture is session-scoped per xdist worker — so an MCP test's own loop start raises in any worker that has opened a browser and passes in one that has not, leaving the schedule to decide. Verified directly: `asyncio.events._get_running_loop()` is `None` before `sync_playwright()`, the loop inside it, `None` after. `interact_support.run_async` runs the entry point on a thread with no loop on it, and both MCP modules go through it — #204's three new exchanges included, since they landed on the direct call while this was in flight. **A declared verb no page replayed.** #194 added `lf-diff`'s `review` to the registry without an event for it on the standing-state page — the one fixture built so that exactly this fails rather than going unexercised. It gets a diff and a standing review. **A focus contract two site tests never heard about.** #193 kept the response field passive so a drag leaves the browser's own selection alone and a native copy still has something to take, and aligned the render tests with it. `test_site.py` is nightly-only. Its label test turned on `document.activeElement === field`, which is now false for the page's own words and the site's label alike — the discriminator was gone, not inverted — so it reads whether the field was offered at all, which is the difference it was always after. ## Testing `uv run pytest tests --run-nightly` and `pre-commit run --all-files` on this head. The everyday suite and lint are green on this branch in CI. Every claim about what `main` does was taken from a control run of the same tests in a clean checkout of `main`, not from reading. <details><summary>Where each failure landed</summary> | test | fix | | --- | --- | | `test_render_gate.py::test_the_runtime_holds_a_scroller_the_page_wrote` | reads the diffs that drew lines | | `test_render_export.py::test_an_exported_example_stands_on_its_own[pr-walkthrough]`, `[corpus]` | widget, probe, theme | | `test_render_controls.py::test_every_ring_the_layer_draws_is_shown_whole_somewhere_in_the_corpus` | example, then `.lf-diff-review`'s ring and both ring names | | `test_render_pages.py::test_a_left_sidebar_uses_the_margin_until_the_page_needs_it_back` | scroll position read off the page | | `test_interact_mcp.py::test_stdio_protocol_carries_the_app_resource_and_private_tool_result` | own thread | | `test_mcp_app.py::test_registered_server_prefers_full_page_and_keeps_snapshot_as_fallback` | own thread | | `test_render_projection.py::test_the_render_gate_applies_every_standing_action_a_second_time` | standing fixture (#194's) | | `test_site.py::test_the_label_is_chrome_rather_than_words_to_quote`, `::test_a_comment_lands_in_the_thread_with_its_quote` | the field is offered, not entered (#193's) | | `test_render_anchors.py::test_a_data_bound_diff_aims_and_selects_one_source_line` | **#194's and #193's, taken on merge** | | `test_site.py`, `test_render_export.py` stale `<h1>` | **#202's, taken on merge** | The two exported-copy cases each carried four faults, three of them masked: `offering` asserts before `covered`, which asserts before the copy's axe run, so each fix uncovered the next. The ring sweep did the same — the geometry fault stood in front of a naming check that had been silent about a rule missing its name since it was written. </details> <details><summary>Three this leaves, all of them main's</summary> **`test_render_controls.py::test_examples_have_no_serious_wcag_a_or_aa_violations[pr-walkthrough]` stays red.** The `.lf-pr-description` contrast above is fixed and confirmed gone. What holds the test red is 46 `nested-interactive` findings from #194: each file's "Mark reviewed" `<button>` sits inside that file's `<summary>`, and a disclosure with a focusable descendant is a serious WCAG failure. Clearing it means taking the press out of the disclosure and re-laying the row — `entry.node` is the `<details>` in ten places, and the theme, print and export rules key on it — which is a change to that widget's shape with visual verification of its own, not a line this branch should slip in. **`test_render_anchors.py::test_a_failed_fragment_hydration_waits_for_a_reader_retry` fails whenever anything runs before it.** Confirmed on unmodified `main`: three runs of the file at `-n4`, three failures, and the same failure at `-n0` over the whole file. It passes alone every time. `assert len(requests) == 2` sees one — the reopened disclosure's second fragment fetch never reaches the route. Arrived with #189. **`test_render_anchors.py::test_a_drag_released_mid_word_hugs_words_and_sentences` fails over the whole file and passes alone.** `assert 'inside' == 'inside it'` — the selection did not grow to the word boundary. It looks like the next instance of what #191 diagnosed, Chromium 151 collapsing a synthetic range on mouse release. Also still behind: the gallery's `docs/example-pr-walkthrough.jpg`. Regenerating here rewrites all nine stills in substituted faces, because `theme.css` asks for Charter and this runner's `fc-match` has none of the serif stack — the same reading #175 made. Left for the authoring machine. </details> --- Automated fix for [failed run](https://github.com/max-sixty/leaf/actions/runs/33516407327) > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…out counting directory levels (#203) The review on #200 raised four things; the ISC004 lint fix landed before the merge and these three did not. They are all on `main` now, so this is the follow-up. **The `headless_shell` fixture only works on Linux.** It found Playwright's registry root by counting two levels up from the Chromium executable, which is the Linux layout (`ms-playwright/chromium-<build>/chrome-linux/chrome`). On macOS the binary is inside an app bundle — `chromium-<build>/chrome-mac/Chromium.app/Contents/MacOS/Chromium` — so `parents[1].name` is `Contents` and `"Contents".rsplit("-", 1)[1]` raises `IndexError` before the fixture's own `AssertionError` guard can say anything useful. Four tests request the fixture, so all four error out. CI is ubuntu-only, which is why it stays green here, but `tests/CLAUDE.md` tells a developer to run the complete browser file before handing over a browser-facing change, and that file is this one. Naming the `chromium-` directory instead of counting depth holds on both layouts. **The success line named Chrome on a host that never ran Chrome.** `renders clean in Chrome` is the same false claim on the way out that #200 stopped the failure messages making. `launched_name()` sits beside `browser_hint()` in `render_gate/browser.py` and answers from the same place, so the two cannot drift. **The CLI help named Chrome alone.** `check`'s `--render` option help, its docstring, and `export_page`'s docstring are the surface a host reads with `--help` before it ever sees a failure message. <details><summary>Verification</summary> The two arms of `test_check_render_refuses_what_only_a_browser_can_see` now pin the browser each one names rather than just `"renders clean"`, so the success-line change is what the test is about: ```python assert "renders clean in Chrome" in ok.stdout assert f"renders clean in {headless_shell}" in named.stdout ``` ``` uv run pytest tests/test_render_commands.py # 15 passed uv run pytest tests # 773 passed, 6 skipped ruff 0.16.1 check + format # clean ``` I have not run macOS from CI, so the app-bundle layout is Playwright's documented one rather than one I observed. The fix does not depend on it: searching the parents for `chromium-<build>` holds however deep the executable sits. Fixture change: [`tests/conftest.py`](https://github.com/max-sixty/leaf/blob/47edb743363c5a257d3cf1f491708094bbacd402/tests/conftest.py#L237) </details> --------- Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com>

max-sixty enabled auto-merge (squash) September 1, 2026 20:27

Parenthesize the fixture's implicit string concatenation for ISC004

a5acaf2

leaf-agent commented Sep 1, 2026

View reviewed changes

max-sixty merged commit f98a205 into main Sep 1, 2026
6 checks passed

max-sixty deleted the fix/issue-197 branch September 1, 2026 20:38

leaf-agent mentioned this pull request Sep 1, 2026

Name the browser that drew the page, and find the headless shell without counting directory levels #203

Merged

max-sixty mentioned this pull request Sep 1, 2026

Fix ten nightly failures the everyday gate never ran #192

Merged

leaf-agent mentioned this pull request Sep 2, 2026

review-runs-tracking: 2026-09 #186

Open

Navigation Menu

Launch the browser LEAF_BROWSER_EXECUTABLE names, else the Chrome channel - #200

Launch the browser LEAF_BROWSER_EXECUTABLE names, else the Chrome channel#200
max-sixty merged 2 commits into
mainfrom
fix/issue-197

leaf-agent commented Sep 1, 2026

Uh oh!

leaf-agent left a comment

Uh oh!

Uh oh!

leaf-agent Sep 1, 2026

Uh oh!

Uh oh!

leaf-agent commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	root, build = chromium.parents[2], chromium.parents[1].name.rsplit("-", 1)[1]
	versioned = next(p for p in chromium.parents if p.name.startswith("chromium-"))
	root, build = versioned.parent, versioned.name.rsplit("-", 1)[1]

Conversation

leaf-agent commented Sep 1, 2026

Problem

Solution

Testing

Uh oh!

leaf-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leaf-agent Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leaf-agent commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants