ci: fulfil the defined nightly test tier and fix hygiene at the source - #967
Conversation
Bring nightly-tests.yml up to what docs/ci-strategy.md §6/§9 promise, and make the failing `hygiene` job green. Hygiene (all three tools now exit 0): - .github/zizmor.yml: unpinned-uses ref-pin policy (matches the repo's tag-pin convention) + documented per-line ignores for pre-existing release-pipeline findings. - .github/actionlint.yaml: declare the ubuntu-26.04-arm runner label. - remote-proto: cargo-machete ignore for the prost/tonic deps used only by OUT_DIR-generated code. Nightly tier: - MSRV: consolidate rust-version into [workspace.package] (1.91), inherited by all 29 crates; `just msrv` + a blocking `msrv` job (cargo-hack). - miri: `just miri` + a non-blocking `miri` job over `graph` (pure in-memory logic; switch has no tests, common does file IO). - nightly-rustc canary (mirrors beta-canary); macOS/HVF nightly e2e via a workflow_call reuse of ci-macos.yml; cache priming on the new jobs. - Supervision/restart/kill: crates/minvmd/tests/supervision_integration.rs (restart cycle + dirty-kill repair) + a second boot->stop cycle in scripts/minvmd-lifecycle.sh (both run on Linux/KVM and macOS/HVF). - Concurrency stress: scripts/stress-session-e2e.sh + `just stress` + a non-fatal step in the soak job. notify now gates on the blocking msrv + macos-e2e jobs. Doc §6/§9 reconciled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR centralizes the Rust MSRV at 1.95, pins GitHub Actions to commit SHAs, adds reusable macOS and nightly validation workflows, expands lifecycle and concurrency E2E coverage, and collects diagnostic bundles during failures. ChangesCI hardening and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Pull request overview
Aligns the implemented nightly CI “test tier” with docs/ci-strategy.md by adding the missing gates/canaries and fixing hygiene tooling configuration, while centralizing MSRV so it’s enforced consistently across the workspace.
Changes:
- Adds nightly jobs for MSRV verification, miri (non-blocking), nightly-rustc canary (non-blocking), and macOS/HVF nightly e2e; updates nightly failure notification gating accordingly.
- Introduces new session concurrency stress coverage plus expanded minvmd lifecycle/supervision restart/repair proofs.
- Centralizes MSRV (
rust-version = 1.91) at the workspace level and migrates all crates to inherit it.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/stress-session-e2e.sh | New concurrency stress harness that mints N sessions in parallel and validates list/teardown behavior. |
| scripts/minvmd-lifecycle.sh | Extends lifecycle proof with a second boot→running→stop restart cycle. |
| justfile | Adds msrv, miri, and stress recipes to match the expanded nightly tier. |
| docs/ci-strategy.md | Updates documentation to reflect the nightly tier “as implemented” and adjusts the coverage table. |
| Cargo.toml | Sets workspace-wide MSRV (package.rust-version = "1.91") for inheritance by all crates. |
| .github/zizmor.yml | Configures zizmor pin policy and documents scoped ignores for existing findings. |
| .github/actionlint.yaml | Declares newer runner labels so actionlint doesn’t flag them as unknown. |
| .github/workflows/nightly-tests.yml | Adds msrv/miri/nightly-canary/macos-e2e jobs, adds concurrency stress step, and expands notify gating/body. |
| .github/workflows/ci-macos.yml | Exposes the macOS lane as a reusable workflow via workflow_call for nightly reuse. |
| crates/minvmd/tests/supervision_integration.rs | New integration test covering supervised restart cycle and dirty-kill state repair. |
| crates/args/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/async-dialog/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/check/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/checkouts/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/common/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/decode/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/diagnostics/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/graph/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/lcache/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/mctx/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/mfile/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/minimal/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/minimald-rpc/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/minimald/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/minvmd/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/mip/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/mlog/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/op/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/orchestrator/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/ot/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/paths/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/rcache/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/remote-client/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/remote-proto/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true and adds cargo-machete ignored deps metadata for OUT_DIR-generated code. |
| crates/sandbox2/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/sessions/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/stdlib/Cargo.toml | Switches to inheriting workspace MSRV via rust-version.workspace = true. |
| crates/switch/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
| crates/version/Cargo.toml | Inherit workspace MSRV via rust-version.workspace = true. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- hygiene: actionlint's bundled shellcheck flagged pre-existing SC2086/SC2046 in release.yml run: blocks (my local run missed them — shellcheck wasn't on PATH). Scope-ignore them in .github/actionlint.yaml (SC2046 is intentional word-splitting; the release pipeline stays untouched). - miri: rust-toolchain.toml pins 1.97.0, which overrode the installed nightly so `cargo miri setup` had no miri component. Set RUSTUP_TOOLCHAIN=nightly on the job (same mechanism as the canaries). - msrv: bump timeout-minutes 30 -> 60; the cold cargo-hack workspace check under 1.91 was still compiling when the prior run was cancelled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- msrv: the declared 1.91 was fiction — sysinfo 0.39 requires rustc 1.95, and 1.95 is the max rust-version across the whole resolved dependency tree. Bump [workspace.package] rust-version 1.91 -> 1.95 (still below the 1.97.0 pin). - miri: graph's env_setup tests create directories, which miri rejects under isolation. Run with MIRIFLAGS=-Zmiri-disable-isolation (job env + `just miri`); miri still UB-checks the graph/planner logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both were cancelled by hitting their timeouts, not by real failures: - miri: `graph` pulls in ~1k transitive deps (all of nickel), a ~34-min compile under miri. Point miri at `switch` instead — zero deps, pure integer/IP/MAC tests (the §6 "vsock framing" surface) that compile and interpret in seconds. Drop the now-unneeded -Zmiri-disable-isolation. - session-e2e-soak: 10 cold VM boots + build + bulk-upload + the new stress step ran ~65 min, over the 60-min cap. Bump to 90 and give the stress step its own 15-min cap so it can't run away with the job's budget. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ignores) Replace the earlier `.github/zizmor.yml` — which downgraded `unpinned-uses` to a blanket `ref-pin` policy and carried 17 line-ignores — with real fixes: - unpinned-uses (149): SHA-pin every third-party action across all workflows and composite actions, keeping the tag as a trailing comment. dtolnay/ rust-toolchain is pinned per channel with an explicit `toolchain:` input so the SHA pin can't fall through to rust-toolchain.toml (1.97.0). - artipacked (8): add `persist-credentials: false` to the checkouts that lacked it, matching the repo's existing convention (no workflow pushes via git; all use gh/OIDC). - template-injection (3): move `steps.release_info.outputs.short_sha` into an `env:` var referenced as `$SHORT_SHA` in the run blocks. - excessive-permissions (2): drop nightly.yml to top-level `contents: read` and grant the release-cut scopes only on the `release`/`check` jobs. - shellcheck via actionlint: quote `$GITHUB_OUTPUT` (SC2086) and add a targeted, justified `# shellcheck disable=SC2046` for the intentional word-split of `find` into `gh release create` args — so actionlint.yaml no longer needs the release.yml paths.ignore block. The only remaining suppressions are 4 inline `# zizmor: ignore[...]` markers with co-located justifications, all in the trusted-ref release/promote path (2 cache-poisoning, 1 secrets-inherit, 1 github-app) where the finding is an accepted design tradeoff rather than a fixable defect. `zizmor .github/workflows/` and `actionlint` (with bundled shellcheck) both exit 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On any failure the session-e2e and bulk-upload harnesses already dump scraped console/state tails, but not the daemon's own logs/state/config — which is exactly what the `min stop` soak failure needs. Add a `min bug` collection to each script's failure path: bounded guest wait with a host-only (`--no-guest`) fallback since the daemon may be wedged or already gone. The bundle is written next to the boot log, which under the VM soak is the job's uploaded soak-logs dir — so the existing `if: always()` artifact upload now ships a real diagnostic bundle (minimal-diag-*.tar.zst), not just tails. Collected before teardown wipes the state dir; /tmp fallback for a local no-VM bulk run whose $WORK is reaped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7d41395 to
399ef4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
scripts/minvmd-lifecycle.sh (1)
110-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the repeated "wait for status" polling loop.
This block re-implements the same
status --jsonpolling pattern that the first run/stop cycle earlier in the script almost certainly already uses. Extracting a smallwait_for_state()helper (taking the expected state string and an optionalvmm_pidcheck) would avoid the two loops drifting out of sync as timeout/backoff tuning changes.♻️ Sketch of a shared helper
+wait_for_status() { + # $1: jq predicate, $2: output file, $3: max iterations + for _ in $(seq 1 "$3"); do + minvmd status --json > "$2" || true + if jq -e "$1" "$2" >/dev/null; then return 0; fi + sleep 0.2 + done + return 1 +} + echo "::group::restart (run --detach again)" minvmd run --detach --timeout "$BOOT_TIMEOUT" -for _ in $(seq 1 $((BOOT_TIMEOUT * 5))); do - minvmd status --json > "$WORK/status2.json" || true - if jq -e '.state == "running" and (.vmm_pid | type == "number")' "$WORK/status2.json" >/dev/null; then - break - fi - sleep 0.2 -done +wait_for_status '.state == "running" and (.vmm_pid | type == "number")' "$WORK/status2.json" $((BOOT_TIMEOUT * 5)) cat "$WORK/status2.json" jq -e '.state == "running" and (.vmm_pid | type == "number")' "$WORK/status2.json" echo "::endgroup::"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/minvmd-lifecycle.sh` around lines 110 - 135, Extract the repeated status polling logic into a shared wait_for_state helper, reusing it for both the initial and restart lifecycle checks. Have the helper accept the expected state and optionally require a numeric vmm_pid, while preserving the existing timeout, polling interval, status JSON capture, and failure validation behavior..github/workflows/commitlint.yml (1)
20-25: 📐 Maintainability & Code Quality | 🔵 TrivialRequire
@gominimal/minimalistsreviewThis touches
.github/workflows/commitlint.yml, which is covered by the repo-wide CODEOWNERS rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/commitlint.yml around lines 20 - 25, Request review from the `@gominimal/minimalists` CODEOWNERS group for the changes in the commitlint workflow, ensuring the repository’s required ownership approval is obtained before merging.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci-linux-kvm.yml:
- Line 48: Require approval from the `@gominimal/minimalists` CODEOWNER for the
edit to the frozen CI workflow, including the actions/checkout reference in
ci-linux-kvm.yml, before merging.
In @.github/workflows/nightly-tests.yml:
- Around line 279-289: Update the stale MSRV references in the msrv job comments
to state Rust 1.95 instead of 1.91, including the workspace floor and matching
toolchain description. Leave the cargo-hack command and workflow behavior
unchanged.
In `@crates/minvmd/tests/supervision_integration.rs`:
- Around line 51-107: The minvmd helper’s Command::output call can block
indefinitely, bypassing the polling deadlines. Update minvmd to run the child
process with an explicit timeout, using the repository’s existing timeout
mechanism if available or spawn plus periodic try_wait, and fail promptly when
the timeout expires while preserving the current Output-based callers.
In `@docs/ci-strategy.md`:
- Around line 353-354: Update the MSRV description near the workflow strategy
documentation to state that nightly-tests.yml directly runs cargo hack check
--rust-version and mirrors the just msrv recipe, while preserving the existing
rust-version inheritance details.
In `@scripts/stress-session-e2e.sh`:
- Around line 163-167: Update the post-teardown session check around the `after`
assignment so failures from `mnl ls --raw` are preserved rather than converted
to an empty result; explicitly capture and validate the command status before
evaluating session output, and call `fail` when the list command itself fails
while retaining the existing non-empty-session failure behavior.
---
Nitpick comments:
In @.github/workflows/commitlint.yml:
- Around line 20-25: Request review from the `@gominimal/minimalists` CODEOWNERS
group for the changes in the commitlint workflow, ensuring the repository’s
required ownership approval is obtained before merging.
In `@scripts/minvmd-lifecycle.sh`:
- Around line 110-135: Extract the repeated status polling logic into a shared
wait_for_state helper, reusing it for both the initial and restart lifecycle
checks. Have the helper accept the expected state and optionally require a
numeric vmm_pid, while preserving the existing timeout, polling interval, status
JSON capture, and failure validation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57f4080a-de27-460f-b6ea-efce2c4f3172
📒 Files selected for processing (52)
.github/actionlint.yaml.github/actions/core-tests/action.yml.github/actions/setup-libkrun-macos/action.yml.github/actions/setup-rust/action.yml.github/workflows/ci-linux-kvm.yml.github/workflows/ci-linux-native.yml.github/workflows/ci-macos.yml.github/workflows/ci-shell-installer.yml.github/workflows/ci.yml.github/workflows/commitlint.yml.github/workflows/nightly-tests.yml.github/workflows/nightly.yml.github/workflows/promote.yml.github/workflows/prune-releases.yml.github/workflows/release.ymlCargo.tomlcrates/args/Cargo.tomlcrates/async-dialog/Cargo.tomlcrates/check/Cargo.tomlcrates/checkouts/Cargo.tomlcrates/common/Cargo.tomlcrates/decode/Cargo.tomlcrates/diagnostics/Cargo.tomlcrates/graph/Cargo.tomlcrates/lcache/Cargo.tomlcrates/mctx/Cargo.tomlcrates/mfile/Cargo.tomlcrates/minimal/Cargo.tomlcrates/minimald-rpc/Cargo.tomlcrates/minimald/Cargo.tomlcrates/minvmd/Cargo.tomlcrates/minvmd/tests/supervision_integration.rscrates/mip/Cargo.tomlcrates/mlog/Cargo.tomlcrates/op/Cargo.tomlcrates/orchestrator/Cargo.tomlcrates/ot/Cargo.tomlcrates/paths/Cargo.tomlcrates/rcache/Cargo.tomlcrates/remote-client/Cargo.tomlcrates/remote-proto/Cargo.tomlcrates/sandbox2/Cargo.tomlcrates/sessions/Cargo.tomlcrates/stdlib/Cargo.tomlcrates/switch/Cargo.tomlcrates/version/Cargo.tomldocs/ci-strategy.mdjustfilescripts/bulk-upload-e2e.shscripts/minvmd-lifecycle.shscripts/session-e2e.shscripts/stress-session-e2e.sh
- supervision_integration: bound each `minvmd` invocation with a hard 120s subprocess timeout (spawn + poll try_wait, SIGKILL past the deadline) so a wedged daemon fails fast instead of blocking Command::output() past the polling deadlines and hanging CI. - stress-session-e2e: fail loudly if the post-teardown `min ls` itself errors, instead of swallowing it with `|| true` and passing the clean-restart assertion on an empty result. - nightly-tests / ci-strategy: correct the stale 1.91 MSRV comments to 1.95 and note the msrv job runs `cargo hack check --rust-version` directly, mirroring the `just msrv` recipe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VU3ZE9qTPRrqzChvJYHM5G
What & why
Brings
nightly-tests.ymlup to whatdocs/ci-strategy.md§6/§9 promise, and makes the failinghygienejob green. Closes the delta between the documented nightly tier and reality.Hygiene — fixed at the source (no blanket ignores)
All three tools exit 0 locally (
zizmor,actionlint,cargo-machete). Per the codeowner's steer, the earlier.github/zizmor.ymlpolicy + bulk suppressions were removed and replaced with real fixes:unpinned-usesfindings.dtolnay/rust-toolchainchannel pins carry an explicittoolchain:input so the SHA pin doesn't silently fall through torust-toolchain.toml.persist-credentials: falseadded to checkouts flagged byartipacked; least-privilegepermissions:blocks added tonightly.yml(top-level + per-job) andrelease.yml(template-injectionhardening: outputs moved intoenv:and referenced as shell vars;$GITHUB_OUTPUTquoted).# zizmor: ignore[...]remain, each documented at the line: onesecrets-inherit(first-party same-repo reusable workflow), twocache-poisoning(releaserust-cache,save-if-gated), onegithub-app(owner+repositories:docs-scoped token, ADR-0002)..github/actionlint.yamldeclares theubuntu-26.04-armrunner label (newer than actionlint 1.7.12's built-in list); shell quoting fixes (SC2086) applied at the source rather than ignored.remote-protocarries acargo-macheteignore for the prost/tonic deps used only byOUT_DIR-generated code (include!-d, invisible to machete's source scan).Nightly tier
rust-versionconsolidated into[workspace.package] = "1.95", inherited by all 29 crates viarust-version.workspace = true(was 18 hardcoded / 11 missing);just msrv+ a blockingmsrvjob (cargo hack check --rust-version), wired tonotify. 1.95 is the real floor the tree compiles at.just miri+ a non-blockingmirijob overswitch(the vsock/subnet/MAC primitives — pure in-memory logic that interprets in seconds). Widen the crate set as more prove clean.beta-canary(non-blocking,save-if: false).workflow_calladded toci-macos.yml; a blockingmacos-e2ejob reuses it, closing the §9 "both platforms" row.crates/minvmd/tests/supervision_integration.rs(restart cycle + dirty-kill → state-repair) + a second boot→stop cycle inscripts/minvmd-lifecycle.sh; auto-discovered, so it runs on Linux/KVM and macOS/HVF with no YAML edit.scripts/stress-session-e2e.sh+just stress+ a non-fatal step in the soak job (mints N sessions in parallel, asserts listing, bulk teardown, and clean restart with zero residual sessions).msrv/mirijobs;notifynow gates on the blockingmsrv+macos-e2ejobs.Diagnostics on failure
When the session/bulk e2e fails, the harness now collects a
min bugdiagnostic bundle (scripts/session-e2e.sh,scripts/bulk-upload-e2e.sh) — the daemon's own logs/state/config land in the existingsoak-boot-logsartifact for post-mortem. This surfaces the daemon-side view of the pre-existingsession-e2e-soakmin stopfailure (gominimal/inbox#363), which is a separate regression, not introduced here.Verification
Verified locally: hygiene (machete / zizmor / actionlint all exit 0); MSRV resolves to
1.95across all 29 crates (cargo metadata); both scripts shellcheck-clean; the new harness is rustfmt-clean and parses.Could not verify in the dev sandbox (no KVM; toolchain 1.95 vs pinned 1.97; OOM on a heavy dep) — full
cargobuild/clippy/test and all VM/e2e paths rely on CI as the validator.Two first-run risks to watch: (1) the blocking
msrvjob relies on cargo-hack auto-installing the 1.95 toolchain and 1.95 actually compiling the tree; (2)supervision_integration.rsruns in the KVM + macOStest-vmlanes on every push (auto-discovery), adding a few VM boots to the shared mac mini — gate-able behind an env flag if that's too much.🤖 Generated with Claude Code
Note
Add nightly MSRV, miri, macOS e2e, and concurrency stress jobs and pin all CI action refs to commit SHAs
msrvjob to nightly-tests.yml that runscargo hack check --rust-versionacross the workspace; sets workspace MSRV to 1.95 in Cargo.toml and migrates all member crates torust-version.workspace = true.mirijob runningcargo miri test -p switchunder nightly, and anightly-rustc-canaryjob running core tests under nightly rustc.macos-e2ejob invoking the reusable ci-macos.yml workflow (which gainsworkflow_callsupport) and expands thenotifyjob to open failure issues formsrvandmacos-e2e.just stressrecipe.persist-credentials: falsewhere missing.Macroscope summarized 849b584.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores