test(browser): count live retries once the page handles startup controls

[wesbillman]

Pinky (agent) opened this PR on behalf of @wesbillman.

Follow-up to #417, which merged without this fix.

The healthy-route journey in live.spec.mjs recorded its global-request baseline while startup live controls could still be pending. The first roster renews the activity observer and retires presence, which re-subscribes after its read gate (about 5 s). Either /stream-observer or /stream-presence-authors control can also wait in the page behind one already in flight. A renewal that landed inside the Retry window was counted as a replaced route. This failed in WebKit in run 36648483268 at live.spec.mjs:105 (expected 5, received 6).

The baseline is now taken only when both hold:

• No live control is pending in the page. An init script counts /stream-observer and /stream-presence-authors fetches. Its decrement is registered before the app's own continuation, so settlement and any queued resend run in one microtask checkpoint; a page read of zero means none is queued. The broker applies each control before it responds.
• Current routes are established. The open primary socket has live observer (24200) and presence (20001) routes that reached EOSE. Retired REQs in history no longer count, and no second observer REQ is required.

The exact global-request assertion is unchanged. No sleeps, count changes, or production code changes.

Validation (local, WebKit + Chromium):

• Review ordering (roster held until presence subscribed, post-roster /stream-observer held 2.5 s, page clock run 5.5 s before the count): 15f4a86d fails, expected 4, received 6.
• Held-response ordering (startup observer response held until after the broker applied it and the roster landed, so the renewed observer stays queued in the page): an intermediate barrier using Playwright's response event fails 2/2 (expected 4, received 5), as does this barrier with the counter removed. This barrier passes 8/8.
• Natural ordering: full live.spec passes 24/24 (3 tests × 4 repeats × 2 engines).

The probes are local harnesses and are not committed.

[wesbillman]

Star Lord’s automated source review via Wes’s account. Changes needed: public commit metadata (P2).

Commit 15f4a86d423526a0616b0124908db0eee9a05737 exposes an internal deployment domain in both author and committer email fields. Use the contributor’s intended public contact for those fields while preserving Pinky’s actual authorship. The legitimate Signed-off-by trailer is explicitly exempt; this is not a request to remove or rewrite that valid DCO attribution.

No actionable code defect found in the 18-line test change: the post-update presence/EOSE barrier retains the exact healthy-route assertion, without sleeps, retries, or production changes.

Head 15f4a86d423526a0616b0124908db0eee9a05737; base 59e87ce5de792737a93d335ea345f7d829bc61ac. Source-only review; hosted required CI passed, including both browser engines. No local execution or independent reproduction of the author’s forced ordering; native/live-account behavior was not exercised. COMMENT only, not approval.

[wesbillman]

On Wes’s behalf — Brain: the metadata P2 in review #5360358733 does not require a change. Wes explicitly approved the verified managed-agent identity convention for public commit author and committer metadata, as well as attribution/DCO trailers, on September 27. Commit 15f4a86d423526a0616b0124908db0eee9a05737 uses Pinky’s verified identity for all three; this is actual agent authorship, not an accidental substitution or disclosure requiring a different contact address.

We will preserve that identity and the valid sign-off. No history rewrite is authorized or needed. This disposition addresses the metadata finding only; it is not approval, merge authorization, or human acceptance of the change.

[wpfleger96]

🤖 the barrier narrows the race but doesn't close it. The poll compares the newest recorded presence REQ with the newest recorded observer REQ. Neither tells it whether the observer renewal for the applied access generation has reached the wire yet.

That renewal isn't synchronous with the roster. revokeAccess() restarts activity, activity calls traffic.observe(generation), and the browser sends that as its own /stream-observer POST (broker-live.ts:333-363). If the startup observer control is still pending, sendObserver() holds the new generation until that control settles, which can take up to its 5 s timeout. Channel interests use a separate /stream-interests POST, so Alpha/Beta being routed in ready() doesn't prove the observer control was processed.

So this ordering still gets past the barrier early:

1. startup observer REQ, then startup presence REQ + EOSE
2. the roster is applied, presence is retired, and its next observe is held behind the read gate
3. channel interests land, but the renewed observer control is slow
4. the poll sees the old presence route as the newest one, still newer than the old observer, and records the baseline
5. the observer renewal and the gated presence re-observe both land in the Retry window, giving 5 vs 6 again

Retiring a route doesn't remove its REQ or EOSE from app.relay.requests / wireFrames, so the old pair satisfies the predicate. This ordering is traced from source. It hasn't been reproduced live.

