quest: keep peer-asked PUBLISH_NAMESPACE retries and note async credit backends

[kixelated]

Problem

Grok's second review of #5013 (on c0c079a9) raised two findings that landed unaddressed when #5013 merged:

• js-request-deadline.md drops the IETF publisher's PUBLISH_NAMESPACE re-offer loop entirely. That loop also honours a peer's REQUEST_ERROR retry interval and backs off draft-14/15 refusals, so a peer saying "retry later" would leave the namespace undiscoverable until reconnect.
• rs-request-credit.md asks for a non-waiting open on the transport trait, but the async backends moq-tokio wraps (qmux, iroh, noq) have no credit query in web-transport-trait 0.5.

Approach

• js-request-deadline.md: the loop stops re-offering only after this quest's two failures (no stream credit, the fatal timer); it still re-offers when the peer names a retry interval, and keeps its backoff for draft-14/15 refusals.
• rs-request-credit.md: prefer adding a credit query upstream in moq-dev/web-transport, and say in the PR which backends fail fast.

Decisions

Taken while the maintainer was away, with the recommended options.

PUBLISH_NAMESPACE retries

• ✅ Keep re-offering when the peer asked for it; drop only the re-offers after our own two failures
• Drop the loop and accept losing peer-asked retries

Async backends without a credit query

• ✅ Add one upstream in web-transport-trait
• Leave those backends waiting

Impact

• None (quests only).

Alternatives

• Folding these into a quest implementation PR; kept separate so the plan is correct before work starts.

Follow-ups

