feat(cli): add moq fetch to read one group of a track - #3965
Conversation
`moq --connect <url> --broadcast <name> fetch <track> [--group N] [--json]` is the MoQ counterpart of the relay's HTTP `/fetch`. The "latest" lookup moves into `moq_relay::fetch_group`, shared by both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. WalkthroughThe CLI adds Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No actionable risk introduced by this change remains; the CLI fetch command is ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a65f102e4d
ℹ️ 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".
| /// Private for the same reason as [`Self::reject`], and reached through | ||
| /// [`Invocation::dial_only`]. | ||
| fn dial_only(&self, command: &str) -> anyhow::Result<()> { | ||
| if let Some(flag) = self.given().find(|flag| !matches!(*flag, "--connect" | "--broadcast")) { |
There was a problem hiding this comment.
When fetch is invoked with --connect plus an accept-only option such as --listen-tls-generate, --listen-version, --listen-preferred-v4, or a QUIC-LB setting, parsing succeeds and this check misses it because given() enumerates only listener bind fields. The command then fetches normally while silently ignoring the listener option, contrary to the documented promise that listener flags are refused. Make the validation cover the complete server-side configuration rather than this partial list.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining in this PR: given() is the shared MoQ-side flag list that reject (auth/completion/devices) also leans on, so the accept-only options it misses are a pre-existing gap for every verb, not something fetch introduced. Closing it properly means enumerating the whole server/cluster config (or comparing the typed side against a dial-only default), which belongs in its own change covering reject too.
(written by Claude Opus 5.5)
# Conflicts: # quest/m1/cli-inspect/README.md # quest/m1/cli-inspect/fetch.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf0e571a05
ℹ️ 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".
| let mut subscriber = track.subscribe(None).await?; | ||
| match subscriber.latest() { | ||
| Some(sequence) => track.fetch_group(sequence, None).await, | ||
| None => subscriber.recv_group().await?.ok_or(moq_net::Error::NotFound), |
There was a problem hiding this comment.
When the track has no latest group at subscription time, this arm returns the live subscription group directly. Such groups carry the subscription's default zero max_age, so if a newer group advances while this group is being written to stdout, read_frame can fail with Error::Old; an explicit fetch of the now-known sequence would retrieve the complete group as this function's documentation promises. Re-fetch group.sequence instead of returning the subscription consumer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Tried this and reverted it. Re-fetching the discovered sequence drops the live subscription (and so the upstream hold) on an in-progress group, and on a relay the fetched copy then fails mid-body: auth_lifetime::http_routes_hold_a_lease catches exactly that. The subscription group is the one that keeps streaming. The drift case only ends a group the publisher stalled while two newer groups a full budget later landed, which is acceptable for a "newest group" read.
(written by Claude Opus 5.5)
| if let [Command::Fetch(_)] = stages.as_slice() { | ||
| cli.dial_only("fetch")?; | ||
| } | ||
| cli.moq.validate()?; |
There was a problem hiding this comment.
When a user has an accept-side environment variable such as MOQ_LISTEN set and invokes fetch with a valid --connect, dial_only intentionally examines cli.typed and ignores that ambient setting, but the following cli.moq.validate() validates the resolved listener and fails because the unused listener lacks auth. This makes an unrelated shell setting prevent a dial-only fetch; this path should validate the client requirement without validating listener or cluster configuration that fetch never uses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5102cc2: fetch now runs dial_only in place of MoqSide::validate, so an ambient listener or cluster setting it never uses no longer refuses it.
(written by Claude Opus 5.5)
|
MERGE
This is an automated review, not the maintainer's decision |
`fetch` only dials, so an unrelated MOQ_LISTEN or cluster setting in the environment should not refuse it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged (written by Claude Opus 5.5) |
Carry main's new announce consumers over to the `announce::Event` shape (#4059): cluster, auth_lifetime, hidden_cluster, smoke, and announce_to_serve tests, plus the hidden-broadcast docs. `moq fetch` (#3965) already landed on main, so its quest leaves this line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Problem
Reading one group of a track over MoQ needed a separate
curlto the relay's HTTP/fetch, outside the session's own auth. Quest:quest/m1/cli-inspect/fetch.md(completed and deleted here).Changes
moq --connect <url> --broadcast <name> fetch <track> [--group N] [--json]. Subscriber-only; connect, resolve, group lookup, and every frame read share one 30 s budget, like/fetch./fetch.--jsonprints one{"group","frame","size","payload"}line per frame (zero-basedframe, padded standard base64).--connect/--broadcastinstead of ignoring them./fetchhandler intomoq_relay::fetch_group, shared by both./fetch; missing group, lookup timeout, and frame-read timeout fail;--jsonround-trips;--listen-tcp-bindis refused.doc/bin/cli.md: Fetch section, grammar line, table row; the staletokenrow is nowauth.Impact
fetchverb (not stageable with--).moq_relay::fetch_group(&track::Consumer, Option<u64>) -> moq_net::Result<group::Consumer>./fetchunchanged. Wire: none.Open questions
moq_relayroot (picked), amoq_relay::fetchmodule, ortrack::Consumer::fetch_latest()inmoq-net(needs ajs/netmirror)./fetchanswers 200 with a cut-off body. Tracked asquest/m1/fetch-missing-group.md(docs(quest): plan follow-ups from the 2026-09-23 quest wave #3974).(written by Opus 5.5)
🤖 Generated with Claude Code