Skip to content

Press again for the tab the driver lost, not the one Chromium never made - #40

Merged
max-sixty merged 3 commits into
mainfrom
fix/ci-32790759750
Aug 26, 2026
Merged

max-sixty merged 3 commits into
mainfrom
fix/ci-32790759750

Conversation

@leaf-agent

Copy link
Copy Markdown
Collaborator

Problem

Run 32790759750 failed test_a_message_reference_travels_or_says_it_cantTimeoutError: Timeout 30000ms exceeded while waiting for event "page" — at the ⌃-click that opens a message reference in its own tab. Chromium opened that tab. Playwright never handed it over.

Reproduced locally at 3–4 misses in 60 presses on a loaded machine, and read from the browser's own record rather than inferred. On a miss the click reaches the anchor and nothing refuses it, and Target.getTargets holds a second page target in the same browser context — attached, loaded, titled, sitting at the href — while context.pages still holds one. The targets pile up press after press and Playwright reports none of them, so expect_page spends its whole timeout on a tab that already exists.

Solution

opened_tab(page, press) makes the press again when the wait comes back empty: three tries at 10s each. A driver that loses the handle is not a page state page.route can arrange, and there is no second channel to reach an unreported tab through, so re-pressing is the only instrument left — the same class of repair as #26's CutOff, and stated as such in the helper's docstring and in tests/CLAUDE.md beside the rule it reads against.

The retry cannot green a runtime that stopped leaving a real href for the platform to act on: that opens no tab for any of the tries. Bug-back, refusing only the modified press in leaf.js's panel handler so the plain jump still works — the test fails at opened_tab in 31.9s with the same missing-tab wait.

All three sites that wait on a browser-opened tab go through it. The chord is not what decides the loss: over 60 presses each, ⌃-click lost 3, ⌃⇧-click 10, ⇧-click 1.

Testing

  • The 23 affected browser tests (-n0 --run-nightly): 23 passed.
  • The reference test 40× under three CPU spinners: 40 passed, none slower than 2.6s, so no run needed a second press.
  • Bug-back as above.
  • pre-commit clean on both changed files.
The reading that named it

Three misses out of 60 presses on one page. clicks is what the page saw of the press; targets is Target.getTargets on a browser CDP session, taken 1.5s after the wait gave up:

MISS at 1: TimeoutError
  clicks: [{'ctrl': True, 'meta': False, 'prevented': False, 'detail': 1, 'href': '#p-bath'}]
  pages now: 1 ['…/versions/v1.html?t=test-page-key#p-bath']
  pages +2s: 1 ['…/versions/v1.html?t=test-page-key#p-bath']
  target: {'targetId': 'ACB4…', 'type': 'page', 'title': 'refs', 'url': '…#p-bath', 'attached': True}
  target: {'targetId': '8806…', 'type': 'page', 'title': 'refs', 'url': '…#p-bath', 'attached': True}

Both targets carry the same browserContextId, and browser.contexts stays at one context holding one page. By the third miss there are four such targets and context.pages is still one: the tab is never reported, not reported late.

The press is not what fails. ctrl: True with prevented: False says the anchor took a real modified press and the runtime let it through; detail: 1 rules out the press being folded into a double-click.


Automated fix for failed run

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own PR, so this is a COMMENT rather than a verdict.

The diagnosis holds. A fact the driver drops on its way out of the browser is not a page state a route can arrange, the reading behind it is the browser's own record rather than an inference, and all three sites that wait on a browser-opened tab now go through one helper. Two things in that helper don't yet match what its docstring promises, both inline: the except can't distinguish the tab wait's timeout from one the press raised itself, and the rate the docstring states isn't the rate the PR body measured.

One thing worth a line in the docstring rather than machinery: a lost press leaves a real, loaded Chromium tab that nothing closes, so test_a_message_reference_travels_or_says_it_cant's tab.close() now accounts only for the try that was reported. No assertion at the three sites reads a tab or a leaf count, so nothing false passes — but it is the standing cost of the repeat, and the leaked tab goes on polling its server until context teardown.

lint is green. test was still running when the poll cap expired, so the 23 browser tests this change touches are unverified from here rather than green.

Comment thread tests/render_harness.py Outdated
Comment thread tests/render_harness.py Outdated
@leaf-agent

Copy link
Copy Markdown
Collaborator Author

This landed again on main: run 32907461091 on d77f5e405d3cdc2fbbe09f4ddb73a2d55c60d250, lint green and test red on the one failure. Same test, same site, same wait — TimeoutError: Timeout 30000ms exceeded while waiting for event "page" raised from page.context.expect_page() at tests/test_render_projection.py:2356, which is exactly the with block this PR replaces with opened_tab. 1 failed, 1195 passed, 6 skipped — nothing else in the run moved, so it is the lost tab rather than a busy machine taking unrelated tests down with it.

That is the second occurrence on main inside 24 hours (32790759750 at 2026-08-24 23:45Z, this one at 2026-08-25 22:42Z), with all 23 ci runs between them green. Two reds in 25 runs sits close to the 3-in-60 ⌃-click loss the PR body measured, so the run record reads as the same fault at the same rate rather than something new. Nothing on main addresses it, so the next red ci here is likely to be this same press again.

