quest: apply the 2026-09-30 audit - #4589
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reorder m0 for Seattle interop, delete quests done on dev, prune and park marginal work, merge overlaps, and add the missing ordering links. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
…ioritization-406f95 # Conflicts: # quest/m1/README.md # quest/m1/wt-close-upstream.md # quest/m2/README.md # quest/m2/capture-frame-buffers.md
A quest waiting on the outside world states its gate as a plain-text Required bullet in any milestone, re-checked by /quest-audit. Folded in from #4585, which is abandoned in favor of this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ReviewQuest-only audit (moves, deletes, merges, m0 Seattle reorder). Direction looks mostly right, but a few edits are inaccurate against Reviewed head: Blocking
Non-blocking
CI Check/Test are green. No code/API/wire impact. ITERATE This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: cf3ec11
[P2] Gate the runnable leaves when parking a questline. At quest/m3/cat/README.md:54-59 the new external gate only blocks the README: the pinned quest implementation (8590d2a, src/ready.rs:78-95) traverses a questline's children without inheriting its conditions. quest/m3/cat/verify.md has no Required section, so it still appears ready before claim-key registration. The same problem affects quest/m3/cs/README.md:13-14: its newly parked generator child is still ready despite the missing consumer. Put the external condition on the executable leaf quests too; linking a child back to its parent would create a cycle.
Direction: keep the audit. Consolidating overlapping studies, separating independent test flakes, and dropping the installer's two-file transaction reduce unnecessary work and complexity. The new gate convention is sensible once the actual readiness graph enforces it. No runtime API or wire change in this PR.
Verification: inspected planning changes and the pinned readiness implementation; checked all 407 quest files for missing quest links, Required cycles, and unreferenced quests, finding none. No quest guide/check or builds ran because quest/Nix are unavailable here. The claimed completion of all historical branch work was not independently revalidated.
(Written by OpenAI)
Follow-up reviewNotable push after prior ITERATE on Reviewed head: Fixed since prior review
Still open (blocking accuracy)
Non-blocking (still open)
No code/API/wire impact. Milestone README quest links checked clean earlier. ITERATE This is an automated review, not the maintainer's decision |
ReviewQuest-only audit (m0 Seattle reorder, done/marginal deletes, m1↔m2↔m3 moves, overlap merges, gate convention from abandoned #4585). Milestone README Reviewed head: Blocking
Non-blocking
No code/API/wire impact. CI Check/Test were green before merge. ITERATE (follow-up on This is an automated review, not the maintainer's decision |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a63bb29d2f
ℹ️ 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".
| Decided in the 2026-09-30 audit: two suspected per-group costs merged here | ||
| as candidates to measure with `SESSION_ALLOCS`, not separate quests. |
There was a problem hiding this comment.
Measure cache refresh outside the allocation counter
SESSION_ALLOCS only reports allocations, but the folded egress cache-refresh candidate is specifically read-lock, clock, and atomic work and may allocate nothing. When this quest evaluates candidates, that measurement can therefore declare cache refresh a no-win without observing its cost; retain the original timing/CPU measurements for fast fanout and flow-controlled readers, including SUBSCRIBE and FETCH.
AGENTS.md reference: AGENTS.md:L52-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. #4591 has group-cost time the cache refresh per viewer-group for fast fanout and flow-controlled readers, over SUBSCRIBE and FETCH, instead of counting allocations.
(Written by Claude Opus 5.5)
| Also enable `v4l2` in the Linux ARM release build, so a released binary on a | ||
| Raspberry Pi 4 publishes from `moq import capture` through the V4L2 M2M | ||
| hardware encoder (`rs/moq-video/src/v4l2.rs`, already run on a Pi 4's | ||
| `bcm2835-codec`) with no GStreamer detour. Verify that once on a Pi 4. |
There was a problem hiding this comment.
Preserve the V4L2 runtime validation before release
This verification covers only a Pi 4 publishing once, but the folded V4L2 quest still had unverified acceptance cases for changing set_bitrate on a running encoder and for 1080p's padded 1088-row crop; rs/moq-video/src/encode/backend/v4l2.rs also records that only 640x360 was run. When v4l2 becomes enabled in released ARM binaries, omitting those checks can ship a backend that fails congestion-driven bitrate changes or produces an incorrectly cropped picture, so keep them in the merged plan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. #4591 keeps set_bitrate on a running encoder and the 1080p 1088-row crop in the Pi 4 check.
(Written by Claude Opus 5.5)
| ## Required | ||
|
|
||
| - [Ownership boundary](/quest/m2/mobile-ownership.md) - decides whether Rust owns capture on mobile at all | ||
| - [Ownership boundary](/quest/m1/mobile-ownership.md) - decides whether Rust owns capture on mobile at all |
There was a problem hiding this comment.
Remove the resolved mobile ownership blockers
The new ownership quest says the verdict is already settled and that the Android and iOS capture quests “no longer wait on this,” but both capture quests now list it under Required. Until the XS documentation-only quest lands, the quest dependency graph therefore keeps both implementation quests blocked despite the recorded decision; remove these Required edges or change the ownership quest if it is genuinely still a prerequisite.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. #4591 drops the ownership blockers from iOS capture, Android capture, both MediaCodec audio quests, and mobile-completion, which record the verdict instead.
(Written by Claude Opus 5.5)
| ## Required | ||
|
|
||
| - [IETF FIN semantics](/quest/m0/ietf-fin-not-cancel.md) - MAX_REQUEST_ID refills as requests close, and that change decides when a request closes |
There was a problem hiding this comment.
Wire every interop step into the Required chain
The audited order is legal input → FIN → subgroup refusal → uni stream types → request caps, but subgroup refusal and uni stream types have no Required edges and this final quest points straight back to FIN. A quest scheduler can consequently start those three steps independently instead of enforcing the stated Seattle sequence; add the missing predecessor links and make request caps depend on the final stream-types step.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Disagree. m0's Required list is the priority rank (legal input, FIN, subgroup refusal, stream types, request caps). Subgroup refusal and uni stream types touch different code and can land in any order, so a Required edge between them would only serialize independent work. request-caps' edge to FIN is a real code dependency (MAX_REQUEST_ID refills as requests close). The PR body's "chained with Required" overstated it.
(Written by Claude Opus 5.5)
Brings in the 2026-09-30 quest audit (#4589) and 58 main commits. Conflicts: JS IETF publisher (grant readiness kept on main's reconciliation refactor), lite subscriber tests and the lite draft changelog (both kept), auth quests (line's finished children stay deleted, main's libmoq removal kept), and two ietf session tests that each lacked the other side's new Config field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This merged before the Codex and OpenAI reviews finished. #4591 applies the findings I agree with: group-cost times the cache refresh, cli-packaging keeps the V4L2 bitrate and crop checks, the settled mobile-ownership blockers are dropped, and the CAT and C# gates move onto their first leaf quests so (Written by Claude Opus 5.5) |
Aligns quests after the 2026-09-30 audit (#4589): the branch copies of ietf-subgroup-refusal (now m0) and datagram-range (merged into datagram-unfetchable) are deleted. fetch.md stays deleted; the draft-20 FETCH work main added to it moves to quest/m1/ietf-fetch-location.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's audit (#4589) moved the tooling quests; every child has landed, so the README goes and so does every link to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Quest audit on 2026-09-30, covering 449 quests on
mainand 40quest/*branches, with the settled edits applied. Nothing here touches code.Required. The A/V clock moves to m1, and announce dedupe (shipped in fix(net): skip announce updates the peer cannot tell apart #4423) is gone from the text./quest-plan.Required/Relatedlinks between quests that share code.Also: test-flakes-2 splits into one child per flake (shaper-virtual-time joins it), and mobile-ownership shrinks to [XS] recording the verdict (Rust owns codecs). remove-gossip keeps
mesh_withdraw.rs, which already tests configured peers.Also folds in the gate convention from #4585 (abandoned): any quest waiting on the outside world, in any milestone, states its gate as a plain-text
Requiredbullet, re-checked by/quest-audit. The m3 quests parked here without one gained a gate bullet.May conflict with #4520 (wt-close-upstream: this PR only appends) and #4583 (ts-import-shared-shift: untouched here).
Already done outside this PR: deleted 6 dead quest branches whose PRs were closed, and closed #4134 (qos/stats, replanned as
m1/stats).Decision prompts
Round 1
Round 2
Round 3
Public API and wire impact
None. This PR only edits quests.
Follow-ups
These are spawned as separate sessions:
datagram-rangeandietf-subgroup-refusal, which now live on main.m2/p2p.Not spawned:
## Questswith## Required, merge dev in.ietf-paramsto m2 on its branch.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code