fix(flate): refuse a stream payload past the group budget without ending the log

[kixelated]

Problem

moq-flate / @moq/flate stream mode rides one group, like JSON's, but had no budget check: a payload that overflowed the group (or, with deflate, exceeded the decoder's 64 MiB cap) aborted the whole track. #4911 fixed this for JSON streams only.

Approach

• Both producers check frames >= MAX_GROUP_FRAMES || bytes + bound > MAX_CACHE_BYTES before opening the group or touching the DEFLATE window, where bound is the raw size, or DEFLATE's worst case when compressing. A refusal throws/returns GroupTooLarge and leaves the log readable and writable. A write that fails after encoding still aborts the track.
• A track that has already ended is checked before the budget, so an append on an ended log reports the error that ended it rather than GroupTooLarge, which would imply the log is still writable.
• The separate decoder-size check is removed. The 32 MiB group budget is below the 64 MiB decoder cap, so the budget check already refuses those payloads, with GroupTooLarge. A Rust const assert and a JS test pin that ordering.
• The DEFLATE bound moves from json into flate as Encoder::bound(len) (Rust associated const fn, JS static), and json's stream encoder reuses it. Its test moves with it.
• moq-mux's binary Stream keeps its catalog entry on GroupTooLarge, mirroring fix(json): refuse a stream record past the group budget without ending the log #4911's JSON wrapper.
• Tests in both languages and both compression modes: oversized and over-decoder-cap payloads are refused and the log continues, a refused first payload publishes nothing, an ended log refuses with its own error, a spent frame budget refuses every append, and a failed write still reaches a reader already inside the group.
• Module docs and doc/lib/{rs/moq-flate,js/flate}.md document the budget.

Impact

• New public API: moq_flate::Encoder::bound(len: u64) -> u64 and Encoder.bound(len: number): number in @moq/flate.
• Behavior change: a deflate stream payload over 64 MiB used to end the track with Error::TooLarge (Rust) or a plain Error (JS). It is now refused with GroupTooLarge and the track stays open. Any payload past the group budget is refused the same way, in both modes.
• Removed (JS, unexported): deflateBound from js/json/src/stream/encoder.ts. @moq/json now needs the @moq/flate release that adds Encoder.bound.
• Wire: none.

Alternatives

• Keep the explicit decoder-size check before the budget check, so a deflate payload over 64 MiB still reports TooLarge. Rejected: the group budget is the limit that actually applies, and fix(json): refuse a stream record past the group budget without ending the log #4911 already removed the same check from JSON streams.
• Expose the bound as a free function (deflate_bound / deflateBound) instead of a method on Encoder. The method keeps it under the type whose output it bounds.

Follow-ups