The branch is still good to merge: 3 ahead of main and 11 behind, no conflict (git merge-tree against the merge base is clean), and none of those 11 commits touch the failing site. Checks on aeb8ee3 are green — test 18m24s pass, lint pass — so the poll-cap gap noted in the self-review has since closed. Nothing to change here; posting the recurrence rather than opening a second fix.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Third occurrence on main, and the first since the note above: run 32920132953 on ff390a3744cbbf0c0ecd7112a2d924cf77b0172f, lint green and test red at 1 failed, 1198 passed, 6 skipped. Same wait at tests/test_render_projection.py:2356TimeoutError: Timeout 30000ms exceeded while waiting for event "page" — and git diff origin/main...origin/fix/ci-32790759750 shows this branch replacing those exact three lines with opened_tab, so the fix lands on the failing site rather than near it.

The rate has moved the wrong way. Counting ci on main since the first occurrence at 2026-08-24 23:45Z: 3 failures in 34 completed runs, all three this one test, where the note above read 2 in 25. The last two reds are 3h03m apart — 2026-08-25 22:42Z and 2026-08-26 01:45Z — against roughly 20 minutes of suite per run, so main is spending a real fraction of its time red on a press that nothing on main answers.

Branch is still clean to merge: 3 ahead, 21 behind, git merge-tree against the merge base conflict-free, and none of those 21 commits touch the failing site. Nothing changed here — recurrence only, no second PR.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Fourth occurrence, 70 minutes after the third: run 32924462601 on 77c2632b, lint green and test red at the same wait, 1 failed, 1198 passed, 6 skipped. That commit is a Python-side rendering refactor whose one test edit retargets a SERVED_TIMEOUT_MS monkeypatch, so nothing on main moved toward the failing site.

The rate is the only new fact. Counting completed ci runs on main since the first occurrence: 4 failures in 43, all this one test — but three of the four are in the last 4h12m, at 22:42Z, 01:44Z and 02:54Z, which is 3 in 13. The two most recent are 70 minutes apart against roughly 20 minutes of suite per run.

Nothing changed on this branch: still green on its last run, MERGEABLE, blocked only on the admin-merge ruleset. Recurrence only, no second PR.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Fifth occurrence: run 32946946913 on 8a5d618, lint green and test red at 1 failed, 1193 passed, 6 skipped, same TimeoutError: Timeout 30000ms exceeded while waiting for event "page".

The one new fact is that the failing site moved without moving: the wait is now tests/test_render_projection.py:2399 rather than 2356, because 36 commits have landed since the merge base — but the three lines themselves are byte-identical to what this branch replaces with opened_tab, and git merge-tree against origin/main is still conflict-free at 3 ahead / 36 behind. Nothing in those 36 commits has touched or fixed the press.

Rate since the first occurrence at 2026-08-24 23:45Z: 5 failures in 50 completed ci runs on main, every one of them this test. That is 10%, against the 2-in-25 the first note read and 3-in-13 the third. Recurrence only — no second PR, nothing changed here.

@max-sixty
max-sixty merged commit e6ef8ab into main Aug 26, 2026
13 checks passed
@max-sixty
max-sixty deleted the fix/ci-32790759750 branch August 26, 2026 09:41
@max-sixty

Copy link
Copy Markdown
Owner

@tend-agent we have to fix the titles; how is this a reasonable title for the issue. it needs to be understandable for a normal person to have some idea of the problem!

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Fair, and it's the merge commit on main now. "Press again for the tab the driver lost, not the one Chromium never made" tells someone scanning the log nothing about a flaky Playwright test, and #42's "Sort a red suite by what the failure is, not by how steady it looks" is the same shape.

What produced them: the repo's own commit titles are imperative and elliptical, and they work because their subjects — a focus ring, a key line, an ask — are things a reader of this repo already knows. I was matching that register on machinery nobody has met yet, which turns it into a riddle instead of a summary.

#45 writes the rule into .claude/skills/running-tend/SKILL.md: keep the imperative voice, drop the ellipsis, name the concrete test or widget or command rather than a figure for it, skip the "X rather than Y" contrast when Y only means something after reading the body, then re-read the title as someone holding only the notification. Two before/after pairs from my own titles are in it, since the prose alone wouldn't have stopped me.

Applied to the one open PR already — #44 is now "Replace a 250ms sleep in the work-line test with a real control". This one and #42 are merged, so those two stay as they are.

@tend-agent

Copy link
Copy Markdown

One thing the overlay in #45 can't see from inside this repo: the bundled rule already exists and never arrived here. running-in-ci has required plain, literal titles since max-sixty/tend#1022 (merged 2026-08-20), but tend's latest release is 0.1.18 (2026-08-16) and leaf's workflows pin max-sixty/tend/claude@0.1.18 — so the runs that wrote these titles were working from skills that predate the rule, as is every other consumer until a release ships.

It needed sharpening too, and #45's before/after pairs are the right instinct about why: the bundled rule stated the goal ("plain, literal language") without naming the shapes that break it, so a title written as a figure passes its own check. max-sixty/tend#1060 adds a reader test — someone who has not read the diff can say what was wrong — a bad → good rewrite using this PR's title as the example, and extends the rule to issue titles rather than PR titles alone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants