test(reach-rank): wait for the remounted board before typing; prove Back restores the search

[adminopenclaw8-sketch]

Summary

This fixes the intermittent failure of test-reach-rank-e2e.js at "Back restores search: search restored after Back". The cause was in the test, not in the product. This PR changes only test-reach-rank-e2e.js; public/, workflows and Go code are unchanged.

The flake surfaced in CI on PR #88 (run 35987585041). #88 only touches the Live page and its test, so it was just where the flake was observed, not its cause.

Baseline

The same harness was run on the same server and fixture, alternating master's test and this branch's test, on macOS arm64. The fixture was prepared as the Playwright job does (freshen → seed SQL → migrate).

Test version	Plain public/	Instrumented (as CI)
master e51272d9	35/50 pass (15 failures)	7/20 pass (13 failures)
this branch	50/50 pass	20/20 pass

Every master failure is the identical assertion.

Root cause (from an instrumented event timeline)

The keyboard step does:

1. openBoard() (page.goto('#/reach-rank'));
2. focus the search box and type the query;
3. press Enter;
4. Tab to the target row and open its Reach page;
5. go Back.

Two failing timelines showed that the search was never applied: no submit, no replaceState(...?q=...). Two paths lead there:

1. Stale board. The previous step left the page on #/reach-rank?page=2. page.goto() on a same-document hash resolves as soon as the history entry commits (popstate). app.js re-renders on the later hashchange (window.addEventListener('hashchange', navigate)). So openBoard()'s waits (#rrRows tr, table not aria-busy) could pass on the previous board. The test focused that board's #rrSearch, the router then replaced it, focus fell to <body>, and every keystroke went nowhere.
2. Focus hand-off. One animation frame after each mount, app.js moves focus to the page heading (its accessibility code marked #630-7). When the typing spanned that frame, the first character landed in the new box ("E") and the rest was lost.

In both cases the unfiltered first page still contained the target row (it ranks second). So the test kept going, and Back correctly returned to #/reach-rank without q, leaving an empty box.

Why the product is fine. reach-rank.js renders #rrSearch with the value from the hash synchronously in init(), and keeps q/page in the URL with replaceState. A real user cannot type within the millisecond between popstate and hashchange, or the single frame before the focus hand-off.

A Chromium check at 1200 px and 375 px confirmed that search, field and filtered rows survive Back, Forward+Back, a real reload and an SPA round trip to Packets, with no errors.

Fix (test-only)

settleBoard(page, navigate) is the new readiness contract. When the hash changes, it waits in this order:

1. for a #rrSearch that did not exist before the navigation (the old node is marked first);
2. for the app.js focus hand-off (#630-7): document.activeElement is the first h1, h2, h3, [role="heading"] in #app, the same selector app.js uses;
3. for the mount's first fetch (#rrTable not aria-busy).

If the hash doesn't change, the router doesn't re-render, and the current board is kept.

Stronger keyboard step:

• Before navigating on, the search must be applied in the field, in the hash, and as exactly the API's filtered rows.
• After Back it checks field, hash and filtered rows. It does this twice, with Forward in between.
• It checks that Back was a history traversal in the same document (a window token survives and history.length is unchanged), not a reload.

Deterministic race amplifier.

• installRouteHold wraps app.js's router, the hashchange listener named navigate. The test asserts the hook caught it.
• holdNextRoute holds the keyboard step's route change, and the animation-frame callbacks it schedules (including the focus hand-off), until the page's next key press.
• settleBoard releases them only after seeing that its condition does not hold yet.
• The old waits therefore fail every time instead of now and then. No sleeps, retries or longer timeouts are involved, and other listeners are untouched.

Mutations (isolated scratch copies)

Mutant	Result
T1: master's openBoard waits in the new test	red 3/3: "rows never matched the search … after typing it and pressing Enter"
T2: settleBoard accepts a stale #rrSearch (faked readiness)	red 3/3, same assertion
T3: focus hand-off not awaited	red 5/5, same assertion
A1: app renders the box without q (search only in the URL)	red 2/2: "search field shows "" … after Back 1"
A2: a mount's first fetch ignores q (box shows it, list unfiltered)	red 2/2: "rows never matched the search … after Back 1"
A3: a later async render clears the box	red 2/2: "search field shows "" … after Back 1"
R: app reloads the page on a return to a search	red 6/6: "Back 2 reloaded the page instead of traversing history" (added after review)
This branch	green

Verification (4fc30fc7, re-run after fedee520)

• Target test at 4fc30fc7: 50/50 plain and 20/20 instrumented; master's test with the same harness: 35/50 and 7/20.
• After the review fixes: 20/20 plain and 10/10 instrumented at 4fc30fc7; master 15/20 and 6/10. At fedee520, 5/5 plain more.
• CI order, positions 91–97 of the Playwright step (the channels tests, BYOP modal, reach-mobile, node-reach-coverage, reach-rank), instrumented: 10/10 per test at 4fc30fc7, and 3/3 after the review fixes.
• Reach family:
  • test-node-reach-coverage-e2e.js 20/20;
  • test-issue-1630-reach-mobile-e2e.js 20/20;
  • unit tests test-reach-rank.js, test-node-reach-coverage.js and test-node-reach-coverage-debounce.js 20/20 each.
• Static: node --check and git diff --check are clean. eslint 8 (as in CI) finds 0 issues in the test file, as on master. The XSS diff gate reports no public/ changes.
• Other: the test is still registered exactly once in deploy.yml. No server or Chromium processes were left behind. No staging, demo or production system was contacted.

Limitations

• test-node-reach-e2e.js is not in CI and times out locally waiting for #nqMap .leaflet-container. It fails identically from a clean e51272d9 checkout and is unrelated to this change.
• The route hold is keyed to app.js's router function name navigate. If it is renamed, the test fails loudly ("the route hold did not catch app.js's hashchange router") instead of silently losing the amplifier.
• The existing narrow ignore for the pre-existing Leaflet _leaflet_pos teardown error is unchanged.
• A reload that some code triggered long after Back, once the test has finished its checks, is out of reach without timing. The final view would still be rebuilt correctly from the hash.

Independent review

A fresh reviewer that did not write the change reproduced the root cause from reach-rank.js and app.js and agreed it is a test error, not a product bug.

• Its stability runs: the new test passed 50/50 plain and 25/25 instrumented; master passed 35/50 and 22/25.
• Its mutants: all were red. They included one where search does not write the hash, which failed with "hash lacks the search".
• Findings, fixed in fedee520:
  • a reload on Back was red only through a crash, not a named assertion;
  • three wrappers dropped the original error text.

The reviewer's re-check of fedee520 found no blockers. The PR test passed 8/8, and its reload-on-Back mutant failed with "Back 1 reloaded the page instead of traversing history".

🤖 Generated with Claude Code