Skip to content

fix(net): fail socket dials and accepts behind an older moq-lite hop - #7888

Merged
kixelated merged 5 commits into
mainfrom
quest/m1/socket/old-hop-dial
Nov 4, 2026
Merged

kixelated merged 5 commits into
mainfrom
quest/m1/socket/old-hop-dial

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m1/socket/old-hop-dial.md: a socket dial or accept behind a moq-lite hop older than 07 now fails with Unsupported instead of waiting. Before this, the dial never found its peer, and an acceptor reached by such a dialer waited until the dialer gave up.

Summary

  • Where the hidden broadcast gets lost. An endpoint's .socket/<id> broadcast is hidden. Before lite-07, ANNOUNCE_REQUEST has no hidden opt-in, so a session's publisher never announces hidden routes to such a peer unless its publish half opted in (rs/moq-net/src/lite/publisher.rs, AnnounceServe::start). The cluster already opts in on both ends of every relay-to-relay link, on every version (rs/moq-relay/src/cluster.rs: with_hidden(token.peer) when accepting a peer, with_hidden(true) when dialing one). So the only lossy hop is an endpoint's own client session to its relay, which its origin can see.
  • The session reports it to the origin. A moq-lite client session before 07 whose publish half isn't opted into hidden routes counts itself against its publish origin until the session ends. The count is a new crate-private origin::HiddenWithheld (a kio::Shared<usize> in OriginState), and the session holds it with a HiddenWithholding guard.
  • The endpoint fails while one is counted. Endpoint reads that count. Dialing::poll_ok, Accepting::poll_ok and poll_accept_tcp/poll_accept_track fail with Error::Unsupported while any such session is up. The check wakes when the count changes, so a dial already waiting fails once an older session connects, for example when a client reconnects.
  • The reverse direction waits, as for an absent peer. A lite-07 dialer to a peer behind an older hop never learns why the peer's endpoint is missing, so its dial waits as it would for a peer not online yet; the peer's own accepts fail. Telling the two apart would need a relay signal on the wire. The docs say so, and a_peer_behind_an_older_hop_fails_its_accepts checks both halves.
  • --no-default-features stays warning-free. The HiddenWithholding re-export is gated on async, its only user. The socket rustdoc linked the async-only Endpoint::accept_tcp/accept_track, which broke the no-default-features cargo doc -D warnings leg (already broken on main, run only by nightly features.yml); those links now point at poll_accept_tcp/poll_accept_track.
  • Docs: the Endpoint, dial_tcp, poll_accept_tcp and Accepting::poll_ok rustdoc, doc/concept/socket.md, doc/lib/rs/moq-net.md, and the unreleased CHANGELOG entry.
  • Quests: deletes old-hop-dial.md and its link from the socket epic, and rewrites the epic's note about older hops. JS sockets hasn't landed (moq-dev/moq#7884 is open), so its plan now asks for the same check and a lite-06 test. Python gets the check through moq-ffi.

Choices made without asking

  • Only the first hop is checked, and that covers every lossy hop today. The quest asked to weigh a check that sees only the first hop against one that covers relay-to-relay hops. Relay-to-relay links already carry hidden routes on every version through the cluster's opt-in. A relay that answered an unroutable hidden request with a distinct error would need a wire change, and it couldn't tell a peer behind an older hop from one that hasn't announced yet. So the check reads the endpoint's own session version, which works even against an old relay that knows nothing about this check.

  • Client sessions only. If a server's older sessions counted too, one old viewer of a relay would fail every endpoint on that relay's origin. An endpoint behind a server's older session is unreachable only from that peer, and the peer is a client that detects the problem from its own side.

  • Coarse, origin-wide. The count covers the whole origin, not the paths the session publishes. So, while an older client session is up:

    • An origin that also has a lite-07 session to another relay fails its socket handshakes too.
    • So does a dial between two endpoints on the same origin.

    Neither case exists today, and failing loud beats waiting forever.

  • Accept queues fail too. Behind an older hop, no peer can find the endpoint, so accept_tcp()/accept_track() would wait forever. They now fail with Unsupported as well. The quest's accept case was Accepting; this also covers the queue.

  • Sessions already established keep running. The check covers the handshake (dial, accept queue, accepting). A tcp session's task doesn't consult it, since a session that connected already proved its hops carry it.

  • A publish half opted into hidden routes isn't counted (client_publish = origin.consume().with_hidden(true)), as on a relay's cluster dial: that session does carry the broadcast. an_older_hop_opted_into_hidden_connects checks that such an endpoint connects over lite-06.

Impact

  • Public API: none. Existing calls now return the existing Error::Unsupported in the case above. The new types are crate-private.
  • Wire: none.

Tests

On simulated time, through the relay harness (rs/moq-net/tests/integration/socket.rs):

  • an_older_hop_fails_dials_and_accepts: an endpoint on moq-lite-06, and again on moq-lite-03, fails tcp and track dials and accepts with Unsupported within 1 s, and its lite-07 peer never hears a dial.
  • an_older_hop_fails_a_waiting_dial: a dial waiting with no session fails once a lite-06 session connects the origin.
  • a_peer_behind_an_older_hop_fails_its_accepts: a lite-06 (and lite-03) endpoint's accept_tcp fails Unsupported, and a lite-07 dial to it stays pending.
  • an_older_hop_opted_into_hidden_connects: a lite-06 client whose publish half opted into hidden routes still connects a tcp session to a lite-07 peer.

Before the fix, the first two fail (one wait times out, the other gets no Unsupported), and the third passes either way. It guards the opt-in exemption.

Validation

On 70aa311748 (after merging origin/main):

  • cargo test -p moq-net --features async --test integration socket: 26 passed.
  • cargo nextest run -p moq-net --all-features: 3425 passed.
  • The features.yml moq-net legs: cargo clippy -p moq-net --no-default-features --lib -D warnings, cargo doc -p moq-net --no-default-features with -D warnings, and cargo nextest run -p moq-net --no-default-features --tests (1384 passed). Also cargo clippy -p moq-net --all-targets --features async -D warnings, the async rustdoc, cargo fmt --check, sh/markdown.sh check and quest check.
  • Earlier, on de5628dd05: NEXTEST_NO_FAIL_FAST=1 just check origin/main passed, and reverting rs/moq-net/src made an_older_hop_fails_dials_and_accepts and an_older_hop_fails_a_waiting_dial fail. An unrelated moq-gst test, restarts_under_reconnects_interleave_on_one_pad, failed once under that suite's load and passed alone; see the follow-ups.

GitHub reviews

  • pullfrog, unused HiddenWithholding re-export without async: fixed as suggested, along with the no-default-features rustdoc links above.
  • pullfrog, the docs omitted the publish-half opt-in exemption: fixed in the Endpoint rustdoc, both docs pages and the CHANGELOG.
  • OpenAI review [P2], a lite-07 dial to a peer behind an older hop still waits: documented and tested, not fixed; see the reverse-direction bullet above.

Suggested follow-ups

  • moq-gst restarts_under_reconnects_interleave_on_one_pad flakes under load. It failed once in a full just check with 5 never reached a pad (rs/moq-gst/src/source/imp.rs:3829) after its 10 s wall-clock wait, and passes alone. It belongs in the test flakes epic. No quest or open PR tracks it yet.

Review

Local review session 01a1a763-def4-7d10-b46c-aa78df823aab, by Codex gpt-6.1-sol at medium effort.

  • Round 1 (head de5628dd05), one finding:
    • [medium] Scope the reachability check to the endpoint and route, not the whole origin (socket.rs:504). Refuted: an older client session's subscribe half still routes dials, whatever its publish scope covers. A client session asks its peer about the dial's prefix (rs/moq-net/src/discovery.rs:91), and a lite-06 relay announces the endpoint for that narrow request (rs/moq-net/src/lite/publisher.rs:1014-1021); an_older_hop_opted_into_hidden_connects relies on this. So a dial can reach a peer that never learned the dialer's endpoint, and the endpoint can't tell which session a dial will take. Recorded under the coarse, origin-wide choice above.
  • Round 2 (head de5628dd05, with that response): no findings.
  • Round 3 (head 70aa311748, after merging origin/main and addressing the GitHub reviews): no findings.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 3 commits November 4, 2026 08:09
A moq-lite client session before 07 never carries the origin's hidden
broadcasts to its peer, so a socket endpoint behind one is unreachable.
The session now counts itself against its publish origin, and the
endpoint fails dials and accepts with Unsupported while one does,
instead of waiting until the dialer gives up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A socket dial over an older hop fails instead of waiting fix(net): fail socket dials and accepts behind an older moq-lite hop Nov 4, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 16:31
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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 18 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
  • 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: 8be0c0bb-ec2d-4b50-8089-059f0c0bbf4b
Commits

Reviewing files that changed from the base of the PR and between 2be4cc7 and 70aa311.

Files selected for processing: 11
  • doc/concept/socket.md
  • doc/lib/rs/moq-net.md
  • quest/m1/socket/README.md
  • quest/m1/socket/js.md
  • quest/m1/socket/old-hop-dial.md
  • rs/moq-net/CHANGELOG.md
  • rs/moq-net/src/lite/session.rs
  • rs/moq-net/src/model/mod.rs
  • rs/moq-net/src/model/origin.rs
  • rs/moq-net/src/model/socket.rs
  • rs/moq-net/tests/integration/socket.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

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed head de5628d against base 635dd54.

  • [P2] Preserve the unresolved remote-old-hop case (rs/moq-net/src/model/socket.rs:504–507, :834–840). With Alice on lite-07 and Bob on lite-06, Alice's local origin has no withholding session, but Bob's hidden endpoint is never announced. Alice follows that absent endpoint and can still wait indefinitely; Bob's accept_* fails locally but Alice's dial never reaches it. The new tests cover the opposite direction, where the dialer's own hop is old. Could this reverse-direction case remain explicitly tracked and documented, with a regression test, rather than deleting the quest's broader guarantee that a dial through an old hop fails? A remote peer that is merely absent may be indistinguishable without an additional signal, so the chosen local check can still stand while that limitation is made explicit.

Verification limits: GitHub-only static review of the endpoint, session and integration paths; I did not run the relay tests or reproduce the case at runtime.

(Written by review (OpenAI))

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The --no-default-features lib build fails the repo's net-features gate: HiddenWithholding is re-exported unconditionally but only named by the async-only lite::session, so it is an unused import once async is compiled out.

Reviewed changes

  • Hidden-route accounting. origin::HiddenWithheld (a kio::Shared<usize> on OriginState) and a HiddenWithholding guard count client sessions that withhold an origin's hidden routes from their peer.
  • Session wiring. lite::start takes the guard for a client session on a version without hidden support (!version.has_hidden()) whose publish consumer has not opted in (!publish.includes_hidden()), storing it on Driver until the session ends.
  • Endpoint checks. socket::Shared::poll_reachable returns Error::Unsupported while that count is non-zero, wired into poll_accept, Dialing::poll_inner, and Accepting::poll_inner.
  • Tests. an_older_hop_fails_dials_and_accepts (lite-06 and lite-03), an_older_hop_fails_a_waiting_dial, and the opt-in exemption an_older_hop_opted_into_hidden_connects.
  • Docs/quest. doc/concept/socket.md, doc/lib/rs/moq-net.md, CHANGELOG, and the socket quest/JS plan updates.

I verified the cluster premise independently: the dial side sets with_publisher(origin.consume().with_hidden(true)) (moq-relay/src/cluster.rs:2162), and the accept side's Admitted.subscriber (built with .with_hidden(token.peer) at :1533) becomes with_publisher, so hidden routes flow both ways on every version. IETF peers that do not declare MoQ Hidden receive every namespace, so only lite<07 is lossy. The new socket tests pass (25 passed), as do cargo clippy -p moq-net --lib -- -D warnings, cargo fmt --check, and sh/markdown.sh check.

One thing to note: the CHANGELOG and doc/lib/rs/moq-net.md describe lite<07 sessions as never carrying hidden broadcasts and omit the publish-half opt-in exemption that the code and an_older_hop_opted_into_hidden_connects rely on. Not blocking, but the sentence is technically false for an opted-in publish half.

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

Comment thread rs/moq-net/src/model/mod.rs Resolved
kixelated and others added 2 commits November 4, 2026 11:23
…ocument the remote-old-hop wait

- Gate the `HiddenWithholding` re-export on `async`, its only user.
- Point socket rustdoc links at the `poll_accept_*` methods, which exist
  without `async`.
- Say that a publish half opted into hidden broadcasts isn't counted, and
  that a dial to a peer behind an older hop waits as for an absent peer;
  test that case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator Author

Re the OpenAI review's [P2] (remote old hop): agreed it's a real limitation; documented and tested in 70aa311 rather than fixed. With Alice on lite-07 and Bob behind lite-06, Bob's accept_* now fails Unsupported locally, and Alice's dial waits as it does for any peer not announced yet: dial_* follows the peer's endpoint until it appears (rs/moq-net/src/model/socket.rs, Step::Resolving). Nothing on Alice's side can tell "behind an older hop" from "not online yet" without a relay signal on the wire, and even a relay can't tell a peer behind an older hop from one that hasn't announced yet (see "Only the first hop is checked" in the PR body). The Endpoint rustdoc, doc/concept/socket.md, and doc/lib/rs/moq-net.md now say so explicitly, and a_peer_behind_an_older_hop_fails_its_accepts (lite-06 and lite-03) asserts both halves: Bob's accept fails, and Alice's dial stays pending.

Pullfrog's docs note (the publish-half with_hidden(true) exemption) is fixed in the same commit.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 70aa311

No new actionable findings in the six-file follow-up, after separating the main-branch merge.

The prior P2 is addressed as requested documentation and regression coverage, not a behavior fix. The new test exercises lite-07 Alice dialing lite-06/lite-03 Bob: Bob's accept fails with Unsupported while Alice stays pending. The Endpoint and user docs now explicitly retain that remote-old-hop limitation and the hidden-publish opt-in exemption. The async-gated re-export and unconditional poll-method rustdoc links are consistent with the feature boundaries.

Overall direction: the narrower guarantee is now explicit and tested in source. Verification was GitHub-only inspection of the updated files, session harness and endpoint routing, with an independent pass. I did not execute tests or quest context, so the reported build/test results remain author-reported.

(Written by review (OpenAI))

@kixelated
kixelated added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 7df9477 Nov 4, 2026
8 of 9 checks passed
@kixelated
kixelated deleted the quest/m1/socket/old-hop-dial branch November 4, 2026 21:29
kixelated added a commit that referenced this pull request Nov 4, 2026
A subscribe, track info, fetch, or fill now waits for its answer until its
demand ends, its caller cancels, stream credit fails fast, or the session
closes, as in Rust, replacing the ten-second answer budget and the
`.socket/<id>` namespace exemption. Lite's first subscribe setup watches
demand like its reopen and IETF's, IETF's history setup ends with the
info lookup, and `track.Consumer.info()` takes an optional `signal`.

Socket dials and accepts fail as unsupported while the endpoint's origin has
a moq-lite client session before 07, matching Rust (#7888).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant