[kixelated]

Finishes quest/m1/retry-interval-overload.md.

Summary

The moq-transport refusal of a PUBLISH over the listed-track limit (EXCESSIVE_LOAD, "too many listed tracks") now carries the may-pass retry interval (RETRY_INITIAL, 1 s) instead of 0, as draft-18 section 10.6.2 says it SHOULD.

• request::retry_interval (the rule from moq-dev/moq#7993) treats EXCESSIVE_LOAD as a refusal that may pass, whatever the error.
• ServePublish's refusals ask that rule with the error as the peer reads the code (from_code), instead of hard-coding 0. The other refusals there (duplicate, UNINTERESTED, unsupported) still read 0.
• New test an_over_limit_publish_invites_a_retry decodes the refusal on draft-18, -20 and -22.

Choices made without asking

• Judged the refusal from its wire code (from_code) so Offer::Refuse stays unchanged, which keeps the diff in tracks.rs small next to the parallel retry-interval-forward work.

Notes

• model::track::test::pool_debt_sheds_a_flight_without_its_cached_group failed once in a full cargo test -p moq-net run and passed on rerun; it sits in an untouched module.

Review

mechanical fix by quest land watch: merged main at f85fa6ed16a9b08cf1f66141294561a3ec9abbf9 with the quest merge driver (head 6aebc64)

Codex, gpt-6.1-sol, medium effort, round 1 on head daaa13009c0ddf047c2d57ca9f6345076f894f7a: no findings.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The retry interval policy now assigns the initial interval to every EXCESSIVE_LOAD refusal, regardless of error retryability. Refused PUBLISH responses use the policy to set retry_interval. A test verifies the interval for over-limit listed PUBLISH refusals across Draft18, Draft20, and Draft22. The README entry and planning document for this change were removed.

Merge Risk: Minimal · up to 6aebc

The change gives over-limit PUBLISH refusals the initial retry interval, with version-specific coverage. No actionable merge blocker remains.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names the change: an over-limit PUBLISH refusal now invites a retry. The raw summary shows the refusal now carries the initial retry interval instead of 0 through request::retry_interval, so…
Description check		The description matches the changeset. It explains that the over-limit PUBLISH refusal (EXCESSIVE_LOAD) now carries RETRY_INITIAL (1 s) instead of 0. It names request::retry_interval and ServePublish,…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the change: an over-limit PUBLISH refusal now invites a retry. The raw summary shows the refusal now carries the initial retry interval instead of 0 through request::retry_interval, so the title matches the main change. The title uses the fix(moq-net) prefix, which is a conventional-commit style and does not hurt clarity. It is 60 characters, which is within a normal length.

Full details: Description check

Explanation

The description matches the changeset. It explains that the over-limit PUBLISH refusal (EXCESSIVE_LOAD) now carries RETRY_INITIAL (1 s) instead of 0. It names request::retry_interval and ServePublish, which appear in the raw summary, and it lists the new test an_over_limit_publish_invites_a_retry. It also notes the removal of the planning document in the summary, which matches the raw summary's deletion of retry-interval-overload.md.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.

Finishing Touches

Simplify code

• Commit to this branch
• Create a new PR

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

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

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 37f25755-29e9-4e9a-9f32-62c6112bb781

Commits

Reviewing files that changed from the base of the PR and between f85fa6e and 6aebc64.

Files selected for processing: 4

• quest/m1/README.md
• quest/m1/retry-interval-overload.md
• rs/moq-net/src/ietf/error.rs
• rs/moq-net/src/ietf/subscriber/tracks.rs

Files with no reviewable changes: 1

• quest/m1/retry-interval-overload.md

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

[kixelated]

Automated review by review (OpenAI)

Reviewed head daaa13009c0ddf047c2d57ca9f6345076f894f7a. No actionable correctness findings. The overload refusal now uses the shared 1-second retry interval; the other Offer::Refuse codes retain zero. The new test checks the decoded interval across drafts 18, 20, and 22.

Direction: proceed through normal CI/maintainer review. Verification: GitHub-only static review of the changed paths, error mapping, request encoding, and test harness; I did not run tests or interop checks. The current Check workflow is still in progress.

[pullfrog]

ℹ️ No critical issues — one question about where the invited retry lands.

Reviewed changes

• Over-limit refusals invite a retry. request::retry_interval now treats EXCESSIVE_LOAD as a may-pass refusal whatever the error, returning RETRY_INITIAL (1000 ms), and ServePublish derives the refusal's interval from the wire code (from_code) instead of hard-coding 0. The other ServePublish refusals (duplicate, listing cancelled, not supported, malformed properties) still resolve to 0.
• Coverage. New an_over_limit_publish_invites_a_retry decodes the refusal on drafts 18, 20, and 22; the existing rule test still passes.
• Quest finished. Deletes quest/m1/retry-interval-overload.md and its m1 Required line, with no leftover references.

I verified the draft-18 §10.6.2 wording (non-zero Retry Interval = retryable, and EXCESSIVE_LOAD SHOULD carry it), ran ietf::subscriber (257 tests) and ietf::error (16) plus the new test with --features async, and confirmed js/net has no counterpart to update (it refuses every PUBLISH and has no excessive_load condition).

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

The invited retry does not reach a listener: this same arm still calls listings.fail(&key.0, err) (tracks.rs:490), which cancels the covering listing, so a retried PUBLISH for the namespace falls into the "listing cancelled" arm at tracks.rs:464 and is answered UNINTERESTED with interval 0. I measured it with a one-slot announces cap: the first offer gets EXCESSIVE_LOAD/1000 and the retry UNINTERESTED/0. Since the refused track is never inserted into entries, dropping that fail would let the retry land once a slot frees, and would stop one transient load refusal from dropping every other track under the namespace and aborting its readers.

Technical details

# The over-limit refusal cancels the listing it just asked the peer to retry on

## Affected sites
- `rs/moq-net/src/ietf/subscriber/tracks.rs:490` — `listings.fail(&key.0, err)` runs on the over-limit arm (`Error::TooManyRequests` from `announces.acquire()`).
- `rs/moq-net/src/ietf/subscriber/tracks.rs:464` — `Offer::Refuse(UNINTERESTED, "listing cancelled")` answers any offer under a request whose `live` is `None`, which `fail` just set.
- `rs/moq-net/src/ietf/subscriber/tracks.rs:528` — the interval the peer is now told to come back after.

## Observed
With `subscriber.announces = Slots::new(1)`, a listing on `bcast`, one accepted track: the second `PUBLISH` is refused `0x9`/1000; retrying that same `PUBLISH` is refused `0x20`/0 because `Listings::fail` cancelled the request and `covering` now returns `None`.

## Required outcome
A retried over-limit `PUBLISH` should reach the listing that is still standing, so it is either accepted once a slot frees or refused `EXCESSIVE_LOAD` again with the interval. Not failing the listing also stops the over-limit arm from dropping the other tracks the listing holds (via `release()`) and aborting its readers.

## Suggested approach
Drop the `listings.fail` call from the over-limit arm (`Err(_) => ...`) and keep only the `Offer::Refuse`. The property and listing-cancelled arms keep `fail` because the peer sends nothing more for those.

[moq-bot]

quest land watch: the merge queue removed this pull request at daaa130 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

[moq-bot]

quest land watch: armed auto-merge on this pull request at 6aebc64, so it joins the back of the merge queue once its required checks pass, with main at f85fa6e merged in by the quest merge driver.

(written by quest land watch)

[pullfrog]

Your OpenRouter API key was rejected. Issue a new key with the provider, then update the copy Pullfrog uses, in Pullfrog or in this repo's GitHub Actions secrets.

The provider's response

provider error: User not found.

Pullfrog provider settings → · GitHub Actions secrets →

  ^{｜ Rerun failed job ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS)}

[pullfrog]

No code change was needed: the failed suite was Pullfrog's own agent run, not the PR's CI.

• The failing check was the previous pullfrog run (42325343452), which stopped before doing any work with action failed: provider error: User not found. (logged as provider error detected (auth error (invalid/disabled key))). That is a provider/auth failure, unrelated to the PR.
• The repository Check workflow re-ran on the same head 6aebc64 and completed success (42325342375), so the branch is green.
• Locally verified with cargo test -p moq-net --lib --features async: an_over_limit_publish_invites_a_retry passes, the full ietf::error::request::tests module passes (10/10), and ietf::subscriber::tracks::tests passes (17/17).
• The flake mentioned in the PR body (pool_debt_sheds_a_flight_without_its_cached_group) was already fixed on main by pinning the crossbeam epoch; the merge at 6aebc64 includes that fix.

No commit was pushed because there was nothing to fix.

Task list (5/5 completed)

• Checkout PR fix(moq-net): the over-limit PUBLISH refusal invites a retry #8084 branch
• Fetch failing check suite logs and identify the failure
• Confirm whether the failure was introduced by the PR
• Verify the PR's tests and CI run on the merge head
• Report diagnosis (no code fix needed)

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}