Skip to content

ci: the merge gate proves the session driver Send at the default recursion limit - #8706

Merged
kixelated merged 4 commits into
mainfrom
quest/m0/nightly/send-depth-gate
Nov 14, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m0/nightly/send-depth-gate

Conversation

@kixelated

@kixelated kixelated commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

The merge gate now runs just rs send-depth whenever moq-net is selected. A PR that pushes the session driver's Send proof past the default recursion limit fails the gate, instead of breaking every moq-net and moq-rtmp Fuzz job in the next Nightly.

Root cause

send-depth ran cargo check -p moq-net --test send with default features. async stopped being a default feature, so without it the lib had no Driver and tests/send.rs (#![cfg(feature = "async")]) compiled to nothing. The check passed while proving nothing. The fuzz build turns on fuzz = ["async", "kio/sim"], and that is where the overflow showed up. The sanitizer flags and the profile played no part.

Changes

  • sh/rs/send-depth.sh: checks -p moq-net --features async --lib --test send.
  • sh/rs/select.sh: the lint phase (the queue's and just check's rs step) runs rs send-depth when moq-net or a dependency is selected, next to rs no-async.
  • nightly.yml: drops its own rs send-depth job, since Nightly's gate job already runs the full selection.
  • Updated the comments in rs/justfile and flake.nix, the 2026-10-25 decision in quest/m0/ci/README.md, and sh/rs/select.test.py.
  • Deletes quest/m0/nightly/send-depth-gate.md. Its wait cleared when fix(net): prove the lite session driver Send within the default recursion limit #8657 merged.

Validation

Gate cost

This only applies to PRs that select moq-net. It adds a cargo check of moq-net and its deps on the flake's pinned rust-nightly (minimal).

  • Locally with a cold target dir it took about 54 s. A warm rerun took about 4 s.
  • CI cost depends on mbx cache hits for the nightly build.

It's worth it because the alternative lets a Send break land on main and turns 11 Fuzz jobs red. The maintainer decided on 2026-11-13 to gate it.

Decisions

  • Taken by the agent under /quest-afk, for the maintainer's review: fix send-depth to check with async instead of gating on a fuzz target's build.
    • Rejected: building a fuzz target, which needs a sanitizer build and is much slower.
    • Rejected: checking with --features fuzz too, since the measurement showed the same depth.
  • Taken by the agent under /quest-afk, for the maintainer's review: run it in the existing rs lint step rather than as a separate gate job.
    • Rejected: a separate job, which would need its own runner and cache setup. No existing gate job had the nightly toolchain to reuse; the check fetches the flake's rust-nightly itself.
  • Taken by the agent under /quest-afk, for the maintainer's review: answer Pullfrog's stale-comment note by rewriting the rs/justfile recipe comment to say the gate's rs lint phase runs send-depth when moq-net is selected.
  • Taken by the agent under /quest-afk, for the maintainer's review: answer Pullfrog's selection-gap note for sh/rs/send-depth.sh by making an edit to it seed moq-net (the selector's early exit on an empty selection would skip a bare grep in the lint branch), with a select.test.py case.
    • Rejected: selecting moq-net on every flake.nix edit. Flake edits are frequent and would run moq-net's lint, check, and test closure each time; a rust-nightly pin bump stays caught by Nightly's gate, which selects the workspace. Revisit if a bad pin bump lands.

Public API: none. Wire: none.

Caused by

  • moq-dev/moq#7607: made async default-off without adding --features async to send-depth, so the check went empty.

Follow-ups

None.

Models

Size: S
Implementer: Claude, claude-opus-5-5, unknown effort
Implementer: Claude Code, claude-sonnet-5-5, high effort

Review

Rewritten by quest review after each round.

  • Head: 4cfc73a

  • Base: origin/main at 9b80cfc

  • Reviewer: Codex, gpt-6.1-sol, medium effort

  • Session: 01a1db28-3936-7fb2-a2f4-d4d6f3f707e9, round 1

  • Score: 9/10, Source inspection found no actionable defects in the Send-depth proof activation and moq-net gate integration, but execution validation remains outstanding because builds and tests were not run as instructed.

  • Rounds: 2; findings 0: fixed 0, refuted 0, open 0

  • Findings: none

kixelated and others added 2 commits November 13, 2026 19:44
…rsion limit

send-depth checked moq-net without `async`, so the lib had no driver and the
send test compiled to nothing. Check it with `async` and run it in the lint
phase whenever moq-net is selected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title The merge gate checks the session driver's Send depth ci: the merge gate proves the session driver Send at the default recursion limit Nov 14, 2026
@kixelated
kixelated marked this pull request as ready for review November 14, 2026 04:56
@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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 29 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

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: 4653debf-5fc4-446a-83db-22772081c32f
Commits

Reviewing files that changed from the base of the PR and between 8088d55 and 4cfc73a.

Files selected for processing: 8

  • .github/workflows/nightly.yml
  • flake.nix
  • quest/m0/ci/README.md
  • quest/m0/nightly/send-depth-gate.md
  • rs/justfile
  • sh/rs/select.sh
  • sh/rs/select.test.py
  • sh/rs/send-depth.sh
  • 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

@kixelated
kixelated enabled auto-merge November 14, 2026 04:56

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues. There's one stale comment and one selection gap worth a look.

Reviewed changes

This review covers the move of the session driver's Send-depth proof from a standalone Nightly job into the merge gate's rs lint phase. It also covers the fix that makes the proof check something again.

  • send-depth actually compiles the proof: it now checks -p moq-net --features async --lib --test send. Before this, tests/send.rs (#![cfg(feature = "async")]) compiled to nothing.
  • Gate runs it for moq-net: sh/rs/select.sh calls just rs send-depth from the lint action when moq-net or a dependency is selected. The gate CI runs under nix develop, so the CI_STRICT nix requirement holds.
  • Nightly job dropped: Nightly's gate recipe (just ci --all) selects the whole workspace, so it still runs the proof.
  • Tests and docs: select.test.py expects the fourth call and checks that it is limited to lint. The comments in flake.nix and quest/m0/ci/README.md are updated, and the finished quest file is deleted.

ℹ️ rs/justfile still says the land gate skips send-depth

The PR description says the rs/justfile comments were updated, but that file isn't in the diff. The recipe's doc comment at rs/justfile:438-439 still reads "Nightly runs it; the land gate does not." That is now the opposite of what happens, and just --list shows this text.

Technical details
# Stale `send-depth` recipe comment

## Affected sites
- rs/justfile:438-439 — "Prove downstream futures are Send at the default recursion limit on the flake's pinned nightly. Nightly runs it; the land gate does not."

## Required outcome
- The comment describes the new behavior: the gate's rs lint phase runs it when moq-net is selected, and it proves the session driver (with `async`).

ℹ️ The changes that alter the proof don't select it in the gate

send-depth runs only when wants moq-net. A bump of the rust-nightly pin in flake.nix, or an edit to sh/rs/send-depth.sh, matches neither a moq-net seed nor the ALL regex in select_packages. So the gate skips the proof for exactly the two changes that alter what it checks. Only Nightly's --all gate would catch a broken pin bump or script edit, and by then it has already landed on main. This PR exists to close that kind of gap.

Technical details
# send-depth not selected by pin bumps or its own script

## Affected sites
- sh/rs/select.sh:23 — `ALL` regex does not include `sh/rs/send-depth\.sh` or `flake\.nix`
- sh/rs/select.sh:174 — runs only on `wants moq-net`

## Required outcome
- A change to `sh/rs/send-depth.sh` (and ideally to the `rust-nightly` pin) runs `just rs send-depth` in the gate, without selecting the whole workspace for every `flake.nix` edit.

## Suggested approach (optional)
- In the `lint` branch, also run `send-depth` when the list file contains `^sh/rs/send-depth\.sh$` or `^flake\.nix$`, e.g. a `grep -qE` on `$list` next to the `wants moq-net` test. Add a `select.test.py` case for `sh/rs/send-depth.sh`, which replaces the removed assertion that it selected nothing.

## Open questions for the human (optional)
- Is it fine to leave the pin bump to Nightly, given that the `flake.nix` comment says to bump it on purpose?

Pullfrog  | Fix it ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

@kixelated

Copy link
Copy Markdown
Collaborator Author

@pullfrog review this pull request and submit an approving review if it is ready to merge

Both notes are addressed on 4cfc73a: the rs/justfile comment now says the gate's rs lint phase runs send-depth, and an edit to sh/rs/send-depth.sh now selects moq-net (with a select.test.py case). A flake.nix pin bump is left to Nightly's gate; see Decisions in the PR body.

(Written by Claude Sonnet 5.5 with high)

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found. Both notes from the last review are fixed.

Reviewed changes

This review covers the new commits since 4cb8988. They respond to the two informational notes from the last review.

  • Updated the recipe comment: rs/justfile:438-439 now says the gate's rs lint phase runs send-depth when moq-net is selected. It used to say the land gate skips it.
  • The proof's own script now selects it: select_packages adds moq-net as a seed when sh/rs/send-depth.sh changes, so an edit to the script runs just rs send-depth in the gate. sh/dispatch.sh's rs module already matches sh/rs/, so the rs gate itself runs for that path. A new select.test.py assertion replaces the old one that expected an empty selection.
  • Left the pin bump to Nightly on purpose: per the PR's Decisions section, a flake.nix rust-nightly bump still doesn't select moq-net. Nightly's --all gate covers it. That trade is reasonable given how often flake.nix changes.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 14, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found. Both notes from the prior review are addressed.

Reviewed changes

This run covers what changed since the prior Pullfrog review at 4cb8988: 4cfc73a, plus a merge of main that brought nothing new into this PR's files.

  • rs/justfile recipe comment corrected: the send-depth doc comment now says the gate's rs lint phase runs it when moq-net is selected, and Nightly runs it too. That matches sh/rs/select.sh:176.
  • A send-depth.sh edit seeds moq-net: select_packages maps sh/rs/send-depth.sh to the moq-net seed (sh/rs/select.sh:66). It sits with the other seeds, so it gets past the early return on an empty selection, and sh/dispatch.sh's rs module already routes sh/rs/ to the rs gate.
  • Selector tests extended: test_gate_proves_send_depth_for_net asserts the fourth rs send-depth call for moq-net, kio, and Cargo.lock. It also checks that a send-depth.sh edit triggers the call, that test doesn't run it, and that an unrelated crate (moq-auth) doesn't trigger it. python3 sh/rs/select.test.py -k send_depth passes locally. The full suite's 4 failures come from FeatureTest and RecipeTest. The log shows one of them hitting a sandbox sudo restriction. All four are outside the selection logic this PR changes.
  • rust-nightly pin bumps left to Nightly: this is a recorded decision in the PR body. Selecting moq-net on every flake.nix edit would cost too much, and Nightly's gate --all still catches a bad bump.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

Merged via the queue into main with commit 40431f6 Nov 14, 2026
8 checks passed
@kixelated
kixelated deleted the quest/m0/nightly/send-depth-gate branch November 14, 2026 18:05
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