Skip to content

feat(net)!: transit facts stay read-only - #8007

Merged
moq-bot[bot] merged 13 commits into
mainfrom
quest/m1/api-audit/route/facts
Nov 6, 2026
Merged

moq-bot[bot] merged 13 commits into
mainfrom
quest/m1/api-audit/route/facts

Conversation

@kixelated

@kixelated kixelated commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Hops, via, source, the epoch and the anonymous mark are facts the origin and the relays fill in, but Hop, Hops, InvalidHop and origin::HopLimit were still public in Rust, and @moq/net still exported Hop, HopSchema, isAnonymous and a RouteInfo carrying epoch and auth. Quest: quest/m1/api-audit/route/facts.md, split from moq-dev/moq#7806.

Approach

  • Rust. Hop, Hops and InvalidHop are crate-private and HopLimit is gone. Applications pass and read plain ids: origin::Config::hop is a NonZeroU64 (random by default), Config::max_hops a NonZeroUsize (above 32 acts as 32, the wire's own cap), Producer::hop()/Consumer::hop() return NonZeroU64, RouteInfo::hops() iterates u64 ids, and Source::Peer holds a u64. The peer-hop setters on Client, server::Handshake and moq-tokio take a NonZeroU64. The hidden fuzz helpers (route_info, excluding, Announced) take plain ids, which shortens every bench and test that builds a chain. The NonZeroU64 ids give the origin handles a niche, which shrank Client's handshake steps enough for clippy's large_enum_variant to flag the internal Connect future; its Started variant is now boxed.
  • JS. The table and sessions keep the full route as the internal WireRoute (epoch, auth, branded hops). The public Origin.RouteInfo is { instance, hops: bigint[], cost, anonymous }, projected once where a listing hands an event to the application (Announce.Consumer.next). Hop, HopSchema and isAnonymous leave the Origin namespace.
  • Docs. A line in doc/lib/rs/moq-net.md. The migration notes live in Impact below: main now reserves MIGRATE.md for the release cut (moq-dev/moq#8024), so this PR no longer edits it.
  • Quests. Deletes the finished quest and its epic route/, whose last child this was; prose links to the epic now point at its last version on main.

Impact

  • moq-net (breaking): Hop, Hops, InvalidHop, origin::HopLimit removed from the public API. Migrate Config::new(Hop::new(id)?) to Config::new(NonZeroU64::new(id)?), HopLimit::new(n)? to NonZeroUsize::new(n)?, hop().id() to hop().get(), route.hops().iter().map(|h| h.id()) to route.hops(). Migration note for the release cut:

    Before After
    origin::Config::new(Hop::new(id)?) origin::Config::new(NonZeroU64::new(id).context("…")?)
    origin::Config::new(Hop::random()) origin::Config::default(), which picks a random id
    config.max_hops = origin::HopLimit::new(n)? config.max_hops = NonZeroUsize::new(n).context("…")?; above 32 acts as 32
    origin.hop().id() origin.hop().get()
    route.hops().iter().map(|hop| hop.id()) route.hops(), an iterator of u64 ids where 0 is an anonymous hop
    origin::Source::Peer(hop) with a Hop origin::Source::Peer(id) with a u64
    Client::with_peer_hop(hop), server::Handshake::with_peer_hop(hop) take a NonZeroU64
    server::Handshake::peer_hop() returns Option<Hop> returns Option<NonZeroU64>
  • moq-net (breaking): origin::Source::Peer holds a u64; Client::with_peer_hop and server::Handshake::with_peer_hop take NonZeroU64; server::Handshake::peer_hop returns Option<NonZeroU64>.

  • moq-tokio (breaking): Client::with_peer_hop and server::Request::with_peer_hop take a NonZeroU64, and server::Request::peer_hop returns Option<NonZeroU64>, since moq_net::Hop is no longer public. Pass the id you passed to Hop::new.

  • @moq/net (breaking): Origin.Hop, Origin.HopSchema, Origin.isAnonymous removed; Origin.RouteInfo drops epoch and auth, its hops are bigint[], and it gains anonymous. Replace Origin.isAnonymous(event.route) with event.route.anonymous; epoch and auth stay inside the origin, as in Rust.

  • moq-relay: no change in behavior; --cluster-id and --cluster-max-hops validate as before.

  • moq-ffi, moq-c and wrappers: none (they already exposed plain ids).

  • Wire: none.

Choices made without asking

  • Config::hop is NonZeroU64 and Config::max_hops is NonZeroUsize, so the reserved 0 cannot be configured; a max_hops above 32 acts as 32, since no wire carries a longer path. The relay keeps rejecting values outside 1 to 32 itself.
  • Producer::hop() and Consumer::hop() stay public, returning NonZeroU64: the relay's Cluster::id and the FFI and C tests read them.
  • RouteInfo::hops() returns impl ExactSizeIterator<Item = u64> rather than a Vec, so reading it allocates nothing.
  • Source::Peer(0) means the announcing handle named no hop: a peer-linked handle in-process, or a session whose peer declared no hop and was assigned none (Pullfrog's suggested wording).
  • The quest Plan did not list the peer-hop setters on Client, server::Handshake and moq-tokio; they took Hop, so the Goal moves them to NonZeroU64 too.
  • quest/m1/api-audit/README.md lost its last child, so it is now sized [M], with its own work spelled out as a final API diff against release, as release-perf did when its children landed.

Alternatives

  • Keeping a validated HopLimit-like newtype: it would leave a hop type in the public API, which the quest removes.
  • Hiding JS epoch and auth behind a wireOf lookup on each public RouteInfo object: route objects live in deep-equal signals, so a field kept outside the object would stop an epoch-only change from notifying. A separate internal type, projected at the listing, avoids that.

Validation

  • Head 0c3fdd0 (after merging origin/main f48bc94): quest gate just check origin/main passed every lane, including test rs, test js and test gh.
  • quest check --base origin/main passes.
  • Merging origin/main d505e96 resolved one conflict in rs/moq-net/MIGRATE.md by keeping both entries; moq-dev/moq#8024 then reserved MIGRATE.md for the release cut, so 0c3fdd0 restores all three MIGRATE.md files to main and the notes moved into Impact. No new main code used the changed APIs outside what the merge already compiled.

Review

mechanical fix by codex / Codex: Merged base and migrated six test origin constructors from Hop to NonZeroU64, retaining hop ID 1; passed LIBCLANG_PATH=/nix/store/5ajx1np9zlq7z6ljqq18sibpzhnjakdw-clang-22.1.8-lib/lib cargo nextest run --locked --tests --examples --benches -p moq-audio -p moq-c -p moq-crypto -p moq-ffi -p moq-mux -p moq-net -p moq-net-tests -p moq-relay -p moq-room -p moq-tokio --features 'moq-net/async moq-net/fuzz moq-mux/fuzz moq-crypto/e2ee moq-crypto/sign moq-relay/test-support moq-tokio/driver moq-tokio/ne (head 24c6150)

Codex (quest review), gpt-6.1-sol at medium effort, round 1 on head 9fa4c59 (the hand-resolved merge of d505e96): no findings.

Head 0c3fdd0 adds only a conflict-free merge of f48bc94 and a commit restoring the three MIGRATE.md files to main (deleting this PR's notes, no code); review skipped as too trivial to need one.

Pullfrog's two threads on Source::Peer(0) (r4490770881, r4490770917): fixed by applying its suggested patch unchanged in 0dcd78f.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 6 commits November 5, 2026 22:28
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The NonZero hop ids give the origin handles a niche, shrinking the
handshake steps enough that clippy flags the size gap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Transit facts stay read-only feat(net)!: transit facts stay read-only Nov 6, 2026
@kixelated
kixelated marked this pull request as ready for review November 6, 2026 09:00

@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: 710deba
Base: 1a6ac47

Findings: No actionable regression found in this static review.

Coverage: Reviewed the Rust/JS API changes and updated callers across the 155-file diff, concentrating on nonzero local and peer identities, received zero/anonymous hops, the 32-hop cap and relay validation, lite/IETF session conversions, Rust/C/FFI route projections, the boxed Connect::Started state, JS public listing projection, and the migration/API documentation. The Rust default random-id range is preserved; larger library hop limits are clamped before use while the relay still rejects values outside 1–32. JS keeps epoch/auth in its internal route representation and returns a separate public snapshot with copied hop ids and derived anonymity, retaining internal change detection.

Direction: The internal-wire/public-view separation matches the stated API goal. Keep the documented breaking migrations and the existing loop, hop-limit, wide-id, and anonymity regression coverage. No additional code change is requested by this review.

Validation limits: GitHub-only source review. I did not execute quest context, just check, tests, compilation, benchmarks, or all-language interoperability checks. The PR reports 10,952 passing Rust tests after the boxing fix and a separately blocked benchmark-lock check; those results were not reproduced here. The exact-head Check workflow is still in progress. This COMMENT is not an approval or a claim that all validation is complete.

@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.

ℹ️ One documentation inaccuracy, anchored inline.

Reviewed changes

  • Rust hop types leave the public API. Hop, Hops and InvalidHop are pub(crate), origin::HopLimit is gone, and applications now pass and read NonZeroU64/u64: Config::{hop, max_hops}, Producer/Consumer::hop(), RouteInfo::hops() (an allocation-free ExactSizeIterator), Source::Peer(u64), and the peer-hop setters on Client, server::Handshake and moq-tokio.
  • The 32-hop cap moves into the origin. Producer::new clamps Config::max_hops (a plain NonZeroUsize) to the wire's MAX_HOPS, while moq-relay keeps rejecting --cluster-max-hops outside 1 to 32, so the CLI contract is unchanged.
  • JS splits the internal route from the public facts. WireRoute keeps epoch, auth and branded hops for the table and sessions; Origin.RouteInfo is { instance, hops: bigint[], cost, anonymous }, projected by routeInfo() only in Announce.Consumer.next(). All four public announced() entry points (origin.ts, connection/pool.ts, lite/connection.ts, ietf/connection.ts) reach it through announce.list(), so the projection cannot be bypassed, and the new announced.test.ts case pins the exact key set.
  • Bench/test builders shorten to plain ids. The hidden fuzz helpers (route_info, excluding, Announced) take ids, and the ~100 config sites in tests and benches move to NonZeroU64::new.
  • Bindings and docs. moq-ffi/moq-c already exposed hops: Vec<u64> plus anonymous, so only their internals changed; MIGRATE.md entries cover moq-net, moq-tokio and @moq/net, and the finished quest and its epic are deleted with prose links pinned to their last main revision.

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/origin.rs Resolved
Comment thread rs/moq-net/src/model/origin.rs Resolved
kixelated and others added 2 commits November 6, 2026 07:55
…ute/facts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Applies the Pullfrog suggestion on #8007.

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

@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.

✅ No new issues found.

Reviewed changes

  • Source::Peer(0) doc corrected. The comment now says 0 means the announcing handle named no hop, covering both a peer handle announced in-process and a session whose peer declared no hop and was assigned none — the substantive concern from the prior review's two inline threads, which the author resolved.
  • origin/main merged in. The one conflict resolution in rs/moq-net/MIGRATE.md keeps both Unreleased entries: the PR's Hop chains are plain ids section and main's new track::Consumer::unmetered section, ahead of the existing Socket readers start at the beginning entry.

The PR's own changed lines are unchanged since the previously reviewed head, and the merge replayed cleanly everywhere else (the with_peer_hop/peer_hop signatures in moq-net and moq-tokio are intact at head).

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

kixelated and others added 3 commits November 6, 2026 09:12
# Conflicts:
#	rs/moq-net/MIGRATE.md
Main now reserves MIGRATE.md for the release cut.

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

pullfrog Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Diagnosed the test js failure as a pre-existing load-bound flake, not a regression from this PR, and preserved the fix in moq-dev/moq#8040.

Diagnosis. a session close right after a datagram track's end keeps the end (lite-06) aborts the datagram track with Session: remote error: 0. Instrumenting Subscriber.close showed the entry's ended was still unset at close time: the subscriber had not read the DATAGRAMS_INFO FIN within the test's fixed 1000-microtask drain, so the close aborted the producer instead of finishing it. The same file fails on unchanged main (4 of 8 loaded runs, vs 7 of 8 on this branch), so it is not introduced here.

Fix. Quarantined the case with test.skip (assertions kept) and added the follow-up quest quest/m0/test-flakes-2/lite-datagram-end-close.md with its epic Plan note and ## Required bullet. bun test --only-failures in js/net reports 3044 pass, 6 skip, 0 fail.

Could not push to this PR. The commit is ready, but the push was rejected:

remote: error: GH006: Protected branch update failed for refs/heads/quest/m1/api-audit/route/facts.
remote: - A pull request for this branch has been added to a merge queue. Branches that
remote:   are queued for merging cannot be updated. To modify this branch, dequeue the
remote:   associated pull request.

So the quarantine landed as a separate PR against main instead: moq-dev/moq#8040. Once it merges (or this PR is dequeued and updated with main), this check should pass.

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

@kixelated
kixelated added this pull request to the merge queue Nov 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 6, 2026
@moq-bot

moq-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at 0c3fdd0 because its merge_group build failed (failed_checks).

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.

Queue / Queue (job) failure at step Check:

…calhost:4443/test negotiated ALPN: moq-lite-07-wip
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: publish ok: broadcast=room track=video
@moq/net test: subscribe update: id=0 broadcast=room track=video
@moq/net test: subscribe update: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=remote error: 1
@moq/net test: subscribe update: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: https://localhost:4443/test negotiated ALPN: moq-lite-07-wip
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe update: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: publish ok: broadcast=room track=video
@moq/net test: subscribe update: id=0 broadcast=room track=video
@moq/net test: subscribe update: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=remote error: 1
@moq/net test: publish error: broadcast=room track=video error=session closed
@moq/net test: subscribe update: id=0 broadcast=room track=video
@moq/net test: subscribe update: broadcast=room track=video
@moq/net test: publish error: broadcast=room track=video error=session closed
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: 
@moq/net test: ::endgroup::
@moq/net test: 
@moq/net test: ::group::src/lite/refresh.test.ts:
@moq/net test: https://localhost/refresh negotiated ALPN: moq-lite-06
@moq/net test: announced: prefix=
@moq/net test: announce: prefix=
@moq/net test: announce: broadcast=room active=true instance=0fdd4226efaf8139dde72073c7227e6e
@moq/net test: announced: broadcast=room active=true instance=~aa2b8866a340eb1cabcb91f77fd73f59
@moq/net test: track info: broadcast=room track=data
@moq/net test: subscribe start: id=0 broadcast=room track=data
@moq/net test: track info: broadcast=room track=data
@moq/net test: subscribe ok: id=0 broadcast=room track=data
@moq/net test: publish ok: broadcast=room track=data
@moq/net test: subscribe update: id=0 broadcast=room track=data
@moq/net test: subscribe update: broadcast=room track=data
@moq/net test: publish error: broadcast=room track=data error=session closed
@moq/net test: subscribe close: id=0 broadcast=room track=data
@moq/net test: https://localhost/refresh negotiated ALPN: moq-lite-07-wip
@moq/net test: announced: prefix=
@moq/net test: announce: prefix=
@moq/net test: announce: broadcast=room active=true instance=11aab110491adb61c0b83325838228ca
@moq/net test: announced: broadcast=room active=true instance=11aab110491adb61c0b83325838228ca
@moq/net test: track info: broadcast=room track=data
@moq/net test: subscribe start: id=0 broadcast=room track=data
@moq/net test: track info: broadcast=room track=data
@moq/net test: subscribe ok: id=0 broadcast=room track=data
@moq/net test: publish ok: broadcast=room track=data
@moq/net test: subscribe update: id=0 broadcast=room track=data
@moq/net test: subscribe update: broadcast=room track=data
@moq/net test: subscribe update: id=0 broadcast=room track=data
@moq/net test: subscribe update: broadcast=room track=data
@moq/net test: publish error: broadcast=room track=data error=session closed
@moq/net test: subscribe close: id=0 broadcast=room track=data
@moq/net test: 
@moq/net test: ::endgroup::
@moq/net test: 
@moq/net test: ::group::src/lite/track_stream.test.ts:
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: subscribe start: id=0 broadcast=room track=video
@moq/net test: subscribe ok: id=0 broadcast=room track=video
@moq/net test: subscribe close: id=0 broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: fetch error: broadcast=room track=video group=0 error=The operation was aborted.
@moq/net test: track unknown: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: track info: broadcast=room track=video
@moq/net test: 
@moq/net test: ::endgroup::
@moq/net test: 
@moq/net test: ::group::src/lite/tracks.test.ts:
@moq/net test: tracks failed: broadcast=room error=track listed twice: a
@moq/net test: tracks failed: broadcast=room error=unlisted track removed: a
@moq/net test: tracks failed: broadcast=room error=duplicate SYNCED
@moq/net test: tracks failed: broadcast=room error=remote error: 7 (too many listed tracks)
error: recipe `test-changed` was terminated on line 58 by signal 15
##[endgroup]
##[error]ci: test rs failed
error: recipe `ci` failed on line 115 with exit code 101
##[error]Process completed with exit code 101.
##[group]Run actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9
with:
  name: cargo-timings
  path: target/cargo-timings/cargo-timing-*.html
  if-no-files-found: ignore
  retention-days: 14
  compression-level: 6
  overwrite: false
  include-hidden-files: false
  archive: true
env:
  BASE: 1ae5c5f808f5fb695a42dcb451611edce81802d7
  CHECK_FAIL_FAST: 1
  NEXTEST_TEST_THREADS: 24
  MBX_CACHE_LINKS: 1
  MBX_REMOTE_URL: 
##[endgroup]
With the provided path, there will be 1 file uploaded
Artifact name is valid!
Root directory input is valid!
Uploading artifact: cargo-timings.zip
Beginning upload of artifact content to blob storage
Uploaded bytes 100726
Finished uploading artifact content to blob storage!
SHA256 digest of uploaded artifact is a8e8205a194580509184f67f422cb19f63f71557bc62e7e625e4149356e02a15
Finalizing artifact upload
Artifact cargo-timings successfully finalized. Artifact ID 13537595803
Artifact cargo-timings has been successfully uploaded! Final size is 100726 bytes. Artifact ID is 13537595803
Artifact download URL: https://github.com/moq-dev/moq/actions/runs/42283341157/artifacts/13537595803
Post job cleanup.
##[start-action display=Run jdx/mr-boxington-action@88acf2bb50552c7c508cbac13e5f8c576b95b1de;id=__612f4876-bf14-4306-a508-c2344df7d64e.mbx]
##[end-action id=__612f4876-bf14-4306-a508-c2344df7d64e.mbx;outcome=skipped;conclusion=skipped;duration_ms=0]
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/10cd2d2e-5069-4591-866b-c3d997544aa6' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/moq/moq
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 658 objects and 91 action results (12.7 GiB logical); 49.8 GiB logical remain
dropped 11 session event streams
Cleaning up orphan processes

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 6, 2026 20:16
@moq-bot

moq-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

codex / Codex land fixer note:

The queue failed to compile six test call sites after this PR changed origin::Config::new to accept NonZeroU64. Base merged cleanly; the mechanical fix updates those six constructors while retaining hop ID 1. No production behavior was changed.

Validation passed: 7,520 Rust tests across all ten Rust packages changed by the PR, JavaScript net build/check and 3,060 tests, JavaScript room build/check and 19 tests, formatting and whitespace checks. The Rust build used the installed libclang via LIBCLANG_PATH.

Intermittent failures observed during validation: JavaScript lite-07 answers a bounded window that resolves empty and FINs in reply failed once, then passed alone and in a full net rerun. Under plain cargo test, model::track::test::pool_debt_sheds_a_flight and model::track::test::pool_debt_sheds_a_flight_without_its_cached_group failed; both passed under the CI nextest runner. These tests were left unchanged.

(written by codex / Codex)

@moq-bot

moq-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

quest land watch: armed auto-merge on this pull request at 24c6150, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex / Codex): Merged base and migrated six test origin constructors from Hop to NonZeroU64, retaining hop ID 1; passed LIBCLANG_PATH=/nix/store/5ajx1np9zlq7z6ljqq18sibpzhnjakdw-clang-22.1.8-lib/lib cargo nextest run --locked --tests --examples --benches -p moq-audio -p moq-c -p moq-crypto -p moq-ffi -p moq-mux -p moq-net -p moq-net-tests -p moq-relay -p moq-room -p moq-tokio --features 'moq-net/async moq-net/fuzz moq-mux/fuzz moq-crypto/e2ee moq-crypto/sign moq-relay/test-support moq-tokio/driver moq-tokio/ne.

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 6, 2026

@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

just bench-fanout no longer builds on the current tree. The out-of-workspace bench/fanout/session harness still imports the now-private moq_net::Hop, so its build fails and bench/run.sh aborts the whole fan-out run for the current tree.

Reviewed changes

  • Merged the newer main. 08dcd58ea merges 1ae5c5f8 conflict-free: the three MIGRATE.md files stay byte-identical to main (so main's new Fill::wanted_changed entry arrives without resurrecting this PR's removed notes), and the only shared-file change is main's err.retryable() route-failure path.
  • Migrated the merged test origins. 24c6150 moves the six origin::Config::new(crate::Hop::new(1).unwrap()) calls that the merge reintroduced in ietf::publisher, lite::publisher and lite::subscriber tests to std::num::NonZeroU64::new(1).unwrap(), matching the PR's API.

⚠️ The fan-out session harness no longer compiles

bench/fanout/session/src/main.rs still does use moq_net::{… Hop …} and origin::Config::new(Hop::new(hop).unwrap()). Hop is crate-private in this PR, so cargo check there fails with error[E0603]: struct 'Hop' is private. bench/run.sh reports a base revision's build failure as unsupported, but a failure in the current tree is fatal, so just bench-fanout (default scenarios model session) stops before measuring anything. This break came with the API change itself rather than the last two commits, but it is the one consumer the migration missed.

Technical details
# Fix the out-of-workspace fan-out harness after `Hop` became private

## Affected sites
- `bench/fanout/session/src/main.rs:11` — `use moq_net::{Client, Hop, Server, ...}` no longer resolves; `Hop` is `pub(crate)` (`rs/moq-net/src/model/mod.rs:88`).
- `bench/fanout/session/src/main.rs:16` — `origin::Config::new(Hop::new(hop).unwrap())` must pass a `NonZeroU64`.
- `bench/run.sh:1133-1137` — a failed build under `$RUN/fanout/current/` prints the log and `return 1`; `bench/run.sh:142` defaults `FANOUT_SCENARIOS` to `model session`.
- `bench/run.sh:1108-1120` rewrites the harness's `../../../rs/` paths to the checkout under test and copies that revision's `Cargo.lock`, so the harness is expected to build against this revision's `moq-net`.

## Required outcome
- `bench/fanout/session` builds against this revision's `moq-net`, so `just bench-fanout` once again measures the `session` scenario on the current tree.
- No other file under `bench/` uses the removed hop API; `bench/fanout/{group,model}` are clean.

## Suggested approach
- Drop `Hop` from the `moq_net` import and use `origin::Config::new(std::num::NonZeroU64::new(hop).unwrap())`, matching the other migrated call sites.

## Verification
- `cd bench/fanout/session && cargo check --offline` reproduced `E0603: struct 'Hop' is private` at `24c6150`.

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

Merged via the queue into main with commit f2f9bc9 Nov 6, 2026
9 of 10 checks passed
@moq-bot
moq-bot Bot deleted the quest/m1/api-audit/route/facts branch November 6, 2026 21:22
kixelated added a commit that referenced this pull request Nov 6, 2026
…iber tests

Main's #8007 changed the argument from Hop; these two tests came from this branch.

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