fix(net)!: enforce the loop-free hop chain where chains are built

[kixelated]

Fixes #3049. Targets dev: it changes the error type on published moq-net API.

Root cause

HopPath::validate ran only on decode, so nothing stopped us constructing and sending an outbound HOP_PATH that a conforming receiver must reject. Advert::forward appended our own Hop ID and wrapped the result without revalidating, and the encode path checked nothing, so a broadcast::Route whose hops already carried a duplicate went out as-is.

draft-lcurley-moq-cluster makes that the receiver's problem: "A receiver MUST close the session with a PROTOCOL_VIOLATION if the entries do not exactly fill Length, if the list is empty, or if a non-zero Hop ID appears twice." So a malformed chain we build does not degrade our own routing. It closes someone else's session.

Routes reach the model from more than one place and only the IETF decode path was checked:

• The moq-lite ingress builds hop chains itself. OriginList capped length without rejecting duplicates, so a lite chain could legally carry one and, once shared through the origin, be forwarded to an IETF cluster peer.
• broadcast::Route is public with a public hops field, so any in-process producer could attach one.

The general shape is that validity was enforced where a chain is parsed rather than where one exists, so every new route source had to remember the rule again.

The fix

Move the rule into OriginList, which is where the state lives. push and TryFrom<Vec<Origin>> reject a repeated non-zero id; Decode goes through push, so a looped chain never enters the model rather than being caught later; replace_first refuses a replacement already in the chain, since writing one in names it twice by another door.

Two things fall out:

• HopPath::validate is left with only the empty-list check. The duplicate rule now holds on the outbound path too, not just where a parameter was parsed.
• Advert::forward fails on a chain it cannot legally extend, and Publisher::select already advances to the next route on that error. So a route that cannot be advertised is skipped rather than emitted, and stays servable by exact path — which is what the issue asked for, since a route that is illegal to advertise may still be fine to serve locally.

Origin::UNKNOWN identifies nothing, so duplicate zeros stay legal. That is the whole of a lite-03 chain, and it is what #3060 changes next.

The lite draft never stated the rule

That asymmetry is what let the two dialects disagree, and it was not deliberate. draft-lcurley-moq-lite now says it in the same words as the cluster draft, alongside the existing Hop Count rule. Validated with just drafts check.

Breaking change

• OriginList::push and TryFrom<Vec<Origin>> return InvalidHop (TooMany / Duplicate) instead of TooManyOrigins, which is removed.
• OriginList::replace_first returns Result<bool, InvalidHop>.
• Route::with_hop and cluster::Advert::forward carry the new error.

Outside moq-net these are named only by tests and moq-relay/src/nodes.rs.

Tests

• origin_list_push_rejects_a_repeat and the try_from case cover both construction paths, including that repeated zeros stay legal.
• origin_list_replace_first covers the rewrite refusing an id already in the chain.
• forward_refuses_a_chain_it_cannot_extend covers the outbound half: a chain that already names us, and one with no room left.
• hop_path_rejects_repeated_hop now forges its bytes, because OriginList no longer lets an invalid chain be built. That is the change working, and the test still proves decode rejects one arriving from a non-conforming sender.

Cross-package sync

• drafts/draft-lcurley-moq-lite.md: the new receiver MUST (wire spec change).
• js/net: third commit. checkHops guarded only length and only on encode; it now applies both rules in both directions, with a test that patches a valid encoding into a looped one.
• just test smoke-full passes: all 21 publisher/subscriber pairs across rust, python, js, js-native-node, js-native-bun, c, and gst.

Follow-up

#3060 extends the same rule to ban 0 from chains entirely, which is what closes #3053. This is its prerequisite: it is what makes an invalid chain unbuildable rather than something each ingress path remembers.

🤖 Generated with Claude Code

(written by Claude Opus 5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7e5e75241

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Validate the implicit publisher hop before accepting announces

For Lite05+ announcements, a sender can include its non-zero ANNOUNCE_OK.Hop ID once in the explicit list, making it duplicate only after the implicit trailing hop is reconstructed. decodeHops accepts that list, while Rust's start_announce in rs/moq-net/src/lite/subscriber.rs:271-279 merely returns Ok(false) and the JS subscriber in js/net/src/lite/subscriber.ts:277-286 merely drops it, so neither implementation closes the stream as this new MUST requires. Check the reconstructed list after adding the responder ID and propagate a protocol violation.

AGENTS.md reference: AGENTS.md:L186-L191

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Check for the target before rejecting a replacement

When target is absent but replacement already occurs in the list, this preflight returns InvalidHop::Duplicate even though the documented operation would make no change and should return Ok(false); it also rejects the valid no-op replace_first(x, x) instead of returning Ok(true). Since this is a public method, callers performing conditional rewrites now receive an error for operations that cannot create a duplicate. Locate the target first and exclude its slot from the duplicate check.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Record the hop-chain rule in the draft changelog

This adds a new fatal protocol condition to the in-progress moq-lite-06 semantics, but the moq-lite-06 section of Appendix A has no corresponding bullet. Add a concise entry so readers can discover that repeated non-zero Hop IDs now require PROTOCOL_VIOLATION.

AGENTS.md reference: drafts/AGENTS.md:L77-L81

Useful? React with 👍 / 👎.