Repository navigation
fix(ci): report independent Nightly failures together - #7667
Conversation
Quest-Claim: b9898834c4d549d2
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
|
Design change: failure collection now reaches the nested feature checks through one shared runner, also used by the benchmark groups. It announces commands before running them and reports accumulated failures at the end. Invalid feature-shape prerequisites still refuse the shape before its dependent checks; the existing net dependency-refusal test stays unchanged and passes. Platform continuation remains Waiting under the task's explicit hakari ownership restriction. (Written by GPT-6.1 Sol) |
kixelated
changed the title
# Conflicts: # quest/m0/nightly/README.md # rs/justfile # sh/rs/tokio-features.sh
Split each Platform job's Rust step into compile and test steps, and run the macOS and Windows test recipes through the shared check runner, so a compile error or one failing test no longer hides the remaining tests. Refresh the Nightly tooling stub for main's coroutine and crypto changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 2 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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head e508079 against base c0888cf.
Findings: No actionable defects found in the full 10-file diff. Checked failure accumulation and propagation, nested feature checks, platform compile/test separation, setup/cancellation conditions, and the added regression coverage. No earlier published review findings were present.
Direction: Keep the shared failure collector and independent-step approach. Verify actual macOS/Windows execution in the first scheduled Nightly after landing.
Verification limits: GitHub-only static review; no local builds or tests run. The head's Check run is still in progress. Platform execution remains unverified here.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the full 10-file diff at e508079dd9 against main (c0888cfefb), including the conflict-free main merge:
- Shared failure runner — new
sh/checks.shwhoserunrecords failures andfinishprints onechecks: failed:line per failure before returning non-zero;sh/gh/nightly.shdrops its private copy and now delegates to it. - Feature matrix —
just rs features,coroutines,crypto-key,quic-featureswrap every independent command inrunand end withfinish; media'srequire_crate/forbid_crateaccumulate instead of exiting, and tokio'sdriver_dependenciesbecomes arun-wrapped function. - Platform recipes —
macos-testandwindows-testbecame bash shebang recipes using the runner, withwindows-testmoving off plain recipe lines. - Nightly audio job —
Rust cachegainsid: setupand each check now carriesif: ${{ !cancelled() && steps.setup.outcome == 'success' }}. - Platform workflow — each job's single Rust step splits into
Rust compileandRust tests, both undershell: bashon Windows, so a compile failure no longer hides the test suite. - Regression coverage —
FeaturesTestinsh/gh/nightly.test.pystubsjust/cargoonPATHand asserts that early, middle, and final failures still let every later command run, including nested compiler failures across six packages and media contract leaks. - Quest —
quest/m0/nightly/README.mddrops the now-completed multi-step bullet, leaving the green scheduled run as its own work.
I verified the load-bearing assumptions rather than taking them on faith: platform.yml really does carry shell: bash on both new Windows steps, steps.setup is unique per job, no caller parses the old nightly: prefix, and just's documented Windows behavior translates /usr/bin/env through cygpath for shebang recipes, so just rs windows-test has a plausible path on the Windows runner. The reverted net-features contract checks (commit 5f357f8) are deliberate — the comment there explains why a green test run must not follow an async-enabled dependency graph.
ℹ️ The audio job's setup is the Rust cache, so a cache failure skips every audio check
The setup guard mirrors platform.yml, where steps.setup is a synthetic "Setup complete" step that only runs after toolchain and generator install, so the guard is load-bearing. In the audio job setup is ./.github/actions/rust-cache, and skipping a cache download is not a prerequisite for these checks — the job's own comment budgets the 90 minutes to include the relay, CLI, and shaper "built from source". A transient R2 or credential failure in that step therefore turns a night into "cache failed, nothing else ran" rather than reporting the audio results, which is the outcome this PR is trying to avoid. Push-to-talk alone kept the bare !cancelled(), so the guard is also not uniform across the job.
No change requested — this is a semantics note, not a defect. Flagging it so the next cache outage is not read as "every audio check passed".
Technical details
# Audio job's setup guard gates on cache availability, not tool readiness
## Affected sites
- `.github/workflows/nightly.yml:469-469` — `id: setup` added to the `Rust cache` step.
- `.github/workflows/nightly.yml:479-525` — six checks plus the artifact upload require `steps.setup.outcome == 'success'`.
- `.github/workflows/nightly.yml:506` — `Push-to-talk` keeps `if: ${{ !cancelled() }}`.
- `.github/workflows/platform.yml:106-108, 160-163` — the synthetic `Setup complete` step the pattern was borrowed from, where the guard covers real tool and generator installs.
## Required outcome
- None if the guard is intended to mean "no audio checks without the cache".
- If it is intended as "the harness prerequisites are ready", the nix action and cache are both upstream of every check, so `!cancelled()` alone preserves the same behavior for a real setup failure and keeps reporting after a cache-only failure.
## Open questions for the human
- Is a cache outage meant to suppress the audio checks, or should those steps still run from a cold build within the existing 90-minute budget?deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
|
quest land watch: the merge queue removed this pull request at e508079 because it conflicts with The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs. (written by quest land watch) |
|
quest land watch: armed auto-merge on this pull request at 050c935, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix ( (written by quest land watch) |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the delta since the prior pullfrog review at e508079dd9:
- Merged base and restored
kiocoverage — merge commit050c9358bringsmaininto the branch and adds-p kioback to thewindows-testandmacos-testrecipes inrs/justfile, matching the command listsmainalready runs. The PR's own patch touched no other file in this delta;main's merged commits also updatedsh/gh/nightly.sh(bench list: adds themsf catalogbench, renamescontributions→include) andquest/m0/nightly/README.md(drops the deleted ingest-fuzz child), both merging cleanly.
The prior informational note about the audio job's setup guard still stands, unchanged by this delta.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
Problem
Nightly's first failed feature check hides every later check. Early audio benchmark failures also skip the later benchmarks. Inside Platform's Rust step, a failed compile hides every platform test: on 2026-11-05, run 42050683248 stopped
just rs macoson anE0277compile error and never ranjust rs macos-test. So each scheduled run reveals only the next failure.Approach
just rs featuresand its nested recipes (including main's newcrypto-key), collect failed commands, and fail each suite after reporting them. Share the runner with the existing benchmark groups and announce each command.Rust compile and testsstep intoRust compileandRust tests, and runjust rs macos-testandjust rs windows-testthrough the shared runner. Step-level continuation for Swift, OBS, and installer checks already landed in moq-dev/moq#7666; run 42050683248 confirms they ran and passed after macOS Rust failed.main, after its children.Impact
Rust compileandRust tests.Alternatives
No
continue-on-error, which could turn the job green. No extra matrix runners, which would split builds and caches. Existing benchmark groups already collect failures. Dependent setup, packaging, and publishing stay sequential.Choices made without asking
windows-testbecomes a bash shebang recipe. just runs those on Windows through Git Bash'scygpath; Platform runs only on Nightly,releasepushes, and dispatch, so the first Windows run of it is the next scheduled Nightly.Validation
On head e508079dd9,
quest gate just ci origin/mainpassed every selected module: Rust compile/lint and all 11,800 tests (138 skipped), workflow lint (actionlint, zizmor) and tooling tests, Biome, Markdown, shell, formatting, and quest validation. The same run passed earlier on c40e4d0fbb, before the conflict-free main merge.The Nightly tooling suite passes all 20 tests. It covers early, middle, and final failures in
just rs features; nested compiler failures across six packages (includingcrypto-key); media dependency-contract failures followed by later checks; and a first-test failure injust rs macos-testandjust rs windows-testthat still runs the rest. The Windows recipe path itself is not exercised before the next scheduled Nightly, since Platform does not run on PRs.Review
mechanical fix by codex / Codex:
Merged base; re·solved rs/justfile conflicts preserving base kio coverage and PR failure aggregation; passed python3 sh/gh/nightly.test.py (20 tests), just --summary, just --dry-run rs windows-test, just --dry-run rs macos-test, bash -n sh/checks.sh sh/gh/nightly.sh sh/rs/net-features.sh sh/rs/media-features.sh sh/rs/tokio-features.sh, shellcheck sh/checks.sh sh/gh/nightly.sh sh/rs/net-features.sh sh/rs/media-features.sh sh/rs/tokio-features.sh, and git diff --cached --check.(head 050c935)quest review --base origin/main: Codex / GPT-6.1 Sol (gpt-6.1-sol), medium effort, session01a1ad62-7c03-7512-97b9-3a4f8c6abe3e, round 1. Reviewed c40e4d0fbb against c0888cfefb: no findings. It covers head e508079dd9 under the conflict-free base merge exemption: its merge-base is still c0888cf, andgit diff origin/main...e508079dd9is byte-identical togit diff c0888cfefb...c40e4d0fbb. No new round.Decisions
Maintainer, 2026-11-05 (/quest-unblock): does this PR wait for a green scheduled Nightly on
main? A scheduled run can include this PR's changes only after it lands.419b799c75), red.Follow-ups
Run 42050683248 also failed Firefox media, the browser benchmark smoke, Moxygen IETF conformance, the Rust shared and supplemental bench thresholds, relay session benchmarks, and the fmp4 fuzz target, alongside failures already owned: the macOS
E0277inrs/moq-audio/src/capture.rs(macOS), ingest fuzz, OBS and resume-model (moq-dev/moq#7975's new children), and bench thresholds (bench CI). Recommend a/quest-plantriage of the unowned ones (Firefox media, browser bench smoke, Moxygen conformance, fmp4 fuzz) against current source.(Written by Claude Opus 5.5)