Skip to content

test(stream-credit): a browser tab reads 300 live tracks past its stream cap - #8652

Merged
kixelated merged 4 commits into
mainfrom
quest/m1/stream-credit/README
Nov 13, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m1/stream-credit/README

Conversation

@kixelated

@kixelated kixelated commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Works on quest/m1/stream-credit/README.md: the stream-credit epic's own remaining work, the browser end-to-end test that builds its own harness, now that its last child (moq-dev/moq#8610) merged.

Summary

Adds just test stream-credit (test/stream-credit/). A Chromium or Firefox tab reads 300 live tracks through moq-relay over WebTransport, three times the ~100 streams a browser lets a server open to one session:

  • run.sh starts the relay through test/lib/harness.sh (private run dir, reserved port, process groups) and runs driver.ts for each version (lite-07, lite-06) and engine (Chromium, Firefox).
  • driver.ts publishes from @moq/net in Bun over WebSocket (qmux's raised stream limit, so the browser's cap is the only one in play). Each track keeps one group open at a time, like a live GOP: 40 groups at 200 ms, two frames each.
  • The page (src/, built with Vite, launched with the interop harness's launch) subscribes over WebTransport only, in three tiers: high (priority 2, 30 tracks, 500 ms budget), mid (priority 1, 120 tracks, live edge), low (priority 0, 150 tracks, live edge).

A cell passes when:

  • the high tier reads every group whole;
  • every subscription ends cleanly within 10 s of the publisher finishing, and on lite-07 every track reads its final group whole;
  • frames read follow priority, and the low tier lost some, so the cap bound;
  • on lite-07, the relay log shows paused groups continued.

The harness runs locally only for now. It found two rare browser reset failures on lite-07 (below), and the new child quest browser-resets owns them plus the nightly interop wiring. sh/gh/test-inventory.allow records that owner.

Findings

  1. Chromium, about 1 lite-07 cell in 20: a subscription never ends. The page read the header of a group the relay paused (YIELDED), continued, then dropped as stale. That stream never ended or errored in the page, so the lite-07 tail waits on it forever. Chromium 156 doesn't negotiate RESET_STREAM_AT, so the relay sends a plain RESET_STREAM once the WebTransport header is acknowledged. I checked the relay's held-reset, truncate, and drop-reset paths in moq-quic and moq-tokio, and they look right, but I could not prove where the reset is lost. The child quest's next step is per-stream relay tracing of whether Chromium acknowledges the reset.
  2. Firefox, 1 lite-07 cell in about 25: a starved track's final group aborts. It fails with TypeError: Error in input stream and no stream error code, so the reader can't tell it from a YIELDED reset. Firefox 157 negotiates RESET_STREAM_AT (Reliable Size 3).
  3. lite-06: a starved track may lose its final group. lite-06 has no drops, so a JS reader ends at SUBSCRIBE_FINISH without waiting for a group still queued for credit. This is a protocol limit that lite-07's drop accounting fixes. The harness checks the final group on lite-07 only (see Decisions).

moq-dev/moq#8643's copy-fill duplicate never showed up: no group was handed out twice in any run.

Impact

No public API or wire change. Test-only: a new private workspace package @moq/stream-credit-test and a test::stream-credit recipe. No CI time added.

Validation

  • quest gate just test stream-credit, full matrix, several times: every cell passes. Typical lite-07 numbers: high 100% of frames, mid 61-64%, low 2-3%; every track ends within ~0.5 s of the publisher finishing; the relay continues 600-1,300 paused groups per run.
  • Negative control (all tiers at priority 0, a temporary edit): fails on three independent checks. The high tier misses ~9% of groups past its budget, Chromium's mid tier reads fewer frames than low, and the relay continues no paused group.
  • An earlier variant without the high tier's budget passed at equal priority. That is why the high tier now has a finite budget.
  • Loops of Chromium lite-07 cells (30, 40, 40, 50): failure 1 above in about 1 cell in 20. Firefox loops: failure 2 once in about 25 cells. Diagnosed with temporary in-page tracing of js/net's group-stream lifecycle, which is not committed.
  • quest gate just check: passes.

Decisions

  • Maintainer, 2026-11-13: ✅ merge, and keep the browser-resets follow-up (rejected: dropping it).

Made without asking, since the maintainer was away:

  • ✅ Keep the epic open with a child quest for the two browser failures, and leave the harness out of CI until it lands. Rejected: wiring a ~10%-per-night flake into the interop workflow; loosening the drain or final-group checks to hide it.
  • ✅ Publisher in Bun over WebSocket, not a Rust publisher. The quest asks for a browser reader and a Rust relay; the publisher only has to keep the browser's cap the sole limit, and this avoids a new Rust binary.
  • ✅ Run moq-relay (the binary), as test/drain does, rather than an in-process moq_tokio relay. It is the real surface the epic names.
  • ✅ Cover lite-06 too, since browsers negotiate it by default today (DEFAULT_VERSIONS excludes lite-07-wip), and Firefox too, since the epic cites its 100-stream cap.
  • ✅ lite-06's final-group loss is lite-06's limit, not a follow-up. Rejected: holding the relay's lite-06 SUBSCRIBE_FINISH until no group waits for credit, a heuristic for a version lite-07 replaces.
  • ✅ Reuse launch from test/interop/clients/js/harness.ts for its Nix Firefox library handling, rather than copying it. The tsconfig includes js/common/worklet.d.ts for that import's type graph.

Follow-ups

  • quest/m1/stream-credit/browser-resets.md [L]: make each browser group-stream reset reach the reader (Chromium hang, Firefox code loss), then add just test stream-credit to the nightly interop workflow and delete the epic README. Kept by the maintainer, 2026-11-13.

Caused by

No single PR: a new test. The browser failures it found may sit in the browsers themselves; that's undetermined.

Models

Size: L
Implementer: Claude Code, claude-opus-5-5, unknown effort

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

Review

Skipped on head 877bbd8: since the last quest review, only comments and test-failure reporting changed, answering Pullfrog's and CodeRabbit's findings; Pullfrog re-reviews this head.

kixelated and others added 2 commits November 13, 2026 09:19
…eam cap

`just test stream-credit` stands up moq-relay, publishes 300 live tracks from
@moq/net over WebSocket, and reads them in a Chromium or Firefox tab over
WebTransport in three priority tiers, on lite-07 and lite-06. It checks the top
tier reads every group within its budget, every subscription ends soon after
the publisher finishes, frames follow priority, and on lite-07 that the relay
continues paused groups.

It found two rare browser reset failures on lite-07, which a new child quest
owns before the harness joins the nightly interop workflow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Stream credit by priority test(stream-credit): a browser tab reads 300 live tracks past its stream cap Nov 13, 2026
@kixelated
kixelated marked this pull request as ready for review November 13, 2026 18:36

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — two small rough edges inline.

Reviewed changes

I reviewed the whole PR: a new local-only browser test of stream credit by priority, plus a quest to track the browser reset failures it found.

  • just test stream-credit harness: run.sh starts moq-relay through test/lib/harness.sh and runs driver.ts for each version (lite-07, lite-06) and engine (Chromium, Firefox).
  • Publisher and judge (driver.ts): a Bun @moq/net publisher over WebSocket writes 300 tracks of live groups. The driver then checks the tab's report: the high tier reads every group whole, every subscription drains, frames follow priority, and on lite-07 the final group arrives whole and the relay log shows continuing paused group.
  • Browser reader (src/main.ts, src/plan.ts): a Vite-built page subscribes over WebTransport only, in three priority tiers, and records each group's frames.
  • Quest bookkeeping: the epic README now describes the test, browser-resets.md owns the Chromium and Firefox reset failures plus the CI wiring, and test-inventory.allow records that quest as the recipe's owner.

The relay log scoping (conn{id=N of the last WebTransport session on the version) uses the same pattern as test/interop/relay-log.sh. Every cell republishes broadcast credit with the same track:seq:frame payloads, so reusing the name between cells keeps each identity's content the same.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread test/stream-credit/driver.ts Resolved
Comment thread test/stream-credit/run.sh Resolved
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Adds a stream-credit test harness that publishes 300 tracks through a relay and reads them in Chromium or Firefox over WebTransport. The driver checks track priority, group delivery, subscription completion, and relay log entries across the configured protocol versions. The runner supports selecting versions and browser engines.

Merge Risk: Minimal · up to bb1da

This adds a local, opt-in browser test harness with no production runtime impact. No actionable merge-blocking issues remain.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the change: a test (test(stream-credit)) in which a browser tab reads 300 live tracks past its stream cap. The raw summary confirms the main change is a new stream-credit end-to-end ha…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description is specific and related to the changeset. It states that the PR adds just test stream-credit under test/stream-credit/, a browser test that reads 300 live tracks through `moq-relay…
Full details: Title check

Explanation

The title names the change: a test (test(stream-credit)) in which a browser tab reads 300 live tracks past its stream cap. The raw summary confirms the main change is a new stream-credit end-to-end harness that does this. The title is specific and concise. It does not list every detail, which is acceptable.

Full details: Description check

Explanation

The description is specific and related to the changeset. It states that the PR adds just test stream-credit under test/stream-credit/, a browser test that reads 300 live tracks through moq-relay over WebTransport. It names the driver, run script, page, tiers, pass criteria, validation runs, and the child quest for the two browser reset failures. The raw summary and objectives agree with these details, so the description is not off-topic or vague.

Finishing Touches

Simplify code

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2c75a0cf-fdd6-43ba-a776-e1b35570774e
Commits

Reviewing files that changed from the base of the PR and between 8206d7f and bb1da69.

Files ignored due to path filters: 1

  • bun.lock is excluded by !**/*.lock
Files selected for processing: 15

  • package.json
  • quest/m1/stream-credit/README.md
  • quest/m1/stream-credit/browser-resets.md
  • sh/gh/test-inventory.allow
  • test/README.md
  • test/justfile
  • test/stream-credit/README.md
  • test/stream-credit/driver.ts
  • test/stream-credit/index.html
  • test/stream-credit/package.json
  • test/stream-credit/relay.toml
  • test/stream-credit/run.sh
  • test/stream-credit/src/main.ts
  • test/stream-credit/src/plan.ts
  • test/stream-credit/tsconfig.json

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior Pullfrog review, which only addresses that review's two findings.

  • Relay log miss is a recorded failure: judge in driver.ts now pushes to failures and returns when the relay log names no WebTransport session, instead of throwing, so the collected FAIL output stays whole.
  • run.sh header names Firefox: the comment now says a Chromium or Firefox tab.

Pullfrog  | View workflow run | Using claude-sonnet-5-5 | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 13, 2026
Merged via the queue into main with commit 3cb1c63 Nov 13, 2026
7 checks passed
@kixelated
kixelated deleted the quest/m1/stream-credit/README branch November 13, 2026 20:17
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.

1 participant