Skip to content

feat(relay): merge config with recorded provenance - #3587

Merged
kixelated merged 4 commits into
devfrom
quest/m1/config-provenance
Sep 12, 2026
Merged

kixelated merged 4 commits into
devfrom
quest/m1/config-provenance

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

  • moq-relay and moq-bench merge CLI, env, and TOML with recorded provenance. Presence comes from CliLayer / EnvLayer / the file, never from whether a standing value looks empty, so a file that sets [] or false survives.
  • Precedence is CLI > env > file > defaults, declared in moq_tokio::cli::merge. Env now outranks the file.
  • Dual declaration: setting = "dotted.key" on Args plus a matching usage::Config field; Registry::drift tests they stay in step.
  • Dropped the Option<bool> merge workaround for stats.enabled, runtime.pin, runtime.io_uring, web.ws, and cluster.lan.enabled.
  • Table-driven regressions cover empty lists, false booleans, a declared-default true flipped false by file, nested flattened structs, env overrides, and explicit CLI overrides, each asserting value and source.

Closes #3051
Closes #3221

Public API

Breaking:

  • StatsConfig.enabled, RuntimeConfig.pin, RuntimeConfig.io_uring, WebConfig.ws, LanConfig.enabled: Option<bool> to bool.
  • A usage::Cli that flattens a group with setting= needs #[usage(settings)].

Additive:

  • moq_tokio::cli::merge and moq_tokio::cli::FileSource.
  • moq_relay::Config::source and bench Config::source.
  • moq_tokio::settings (doc hidden).

Wire

None.

(written by grok-4.6)

@kixelated
kixelated marked this pull request as ready for review September 10, 2026 08:41
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-12T05:51:16.687807Z 43604c5 New commits
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3645a9ced9

ℹ️ 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".

Comment thread rs/moq-relay/src/auth.rs
Comment thread rs/moq-relay/src/stats.rs
Comment thread rs/moq-tokio/src/cli.rs Outdated
@kixelated

Copy link
Copy Markdown
Collaborator Author

Disclaimer: This review was posted by Grok Bot (an automated assistant), not a human reviewer. Treat findings as advisory — verify before acting.

PR #3587 — feat(relay): merge config with recorded provenance

Summary

Reworks relay/bench (and shared moq-tokio) config merge so presence comes from layers (CliLayer / EnvLayer / TOML keys), not from “does this value look empty?”. Fixes TOML [] / false being overwritten.

Findings

Blockers

None for the PR alone (CI green, mergeable).

Risks

  1. Operational precedence change: env now beats file. Deployments that relied on a TOML file overriding ambient MOQ_* env will silently change behavior. Release / ops note required; PR body states it clearly.
  2. Option<bool>bool is a Rust API break for anyone constructing StatsConfig / RuntimeConfig / WebConfig / LanConfig in-process (tests and embedders). Serde TOML shape is fine; code constructors need updates (PR updates in-tree call sites).
  3. Stacks with feat(relay)!: construct the cluster origin once #3582. Real content conflicts on cluster.rs, cache.rs, web.rs, auth/smoke tests, and doc/bin/relay/config.md (confirmed via merge-tree). Land one, rebase the other — do not merge both raw.

Nits

  1. keep callback in merge is a necessary escape hatch (file path / non-setting fields); worth a one-line comment at each call site listing what it preserves.
  2. Connect/client key aliasing in occupied_keys is clever; a test that CLI --connect-* blocks file client.* (and the reverse) would lock that edge.

CI

Check Result
Check SUCCESS
Test SUCCESS
WASM SUCCESS
Swift SUCCESS
OBS SUCCESS

Verdict

Approve (logic + tests look solid), but sequence after or rebase onto #3582 if both ship, and call out the env>file ops break in the merge note.

kixelated and others added 3 commits September 11, 2026 22:32
Only a CLI or env value that was actually supplied overrides the TOML file.
Precedence is CLI > env > file > defaults, declared in moq_tokio::cli::merge.

Closes #3051
Closes #3221

Co-Authored-By: grok-4.6 <noreply@x.ai>
merge round-trips through TOML and drops #[serde(skip)] fields.
keep_parse_only restored only client_tls, so --auth-public-subscribe,
--auth-public-publish, and --auth-public-api vanished and the relay
started as though authentication were empty.

File provenance now maps bench [client] keys onto connect.* so source()
records the file origin. SNI cert helpers compile only for the backends
that call them, which default-feature clippy requires after the noq default.

Co-Authored-By: grok-4.6 <noreply@x.ai>
@kixelated
kixelated force-pushed the quest/m1/config-provenance branch from 3645a9c to 65e41bf Compare September 12, 2026 05:33
@kixelated
kixelated enabled auto-merge (squash) September 12, 2026 05:33

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65e41bf51c

ℹ️ 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".

Comment thread rs/moq-relay/src/settings.rs
origin/dev's wasm-bindgen-cli is 0.2.127; the crate was still pinned
to 0.2.121, so wasm-bindgen refused the bindgen format.

Co-Authored-By: grok-4.6 <noreply@x.ai>
@kixelated
kixelated merged commit 6461791 into dev Sep 12, 2026
6 checks passed
@kixelated
kixelated deleted the quest/m1/config-provenance branch September 12, 2026 06:01
kixelated added a commit that referenced this pull request Sep 12, 2026
#3587 made stats.enabled a bool, so the construction-time cache test
cannot wrap it in Some.

Co-Authored-By: grok-4.6 <noreply@x.ai>
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