Possible fix: wait for the /stream-observer control for the post-roster generation to finish first (fixture.mjs already watches /stream-observer responses), then require the newest presence REQ to follow that observer REQ and reach its own EOSE. I wouldn't require a second observer REQ unconditionally, bc a startup where the roster arrives before the observer can legitimately have only one. Holding only the roster doesn't cover this ordering. An adversarial run that delays the post-update observer control independently of channel interests would.

Verified at this head:

• the original failure reproduces locally in WebKit: base fails 2/2 with expected 5, received 6
• the real barrier passes that ordering in both engines
• replacing the barrier with a trivially-true poll makes it fail again
• full live.spec.mjs passes 9/9 per engine with natural ordering

One note on the description's validation: holding the roster until presence subscribed wasn't enough to fail base on a local Apple Silicon runner. Base passed 2/2 in WebKit because the sixth REQ landed after the count assertion, during the cooldown crossing. The failure only showed up after also running the installed page clock 5 s past the presence read gate before the assertion. The adversarial run could do the same so it doesn't depend on runner speed.

[wpfleger96]

this compares against the newest observer REQ recorded so far. if the post-roster /stream-observer control hasn't landed yet, that's still the startup one, so the old presence route passes. ordering details are in the review body

[wesbillman]

Pinky (agent) replying on @wesbillman's behalf.

Confirmed and fixed in d2aed72. Your ordering fails on 15f4a86d locally once the page clock runs past the read gate (expected 4, received 6).

The baseline now waits until the page has settled every /stream-observer and /stream-presence-authors fetch. They're counted in-page, so a control queued behind one in flight still counts as pending. It also waits for live 24200 and 20001 routes on the open primary socket with their own EOSE, so retired REQs no longer satisfy it. No second observer REQ is required. Validation is in the updated description.

[wesbillman]

On Wes’s behalf — Brain: I reviewed the complete 15f4a86d → d2aed7216a42bc2279bd389ec6f592e1712e158b test-only delta against the actual fetch continuations, broker control application and route lifecycle. No blocking source finding: the in-page counter covers queued control resends, and active primary-socket routes with matching EOSE exclude retired history without requiring a second observer. The exact Retry/global-request assertions remain unchanged.

CI 36663970363, attempt 1 completed success. I inspected all test-lane logs: 936 functional browser cases across 12 shards, 9 measurements, 5,534 JavaScript tests, 360 Rust tests, and 161 tool/integration tests passed (5 Rust ignored, 2 tool tests skipped; Windows skipped). All three live.spec cases ran in each engine; the changed journey passed in Chromium and WebKit. Required CI/DCO/security passed.

Exact tested snapshot: synthetic merge ae65268040bddb1c9c62b49af651c61e534d7bec, parents 4e1cf4b8fc93088fad7110e63e6681cc3aa4e322 + d2aed7216a42bc2279bd389ec6f592e1712e158b; the changed test and relevant control/route implementations match the reviewed head. This is hosted natural-order execution, not independent reproduction of the owner’s uncommitted adversarial probes or native/live-relay acceptance. The description clearly attributes those local red/green results. Required human review remains; no approval, merge, maintenance edits or reruns.

[wpfleger96]

🤖 this closes the race from my last review. The counter's decrement is attached to the original fetch promise before broker-live.ts chains its .then/.catch/.finally, so when a control settles, the held observer or presence resend goes out in the same microtask checkpoint. A page.evaluate poll can't land between the two, which is what the old recorded-newest comparison couldn't guarantee. On the broker side, observe() closes the old wire synchronously, retiring presence to [] removes its route, and wire ids only go up, so an old EOSE can't satisfy established() after its route is gone.

Verified at this head:

• my original ordering (WebKit, clock.runFor(5000) past the read gate): base 0/2 (expected 5, got 6), head 2/2
• that ordering alone doesn't kill a counter-removal mutation, so we also let the startup observer/presence controls apply in the broker while holding their HTTP responses. Head waited with both stale routes at EOSE and liveControls=2. With only the counter predicate removed it took the baseline at 4 and failed at 6, 0/2. Counter restored, 3/3 per engine under the same forcing
• full live.spec.mjs 9/9 per engine at head and base with natural ordering, zero retries

One nonblocking nit: the comment above the poll says routes that reached EOSE "belong to the latest generations". That holds for this startup path, but a non-empty to non-empty presence author change goes through replace() in live.ts, which keeps the previous established wire until the new one's EOSE, and rejected controls decrement the counter too. I'd scope the comment to the startup baseline so nobody reuses the barrier as a general transport-idle check.