From 366972574b9a56d7752e11cdcc90f987353e7761 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 10:08:15 -0700 Subject: [PATCH 1/7] docs: fix discontinuity and version-source notes The py and swift examples bind `video` to an encoder, which has no discontinuity(); name the publish_video track instead. List uv.lock as a Python version source and say where OBS release versions come from. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 4 ++-- doc/lib/py/index.md | 2 +- doc/lib/swift/index.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 28673e0f47..c868dbbb3e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,7 +64,7 @@ Releases are cut separately; bump only when asked. Each package's version lives - **Rust**: release-plz owns crate versions and Rust dependency requirements. - **JavaScript**: `js/*/package.json` packages with a `scripts.release` entry (skip private ones like `@moq/clock`, `@moq/wasm`), plus the matching workspace version in `bun.lock`. -- **Python**: `py/moq-rs/pyproject.toml` only; `py/moq-ffi` follows the `moq-ffi-v*` tag and Rust crate. +- **Python**: `py/moq-rs/pyproject.toml`, plus the matching `moq-rs` entry in the root `uv.lock`; `py/moq-ffi` follows the `moq-ffi-v*` tag and Rust crate. - **Swift**: `swift/VERSION`. **Kotlin**: `moq.version` in `kt/gradle.properties`. **Dart**: `version` in `dart/moq/pubspec.yaml`. Their FFI counterparts track the Rust crate. - **Go**: `go/wrapper/VERSION` holds a human-owned `MAJOR.MINOR` line; CI derives the patch, so only edit it for a breaking API. Leave the placeholder FFI version in `go.mod` alone. -- **OBS**: the plugin takes the `libmoq` crate's version (`cpp/obs/CMakeLists.txt`), so release-plz owns it. +- **OBS**: release builds take their version from the `libmoq-v*` tag that release-plz cuts for the `libmoq` crate (`cpp/obs/build.sh --libmoq-release`), not from any manifest, so there is nothing to bump. diff --git a/doc/lib/py/index.md b/doc/lib/py/index.md index 109d8bab30..cbbc824a0f 100644 --- a/doc/lib/py/index.md +++ b/doc/lib/py/index.md @@ -71,7 +71,7 @@ asyncio.run(main()) For already-encoded live output, call `audio.flush(timestamp_us)` after each `audio.write_frame` with the same broadcast-clock PTS. It samples the transport handoff for catalog jitter. File, pipe, and network imports should omit `flush`; raw-pixel and PCM encoders inside the binding measure their own output. -Call `audio.discontinuity()` when the source seeks, pauses, or changes its time base. It publishes a timeline marker and restarts handoff measurement without lowering advertised jitter. Resume with timestamps that continue forward on the broadcast media clock; this does not permit timestamp rewinds. On a video track, resume with a keyframe: a delta frame before it fails. +Call `audio.discontinuity()` when the source seeks, pauses, or changes its time base. It publishes a timeline marker and restarts handoff measurement without lowering advertised jitter. Resume with timestamps that continue forward on the broadcast media clock; this does not permit timestamp rewinds. On a track from `publish_video`, resume with a keyframe: a delta frame before it fails. The three advertising operations, as the other bindings spell them: `client.create_broadcast(path)` (or `OriginProducer.create_broadcast`) returns diff --git a/doc/lib/swift/index.md b/doc/lib/swift/index.md index f9a6f6263b..8cc2695afe 100644 --- a/doc/lib/swift/index.md +++ b/doc/lib/swift/index.md @@ -57,7 +57,7 @@ session.shutdown() For already-encoded live output, call `audio.flush(timestampUs:)` after `writeFrame` with the same broadcast-clock PTS. It measures catalog jitter at the transport handoff. File, pipe, and network imports should omit `flush`; built-in encoders observe their own output. -Call `audio.discontinuity()` when the source seeks, pauses, or changes its time base. It publishes a timeline marker and restarts handoff measurement without lowering advertised jitter. Resume with timestamps that continue forward on the broadcast media clock; this does not permit timestamp rewinds. On a video track, resume with a keyframe: a delta frame before it fails. +Call `audio.discontinuity()` when the source seeks, pauses, or changes its time base. It publishes a timeline marker and restarts handoff measurement without lowering advertised jitter. Resume with timestamps that continue forward on the broadcast media clock; this does not permit timestamp rewinds. On a track from `publishVideo`, resume with a keyframe: a delta frame before it fails. The three advertising operations: `session.publish.createBroadcast(path:)` returns an unannounced producer, invisible to everyone; `broadcast.announce(route:)` / From 3cefab5db9ed7c312101875994b2069b79147e12 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 10:09:21 -0700 Subject: [PATCH 2/7] fix(net): enforce the read-size limit on buffered decodes A length-prefixed value already in the buffer never reached #fillTo, the only place the 64 MiB guard lived, so Cursor.read and string accepted it. Co-Authored-By: Claude Opus 5.5 --- js/net/src/stream.test.ts | 8 ++++++++ js/net/src/stream.ts | 2 ++ 2 files changed, 10 insertions(+) diff --git a/js/net/src/stream.test.ts b/js/net/src/stream.test.ts index b84af33f50..38422aba52 100644 --- a/js/net/src/stream.test.ts +++ b/js/net/src/stream.test.ts @@ -444,6 +444,14 @@ test("Reader decode rejects a stream that ends inside a message", async () => { await expect(reader.decode(sized)).rejects.toThrow("unexpected end of stream"); }); +test("Reader refuses an oversized value even when it is already buffered", async () => { + const size = 64 * 1024 * 1024 + 1; + const buffer = new Uint8Array(4 + size); + buffer.set([0x84, 0x00, 0x00, 0x01]); // the 4-byte varint for size + await expect(new Reader(undefined, buffer).string()).rejects.toThrow("exceeds max size"); + await expect(new Reader(undefined, buffer.subarray(4)).read(size)).rejects.toThrow("exceeds max size"); +}); + /** A stream reset as a transport delivers one: the peer's code, and nothing else useful. */ class Reset extends Error { readonly source = "stream" as const; diff --git a/js/net/src/stream.ts b/js/net/src/stream.ts index 5848accad2..67f7390c33 100644 --- a/js/net/src/stream.ts +++ b/js/net/src/stream.ts @@ -451,6 +451,8 @@ export class Cursor { /** Read `size` bytes, as a view onto the buffer rather than a copy. */ read(size: number): Uint8Array { + // Checked here too, since a value that is already buffered never reaches the fill. + if (size > MAX_READ_SIZE) throw new Error(`read size ${size} exceeds max size ${MAX_READ_SIZE}`); this.#ensure(size); const start = this.#offset; this.#offset += size; From 2fcab5764c32f465b03a421429943733b4b50cee Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 10:12:46 -0700 Subject: [PATCH 3/7] docs(quest): correct quest details from recent reviews Fix claims agents would follow: the TS export outcome, the redirect target type, the CAT test scope, legacy raw stream codes, and the Go per-file limit. Add the Go and Python binary producers to data capture, fold BBR idle burst into BBR app-limited, and park the m3 libmoq quests behind a Required condition. Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 3 +-- quest/m1/bbr-idle-burst.md | 31 ------------------------------- quest/m1/data-capture-bindings.md | 13 ++++++++++--- quest/m1/go-mirror-delivery.md | 2 +- quest/m1/quic/bbr-app-limited.md | 20 ++++++++++++++++++++ quest/m1/raw-stream-codes.md | 16 ++++++++++------ quest/m1/redirect-resolve.md | 5 +++-- quest/m2/cat/present.md | 2 +- quest/m3/libmoq-cmake-lib.md | 6 +++++- quest/m3/libmoq-fetch.md | 6 +++++- quest/m3/libmoq-hidden.md | 6 +++++- quest/m3/libmoq-shutdown.md | 6 +++++- 12 files changed, 66 insertions(+), 50 deletions(-) delete mode 100644 quest/m1/bbr-idle-burst.md diff --git a/quest/m1/README.md b/quest/m1/README.md index 50e3d328e9..a236b05205 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -56,7 +56,7 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Accept-side flags](/quest/m1/cli-given-flags.md) - dial-only and local verbs refuse every `--listen-*` flag instead of ignoring it - [JS catalog path](/quest/m1/js-catalog-path.md) - `@moq/net` broadcast consumers expose their path and `Catalog.watch` rejects escaping references, like Rust - [Full codec string](/quest/m1/publish-codec-string.md) - browser-published video carries the encoder's full RFC 6381 codec string, so native players decode it -- [TS export jitter](/quest/m1/ts-export-jitter.md) - the video reorder bound follows later catalogs and observed reordering, so a late B-frame never reorders TS output +- [TS export jitter](/quest/m1/ts-export-jitter.md) - the video reorder bound follows later catalogs and the declared reorder depth, so a late B-frame never reorders TS output; an undeclared stream can reorder once per new maximum depth - [TS import shared shift](/quest/m1/ts-import-shared-shift.md) - unflagged loop wraps move audio and video by one shift, so A/V sync holds across wraps - [PipeWire duplicate cameras](/quest/m1/pipewire-dup-cameras.md) - a webcam lists once with PipeWire enabled - [Catalog wall clock](/quest/m1/catalog-wall-clock.md) - `Clock::wall_clock` keeps the catalog's full precision instead of truncating to milliseconds @@ -106,7 +106,6 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Own the QUIC stack](/quest/m1/quic/README.md) - the moq-noq fork carries ACK progress, reliable reset, hierarchical scheduling, deadlines, probing, keep-alive, peer limits, careful resume, ECN, and qmux -- [BBR idle burst](/quest/m1/bbr-idle-burst.md) - a BBRv3 burst after a long idle paces near the learned bandwidth, proven by a fork regression - [P2P](/quest/m1/p2p/README.md) - opted-in clients serve each other over data channels and iroh while the relay stays the rendezvous and the fallback, under application policy - [One port](/quest/m1/one-port/README.md) - a relay speaks QUIC, STUN, WebRTC media, and SRT on one UDP port and HTTP, RTMP, and RTMPS on one TCP port - [Signed priority](/quest/m1/signed-priority.md) - on dev, every API priority is an `i8` with 0 as the unset midpoint, and hang's built-ins sit above it diff --git a/quest/m1/bbr-idle-burst.md b/quest/m1/bbr-idle-burst.md deleted file mode 100644 index 480cbad830..0000000000 --- a/quest/m1/bbr-idle-burst.md +++ /dev/null @@ -1,31 +0,0 @@ -# [S] BBR idle burst - -## Goal - -After a long keep-alive-only idle, a BBRv3 (`delay`) sender paces its next -burst near the bandwidth it learned before, not at one packet per RTT. A -regression test proves it. - -## Plan - -- The likely fix already shipped: moq-dev/noq#5 tells the controller of - starvation before the next send, released in moq-noq 1.3.1 and pinned by - #4206. The reporter ran 1.3.0. Nobody has reproduced the stall on either. -- In the fork, add a virtual-time transport test: learn the bandwidth, run - keep-alive only for about five minutes, send 250 KB, and assert the pacing - rate stays at or above about 0.9x the earlier max bandwidth. It must fail on - 1.3.0 and pass on 1.3.1. The existing label tests do not check the rate. -- If 1.3.1 still stalls, find the remaining cause (a stale `bw_shortterm` or a - ProbeRTT effect after idle) and fix it in the fork. Dropping the estimate - after long idle is a policy change for the m2 study, not this quest. -- The `iroh` feature uses upstream noq, which lacks the fix; offering it there - belongs to the upstream quest. - -## Closes - -- [#4219](https://github.com/moq-dev/moq/issues/4219) - the first send after an idle period is paced at a trickle - -## Related - -- [Upstream the fork](/quest/m1/quic/upstream.md) - offer the starvation fix to n0-computer/noq -- [BBR3 app-limited](/quest/m2/quic-bbr-app-limited.md) - broader media measurements diff --git a/quest/m1/data-capture-bindings.md b/quest/m1/data-capture-bindings.md index 11f969227f..dcb3ecc3dd 100644 --- a/quest/m1/data-capture-bindings.md +++ b/quest/m1/data-capture-bindings.md @@ -5,7 +5,8 @@ A moq-ffi publisher, and every wrapper over it (Python, Swift, Kotlin, Go, Dart), can pass a capture time with a JSON or binary snapshot `update` or stream `append`, so its data tracks advertise `delay` and `jitter` like a Rust -publisher's. Leaving it out keeps today's behaviour. `moq-json`'s `window` +publisher's. Leaving it out keeps today's behaviour. The Go and Python +wrappers gain the binary snapshot and stream producers they lack. `moq-json`'s `window` producer takes a capture time too. Settled scope: moq-ffi and its wrappers, not libmoq. @@ -30,13 +31,19 @@ not libmoq. stream producers do. Nothing in `moq-mux` publishes window mode, so there is no estimator to feed. - Wrappers follow per the cross-package sync table, each with a test that a - past capture time is accepted and a future one refused. Update + past capture time is accepted and a future one refused. +- Go (`go/wrapper/moq`) and Python (`py/moq-rs`) wrap only the JSON + producers; [#4137](https://github.com/moq-dev/moq/pull/4137) added + `publish_binary_snapshot` / `publish_binary_stream` to moq-ffi without + them. Add hand-written binary wrappers there, capture time included, so + the capture tests cover binary too. The maintainer asked for this. Update `doc/lib/{py,swift,kt,go,dart}`. Public API: breaking, so it lands on `dev`. A new parameter on the generated `update` and `append` breaks every published binding caller (Go, for one, has no optional arguments), and a `_with_x` twin is ruled out. The broadcast clock -`now()` is additive; `window::Producer::push` accepts `Timed`, source-compatible. +`now()` and the Go and Python binary producers are additive; +`window::Producer::push` accepts `Timed`, source-compatible. Wire: none. ## Related diff --git a/quest/m1/go-mirror-delivery.md b/quest/m1/go-mirror-delivery.md index 4575713a8b..df439a1d7b 100644 --- a/quest/m1/go-mirror-delivery.md +++ b/quest/m1/go-mirror-delivery.md @@ -5,7 +5,7 @@ A recommendation, with a prototype, for delivering the Go binding's staticlibs without committing them to git. `moq-dev/moq-go-ffi` commits them straight into git today, at 60.2 MiB for linux, 52.7 for windows, and -39.1 for darwin. That puts the largest file at 60% of GitHub's 100 MB push +39.1 for darwin. That puts the largest file at 60% of GitHub's 100 MiB per-file limit, and each release adds about 210 MiB of history. ## Plan diff --git a/quest/m1/quic/bbr-app-limited.md b/quest/m1/quic/bbr-app-limited.md index dca67a99a8..17a9c42a7a 100644 --- a/quest/m1/quic/bbr-app-limited.md +++ b/quest/m1/quic/bbr-app-limited.md @@ -31,6 +31,26 @@ any controller event changes with the packet identity and ACK sampling fixes. Keep state private where possible; document any public Controller change and its consumers. No wire change is intended. Wire regressions into fork CI. +moq-dev/noq#5 added `Controller::on_app_limited` for this and shipped in +moq-noq 1.3.1; check what it leaves open before writing more code. + +The same defect is the likely cause of +[#4219](https://github.com/moq-dev/moq/issues/4219): after a long +keep-alive-only idle, a BBRv3 (`delay`) sender paced its next burst at one +packet per RTT instead of near the bandwidth it had learned. The reporter ran +1.3.0 and nobody has reproduced it on either version. Add a virtual-time +transport test: learn the bandwidth, idle on keep-alives for about five +minutes, send 250 KB, and assert the pacing rate stays at or above about 0.9x +the earlier max bandwidth. It should fail on 1.3.0. If it still stalls on the +fix, find the remaining cause (a stale `bw_shortterm` or a ProbeRTT effect +after idle). Dropping the estimate after a long idle is a policy change for +the m2 study, not this quest. The `iroh` feature uses upstream noq, which +lacks the fix; offering it there belongs to the upstream quest. + +## Closes + +- [#4219](https://github.com/moq-dev/moq/issues/4219) - the first send after an idle period is paced at a trickle + ## Related - [Finish each BBR ACK sample](/quest/m1/quic/bbr-ack-sampling.md) - a separate ordering defect in the same callback lifecycle diff --git a/quest/m1/raw-stream-codes.md b/quest/m1/raw-stream-codes.md index a4c1b517da..7338196d3e 100644 --- a/quest/m1/raw-stream-codes.md +++ b/quest/m1/raw-stream-codes.md @@ -23,12 +23,16 @@ on stream errors. WebTransport sessions keep the mapping they need. crates depend on crates.io releases, never a patch. A moq-tokio test over `moqt://` asserts a reset code arrives verbatim, beside `close_code.rs`. - Mixed versions: two moq peers on raw QUIC agree today because both map, and - wire changes must stay compatible with published versions. A fixed peer can - read both forms, since a mapped code lands in the HTTP/3 WebTransport range - that no application code reaches. An older peer misreads a fixed peer's raw - codes. Check which codes moq-net acts on (group stream resets, subscribe - STOP_SENDING): if any drives behaviour beyond reporting, this is a wire - break and retargets to `dev`. + wire changes must stay compatible with published versions. An older peer + still sends `error_to_http3(code)`, so a fixed peer that only skips the + mapping reports the large HTTP/3 value, not the code. The legacy form is + detectable, since it lands in the HTTP/3 WebTransport range that no moq + code reaches (`error_from_http3` returns `None` outside it), so the raw + receive path unmaps a code in that range and passes the rest through, with + a legacy-sender test per adapter. The other direction has no fix at the + receiver: an older peer misreads a fixed peer's raw codes. Check which codes + moq-net acts on (group stream resets, subscribe STOP_SENDING): if any drives + behaviour beyond reporting, this is a wire break and retargets to `dev`. Public API: none expected. Wire: raw QUIC stream error codes become the application's own values; compatible only if older peers merely report them. diff --git a/quest/m1/redirect-resolve.md b/quest/m1/redirect-resolve.md index 5c3f7e56d9..7ee9ef7014 100644 --- a/quest/m1/redirect-resolve.md +++ b/quest/m1/redirect-resolve.md @@ -19,8 +19,9 @@ the connection it documents. Recommendation: make it private. Nothing outside `moq-tokio` calls it (only its own unit tests), and the repo keeps things private until a consumer needs them. If a consumer turns up, the alternative is returning the same -`Result>` as the internal `target`, so empty and refused stay -distinct. Removing or changing a published method is a break, so this +`Result>` as the internal `target` does once the drain line lands +(on `main` it is still `Option`, folding a refusal into "keep the +current addresses"), so empty and refused stay distinct. Removing or changing a published method is a break, so this targets `dev`; update `doc/lib/rs` if it mentions the method. ## Required diff --git a/quest/m2/cat/present.md b/quest/m2/cat/present.md index 82d11a0334..8fcd518586 100644 --- a/quest/m2/cat/present.md +++ b/quest/m2/cat/present.md @@ -30,7 +30,7 @@ the option fails loud instead of dropping the credential. - `moq` CLI publish and subscribe get the flag through the shared connect config; `doc/bin/cli.md` and `doc/lib/rs/moq-net.md` gain it. - Tests: a Rust server's `Handshake::token()` sees kind `0x01` and the - bytes on every draft from both a Rust and a JS client (a JS server is out + bytes on every draft that carries the setup option, from both a Rust and a JS client (a JS server is out of scope: #4278 added no JS accept-side API, since nothing in `js/net` authorizes an IETF session); a JWT rides its AUTH stream and not the URL when a CAT holds the option; a lite offer with a CAT refuses at init; end to end against `moq auth serve` with a CAT from diff --git a/quest/m3/libmoq-cmake-lib.md b/quest/m3/libmoq-cmake-lib.md index 9a9887f2fe..ddd27b83d8 100644 --- a/quest/m3/libmoq-cmake-lib.md +++ b/quest/m3/libmoq-cmake-lib.md @@ -8,8 +8,8 @@ Today `BUILD_RUST_LIB` assumes `target//libmoq.a`, so any other layout links a stale library or fails to find one. ## Plan -Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. +Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. - `cmake/cargo-build.cmake` already parses cargo's JSON messages to find `moq.h` in its hashed `OUT_DIR`. Read the libmoq `compiler-artifact` @@ -24,6 +24,10 @@ Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-writ the build. Update `rs/libmoq/README.md` and `doc/lib/c` if they describe the path. +## Required + +- The maintainer keeps the hand-written libmoq instead of retiring it in the Generated C bindings line + ## Related - [libmoq shutdown](/quest/m3/libmoq-shutdown.md) - the other libmoq packaging fix OBS needs diff --git a/quest/m3/libmoq-fetch.md b/quest/m3/libmoq-fetch.md index a6c8430696..89ae92e986 100644 --- a/quest/m3/libmoq-fetch.md +++ b/quest/m3/libmoq-fetch.md @@ -6,8 +6,8 @@ A C embedder can fetch one cached group by sequence through the existing frame, handle, and terminal-status conventions. The new entry point is additive. ## Plan -Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. +Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. Mirror `MoqTrackConsumer::fetch_group` from `rs/moq-ffi/src/consumer.rs` in `rs/libmoq`, supporting raw and container-decoded delivery as the FFI does. @@ -18,3 +18,7 @@ Regenerate `moq.h` and update `doc/lib/c/index.md`, whose capability list alread claims group fetch. Decoder output configuration landed separately on the dev line. The `moq_group_request_*` tests in `rs/libmoq/src/test.rs` fetch from Rust for lack of this entry point; switch them to it. + +## Required + +- The maintainer keeps the hand-written libmoq instead of retiring it in the Generated C bindings line diff --git a/quest/m3/libmoq-hidden.md b/quest/m3/libmoq-hidden.md index 4095d694f6..87be221f7e 100644 --- a/quest/m3/libmoq-hidden.md +++ b/quest/m3/libmoq-hidden.md @@ -8,9 +8,13 @@ prefix) the way every other binding can: `moq_origin_announced` takes a caller can only list them by naming the dot segment in `prefix`. ## Plan -Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. +Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. - Adding a parameter breaks the C ABI, so this lands on `dev`. - Update `rs/libmoq/src/{api,origin}.rs`, the libmoq tests, `cpp/obs` if it calls `moq_origin_announced`, and `doc/lib/c/index.md`. + +## Required + +- The maintainer keeps the hand-written libmoq instead of retiring it in the Generated C bindings line diff --git a/quest/m3/libmoq-shutdown.md b/quest/m3/libmoq-shutdown.md index 8b4cc4870e..5fa707262b 100644 --- a/quest/m3/libmoq-shutdown.md +++ b/quest/m3/libmoq-shutdown.md @@ -10,8 +10,8 @@ libmoq statically, so the thread's code is unmapped under it. Any host that unloads the library has the same exposure. ## Plan -Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. +Parked in m3 behind [Generated C bindings](/quest/m1/c/README.md): the hand-written libmoq is being replaced by C generated from moq-ffi, which carries this for free. Do it only if the hand-written crate outlives that line. Starts on `main`; libmoq's runtime (`rs/libmoq/src/ffi.rs`) is the same on both branches and the change is additive. @@ -49,6 +49,10 @@ both branches and the change is additive. Public API: additive on the libmoq C ABI (`moq_shutdown`). Wire: none. +## Required + +- The maintainer keeps the hand-written libmoq instead of retiring it in the Generated C bindings line + ## Related - [Kotlin JVM exit](/quest/m1/kt-jvm-exit.md) - the same hazard class for the moq-ffi bindings From ffaa97f921bf794746590f076ce708c9da3dbfea Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 10:16:37 -0700 Subject: [PATCH 4/7] docs(quest): plan follow-ups and fix rs2ts and wildcard gaps New quests: a stats producer fan-out benchmark, draining queued stream data before a client closes, and interop matrices that run side by side. The rs2ts line keeps nested Option states apart, gates Generated lite on its own async-feature quest, and requires the browser benchmarks. The wildcard resolve quest must carry specificity across relay hops. Co-Authored-By: Claude Opus 5.5 --- quest/m0/wildcard/resolve.md | 15 ++++++++++ quest/m1/README.md | 3 ++ quest/m1/drain-before-close.md | 40 +++++++++++++++++++++++++ quest/m1/interop-contention.md | 29 ++++++++++++++++++ quest/m1/rs2ts/README.md | 5 +++- quest/m1/rs2ts/lite.md | 1 + quest/m1/rs2ts/sans-io/README.md | 4 +-- quest/m1/rs2ts/sans-io/async-feature.md | 27 +++++++++++++++++ quest/m1/rs2ts/sans-io/ietf.md | 2 ++ quest/m1/rs2ts/translator.md | 8 ++++- quest/m1/stats-producer-bench.md | 33 ++++++++++++++++++++ 11 files changed, 163 insertions(+), 4 deletions(-) create mode 100644 quest/m1/drain-before-close.md create mode 100644 quest/m1/interop-contention.md create mode 100644 quest/m1/rs2ts/sans-io/async-feature.md create mode 100644 quest/m1/stats-producer-bench.md diff --git a/quest/m0/wildcard/resolve.md b/quest/m0/wildcard/resolve.md index c582df54ad..69eb10d343 100644 --- a/quest/m0/wildcard/resolve.md +++ b/quest/m0/wildcard/resolve.md @@ -40,6 +40,19 @@ it. A concrete announcement is maximally specific and shadows every wildcard regardless of cost. A terminal refusal from that concrete claim never falls through to a wildcard; it shadows until the claim is withdrawn. +Specificity must survive relay hops. Nothing on the wire spells a +pattern, so past the first relay `**/transcode.pro` and `**` are both the +root prefix, and `Announcing::announce` (`rs/moq-net/src/model/origin.rs`) +attaches only the receiving cluster session's scope. A relay two hops from +the suffix worker therefore cannot tell its pool from the archive's +catch-all, and may send a transcode request to the archive (Codex on +[#4213](https://github.com/moq-dev/moq/pull/4213)). Carry each route's +specificity across cluster hops, or narrow the promise to what one relay can +see; decide with the maintainer before building. Recommendation: carry a +specificity rank beside the route cost rather than the pattern itself, which +keeps patterns off the wire per #3770. Either way it is a wire change to the +lite draft. + Both lookup kinds route through this table: subscribe via `recv_subscribe`'s existing fallback, and FETCH the same way, since the archive's whole use case is serving stored groups to FETCH for paths nothing announces. A FETCH selects @@ -100,6 +113,8 @@ Tests, at the process level with real sessions rather than an in-process stand-i - A path matched by both a suffix pattern and the catch-all resolves against the suffix pattern's pool only, and a terminal refusal from it never reaches the catch-all advertiser. +- The same holds when the suffix worker and the catch-all archive connect to + different relays and the request arrives at a third. - A refused subscribe resets rather than hanging, and leaves no state behind. - A wildcard retracted mid-serve does not disturb the subscription already running. diff --git a/quest/m1/README.md b/quest/m1/README.md index a236b05205..1ab2ce6b14 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -32,6 +32,7 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Remove finish](/quest/m1/broadcast-remove.md) - on dev, the deprecated broadcast end APIs are gone and `closed()` carries no cause - [CLI inspection](/quest/m1/cli-inspect/README.md) - `moq ls` lists what is live and `moq fetch` reads a group over MoQ, and a guide shows how to inspect a relay - [Session close](/quest/m1/session-close.md) - a graceful session end withdraws announces and waits one second for the ack +- [Drain before close](/quest/m1/drain-before-close.md) - a closing client delivers its queued stream finishes, so `moq import` ends the catalog cleanly over a real relay - [Close codes](/quest/m1/close-codes.md) - a client sees the peer's application close code over WebSocket and raw QUIC, like WebTransport - [Raw stream codes](/quest/m1/raw-stream-codes.md) - raw QUIC stream resets and stops carry the application's code, not an HTTP/3-mapped one - [JS caught up](/quest/m1/js-announce-caught-up.md) - @moq/net's announce consumer says when the initial set has landed, like Rust @@ -50,6 +51,7 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [moq play decode schedule](/quest/m1/play-decode-schedule.md) - `moq play` video keeps valid pictures across rewinds, reordering deeper than 100 ms, and decoder batches larger than three - [moqsrc stop](/quest/m1/moqsrc-stop.md) - moqsrc's stop blocks until its session ends, without deadlocking on a blocked pad push - [More tests under load](/quest/m1/test-flakes-2.md) - the second round of load-only failures, fixed at the cause +- [Interop contention](/quest/m1/interop-contention.md) - two `just test interop --all` matrices pass side by side, and `just test harness` runs from a clean checkout - [UnknownSession log flood](/quest/m1/unknown-session-logs.md) - streams reset before their WebTransport header stop being reported as UnknownSession at WARN - [Merge queue](/quest/m1/merge-queue.md) - the required checks run on `merge_group`, so a stale green check can no longer break main - [Wire compatibility](/quest/m1/wire-compat.md) - a nightly run tests this checkout against the last published release for tokens, session wire, and catalog/container @@ -121,6 +123,7 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [#3126](/quest/m1/3126-moq-bench-every-readme-example-fails-to-parse-and.md) - moq-bench reports per-interval latency percentiles so the ramp leaves the steady state - [Relay session bench](/quest/m1/bench-relay.md) - the same scenario through moq-relay's own connection handling - [Bench coverage](/quest/m1/bench-coverage.md) - Criterion targets for moq-mux containers, the hang catalog, moq-auth verification, and moq-pattern matching +- [Stats producer bench](/quest/m1/stats-producer-bench.md) - the stats drain and encode cost per tick, swept over held paths and tiers and run nightly - [Relay profiling](/quest/m1/performance-profiles.md) - reproducible CPU and allocation captures under the existing workloads - [Browser benchmarks](/quest/m1/browser-benchmarks.md) - measure JS transport, container, decode, and render costs in an identified browser - [Generated @moq/net](/quest/m1/rs2ts/README.md) - the browser runs moq-net as TypeScript generated from the Rust source, retiring js/net's hand-written protocol and model code diff --git a/quest/m1/drain-before-close.md b/quest/m1/drain-before-close.md new file mode 100644 index 0000000000..f24ff82b57 --- /dev/null +++ b/quest/m1/drain-before-close.md @@ -0,0 +1,40 @@ +# [M] Deliver queued stream data before a client closes + +## Goal + +A process that finishes its tracks and then closes its `moq_tokio::Client` +delivers what it already queued, including each stream's FIN, before the +connection closes, bounded by a deadline. `moq import` at stdin EOF is the +consumer: a subscriber over a real relay sees the catalog finish instead of +`Error::Dropped`. + +## Plan + +- [#4303](https://github.com/moq-dev/moq/pull/4303) made `moq import` finish + the catalog at EOF, but only in-process: over a real session the process + exits and the close discards the queued finish. + [#4287](https://github.com/moq-dev/moq/pull/4287) added `Client::close`, + which sends the CONNECTION_CLOSE but does not wait for stream data. +- A QUIC close discards unacknowledged stream data, so the session has to + wait until its open send streams are written, finished, and acknowledged, + not only handed to the transport. The pending group and control writes live + in moq-net's session tasks; the transport wait lives in moq-tokio. +- Bound the wait so a stalled peer cannot hang an exiting process. Share the + deadline and the graceful path with + [Session close](/quest/m1/session-close.md), which waits for announce + acknowledgements the same way; `abort` and drop stay immediate. +- Cover every backend `Client::close` covers, and say which do not drain + (WebSocket and iroh end on drop today). +- Regression tests: a moq-tokio test, shaped like + `noq_client_close_reaches_server`, where the client finishes a track and + closes on a runtime dropped right after, and the server's subscriber reads + the finish rather than a drop. Then a CLI test piping a file through + `moq import` to a relay. + +Public API: likely additive (`Client::close` gains the drain, or a graceful +close sits beside `abort`). Wire: none. + +## Related + +- [Session close](/quest/m1/session-close.md) - the graceful end that withdraws announces +- [Graceful relay drains](/quest/m1/drain/README.md) - the server-side drain over GOAWAY diff --git a/quest/m1/interop-contention.md b/quest/m1/interop-contention.md new file mode 100644 index 0000000000..f1bb5fe6e3 --- /dev/null +++ b/quest/m1/interop-contention.md @@ -0,0 +1,29 @@ +# [S] Interop matrices run side by side + +## Goal + +Two `just test interop --all` matrices can run at once on one machine, even +from one checkout, and both pass. `just test harness` runs from a clean +checkout. + +## Plan + +[#4228](https://github.com/moq-dev/moq/pull/4228) fixed the port +reservations and the canvas-blocked pause click, and stages Go per run. What +remains: + +- Under two concurrent matrices, `python -> js` and `go -> js` sometimes stall + the browser subscriber: the Pause control never appears within 30 s. It + was not seen in a single run. Find the cause (CPU starvation of headless + Chromium, a shared resource, or a real stall) before touching timeouts. +- `prepare_python` runs `just py build` in the workspace, so concurrent runs + rewrite the same `.venv` and maturin output (Codex on #4228). Build into the + run directory, as Go now does, or serialize the build. +- `just test harness` runs `harness.browser.ts` without installing workspace + dependencies or Playwright Chromium, so it only passes where CI's earlier + step prepared them. Provision them in the recipe, or reuse the path + `interop.sh` already takes. +- Prove it by running two `--all` matrices at once, several times, from one + checkout. + +Public API: none. Wire: none. diff --git a/quest/m1/rs2ts/README.md b/quest/m1/rs2ts/README.md index 726f33ff0f..855723abd1 100644 --- a/quest/m1/rs2ts/README.md +++ b/quest/m1/rs2ts/README.md @@ -69,7 +69,10 @@ js/net it replaces, measured with the [browser benchmarks](/quest/m1/browser-ben - [#2822](https://github.com/moq-dev/moq/issues/2822) - close this issue when the quest finishes - [#2835](https://github.com/moq-dev/moq/issues/2835) - close this issue when the quest finishes +## Required + +- [Browser benchmarks](/quest/m1/browser-benchmarks.md) - the harness the no-downgrade report uses + ## Related - [#2850](/quest/m1/2850-js-net-give-reader-a-synchronous-decode-so-the-publisher.md) - the same synchronous decode shape, in hand-written js/net today -- [Browser benchmarks](/quest/m1/browser-benchmarks.md) - the harness the no-downgrade report uses diff --git a/quest/m1/rs2ts/lite.md b/quest/m1/rs2ts/lite.md index 6cade8266c..256dd08783 100644 --- a/quest/m1/rs2ts/lite.md +++ b/quest/m1/rs2ts/lite.md @@ -28,4 +28,5 @@ Public API: breaks `@moq/net`; retargets to `dev`. Wire: none. - [rs2ts](/quest/m1/rs2ts/translator.md) - the translator - [Sans-IO lite session](/quest/m1/rs2ts/sans-io/lite.md) - the session shape it translates - [Sans-IO model](/quest/m1/rs2ts/sans-io/model.md) - the model shape it translates +- [The async feature](/quest/m1/rs2ts/sans-io/async-feature.md) - rs2ts reads moq-net without it - [Mock-clock tests](/quest/m1/rs2ts/mock-clock.md) - the tests that prove parity diff --git a/quest/m1/rs2ts/sans-io/README.md b/quest/m1/rs2ts/sans-io/README.md index 223b68d8af..47a048c76a 100644 --- a/quest/m1/rs2ts/sans-io/README.md +++ b/quest/m1/rs2ts/sans-io/README.md @@ -13,11 +13,11 @@ reimplements the async helpers natively with Promises, so the translator reads the crate without the `async` feature. Split by layer so each lands on `dev` independently. -This README's own work is the `async` feature and the CI lane that builds -moq-net without it. +The line has no work of its own beyond its children. ## Quests - [Sans-IO lite session](/quest/m1/rs2ts/sans-io/lite.md) - the lite session is driven by bytes, stream events, and `tick(now)` - [Sans-IO model](/quest/m1/rs2ts/sans-io/model.md) - origin, broadcast, track, and group handles run without a runtime, with time supplied by the caller +- [The async feature](/quest/m1/rs2ts/sans-io/async-feature.md) - the async helpers sit behind an `async` feature and a CI lane builds and tests moq-net without it - [Sans-IO IETF session](/quest/m1/rs2ts/sans-io/ietf.md) - the moq-transport session is driven the same way as lite diff --git a/quest/m1/rs2ts/sans-io/async-feature.md b/quest/m1/rs2ts/sans-io/async-feature.md new file mode 100644 index 0000000000..f7b7e35046 --- /dev/null +++ b/quest/m1/rs2ts/sans-io/async-feature.md @@ -0,0 +1,27 @@ +# [S] moq-net's async helpers sit behind a feature + +## Goal + +moq-net's async helper methods sit behind an `async` cargo feature, and a CI +lane builds and tests the crate without it, so rs2ts reads the no-runtime +crate that [Generated lite](/quest/m1/rs2ts/lite.md) translates. + +## Plan + +- The feature is on by default, so Rust callers see no change. JS + reimplements the helpers with Promises over the poll API. +- Generated lite needs the lite session and the model without the feature, + not IETF. Until the [Sans-IO IETF session](/quest/m1/rs2ts/sans-io/ietf.md) + lands, the IETF session can sit behind the feature too; that quest then + moves it out. +- The lane runs at least the tests that do not exercise the helpers; tests + that do stay behind the feature. + +Public API: moq-net's async helpers move behind a default feature, so a +`default-features = false` caller loses them; lands on `dev` with the line. +Wire: none. + +## Required + +- [Sans-IO lite session](/quest/m1/rs2ts/sans-io/lite.md) - the session builds without a runtime +- [Sans-IO model](/quest/m1/rs2ts/sans-io/model.md) - the model builds without a runtime diff --git a/quest/m1/rs2ts/sans-io/ietf.md b/quest/m1/rs2ts/sans-io/ietf.md index df8fad3ab9..542c1965a1 100644 --- a/quest/m1/rs2ts/sans-io/ietf.md +++ b/quest/m1/rs2ts/sans-io/ietf.md @@ -11,6 +11,8 @@ Follow whatever shape the lite session settles on. The IETF code is the largest module (about 12.7k non-test lines) and today compiles part of itself twice (for `Session` and `ControlStreamAdapter`); collapse that while here. +If the [async feature](/quest/m1/rs2ts/sans-io/async-feature.md) landed +first with the IETF session behind it, move the session out. Public API: breaks moq-net's IETF session API; retargets to `dev`. Wire: none. diff --git a/quest/m1/rs2ts/translator.md b/quest/m1/rs2ts/translator.md index 1eecf983f1..8459752e54 100644 --- a/quest/m1/rs2ts/translator.md +++ b/quest/m1/rs2ts/translator.md @@ -19,6 +19,12 @@ Mapping decided in planning: - Structs become classes, enums discriminated unions, traits interfaces. `Option` is `T | undefined`, `&[u8]` a `Uint8Array` view with no copy. + That collapses a nested `Option`, whose states the source relies on: + `model/track.rs::first_start` returns `Option>` to tell + "no successor" from an unstamped one, and `reach` behaves differently for + each. Recommendation: the subset lint rejects nested `Option`, and the source + names those states with an enum; a tagged TypeScript form for the inner + `Option` is the alternative. Either way nested states never merge silently. - `Drop` becomes an explicit `drop()` at each MIR drop point, exposed as `[Symbol.dispose]`; `Arc`/`Rc` of a type with drop glue become an explicit refcount. JS is single-threaded, so `Mutex` and atomics become plain @@ -37,7 +43,7 @@ Guidance: - Readability pass: inline single-use temporaries and keep source branch order, so a reviewer can read a generated diff. - Add a lint on moq-net (clippy or dylint) for the accepted subset: no - `unsafe`, no `async` outside the `async` feature, no `u64` bit operations, + `unsafe`, no `async` outside the `async` feature, no nested `Option`, no `u64` bit operations, no trait impls on foreign or primitive types, no `&mut` out-params to scalar or `Option` locals. Translator gaps become compile errors, not runtime `todo()`s. Document the subset in `rs/rs2ts/README.md`. diff --git a/quest/m1/stats-producer-bench.md b/quest/m1/stats-producer-bench.md new file mode 100644 index 0000000000..869404f06b --- /dev/null +++ b/quest/m1/stats-producer-bench.md @@ -0,0 +1,33 @@ +# [S] Stats producer fan-out benchmark + +## Goal + +A `moq-stats` benchmark measures what one relay pays per stats tick to drain +its registry and encode the traffic tracks, swept over held paths and tiers, +so a cost that grows with the whole table instead of the paths that changed +shows up as a slope. It runs at least nightly. + +## Plan + +- [#4299](https://github.com/moq-dev/moq/pull/4299) keeps an idle path in + every frame while the registry holds its counters, adding about 430 B per + idle path to each plain frame (maintainer's note on the PR). Nothing + measures what that, or the per-tick drain, costs as held paths grow. + `rs/moq-stats/benches/decode.rs` covers only the reader. +- Sweep held paths (for example 100 to 50k) against tiers, and the share of + paths that are idle versus changed this tick. Report time and allocations + per tick, and plain and compressed bytes per frame, the way `decode.rs` + reports its table. Include the point where a plain frame nears its size + cap, since a frame that is too large leaves stale counters behind. +- Drive the real producer path (`process_slot`, the snapshot encoders) over + a synthetic `Registry`, not a re-implementation of it. Exposing a bench + hook is fine if it stays out of the public API. +- `decode.rs` is not in CI either. Run both targets once in the nightly + benchmark smoke, as `moq-net`'s are. + +Public API: none. Wire: none. + +## Related + +- [Binary delta stats flavor](/quest/m2/stats-delta.md) - its gate needs the encode baseline this measures +- [Benchmark regressions in CI](/quest/m1/bench-ci.md) - compares these targets across PRs once it lands From 0af55e3b70b271283b63d1d7d2561b760652cef5 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 10:37:07 -0700 Subject: [PATCH 5/7] docs(quest): drop the cross-hop specificity requirement Suffix-based routing is no longer planned, so the wildcard resolve quest should not require carrying specificity across relay hops. Co-Authored-By: Claude Opus 5.5 --- quest/m0/wildcard/resolve.md | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/quest/m0/wildcard/resolve.md b/quest/m0/wildcard/resolve.md index 69eb10d343..c582df54ad 100644 --- a/quest/m0/wildcard/resolve.md +++ b/quest/m0/wildcard/resolve.md @@ -40,19 +40,6 @@ it. A concrete announcement is maximally specific and shadows every wildcard regardless of cost. A terminal refusal from that concrete claim never falls through to a wildcard; it shadows until the claim is withdrawn. -Specificity must survive relay hops. Nothing on the wire spells a -pattern, so past the first relay `**/transcode.pro` and `**` are both the -root prefix, and `Announcing::announce` (`rs/moq-net/src/model/origin.rs`) -attaches only the receiving cluster session's scope. A relay two hops from -the suffix worker therefore cannot tell its pool from the archive's -catch-all, and may send a transcode request to the archive (Codex on -[#4213](https://github.com/moq-dev/moq/pull/4213)). Carry each route's -specificity across cluster hops, or narrow the promise to what one relay can -see; decide with the maintainer before building. Recommendation: carry a -specificity rank beside the route cost rather than the pattern itself, which -keeps patterns off the wire per #3770. Either way it is a wire change to the -lite draft. - Both lookup kinds route through this table: subscribe via `recv_subscribe`'s existing fallback, and FETCH the same way, since the archive's whole use case is serving stored groups to FETCH for paths nothing announces. A FETCH selects @@ -113,8 +100,6 @@ Tests, at the process level with real sessions rather than an in-process stand-i - A path matched by both a suffix pattern and the catch-all resolves against the suffix pattern's pool only, and a terminal refusal from it never reaches the catch-all advertiser. -- The same holds when the suffix worker and the catch-all archive connect to - different relays and the request arrives at a third. - A refused subscribe resets rather than hanging, and leaves no state behind. - A wildcard retracted mid-serve does not disturb the subscription already running. From 00e27e8be3a5a8c808ba246d27fad83eafa33ee4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 11:09:41 -0700 Subject: [PATCH 6/7] docs(quest): trim BBR app-limited to the long-idle regression moq-dev/noq#5 shipped the starvation marker and its label tests in moq-noq 1.3.1. What remains is the #4219 long-idle regression, which exists in neither repo. Co-Authored-By: Claude Opus 5.5 --- quest/m1/quic/README.md | 2 +- quest/m1/quic/bbr-app-limited.md | 72 ++++++++++++-------------------- quest/m1/quic/bbr-release.md | 2 +- 3 files changed, 29 insertions(+), 47 deletions(-) diff --git a/quest/m1/quic/README.md b/quest/m1/quic/README.md index f3234590a4..2752d71990 100644 --- a/quest/m1/quic/README.md +++ b/quest/m1/quic/README.md @@ -53,7 +53,7 @@ This is a transport API change, not a MoQ wire change. - [Preserve QUIC packet identity in BBR](/quest/m1/quic/bbr-packet-identity.md) - ACKs and losses identify the right packet across QUIC spaces - [Finish each BBR ACK sample before using it](/quest/m1/quic/bbr-ack-sampling.md) - current delivery samples reach the model once with consistent metadata -- [Mark application starvation before the next BBR send](/quest/m1/quic/bbr-app-limited.md) - resumed bursts retain correct sample labels +- [BBR idle burst](/quest/m1/quic/bbr-app-limited.md) - a fork regression proves a burst after a long idle is paced at the learned bandwidth, closing #4219 - [Finish BBR bandwidth-probe feedback once](/quest/m1/quic/bbr-probe-feedback.md) - cruise rounds neither age probe history repeatedly nor retain probe-loss classification - [Recalibrate BBR startup pacing from measured RTT](/quest/m1/quic/bbr-startup-pacing.md) - measured RTT replaces the nominal startup rate for media senders - [Protect bandwidth samples during BBR ProbeRTT](/quest/m1/quic/bbr-probe-rtt.md) - intentionally reduced sending cannot masquerade as reduced capacity diff --git a/quest/m1/quic/bbr-app-limited.md b/quest/m1/quic/bbr-app-limited.md index 17a9c42a7a..ea26b55a4a 100644 --- a/quest/m1/quic/bbr-app-limited.md +++ b/quest/m1/quic/bbr-app-limited.md @@ -1,51 +1,34 @@ -# [M] Mark application starvation before the next BBR send +# [S] Prove a BBR burst after a long idle is paced at the learned bandwidth ## Goal -Packets sent after application starvation carry the correct historical -application-limited label, even when no ACK arrives during the idle gap. -The bandwidth model cannot mistake a source-limited sample for capacity. +After minutes of keep-alive-only idle, a BBRv3 (`delay`) sender paces its +next burst near the bandwidth it learned before, not at a trickle, and a +regression test in the fork proves it. ## Plan -In noq `1a26a8b064d21e316fe6769f068617975bd8a27b`, an empty unblocked -[transmit poll](https://github.com/n0-computer/noq/blob/1a26a8b064d21e316fe6769f068617975bd8a27b/noq-proto/src/connection/mod.rs#L1337) records starvation, but BBR receives the marker only in -[on_end_acks](https://github.com/n0-computer/noq/blob/1a26a8b064d21e316fe6769f068617975bd8a27b/noq-proto/src/congestion/bbr3/mod.rs#L1732). -A resumed send can be stamped before that notification. Google's -[QUICHE BBR3](https://github.com/google/quiche/blob/535a2730e77d47e0dc03746555cc9c34b17bc9e9/quiche/quic/core/congestion_control/bbr3_sender.cc#L505) notifies its sampler immediately when application limited. - -Reproduce through the transport boundary: send and ACK one 1200-byte packet -with a 10-ms RTT, run an empty unblocked transmit poll, wait until 30 ms to -send another packet, then ACK it 10 ms later. There is no intervening ACK -to notify the controller of starvation. The existing public-callback -reproduction yields a non-limited sample; preserve a failing regression -without privately seeding the sampler marker. This establishes a label bug, -not a measured throughput regression. - -Communicate starvation before subsequent sends, preserving the delivery -boundary that ends the sampler's limited phase. Distinguish producer -starvation from cwnd, pacing, anti-amplification, receiver credit, and local -buffer limits; do not silently change receiver-limited policy. Cover streams, -datagrams, resumed backlog, repeated empty polls, and ACK batching. Coordinate -any controller event changes with the packet identity and ACK sampling fixes. -Keep state private where possible; document any public Controller change and -its consumers. No wire change is intended. Wire regressions into fork CI. - -moq-dev/noq#5 added `Controller::on_app_limited` for this and shipped in -moq-noq 1.3.1; check what it leaves open before writing more code. - -The same defect is the likely cause of -[#4219](https://github.com/moq-dev/moq/issues/4219): after a long -keep-alive-only idle, a BBRv3 (`delay`) sender paced its next burst at one -packet per RTT instead of near the bandwidth it had learned. The reporter ran -1.3.0 and nobody has reproduced it on either version. Add a virtual-time -transport test: learn the bandwidth, idle on keep-alives for about five -minutes, send 250 KB, and assert the pacing rate stays at or above about 0.9x -the earlier max bandwidth. It should fail on 1.3.0. If it still stalls on the -fix, find the remaining cause (a stale `bw_shortterm` or a ProbeRTT effect -after idle). Dropping the estimate after a long idle is a policy change for -the m2 study, not this quest. The `iroh` feature uses upstream noq, which -lacks the fix; offering it there belongs to the upstream quest. +moq-dev/noq#5 (in moq-noq 1.3.1; main pins 1.3.2) fixed the label bug this +quest was opened for. The transport calls `Controller::on_app_limited` on +every empty poll that nothing held back, and BBR marks starvation before the +next send. Its tests cover streams, datagrams, batched ACKs, and backlogs +held by the window or the pacer. What is left is +[#4219](https://github.com/moq-dev/moq/issues/4219), reported on 1.3.0 and +not yet reproduced on either version: the first 250 KB after five idle +minutes took 5.2 s at 50 ms RTT, against 0.5 s with CUBIC. + +The fix plausibly covers it. The fork's max-bw filter ages only on +non-app-limited samples, and before #5 every other keep-alive was stamped +non-app-limited. Nothing measures it, though. Add a virtual-time transport +test in the fork: learn the bandwidth, idle on keep-alives for about five +minutes, send 250 KB, and assert the pacing rate stays at or above about +0.9x the earlier max bandwidth. Check that it fails on 1.3.0. + +If it still stalls, find the remaining cause, such as ProbeRTT entered during +the idle or a stale `bw_shortterm`, and fix it in the fork. Dropping the +estimate after a long idle is a policy change for the m2 study, not this +quest. The `iroh` feature uses upstream noq, which lacks #5; offering it +there belongs to the upstream quest. ## Closes @@ -53,6 +36,5 @@ lacks the fix; offering it there belongs to the upstream quest. ## Related -- [Finish each BBR ACK sample](/quest/m1/quic/bbr-ack-sampling.md) - a separate ordering defect in the same callback lifecycle -- [Release BBR fixes](/quest/m1/quic/bbr-release.md) - deliver correct labels before policy experiments -- [Upstream the fork](/quest/m1/quic/upstream.md) - offer the general fix upstream +- [Release BBR fixes](/quest/m1/quic/bbr-release.md) - ships any remaining fix +- [Upstream the fork](/quest/m1/quic/upstream.md) - offer the starvation fix upstream diff --git a/quest/m1/quic/bbr-release.md b/quest/m1/quic/bbr-release.md index 8966e3ae3d..41365f1707 100644 --- a/quest/m1/quic/bbr-release.md +++ b/quest/m1/quic/bbr-release.md @@ -24,7 +24,7 @@ and Google comparison do not gate these bug fixes. - [Preserve QUIC packet identity in BBR](/quest/m1/quic/bbr-packet-identity.md) - [Finish each BBR ACK sample before using it](/quest/m1/quic/bbr-ack-sampling.md) -- [Mark application starvation before the next BBR send](/quest/m1/quic/bbr-app-limited.md) +- [BBR idle burst](/quest/m1/quic/bbr-app-limited.md) - the starvation fix shipped in moq-noq 1.3.1; any remaining idle cause ships here - [Finish BBR bandwidth-probe feedback once](/quest/m1/quic/bbr-probe-feedback.md) - [Recalibrate BBR startup pacing from measured RTT](/quest/m1/quic/bbr-startup-pacing.md) - [Protect bandwidth samples during BBR ProbeRTT](/quest/m1/quic/bbr-probe-rtt.md) From 2863f347dac887112effbc1fcee42f027160d33f Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 28 Sep 2026 11:51:56 -0700 Subject: [PATCH 7/7] fix(net): cap the whole buffered decode, not each read Compare the cursor's cumulative offset against MAX_READ_SIZE, as the fill path does, so a buffered message can't pass the cap one field at a time. Name publish_video_on_track in the py keyframe warning too. Co-Authored-By: Claude Opus 5.5 --- doc/lib/py/index.md | 2 +- js/net/src/stream.test.ts | 6 ++++++ js/net/src/stream.ts | 5 +++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/doc/lib/py/index.md b/doc/lib/py/index.md index cbbc824a0f..609f957aee 100644 --- a/doc/lib/py/index.md +++ b/doc/lib/py/index.md @@ -71,7 +71,7 @@ asyncio.run(main()) For already-encoded live output, call `audio.flush(timestamp_us)` after each `audio.write_frame` with the same broadcast-clock PTS. It samples the transport handoff for catalog jitter. File, pipe, and network imports should omit `flush`; raw-pixel and PCM encoders inside the binding measure their own output. -Call `audio.discontinuity()` when the source seeks, pauses, or changes its time base. It publishes a timeline marker and restarts handoff measurement without lowering advertised jitter. Resume with timestamps that continue forward on the broadcast media clock; this does not permit timestamp rewinds. On a track from `publish_video`, resume with a keyframe: a delta frame before it fails. +Call `audio.discontinuity()` when the source seeks, pauses, or changes its time base. It publishes a timeline marker and restarts handoff measurement without lowering advertised jitter. Resume with timestamps that continue forward on the broadcast media clock; this does not permit timestamp rewinds. On a track from `publish_video` or `publish_video_on_track`, resume with a keyframe: a delta frame before it fails. The three advertising operations, as the other bindings spell them: `client.create_broadcast(path)` (or `OriginProducer.create_broadcast`) returns diff --git a/js/net/src/stream.test.ts b/js/net/src/stream.test.ts index 38422aba52..8ecdede0ca 100644 --- a/js/net/src/stream.test.ts +++ b/js/net/src/stream.test.ts @@ -452,6 +452,12 @@ test("Reader refuses an oversized value even when it is already buffered", async await expect(new Reader(undefined, buffer.subarray(4)).read(size)).rejects.toThrow("exceeds max size"); }); +test("Reader refuses a buffered decode whose fields together exceed the max size", async () => { + const half = 32 * 1024 * 1024; + const reader = new Reader(undefined, new Uint8Array(2 * half + 1)); + await expect(reader.decode((c) => [c.read(half), c.read(half + 1)])).rejects.toThrow("exceeds max size"); +}); + /** A stream reset as a transport delivers one: the peer's code, and nothing else useful. */ class Reset extends Error { readonly source = "stream" as const; diff --git a/js/net/src/stream.ts b/js/net/src/stream.ts index 67f7390c33..8093e4495c 100644 --- a/js/net/src/stream.ts +++ b/js/net/src/stream.ts @@ -446,13 +446,14 @@ export class Cursor { #ensure(size: number) { const need = this.#offset + size; + // Checked here too, and on the whole decode like the fill, since bytes that are already + // buffered never reach the fill. + if (need > MAX_READ_SIZE) throw new Error(`read size ${need} exceeds max size ${MAX_READ_SIZE}`); if (need > this.#buffer.byteLength) throw new Short(need); } /** Read `size` bytes, as a view onto the buffer rather than a copy. */ read(size: number): Uint8Array { - // Checked here too, since a value that is already buffered never reaches the fill. - if (size > MAX_READ_SIZE) throw new Error(`read size ${size} exceeds max size ${MAX_READ_SIZE}`); this.#ensure(size); const start = this.#offset; this.#offset += size;