Repository navigation
test(ietf): verify pinned Moxygen conformance under a relay profile - #6497
Conversation
Quest-Claim: 26ad77ae414c5d55
Co-Authored-By: GPT-6 <codex@openai.com>
kixelated
changed the title
The pinned client now accepts the track's advertised priority on every group and one END_OF_TRACK directly after the last object, and keeps every other upstream check. Both single-relay cells pass on drafts 16 and 18. The quest's Goal moves to that profile; the cluster's metadata query and the direct draft-18 control remain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
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: 9cfeb323b0
ℹ️ 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".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe change adds a pinned Moxygen package with a relay conformance profile, a harness for running selected conformance cases, and tests for relay routing. The harness covers drafts 16 and 18 with direct-origin controls, single-relay runs, and two-relay cluster runs. New integration tests exercise routing through one relay and a lite cluster. A reusable workflow runs the harness nightly and retains available reports and endpoint logs. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (11 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title concisely identifies the main change: testing pinned Moxygen conformance with a relay profile. It matches the changeset. 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. Description check The pull request description directly explains the Moxygen pin, conformance profile, nightly workflow, integration tests, measured results, validation, and remaining work described in the changeset.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (11 skipped: 11 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ 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.
Comment @coderabbitai help to get the list of available commands.
Two-relay cells fail every case until a pinned test publisher answers TRACK_STATUS, which would keep the whole nightly red and mask unrelated regressions. Run the direct-origin controls and the single-relay cells, which pass under the profile, and track the cluster cell in the quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merging. Adds the Moxygen conformance profile; the nightly gates on the direct and single-relay cells until upstream Moxygen answers TRACK_STATUS, as the maintainer chose. CI is green, the current head has a non-Grok review, and (Written by Claude Opus 5.5) |
Problem
The passthrough quest required pinned Moxygen sections 1-7 to pass through one relay and a two-relay cluster. The literal suite conflicts with two settled relay behaviors (track-level egress priority, and END_OF_TRACK), and its publisher cannot answer the cold metadata query the cluster sends. This PR lands the pinned harness, verifies under a relay conformance profile instead of the literal suite, gates the nightly on the cells that pass, and rewrites the quest to the work that remains.
Advances quest/m1/ietf-extension/README.md; the quest stays open with its remaining work.
Approach and consumer
c27ae6358df62a1195da479b2e07c50dd8174279and its six matching dependency sources with fixed Nix hashes. Reuse upstream's standalone build, add its missing Folly ManualExecutor link dependency, and fix dependency script interpreters for Nix.nix/moxygen-profile.patch, client verifier only). It also accepts the track's advertised publisher priority (200) on any group, besides upstream's per-group value. It accepts one END_OF_TRACK on the subscription directly after the last object, either inline or opening the next group, and skips the END_OF_GROUP header expectation for that terminal stream, which ends no payload group. Any other priority, a second END_OF_TRACK, or one anywhere else still fails. Every other upstream check is unchanged: object IDs, properties, payloads, END_OF_GROUP status objects and header bits, FIRST_OBJECT.just test ietf-conformanceand a nightly workflow. Each draft runs a direct-origin control and one relay. The client uses the actual 16-field namespaces under the publishedmoq-test-00prefix. Two-relay cells are not run until a pinned publisher answers TRACK_STATUS (below); the quest records adding them back.SKIP_FETCH. Explicitly exclude only modes 1/2, whose cases use nonzero subgroups. Keep all selected arguments, deadlines, and assertions. Refuse changed selection anchors, a crash after SUCCESS, and publisher reconnects.ietf-conformancejob, running the pinnedmoqtest_client/moqtest_server. Ordinary checks and shells do not build the opt-in C++ package.Measured conformance
Six consecutive runs, all passing:
Before the profile, both single-relay cells failed 29/29 on priority and terminal status.
just test ietf-conformancenow exits 0.Remaining in the quest:
a_refused_track_status_never_subscribesforbids a SUBSCRIBE fallback, and moq-dev/moq#6465 owns the broader cold-metadata boundary. The mock-clock routing test still covers the lite-07 cluster path.Group Mismatch: Actual=1 Expected=0without a relay. It has not recurred in six runs since. The verifier expects groups in order although each arrives on its own stream, which its own source comment calls a spurious failure.Decisions
Revisit per-group priority transport and terminal representation(not chosen).Validation
just test ietf-conformance: table above, exit 0; selection tests passed (29 cases selected).shellcheckonrun.shpassed.just check: JS, Rust, quest, drill, OBS compile and OBS checks passed. The unchanged runner-image flake check fails on this host becausesystemd-repartcannotunshareits UID map (Operation not permitted); CI runs it.just test interop --all: all checks passed (172 PASS lines, 0 FAIL).Public API and wire impact
None. Production code, catalog formats, wire encoding, priorities and terminal behavior are unchanged. No migration entry is needed.
Follow-ups
Kept in the quest's Plan rather than new quests, per the maintainer: a Moxygen revision whose test publisher answers TRACK_STATUS (then bump pins and rebase the profile), and resolving the order-sensitive direct draft-18 control at the verifier.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code