• None.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[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 1 minute.

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: 30ee8551-505f-4192-98fb-39bfce20bbc8

📥 Commits

Reviewing files that changed from the base of the PR and between ac72dcd and f731a23.

📒 Files selected for processing (2)

• quest/m1/js-request-deadline.md
• quest/m1/rs-request-credit.md

• Autopilot · 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 by review (OpenAI)

Reviewed commit: 9bf0eac

No actionable bugs found in these plan changes. The direction is sound: preserve peer-directed namespace retries and legacy refusal backoff while making local credit/deadline failures terminal; expose the needed transport capability upstream rather than equating an async Pending result with exhausted credit. These changes address the two request-related points in the earlier review.

Verification: inspected both complete quests, the publisher's retry/refusal/reconciliation paths, and the Rust async-to-poll adapter at this commit. Documentation-only review via GitHub; no tests or runtime verification performed.

(Written by OpenAI)

[kixelated]

Grok review of 9bf0eac0

This follows up the second #5013 review (#issuecomment-6043825133). Both of its findings are addressed, and I checked the new claims against the code at this head.

What checks out

• The retry behaviour the quest now keeps matches js/net/src/ietf/publisher.ts. #refusal (:1175-1179) returns "dropped" on draft-14/15, which #pending (:1164-1166) treats as still outstanding, so the 100 ms to 5 s backoff applies. On draft-16+, a non-zero retryInterval becomes a deadline that #offerable (:1150-1154) waits out, and 0 becomes "never".
• web-transport-trait 0.5.0, the latest release and the version the workspace pins, has no credit or "can open" query in either its async or its poll Session. rs/moq-tokio/src/transport.rs:1-9 names qmux, iroh and noq as the async-only backends it wraps.
• No new links. CI (Check, Test, Quest) is still pending, which doesn't matter much for a quest-only PR.

Non-blocking

1. Two loops share the retry, but the quest names only one (js-request-deadline.md:37-38). The draft-14/15 solicited path, #runSubscribeNamespace (publisher.ts:916-958), goes through the same #advertise, #reconcile and backoff as runPublishNamespaces. Both failures this quest adds happen inside #advertise: the openBi at :1200 and the ADVERTISE_TIMEOUT_MS race at :1207-1240. So the change will reach both loops whether or not the quest says so. Name both, so the tests cover the legacy SUBSCRIBE_NAMESPACE path too.

2. "Stops re-offering" needs a new answer, and it only lasts until a republish (:37-43). Today a missing credit (:1249), a timeout (:1249), a missing request id (:1192) and a draft-14/15 refusal (:1177) all return the same "dropped". That is exactly the value that keeps the backoff running. The implementer has to give the two local failures their own terminal answer, for example by recording "never", while draft-14/15 refusals stay "dropped". Also, #reconcile clears a refusal when the broadcast goes away, is replaced, or changes epoch (:1090-1098). So the namespace stays unadvertised through that peer only until it is republished or the session reconnects, not forever. That is consistent with "nothing retries either failure", but the quest should say it outright. A credit shortage at publish time now costs that peer discovery, which is the same harm the new text cites for dropping peer-asked retries.

3. The timer length disagrees with the code (pre-existing, js-request-deadline.md:5-8 vs :35). The Goal puts PUBLISH_NAMESPACE under "one fatal 10 s timer", and the Plan says PUBLISH_NAMESPACE keeps "its own" timer. That timer is ADVERTISE_TIMEOUT_MS = 5000 (publisher.ts:101). Say whether it moves to 10 s or stays at 5 s.

4. Stale crate name in the paragraph this PR extends (pre-existing, rs-request-credit.md:21-23). It cites web-transport-quinn, which isn't in the workspace. WebTransport goes through web-transport-moq, the moq-noq adapter (Cargo.toml:240-241). Check the "opens the stream, then writes the session header, which can be pending" claim against that crate. Separately, the method would be added to moq-net's own transport::poll traits (rs/moq-net/src/transport.rs). The upstream web-transport-trait change is only what lets moq-tokio's wrapper implement it for the async backends. "Possibly a transport-trait method" could say which trait it means.

Verdict: MERGE. Both carried-over findings are fixed and the claims are accurate. Items 1 and 2 are worth a line each so the implementer doesn't have to rediscover them.

Reviewed head: 9bf0eac0c42d9ce1a5fc2c669129471c9752e7a0

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

[kixelated]

Addressed the Grok review of 9bf0eac0 in f731a23:

1. Both advertise loops (runPublishNamespaces and the draft-14/15 #runSubscribeNamespace) are named and tested.
2. The two local failures need their own terminal answer (both return "dropped" today), and the namespace stays unadvertised only until a republish or reconnect.
3. ADVERTISE_TIMEOUT_MS moves from 5 s to the shared 10 s, matching the Goal.
4. web-transport-moq replaces the stale web-transport-quinn (its open_bi does write the header after opening), and the method goes on moq-net's transport::poll session trait.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: f731a23

No actionable bugs found in this follow-up. It addresses the four clarifications in Grok's earlier review: both advertise loops are covered; local credit/deadline failures get a terminal result distinct from retryable legacy refusals; republish/reconnect bounds that terminal state; and ADVERTISE_TIMEOUT_MS explicitly moves from 5 s to 10 s. The Rust text now names the correct WebTransport adapter and moq-net poll trait.

Direction remains sound: preserve peer-directed retries while making these local failures terminal, and distinguish exhausted stream credit from an async open still writing its session header.

Verification: compared the one-commit/two-file delta, read both complete quests, checked the publisher's two loops and refusal/reconciliation paths, the Rust adapter and poll trait, and the upstream WebTransport header-write path. Rechecked head/state/reviews. Documentation-only GitHub review; no builds, tests or runtime verification performed. Check is in progress.

[kixelated]

Merge summary: keeps peer-asked PUBLISH_NAMESPACE retries (both advertise loops; the two local failures get their own terminal answer until republish or reconnect; ADVERTISE_TIMEOUT_MS moves to 10 s) and notes that the async Rust backends need an upstream web-transport-trait credit query, with the method on moq-net's transport::poll session trait. Decisions are in the description; reviewed clean by OpenAI at f731a235. Merge approved by the maintainer via /quest-merge.

(Written by Claude Opus 5.5)