diff --git a/quest/m0/ci/README.md b/quest/m0/ci/README.md index 3fee91ca6af..88198329669 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` 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 new file mode 100644 index 00000000000..140ec637a6b --- /dev/null +++ b/quest/m0/ci/interop-filter-lines.md @@ -0,0 +1,37 @@ +# [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` 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 + +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. +- 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`) + 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 00000000000..749d55efb0a --- /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 fd5086a45a0..1651f37ed68 100644 --- a/quest/m1/test-flakes-2/README.md +++ b/quest/m1/test-flakes-2/README.md @@ -32,13 +32,15 @@ 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-21 and 2026-10-22 (maintainer): 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 - [Media resource instrumentation](/quest/m1/test-flakes-2/resource-instrumentation.md) - the media test stops seeing a playing player report nothing to release - [Interop cpp to js budget](/quest/m1/test-flakes-2/interop-cpp-js.md) - the cpp -> js cell finishes inside its 30 s budget, fixed at its cause 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 00000000000..2a0f0155158 --- /dev/null +++ b/quest/m1/test-flakes-2/auth-union-renewal.md @@ -0,0 +1,44 @@ +# [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. + +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 new file mode 100644 index 00000000000..11cb3edac93 --- /dev/null +++ b/quest/m1/test-flakes-2/one-process.md @@ -0,0 +1,49 @@ +# [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. + +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. diff --git a/quest/m2/README.md b/quest/m2/README.md index fb440f3d991..6af78d71fa0 100644 --- a/quest/m2/README.md +++ b/quest/m2/README.md @@ -118,6 +118,7 @@ ranked just below deterministic TS legs, which it requires. - [Wire fuzz coverage](/quest/m2/wire-fuzz-coverage.md) - the audited moq-net decoders have targets that assert an invariant, and the unsafe peer input reaches runs under Miri - [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 - [qmux receive leases](/quest/m2/qmux-receive-lease.md) - WebSocket, TCP, and WebRTC sessions bound incomplete messages by their receive window, at a per-session cost the maintainer picks +- [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 00000000000..d3c244b9a14 --- /dev/null +++ b/quest/m2/quic-stuck-scan.md @@ -0,0 +1,37 @@ +# [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 [qmux receive leases](/quest/m2/qmux-receive-lease.md) 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