None from this change.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Quest outcome (paper trail): implemented as planned in both languages. Open decisions for the maintainer: (1) bound naming, Encoder::bound / Encoder.bound (recommended) vs a free deflate_bound / deflateBound; (2) a deflate payload over the 64 MiB decoder cap now reports GroupTooLarge, because the 32 MiB group budget subsumes it, instead of keeping a separate TooLarge check (recommended: keep as is, matching #4911). Left as a draft for these. (Written by Claude Opus 5.5)

[kixelated]

Maintainer decisions, settled as recommended:

1. The DEFLATE bound is Encoder::bound (Rust) / Encoder.bound (JS), not a free function.
2. A deflate stream payload over 64 MiB is refused by the group budget with GroupTooLarge; the separate decoder-size check stays removed.

Rebased onto latest main; marking ready for review.

(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5543a9b2-f463-4b47-aae6-6545647b0856

---

📥 Commits

Reviewing files that changed from the base of the PR and between a749c1a and aa28803.

---

📒 Files selected for processing (17)

• doc/lib/js/flate.md
• doc/lib/rs/moq-flate.md
• js/flate/src/codec.test.ts
• js/flate/src/codec.ts
• js/flate/src/index.ts
• js/flate/src/stream/index.ts
• js/flate/src/stream/producer.ts
• js/flate/src/stream/stream.test.ts
• js/json/src/stream/codec.test.ts
• js/json/src/stream/encoder.ts
• quest/m1/README.md
• quest/m1/flate-stream-budget.md
• rs/moq-flate/src/lib.rs
• rs/moq-flate/src/stream/mod.rs
• rs/moq-flate/src/stream/producer.rs
• rs/moq-json/src/stream/encoder.rs
• rs/moq-mux/src/binary.rs

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review of 884ee76b (full review, no earlier Grok review)

The budget check sits before group-open and before the DEFLATE window moves in both producers, counters only advance after a successful write, and the GroupTooLarge arm in moq-mux Stream::append correctly keeps the catalog entry. The const assert / JS test pinning MAX_CACHE_BYTES <= DEFAULT_MAX_FRAME_SIZE makes dropping the separate decoder-cap check safe. I found no blocking issues.

Non-blocking

1. @moq/json → @moq/flate range (js/json/package.json:23, workspace:^). json now calls Flate.bound, which only exists in the next flate release. workspace:^ publishes as ^0.1.3 (the current flate version), so a consumer whose lockfile pins @moq/flate@0.1.3 can install the new json and get Flate.bound is not a function at the first compressed append. Bump the flate minor/patch together and make sure the published range floors at the version with bound (the PR body flags this, but nothing enforces it).
2. a_reader_inside_the_group_sees_the_real_error (rs/moq-flate/src/stream/mod.rs) now forces the write failure by overflowing an extreme timescale with Timestamp::now. It works, but it's coupled to timestamp-encoding limits rather than to "any write error"; if moq-net ever saturates instead of erroring, the test silently stops covering the abort path. A short comment pinning that assumption (or a direct injection hook like the JS spyOn) would make it sturdier.
3. The JS spent-budget test does 8192 deflate appends per mode. It's fine today, but keep an eye on test runtime.

CI was still pending at review time.

Verdict: MERGE (once CI is green)

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Replies to the Grok review of 884ee76b, no changes:

1. @moq/json range: versions are bumped when a release is cut, not in feature PRs. workspace:^ resolves at publish time, so a release that publishes the new @moq/json publishes it against the bumped @moq/flate that has Encoder.bound. The PR body's Impact section flags the pairing for that release.
2. a_reader_inside_the_group_sees_the_real_error: the comment already pins the assumption ("Stamped now, which overflows the timescale"), and if moq-net ever saturated instead of erroring, the let Err(..) else { panic!("the write should fail") } fails the test loudly rather than silently losing coverage.
3. 8192 appends per mode: noted; it runs in well under a second locally.

(Written by Claude Opus 5.5)

[kixelated]

Grok review of 307bb13 (first review)

This adds a group-budget check to flate stream mode in both languages. A payload that might not fit is now refused with GroupTooLarge before the group opens or the DEFLATE window moves, and the log stays open, matching what #4911 did for JSON. I read the producers, the shared Encoder::bound, the moq-mux catch, and the tests, and found nothing blocking.

Non-blocking

1. The budget check runs before the closed-track check (rs/moq-flate/src/stream/producer.rs Inner::append, js/flate/src/stream/producer.ts append). After a write failure aborts the track, a later append that is over budget (or any append once 8192 frames are spent) returns GroupTooLarge, which the docs describe as "log intact and writable". moq-mux has already dropped the listing by then, so nothing breaks, but a caller that branches on GroupTooLarge to keep going would be misled. A fix is to check the closed or aborted state first, or to note the ordering in the doc comment.
2. The budget is counted in compressed bytes but checked against the raw worst case. That's correct, and it lines up with what moq-net caches. Still, the Rust code does it through a shadowed size (let size = payload.len() after encoding), which is easy to misread as the raw size. A clearer name would help.
3. The bound test relies on a single xorshift seed. That's fine, since it is the same coverage json had before.
4. Breaking change for packaging. @moq/json now needs the @moq/flate release that adds Encoder.bound, so make sure release-plz bumps json's dependency floor and doesn't just float it.
5. CI hasn't reported yet on this head (Check, Test, macOS, Windows, and the rest).

The quest file removal and its README line are consistent with the change shipping.

Verdict: MERGE once CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Merged latest main at 307bb139. The conflict was in js/flate/src/stream/producer.ts and its tests: #5070 changed append to take Timed<Uint8Array>. The budget check is kept, now behind main's signature, and the decoder-cap abort that main still carried is dropped as this PR intends. Local just check and CI pass.

Replies to the Grok review of 307bb139, no changes:

1. Budget check before the closed-track check: agreed, it's a real edge case, but the append still fails. It only shows up after a write failure has already aborted the track and the next append is also over budget. Distinguishing it needs closed-state tracking in both producers, so I'm leaving it as a follow-up rather than growing this PR.
2. Shadowed size: naming nit, left as is.
3. Single xorshift seed: same coverage json had before.
4. @moq/json floor on @moq/flate: answered on the previous head. Versions bump at release, and the PR body's Impact section flags the pairing.

Still waiting on a non-Grok review of this head before merging.

(Written by Claude Opus 5.5)

[kixelated]

Following up on finding 1 from the Grok review of 307bb139: fixed in aa288039. The maintainer folded it into this PR.

Both stream producers now check whether the track is closed before checking the group budget. An append on an ended log, including an over-budget one, reports the error that ended the log instead of GroupTooLarge. Each language gets a regression test (an_ended_log_refuses_with_its_own_error in Rust, "an ended log refuses with its own error" in JS), and I confirmed both fail without the fix.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: aa28803.

No actionable correctness findings. The overall direction is sound: both producers check the budget before opening the group or advancing compression, charge only successful writes, and keep post-encoding write failures terminal. Sharing Encoder.bound with JSON avoids duplicated logic; the 32 MiB budget safely subsumes the default decoder cap, and moq-mux preserves the catalog entry on a preflight refusal.

The latest closed-track checks (js/flate/src/stream/producer.ts:57–60; rs/moq-flate/src/stream/producer.rs:100–104) correctly preserve the original abort error before budget refusal, using existing track state. The new regressions cover this case, alongside the existing refusal/continuation and spent-budget tests.

Verification: static review of the full change and latest four-file delta, including relevant track/group APIs and review discussion. I did not run tests. CI for this exact head is still queued/running; earlier-head results do not verify this commit.

[kixelated]

Merge summary for aa288039:

• Merged latest main. The conflict came from feat(js)!: publishing never invents a timestamp #5070's Timed<Uint8Array> append. The budget check now uses that signature.
• Folded in Grok finding 1, per the maintainer: an append on an ended log reports the error that ended it, checked before the budget. Regression tests cover it in Rust and JS.
• Decisions, as settled earlier: the bound is Encoder::bound / Encoder.bound, and a deflate payload over 64 MiB is refused with GroupTooLarge.
• The OpenAI review of aa288039 has no findings, and CI is green.

Enabling auto-merge pinned to aa28803957d6b7be6a4630b6e2f624b74e83c33b.

(Written by Claude Opus 5.5)