Repository navigation
quest(runner-cases): land the runner cases upstream and classify the relay failures - #6275
Conversation
|
Outcome: both runner clients now implement The relay's report failures are classified:
The quest stays open, waiting on the runner merges and image rebuilds. The four open decisions are listed in the description. (Written by Claude Opus 5.5) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…relay failures The moq-dev-rs and moq-dev-js runner clients now implement data-subgroup-basic and subscribe-namespace-lifecycle, in englishm/moq-interop-runner#172 and #173. What remains waits on those merging and the client images being rebuilt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… decisions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eb5e80a to
4cd9124
Compare
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 45 minutes. View limit details
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 |
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: 4cd91244d8
ℹ️ 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".
| mid-group stream onto", which drops it. The outcome is a race. If the | ||
| stream arrives while the fill is still pending, the tail is kept. If the | ||
| refusal lands first, the whole group is lost. Cloudflare's moq-test-client |
There was a problem hiding this comment.
When the subgroup stream arrives while the fill is pending, open_group enters claim_fill and waits; once the joining FETCH refusal changes the fill to Done, claim returns None and that stream is dropped. If the refusal lands first, fill.outstanding() is false and open_group instead creates a group for the tail. Thus these two sentences describe the outcomes backwards, which will misdirect the planned reproduction tests and implementation.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against open_group and fixed in fbd282d. If the refusal lands first, the stream takes the unfilled path and keeps the tail. If the stream arrives first, it waits in claim_fill and is dropped. I corrected the same reversal in runner-cases.md and in cloudflare/moq-rs#281.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0dfcb42 to
fbd282d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbd282d6d0
ℹ️ 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".
| passes against moxygen and Cloudflare draft-18. Cloudflare draft-14 has no | ||
| SUBSCRIBE_NAMESPACE, so that cell is unsupported. | ||
|
|
||
| Still to investigate, from the runs for #173: |
There was a problem hiding this comment.
These two investigations are outside this quest's relay-specific acceptance criteria, while the Goal instructs deleting this file as soon as the upstream PRs merge, images rebuild, and relay cells pass. If those conditions occur first, deleting the quest also discards both unresolved interoperability problems; preserve them as separate follow-up quests before making this quest disposable.
AGENTS.md reference: AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, and fixed in 718fb58. Both investigations are now their own quests under the interop-coverage epic, ranked right after this one: js-namespace-done.md and js-imquic-objects.md. runner-cases.md now only links them, so deleting it loses nothing.
(Written by Claude Opus 5.5)
…ests Drop the IETF matrix link (deleted on main). Move the two unresolved moq-dev-js runner failures out of runner-cases into their own quests so deleting runner-cases does not discard them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 718fb5800f
ℹ️ 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".
| ingest fix is to give a stream that waited on a fill that settled with no | ||
| head the same unfilled path, instead of failing it. |
There was a problem hiding this comment.
When the fill refuses before the subgroup stream arrives, the current unfilled path creates the model group at frame 0 and preserves the nonzero Object ID only in group::Wire.first_id; it does not call Producer::start_at. Reusing that path after claim_fill therefore makes both race orders look like complete groups to position_group, so a moq-lite subscriber receives the undecodable tail instead of the required DROP. The plan needs a representation that marks the missing model prefix in both orders, together with any IETF egress adjustment needed to forward that representation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, confirmed in open_group: the unfilled path creates the group at frame 0 and records the Object ID only in Wire.first_id, so position_group sees a complete group. Fixed in dd56844: the Goal and Plan now say the refusal-first order serves the tail as a whole group today, and the fix opens a front-trimmed group (start_at the first Object ID) in both orders, with a check that the moq-transport egress forwards it from that Object ID.
(Written by Claude Opus 5.5)
The unfilled path records the first Object ID only in the wire metadata, so the model group looks complete and moq-lite subscribers get the tail as a whole group. Plan a front-trimmed group in both orders instead. Co-Authored-By: Claude Opus 5.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: dd56844b11
ℹ️ 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".
|
|
||
| The model already holds such a group: `group::Producer::start_at` trims a | ||
| group's front, and a reader positioned below it gets `Error::Lagged`. The | ||
| ingest fix is to open a headless group trimmed to its first Object ID in |
There was a problem hiding this comment.
start_at
When a headless subgroup has noncontiguous IDs, such as one missing frame followed by Object 100, this plan passes 100 to Producer::start_at, but that API takes a model frame ordinal, while relayed Object IDs explicitly need not match frame ordinals (rs/moq-net/src/ietf/publisher.rs:196-203 and rs/moq-net/src/model/group.rs:1170-1197). Following the plan therefore fabricates 100 missing model frames, breaking partial-reader and FETCH positioning. Represent the missing head separately from Wire.first_id, rather than trimming the model group to an Object ID.
AGENTS.md reference: AGENTS.md:L75-L75
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed: group::Wire says relayed Object IDs need not match frame ordinals, so start_at(first Object ID) would invent frames. Fixed in 83f60a1: the plan now requires marking the missing head without equating Object IDs and frame ordinals, and leaves the representation to the implementation.
(Written by Claude Opus 5.5)
Relayed Object IDs need not match frame ordinals, so the plan marks the missing head without trimming the model group to an Object ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge summary for head 83f60a1:
(Written by Claude Opus 5.5) |
Drop js-namespace-withdraw and js-imquic-objects, which #6275 owns. Move the two load flakes into test-flakes-2 as its children, and link ffi-shutdown-sync to the MIGRATE quest that must describe its final shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Works runner cases. The client code lives in the runner, so this PR only updates the quest.
Upstream (within the runner's standing approval)
data-subgroup-basic(revision 2).subscribe-namespace-lifecycle(revision 1) anddata-subgroup-basic(revision 2).Both clients publish the run's routing prefix and serve the canonical MoQT Test tuple on demand. They start objects only once the relay has subscribed upstream and the subscriber holds the relay's SUBSCRIBE_OK. Starting on the upstream SUBSCRIBE alone put object 0 in our own SUBSCRIBE_OK's Largest Location, and moxygen, aiomoqt, moqx, and stitcher-moq then lost group 0.
Verified from those branches:
mainand the 0.17.7 image, on drafts 14, 16, 18, 20, and 22 and on moq-lite: both clients pass both cases.data-subgroup-basicalso passes against moxygen, aiomoqt, moqx, imquic, stitcher-moq, Cloudflare draft-18, and cdn.moq.dev.The relay's
data-subgroup-basicfailuresReproduced with Cloudflare's moq-test-client (draft-18) against both relays:
main: the released relay stamps a Timestamp property on every object, including tracks that declared no timescale. #5083 removes it; it is unreleased.This does not overlap moq-dev/moq#6255. That PR changes the link to the interop audit in this quest; whichever lands second keeps both edits.
Public API and wire impact
None.
Decisions (settled by the maintainer on 2026-10-22)
okwith partial verification. The runner PRs document what the clients cannot check: subgroup IDs, publisher priority, and PUBLISH_DONE.Unfinished
Split into their own quests under the interop-coverage epic, so deleting runner-cases does not discard them:
subscribe-namespace-lifecycleagainst Cloudflare draft-18 and aiomoqt never sees NAMESPACE_DONE, but only with both sessions in one process.data-subgroup-basicagainst imquic receives no objects.Recorded in the runner-cases quest:
Checks
just checkpasses (quest, markdown, shell, toml, nix, gh), andquest checkpasses after the follow-up quest.cargo build --locked,cargo fmt --check,cargo clippy,tsc --noEmit, andbiome checkpass.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)