docs(quest): plan the LAN app id, one LAN mesh, and the relay verb - #3590
Conversation
Four quests replace the moq-token one. The mDNS discovery gains an app id (a DNS-SD subtype bound into the proofs) so unrelated apps on one network never meet; moq-cli and moq-relay stop running two incompatible LAN meshes by sharing the relay's Cluster; `moq relay` hosts the relay under a verb while the minimal binary stays published; and the CLI's listener becomes the relay's serve path so the only difference left is the listening default. The moq-token quest is abandoned: minimal binaries stay on purpose, so retiring one contradicts the direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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: 3e57993842
ℹ️ 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".
| library and a `Command::Relay(moq_relay::Config)` verb, answered before any | ||
| transport binds like `token` and `completion`, with `MoqSide::reject` |
There was a problem hiding this comment.
When a user runs moq relay relay.toml, nesting moq_relay::Config only sets Config.file; the actual file read and precedence merge currently happen exclusively in Config::load through parse_and_merge (rs/moq-relay/src/config.rs:167-177, 222-230), while Relay::load only resolves an already-built config. Following this plan therefore ignores the TOML, and calling Config::load afterward cannot work because the process argv still contains the relay verb. Add a composable merge step for the parsed config and cover the same TOML invocation through both binaries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e9b9dcd: the plan splits the TOML read and precedence merge out of Config::load into a step that takes a parsed Config, shared by both binaries, with the same TOML invocation tested through each.
(written by Claude Opus 5)
| - `mdns::App`: a newtype over 1..=63 lowercase ASCII letters, digits, and | ||
| hyphens, with `FromStr` and `Display`, refusing anything else at parse time. |
There was a problem hiding this comment.
A 63-character app produces a 64-byte first DNS label because the subtype is encoded as _<app>. DNS labels are limited to 63 octets, so this value passes the proposed FromStr validation but fails later when registering or browsing the service. Limit the app portion to 62 characters and add boundary tests so the malformed value is refused at parse time.
AGENTS.md reference: AGENTS.md:L59-L59
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e9b9dcd: 1..=62, with the boundary test called out.
(written by Claude Opus 5)
| - [LAN discovery app id](/quest/m1/lan-app.md) - every advertisement names an application as a DNS-SD subtype bound into the proofs, so unrelated apps on one network never meet | ||
| - [One LAN mesh](/quest/m1/lan-mesh.md) - moq-cli drives the relay's Cluster, LAN peers authenticate by mDNS credential, and the two binaries mesh with each other |
There was a problem hiding this comment.
These quests rely on code that exists only on dev, but their described API break is explicitly unpublished and their remaining changes are additive or wire-only. They therefore fit m2's documented rule for work that starts on main after the merge, rather than m1 work that must gate the merge. Keeping both ready entries ahead of Merge dev makes quest priority order delay that merge unnecessarily; move them to m2 and make the first one require Merge dev.
AGENTS.md reference: AGENTS.md:L86-L88
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declined, the maintainer placed these in m1 deliberately. The merge is what publishes moq_tokio::mdns and the peer contract (/.cluster/<credential>, the advert fields); breaking either afterwards costs a dev cycle plus a relay/CLI compatibility cycle, which is exactly the cost the m1 rule exists to avoid. They rank below the merge gates in the list, so they do not delay the merge.
(written by Claude Opus 5)
…TOML Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9b9dcdbdb
ℹ️ 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".
| - moq-cli gains a default-on `relay` feature depending on the moq-relay | ||
| library and a `Command::Relay(moq_relay::Config)` verb, answered before any |
There was a problem hiding this comment.
Making moq-relay a default-on dependency raises every moq-cli build to Rust 1.95: rs/moq-cli/Cargo.toml:10 inherits the workspace's 1.91 declaration, while rs/moq-relay/Cargo.toml:10-15 explicitly requires 1.95 because of sysinfo. Consequently, users building the CLI with its advertised minimum toolchain will be rejected by Cargo. Include either a moq-cli MSRV bump or a split/feature arrangement that keeps the reusable relay dependency compatible with 1.91.
Useful? React with 👍 / 👎.
| layers in one step (`rs/moq-relay/src/config.rs`, `parse_and_merge`), so a | ||
| nested `Config` would carry only the flags and ignore its TOML. Split the | ||
| file read and precedence merge into a step that takes an already-parsed | ||
| `Config`, used by both `Config::load` and the verb, and test the same TOML |
There was a problem hiding this comment.
The composable step must also settle logger ownership, not only merge TOML. moq_relay::Config contains its own log field, and Config::load currently initializes it at rs/moq-relay/src/config.rs:164-180, whereas moq initializes the separate outer cli.log before dispatching commands at rs/moq-cli/src/main.rs:185-187; thus moq relay --log-level debug or a relay TOML log setting will either be ignored if only the outer logger is initialized, or fail if both call the process-wide, single-use Log::init. Plan and test a single initialization using the nested relay value so the advertised relay flags are supported rather than silently discarded.
AGENTS.md reference: AGENTS.md:L59-L59
Useful? React with 👍 / 👎.
1 participant
Summary
Plans four quests and abandons one, from a
/plan-questsinterview.quest/m1/lan-app.md): every mDNS advertisement names an application as a DNS-SD subtype (_<app>._sub._moq._udp.local., RFC 6763 §7.1, supported by mdns-sd 0.20) and the app is folded into both HMAC proofs, so unrelated apps on one network never resolve each other and one secret reused across two apps still yields two meshes.mdns::Config::new(app, port),--cluster-lan-app,[cluster.lan] app, shared defaultdefault.quest/m1/lan-mesh.md):moq --cluster-lanandmoq-relay [cluster.lan]cannot connect today (the CLI dials/.cluster/<credential>with a pinned fingerprint and no node; the relay skips node-less peers, dials with?jwt=cluster.token, and never reads the credential). The CLI drivesmoq_relay::Clusterinstead, LAN peers authenticate by mDNS credential,cluster.tokennever reaches a LAN peer, and the secret is optional on both.quest/m2/moq-relay-subcommand.md):moq relaynests the relay library the waymoq tokennests moq-token-cli;moq-relaystays a minimal binary. Requires the dev merge (usage lives on dev).quest/m2/cli-serve.md):moq --listenuses the relay's serve path (auth, scoping, hops, stats, drain), so the only difference left is the listening default.quest/m1/3046-...(retire the moq-token binary). Minimal binaries stay published on purpose, so the quest contradicts the direction. Fold moq-token into moq token via a Usage executable view #3046 is closed as won't-do and loses thequestlabel.quest/m2/cluster-flags.mdreconciled: the mesh quest deletes the--cluster-lanneeds--cluster-node/--cluster-lan-secretchecks it listed, so it now requires that quest.Targets
devlike #3564: the groomed m1 index and the mDNS code exist only there.Public API and wire impact
None; documentation only. The quests themselves plan a break to the unpublished
moq_tokio::mdns::Config::newand a new mDNS subtype plus TXT-proof binding.🤖 Generated with Claude Code
(written by Claude Opus 5)