[kixelated]

Problem

Several relay io_uring tests run outside nextest's ring concurrency group, so the parallel tests of one just ci can exhaust the shared RLIMIT_MEMLOCK. When a worker does hit the limit, the actionable error is lost: Ready::wait reports only relay stopped before becoming ready: channel closed, and fixtures that skip the wait time out instead.

Approach

• Group every moq-relay test with uring as a whole name segment ((^|[:_])uring([:_]|$)), including unit tests, so a new test joins by its name. Words like "during" no longer match. The four-test limit is unchanged.
• Fix the lost error at its source. Relay::run records its startup error in the readiness watch before returning, and Ready::wait returns that error's full chain. This covers every embedder and fixture, including moq-cli's and moq-studio's.
• Every ring-opening relay fixture waits on Ready before clients connect, so a refused worker fails with its own error instead of a timeout.
• A new isolated-process test lowers RLIMIT_MEMLOCK to 0. It requires RLIMIT_MEMLOCK, ulimit -l, and LimitMEMLOCK= to reach the waiter, and it skips loudly under CAP_IPC_LOCK, which lifts the limit.
• The quest is finished, so it is removed.

Impact

• Public API: no signature change. relay::Ready::wait's error now carries the startup error's chain instead of only "channel closed".
• Wire: none.

Choices made without asking

• /quest-iterate replaced the earlier private start_relay test helper, which lib.rs compiled from tests/ through #[path], with the Ready fix above, following the local review's root-cause finding. Fixtures use the documented ready() / spawn(run()) / wait() pattern.
• Tightened the group regex after cargo nextest show-config test-groups showed a_signal_during_issuance_stops_startup and a_stop_during_startup_skips_readiness in the group.

Validation

• quest gate nix develop --command just ci on 5e98f21623: passed, 10995 tests, 0 failed. The mbx E2BIG was avoided by deduplicating PATH in the shell first, the same line moq-dev/moq#7594 adds to flake.nix. check flake was not selected for this diff.
• just rs test -p moq-relay --features io-uring,qlog: 441 passed, 9 skipped.
• cargo nextest show-config test-groups with moq-relay/io-uring,moq-relay/qlog: 33 relay tests in io-uring, all of them uring tests, with no "during" match. The pending bitrate_caps::uring_* names from moq-dev/moq#7535 match.
• Mutation: with the failure record removed from Relay::run, both relay::tests::a_startup_failure_reaches_the_waiter and runtime_uring::uring_startup_reports_the_memlock_limit fail with channel closed.

Review

Round 1 used Claude Code, claude-opus-5-5, medium effort, on 94ada66c5f, chosen because Codex wrote the original implementation. It reported 3 findings, all fixed:

• medium, start.rs: the startup error was kept only in a test helper, while Ready::wait still dropped it. Fixed at the source in Relay::run and Ready::wait, and the helper was deleted.
• low, lib.rs: the library compiled an integration-test file through #[path]. Fixed by the same change, which removes the include.
• medium, runtime_uring.rs: the memlock test fails under CAP_IPC_LOCK, which root holds. Fixed: the test now skips when CapEff bit 14 is set.

Round 2 used Claude Code, claude-opus-5-5, medium effort, on 5e98f21623601f05ef558577af2903861e014418 with the responses above: 0 findings.

Because the Ready fix is Claude's own code, Codex also reviewed the same head (gpt-6.1-sol, medium effort, 5e98f21623601f05ef558577af2903861e014418): 0 findings.

Follow-ups

None.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Relay startup readiness now distinguishes startup, serving, and failure. If startup fails before serving, readiness waiters receive the recorded error. Relay tests now wait for readiness before proceeding, and the io_uring integration tests include coverage for a zero RLIMIT_MEMLOCK startup failure. The Nextest filter and related CI planning text also changed.

Merge Risk: Minimal · up to 5e98f

