From cceb5f53e13743eccf83763ccfa87da930a8c1ec Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 22 Oct 2026 11:23:25 -0700 Subject: [PATCH 1/3] quest: plan follow-ups from the 2026-10-22 merge run Queue gate (ALLGREEN) and one-per-line interop filters under m0/ci, two test-flakes-2 children (torn union-renewal update, process-wide test state), and the per-packet stuck-window check in m2. Co-Authored-By: Claude Opus 5.5 --- quest/m0/ci/README.md | 2 + quest/m0/ci/interop-filter-lines.md | 28 +++++++++++++ quest/m0/ci/queue-allgreen.md | 38 ++++++++++++++++++ quest/m1/test-flakes-2/README.md | 11 +++-- quest/m1/test-flakes-2/auth-union-renewal.md | 38 ++++++++++++++++++ quest/m1/test-flakes-2/one-process.md | 42 ++++++++++++++++++++ quest/m2/README.md | 1 + quest/m2/quic-stuck-scan.md | 38 ++++++++++++++++++ 8 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 quest/m0/ci/interop-filter-lines.md create mode 100644 quest/m0/ci/queue-allgreen.md create mode 100644 quest/m1/test-flakes-2/auth-union-renewal.md create mode 100644 quest/m1/test-flakes-2/one-process.md create mode 100644 quest/m2/quic-stuck-scan.md diff --git a/quest/m0/ci/README.md b/quest/m0/ci/README.md index 3fee91ca6a..3423155d05 100644 --- a/quest/m0/ci/README.md +++ b/quest/m0/ci/README.md @@ -80,7 +80,9 @@ The historical samples locate candidates; they are not controlled benchmarks. ## Required +- [Every queued PR passes its own group](/quest/m0/ci/queue-allgreen.md) - the main queue switches from HEADGREEN to ALLGREEN, so a failed entry never merges behind a green one - [Require the release gate](/quest/m0/ci/release-gates.md) - condition: the next cut carries `release-gate.yml`; then release PRs require it +- [One test filter per line](/quest/m0/ci/interop-filter-lines.md) - parallel interop filter additions to `test/justfile` merge without conflict - [Share local check capacity](/quest/m0/ci/local-gate.md) - gate expensive phases and bound internal fanout - [Verify the R2 Rust cache](/quest/m0/ci/mbx-r2-verify.md) - condition: the maintainer bootstrap; then measure the hosted Nightly jobs' hits, restore time, and expiry - [Verify the Nix cache](/quest/m0/ci/nix-cache-verify.md) - confirm CI and releases use `nix.moq.dev` and retire Cachix diff --git a/quest/m0/ci/interop-filter-lines.md b/quest/m0/ci/interop-filter-lines.md new file mode 100644 index 0000000000..1f686c098e --- /dev/null +++ b/quest/m0/ci/interop-filter-lines.md @@ -0,0 +1,28 @@ +# [XS] One test filter per line in test/justfile + +## Goal + +Two PRs that each add an interop test filter or harness file to +`test/justfile` merge without a conflict. Every list on a single long line +(the `interop` recipe's `just rs test -p moq-net --lib ...` filters and the +`harness` recipe's browser test files) puts one entry per line, and the +selected tests stay exactly the same. + +## Plan + +Decided 2026-10-22 (maintainer): many PRs on 2026-10-22 conflicted on the +204-character filter line (`test/justfile:50`), for example +[moq-dev/moq#6158](https://github.com/moq-dev/moq/pull/6158), +[moq-dev/moq#6233](https://github.com/moq-dev/moq/pull/6233) and +[moq-dev/moq#6228](https://github.com/moq-dev/moq/pull/6228). +Rejected for now: MIGRATE.md entry fragments, and `merge=union` in +`.gitattributes`, which silently merges real conflicts. + +- Use just's line continuation (`\`) or a recipe variable, whichever keeps + `just --fmt --check` and the `gh`/justfile lints clean. +- Keep the prose comment above the recipe, but stop it repeating the filter + list, so a new filter edits one place. +- Prove nothing changed: compare the expanded commands (`just --dry-run`) + before and after. + +Public API: none. Wire: none. diff --git a/quest/m0/ci/queue-allgreen.md b/quest/m0/ci/queue-allgreen.md new file mode 100644 index 0000000000..749d55efb0 --- /dev/null +++ b/quest/m0/ci/queue-allgreen.md @@ -0,0 +1,38 @@ +# [XS] Every queued PR passes its own group + +## Goal + +The main merge queue merges a PR only when that PR's own group passes `Check`. +A failed entry leaves the queue instead of riding along behind a later green +group. + +## Plan + +Decided 2026-10-22 (maintainer): switch ruleset 2420853's merge queue from +`grouping_strategy: HEADGREEN` to `ALLGREEN`. Rejected: checking every group's +whole diff since main, which keeps HEADGREEN but widens each run's selection. + +Why: main broke twice on 2026-10-22, and in both cases the queue had already +caught it. The group for [moq-dev/moq#6318](https://github.com/moq-dev/moq/pull/6318) +failed `quest check` on a link to a quest that +[moq-dev/moq#6142](https://github.com/moq-dev/moq/pull/6142) deleted, and the +group for [moq-dev/moq#6222](https://github.com/moq-dev/moq/pull/6222) failed +typechecking a call that [moq-dev/moq#6230](https://github.com/moq-dev/moq/pull/6230) +added. Under HEADGREEN a later group passed, so the failed entries merged anyway. +A later group cannot cover an earlier failure, because `sh/changed.sh` diffs each +group against `merge_group.base_sha` (the previous group), so each run selects +only its own PR's packages. + +- This is a repository setting, so the maintainer applies it. Record the change + with `gh api repos/moq-dev/moq/rulesets/2420853` before and after. +- Keep `max_entries_to_build` and `max_entries_to_merge` unless the change + shows a throughput problem. Compare queue wait over the first busy day + against 2026-10-22's run. +- `CONTRIBUTING.md` already describes what each group checks, and that + stays true. + +Public API: none. Wire: none. + +## Related + +- [CI workflow audit](/quest/m1/audit-ci.md) - owns per-job cost; this quest only changes the gate diff --git a/quest/m1/test-flakes-2/README.md b/quest/m1/test-flakes-2/README.md index 4a756d41dd..d8c485ca8f 100644 --- a/quest/m1/test-flakes-2/README.md +++ b/quest/m1/test-flakes-2/README.md @@ -1,4 +1,4 @@ -# [S] More tests hold up under load +# More tests hold up under load ## Goal @@ -32,11 +32,16 @@ times on a loaded machine, as the first round did. The redundant failover drift moved to m0 on 2026-10-20 (maintainer) and is fixed by [moq-dev/moq#6113](https://github.com/moq-dev/moq/pull/6113). -Added 2026-10-21 (maintainer): small flakes from the m0/m1 merge run, kept -in one child. +Added 2026-10-22 (maintainer): two failures from the m0/m1 merge runs, one +child each. Public API: none. Wire: none. +## Required + +- [Union renewal](/quest/m1/test-flakes-2/auth-union-renewal.md) - the JS subscriber never leaves a torn REQUEST_UPDATE on the stream +- [One process](/quest/m1/test-flakes-2/one-process.md) - no test writes process-wide env or unwraps the crypto provider install + ## Related - [Controlled time](/quest/m1/time/README.md) - the systemic fix these rules ask for: every crate on a controlled clock diff --git a/quest/m1/test-flakes-2/auth-union-renewal.md b/quest/m1/test-flakes-2/auth-union-renewal.md new file mode 100644 index 0000000000..6b94b99a1b --- /dev/null +++ b/quest/m1/test-flakes-2/auth-union-renewal.md @@ -0,0 +1,38 @@ +# [S] A union renewal never sends a torn request update + +## Goal + +`js/net/src/auth.test.ts`'s "a request waits for its token and survives union +renewal" passes on every moq-transport draft every time. The subscriber never +leaves a request stream holding a partial message, so the publisher never +closes the session with "malformed request stream message" (ProtocolViolation, +code 3). + +## Plan + +Seen on 2026-10-22 on moqt-17 and moqt-22: about 4 failures in 120 runs on +main. The failure follows a logged `subscribe update priority=255`. + +Leads from reading the code (unverified): + +- `js/net/src/ietf/subscriber.ts:1098-1099` writes the message type + (`stream.writer.u53(SubscribeUpdate.id)`) and then the body in two awaits. + If the body's encode throws, the catch at `:1100-1103` returns silently with + the type already sent. Teardown then FINs the stream (`:970`), and the + publisher fails reading the size, which is exactly this error + (`js/net/src/ietf/request_stream.ts:124`, `:312-326`). +- The test sets no priority, and wire 255 means model priority -128 + (`js/net/src/ietf/priority.ts`). The idle aggregate is 0 (wire 127), so + either the aggregate is wrong mid-revocation or the log came from another + session. +- `first.close()` shrinks the grant union while `#runUpdates` may be writing + (`subscriber.ts:920-924`, `:961-973`). + +Reproduce it first: loop the test until it fails, capture which session logged +255 and where the write stopped. Fix it at the source. A message goes out +whole or not at all, and an encode failure fails loudly instead of being +swallowed. Then, if the 255 is real, correct the aggregate. Add a test with a +mocked clock that tears the stream mid-update. Check the Rust subscriber for +the same split write. + +Public API: none. Wire: none. diff --git a/quest/m1/test-flakes-2/one-process.md b/quest/m1/test-flakes-2/one-process.md new file mode 100644 index 0000000000..1d2a0cdf82 --- /dev/null +++ b/quest/m1/test-flakes-2/one-process.md @@ -0,0 +1,42 @@ +# [S] Tests pass when they share one process + +## Goal + +The Rust tests pass under plain `cargo test`, where every test in a binary +runs as a thread of one process, as well as under nextest's +process-per-test. No test writes process-wide state: none calls +`std::env::set_var` or `remove_var`, and none unwraps an install of the +process-wide rustls `CryptoProvider`. + +## Plan + +Decided 2026-10-22 (maintainer): read `SSLKEYLOGFILE` in one place and pass +the resulting path down as an argument. Tests pass a path and never touch the +environment. Public behavior and API are unchanged. Rejected: a shared env +lock (concurrent `setenv` and `getenv` on other threads stays undefined +behavior on glibc), and a public keylog path in the QUIC config (nothing +consumes it). + +Keylog tests that set or clear `SSLKEYLOGFILE` today, each commented +"nextest runs each test in its own process": + +- `rs/moq-tokio/src/quic.rs` `keylog_requires_a_path` (the env read is in + `validate`, around `:576`) +- `rs/moq-tokio/tests/integration/keylog.rs`: three tests share `handshake()` +- `rs/moq-ffi/src/test.rs` `client_keylog_writes_session_keys` +- `rs/moq-c/src/test.rs` `server_keylog_requires_sslkeylogfile` +- `rs/moq-relay/src/uring.rs` `bind_refuses_a_keylog_without_sslkeylogfile` +- `rs/moq-gst/src/source/imp.rs` `keylog_reaches_the_client` + +Crypto provider: `rs/moq-relay/tests/integration/wire_stats.rs:20` unwraps +`install_default()` and `stack.rs:8` expects it. Every other module in that +binary ignores the result, so a second install panics. Match the rest, or +install once for the binary. + +Grep the workspace for other `set_var`/`remove_var` calls in tests (moq-cli's +`EnvGuard` serializes its own) and fix any that leak. Verify with a +`cargo test` run of each touched crate as well as nextest. Make the +`set_var` ban stick if a lint can do it cheaply, for example clippy's +`disallowed_methods` in test code. + +Public API: none. Wire: none. diff --git a/quest/m2/README.md b/quest/m2/README.md index ac9db1191e..7ec0de0085 100644 --- a/quest/m2/README.md +++ b/quest/m2/README.md @@ -104,6 +104,7 @@ ranked just below deterministic TS legs, which it requires. - [Bindings in the interop matrix](/quest/m2/interop-bindings.md) - Kotlin, Swift, and Dart publish and subscribe against every other interop client - [WebGPU HDR](/quest/m2/webgpu-hdr.md) - HDR renditions play as HDR where the browser and display can show it, and tone-map to SDR elsewhere - [One port](/quest/m2/one-port/README.md) - one UDP port carries QUIC, SRT, and WebRTC media through an embedder hook, and one TCP port carries HTTP, RTMP, and RTMPS +- [One stuck check per packet](/quest/m2/quic-stuck-scan.md) - a RESET_STREAM storm into a shrunk window costs one stream scan per packet, after #5878 - [QUIC caps](/quest/m2/rate-quic.md) - paced MAX_DATA credit in and a capped pacer out hold a QUIC session to its token's bitrate - [WebSocket caps](/quest/m2/rate-websocket.md) - paced reads and writes over bounded socket buffers hold WebSocket to the same caps - [Publishers learn their cap](/quest/m2/rate-grant.md) - the AUTH grant carries the caps and publishers clamp their encoder to them diff --git a/quest/m2/quic-stuck-scan.md b/quest/m2/quic-stuck-scan.md new file mode 100644 index 0000000000..f3a28b56c2 --- /dev/null +++ b/quest/m2/quic-stuck-scan.md @@ -0,0 +1,38 @@ +# [M] One stuck-window check per packet + +## Goal + +A moq-quic session runs its stuck-receive-window check at most once per +received packet, however many frames in that packet move connection credit. +A peer can no longer make us scan every stream for each RESET_STREAM it sends +into a shrunk window, and a benchmark shows the cost staying flat as streams +grow. + +## Plan + +Starts after [moq-dev/moq#5878](https://github.com/moq-dev/moq/pull/5878) +merges, which adds the receive leases and the stuck check. Its last fix +already skips the check for arrivals that move no credit +(`creditless_arrivals_skip_the_stuck_check`). What remains: a RESET_STREAM +that frees credit into shrink debt still pays one scan of every stream, +which needs a local window shrink plus many streams each holding data. + +Decided 2026-10-22 (maintainer): m2, because it bounds CPU rather than fixing +a correctness bug. + +- Benchmark first: a RESET_STREAM storm under a shrunk window with N streams + holding data, swept over N, to show the per-frame slope before and the + flat cost after. +- Then mark the check dirty per frame and run it once at the end of packet + processing. This touches the stuck-check call sites in moq-quic's tests + (about 50) and the qmux mux path that shares it. +- Re-read the landed code first; the lease API may have moved since #5878. + Coordinate with the qmux receive-leasing quest from + [moq-dev/moq#6352](https://github.com/moq-dev/moq/pull/6352) if qmux + adopts leases on the same mux path. + +Public API: none. Wire: none. + +## Related + +- [QUIC caps](/quest/m2/rate-quic.md) - paced receive credit on the same sessions From 5ab5cedd23511db6754f28410c73d8da8573ba24 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 22 Oct 2026 13:40:50 -0700 Subject: [PATCH 2/3] quest: link the merged qmux receive-lease quest Co-Authored-By: Claude Opus 5.5 --- quest/m2/quic-stuck-scan.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/quest/m2/quic-stuck-scan.md b/quest/m2/quic-stuck-scan.md index f3a28b56c2..d3c244b9a1 100644 --- a/quest/m2/quic-stuck-scan.md +++ b/quest/m2/quic-stuck-scan.md @@ -27,9 +27,8 @@ a correctness bug. processing. This touches the stuck-check call sites in moq-quic's tests (about 50) and the qmux mux path that shares it. - Re-read the landed code first; the lease API may have moved since #5878. - Coordinate with the qmux receive-leasing quest from - [moq-dev/moq#6352](https://github.com/moq-dev/moq/pull/6352) if qmux - adopts leases on the same mux path. + Coordinate with [qmux receive leases](/quest/m2/qmux-receive-lease.md) if + qmux adopts leases on the same mux path. Public API: none. Wire: none. From 72777fdfa2fe373ebdd344edd969a0ce7b74cd1c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 22 Oct 2026 15:30:47 -0700 Subject: [PATCH 3/3] quest: narrow filter-lines goal, migrate guarded env writes, cover draft 14-16 update Co-Authored-By: Claude Opus 5.5 --- quest/m0/ci/README.md | 2 +- quest/m0/ci/interop-filter-lines.md | 17 +++++++++++++---- quest/m1/test-flakes-2/auth-union-renewal.md | 6 ++++++ quest/m1/test-flakes-2/one-process.md | 17 ++++++++++++----- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/quest/m0/ci/README.md b/quest/m0/ci/README.md index 3423155d05..8819832966 100644 --- a/quest/m0/ci/README.md +++ b/quest/m0/ci/README.md @@ -82,7 +82,7 @@ The historical samples locate candidates; they are not controlled benchmarks. - [Every queued PR passes its own group](/quest/m0/ci/queue-allgreen.md) - the main queue switches from HEADGREEN to ALLGREEN, so a failed entry never merges behind a green one - [Require the release gate](/quest/m0/ci/release-gates.md) - condition: the next cut carries `release-gate.yml`; then release PRs require it -- [One test filter per line](/quest/m0/ci/interop-filter-lines.md) - parallel interop filter additions to `test/justfile` merge without conflict +- [One test filter per line](/quest/m0/ci/interop-filter-lines.md) - parallel interop filter additions to `test/justfile` rarely conflict - [Share local check capacity](/quest/m0/ci/local-gate.md) - gate expensive phases and bound internal fanout - [Verify the R2 Rust cache](/quest/m0/ci/mbx-r2-verify.md) - condition: the maintainer bootstrap; then measure the hosted Nightly jobs' hits, restore time, and expiry - [Verify the Nix cache](/quest/m0/ci/nix-cache-verify.md) - confirm CI and releases use `nix.moq.dev` and retire Cachix diff --git a/quest/m0/ci/interop-filter-lines.md b/quest/m0/ci/interop-filter-lines.md index 1f686c098e..140ec637a6 100644 --- a/quest/m0/ci/interop-filter-lines.md +++ b/quest/m0/ci/interop-filter-lines.md @@ -3,10 +3,12 @@ ## Goal Two PRs that each add an interop test filter or harness file to -`test/justfile` merge without a conflict. Every list on a single long line -(the `interop` recipe's `just rs test -p moq-net --lib ...` filters and the -`harness` recipe's browser test files) puts one entry per line, and the -selected tests stay exactly the same. +`test/justfile` rarely conflict: they conflict only when both insert at the +same point of a sorted list, and that resolves by keeping both lines. Every +list on a single long line (the `interop` recipe's +`just rs test -p moq-net --lib ...` filters and the `harness` recipe's +browser test files) puts one entry per line, and the selected tests stay +exactly the same. ## Plan @@ -20,6 +22,13 @@ Rejected for now: MIGRATE.md entry fragments, and `merge=union` in - Use just's line continuation (`\`) or a recipe variable, whichever keeps `just --fmt --check` and the `gh`/justfile lints clean. +- Sort each list, so new entries land at different points instead of all at + the end, and close it with a fixed line that holds no entry (the + `--run-ignored all` flags, for example), so adding an entry never edits a + neighbor's line to add or drop its continuation. +- Line layout alone cannot make two inserts at the same point merge cleanly; + only one file per entry could, which costs more than the conflicts it + saves. - Keep the prose comment above the recipe, but stop it repeating the filter list, so a new filter edits one place. - Prove nothing changed: compare the expanded commands (`just --dry-run`) diff --git a/quest/m1/test-flakes-2/auth-union-renewal.md b/quest/m1/test-flakes-2/auth-union-renewal.md index 6b94b99a1b..2a0f015515 100644 --- a/quest/m1/test-flakes-2/auth-union-renewal.md +++ b/quest/m1/test-flakes-2/auth-union-renewal.md @@ -35,4 +35,10 @@ swallowed. Then, if the 255 is real, correct the aggregate. Add a test with a mocked clock that tears the stream mid-update. Check the Rust subscriber for the same split write. +The test's draft matrix runs only drafts 17 and 22, but +`SubscribeUpdate`'s encode has a separate draft 14-16 path (an extra request +ID and different fields). Add a representative draft 14-16 case, or every +supported IETF ALPN, to both the existing test and the new one, so the +legacy encoding is covered too. + Public API: none. Wire: none. diff --git a/quest/m1/test-flakes-2/one-process.md b/quest/m1/test-flakes-2/one-process.md index 1d2a0cdf82..11cb3edac9 100644 --- a/quest/m1/test-flakes-2/one-process.md +++ b/quest/m1/test-flakes-2/one-process.md @@ -33,10 +33,17 @@ Crypto provider: `rs/moq-relay/tests/integration/wire_stats.rs:20` unwraps binary ignores the result, so a second install panics. Match the rest, or install once for the binary. -Grep the workspace for other `set_var`/`remove_var` calls in tests (moq-cli's -`EnvGuard` serializes its own) and fix any that leak. Verify with a -`cargo test` run of each touched crate as well as nextest. Make the -`set_var` ban stick if a lint can do it cheaply, for example clippy's -`disallowed_methods` in test code. +Guarded writes count too: `moq-cli`'s `EnvGuard` (`rs/moq-cli/src/test_env.rs`) +and the similarly guarded `moq-relay` config tests (`rs/moq-relay/src/config.rs`) +serialize their own writes, but a guard only orders the tests that take it, +not a `getenv` on any other thread. These test clap's `env` fallbacks, and +clap reads the process environment with no injection hook, so run each such +test in a child process given its environment by `Command::env` and +`env_remove` (for example the test binary re-run on that one test), and drop +`EnvGuard`. Grep the workspace for every other `set_var`/`remove_var` call in +tests and move it the same way. Verify with a +`cargo test` run of each touched crate as well as nextest. With no write +left, make the ban stick with clippy's `disallowed_methods` on +`std::env::set_var` and `remove_var`, workspace-wide. Public API: none. Wire: none.