Fix an arg collision with --tls-root and --cluster-root

[kixelated]

And rename --listen to --server-bind

Summary by CodeRabbit

• Chores
  • Updated server configuration naming convention from listen to bind across multiple modules. Environment variable changed from MOQ_SERVER_LISTEN to MOQ_SERVER_BIND. Users must update deployment configurations accordingly.

[coderabbitai]

Walkthrough

This pull request standardizes configuration naming conventions across multiple modules by replacing listen field names and environment variables with bind. Changes include renaming the public ServerConfig.listen field to bind in multiple server implementations, updating initialization logic to reference the new field name, and replacing corresponding environment variable names from MOQ_SERVER_LISTEN to MOQ_SERVER_BIND. Additionally, explicit Clap argument IDs are added to various configuration fields for improved argument identification and metadata consistency. The underlying functionality and default values remain unchanged.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Fix an arg collision with --tls-root and --cluster-root" is partially related to the changeset. The title accurately refers to one significant aspect of the changes: adding explicit Clap argument IDs to resolve potential argument collisions (visible in rs/moq-native/src/client.rs and rs/moq-relay/src/cluster.rs). However, the changeset also includes a substantial but undocumented component: renaming the --listen flag to --server-bind across multiple files (rs/moq-native/src/server.rs, rs/moq-relay files, and configuration systems). While the title captures a real part of the change, it does not represent the full scope or relative significance of the modifications.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cluster-root

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33a081f and c1e4622.

📒 Files selected for processing (7)

• rs/hang-cli/src/server.rs (1 hunks)
• rs/moq-native/src/client.rs (1 hunks)
• rs/moq-native/src/server.rs (2 hunks)
• rs/moq-relay/fly.toml (1 hunks)
• rs/moq-relay/src/cluster.rs (1 hunks)
• rs/moq-relay/src/main.rs (1 hunks)
• rs/nix/modules/moq-relay.nix (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

rs/moq-relay/**

📄 CodeRabbit inference engine (CLAUDE.md)

In the CDN/relay (moq-relay), do not include application logic, media codecs, or track-specific details; keep it transport-agnostic.

Files:

• rs/moq-relay/src/cluster.rs
• rs/moq-relay/src/main.rs
• rs/moq-relay/fly.toml

rs/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Write Rust tests integrated within source files (unit tests alongside code).

Files:

• rs/moq-relay/src/cluster.rs
• rs/hang-cli/src/server.rs
• rs/moq-relay/src/main.rs
• rs/moq-native/src/server.rs
• rs/moq-native/src/client.rs

🔇 Additional comments (8)

rs/moq-relay/src/cluster.rs (1)

17-51: LGTM! Explicit Clap argument IDs properly resolve the collision.

The explicit id attributes on these four cluster configuration fields (cluster-root, cluster-token, cluster-node, cluster-prefix) prevent Clap from deriving IDs from field names. This fixes the collision between ClusterConfig.root and ClientTls.root mentioned in the PR title, where both would have defaulted to id="root" despite having different long names.

rs/moq-native/src/client.rs (1)

17-42: LGTM! Explicit IDs complete the collision fix and improve consistency.

The explicit id = "tls-root" on the ClientTls.root field (line 17) is the other half of the collision fix, preventing conflict with ClusterConfig.root. The reformatting to multi-line blocks for tls-disable-verify and client-bind improves readability while adding explicit IDs for consistent Clap argument identification across the codebase.

rs/moq-relay/src/main.rs (1)

17-17: LGTM! Field access updated to match the renamed ServerConfig field.

The change from config.server.listen to config.server.bind correctly reflects the field rename in ServerConfig, while maintaining the same default binding address [::]:443.

rs/moq-relay/fly.toml (1)

19-19: LGTM! Environment variable renamed to align with the new bind field.

The environment variable key change from MOQ_SERVER_LISTEN to MOQ_SERVER_BIND maintains consistency with the ServerConfig.bind field and its Clap env attribute.

rs/hang-cli/src/server.rs (1)

22-22: LGTM! Field access updated consistently with other modules.

The change from config.listen to config.bind aligns with the ServerConfig field rename, maintaining the same default behavior.

rs/nix/modules/moq-relay.nix (1)

211-211: LGTM! NixOS module updated to use the new environment variable.

The environment variable key change from MOQ_SERVER_LISTEN to MOQ_SERVER_BIND maintains consistency with the field rename across all deployment configurations.

rs/moq-native/src/server.rs (2)

66-68: LGTM! Excellent backward compatibility implementation.

The field rename from listen to bind is well-executed with proper backward compatibility:

• #[serde(alias = "listen")] ensures existing config files with "listen" continue to work
• #[arg(alias = "listen")] ensures the --listen CLI flag remains functional
• id = "server-bind" provides explicit Clap argument identification
• env = "MOQ_SERVER_BIND" aligns with the new naming convention

This approach prevents breaking changes for existing users while establishing the new naming standard.

---

137-137: LGTM! Initialization correctly uses the renamed field.

The change from config.listen to config.bind properly reflects the field rename while maintaining the same default binding address.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}