Relay startup failures now reach readiness waiters with their error text, and the relay tests wait for readiness before connecting clients. No concrete merge-blocking risk was found in this change.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes both primary changes: reporting relay startup errors through Ready and grouping all io_uring tests.
Description check		The description is directly related to the changeset and explains the io_uring test grouping, startup error propagation, readiness updates, test coverage, and validation results.
Docstring Coverage		Docstring coverage is 88.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 88.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 25 functions across 8 files. (1 skipped: 1 unsupported.)

Finishing Touches

Simplify code

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[coderabbitai]

Additional comments: 1

rs/moq-relay/src/relay.rs-55-55: 1

55-55: Major · Data Integrity & Integration · Heavy lift

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Preserve the startup error’s cause for readiness waiters.

Startup::Failed(Arc<str>) stores only format!("{err:#}"). Ready::wait therefore creates a new error whose chain does not contain the original cause. An embedder cannot inspect the waiter’s error kind, even when run() retains that kind. Keep a shareable, typed cause or expose a typed startup failure alongside the message. anyhow supports inspecting causes and downcasting them. (docs.rs)

Based on learnings: “Opaque messages and suppressed errors prevent callers from programmatically branching on error kind.”

Source: Learnings

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9fa5feb7-7409-4b76-9764-840b8f45e2c5

Commits

Reviewing files that changed from the base of the PR and between f6de612 and 5e98f21.

Files selected for processing: 11

• .config/nextest.toml
• quest/m0/ci/README.md
• quest/m0/ci/io-uring-test-group.md
• rs/moq-relay/src/relay.rs
• rs/moq-relay/src/uring.rs
• rs/moq-relay/tests/integration/embed.rs
• rs/moq-relay/tests/integration/peer_limits.rs
• rs/moq-relay/tests/integration/refusals.rs
• rs/moq-relay/tests/integration/runtime_uring.rs
• rs/moq-relay/tests/integration/tenant_matrix.rs
• rs/moq-relay/tests/integration/wire_stats.rs

Files with no reviewable changes: 2

• quest/m0/ci/README.md
• quest/m0/ci/io-uring-test-group.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Group every io_uring test by name — .config/nextest.toml replaces the binary_id(moq-relay::integration) filter with package(moq-relay) & test(/(^|[:_])uring([:_]|$)/), so unit tests and integration tests join the io-uring group whenever uring is a whole module/word segment. during no longer matches; the group stays at max-threads = 4.
• Ready carries the startup failure — rs/moq-relay/src/relay.rs switches the readiness watch to Startup::{Starting,Serving,Failed}. Relay::run holds a sender clone past run_inner, records the flattened error chain while still Starting, and Ready::wait returns it instead of only channel closed. No public signature change.
• Fixtures wait before connecting — uring.rs unit tests and the embed, peer_limits, refusals, runtime_uring, tenant_matrix, and wire_stats integration fixtures now ready().wait() before clients connect.
• Memlock regression test — uring_startup_reports_the_memlock_limit isolates its process, skips under CAP_IPC_LOCK, zeroes RLIMIT_MEMLOCK, and asserts the actionable error reaches the waiter.
• Quest removed — quest/m0/ci/io-uring-test-group.md and its README line are gone with no dangling references.

I traced the watch ordering (send_if_modified guard, sender clone surviving run_inner), the run_inner destructure, the admissions ensure! polarity, the deterministic io_uring worker 0 failure ordering, and the filter's coverage of every config.runtime.io_uring = true test (the runtime_uring:: module name matches even where the test name does not contain uring). All check out.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 5e98f21623601f05ef558577af2903861e014418.

No actionable bugs identified. Reporting startup failure through the shared Ready handle addresses the root cause for embedders, rather than hiding it in test helpers. The sender lifetime and startup-only transition preserve readiness semantics; the package-scoped whole-segment grouping and explicit fixture waits keep the change small without increasing the concurrency limit. No public signature or wire change.

Verification limits: static inspection of the diff and relevant startup/worker code only. I did not run tests, builds, or io_uring/memlock experiments.

(Written by OpenAI)