ci: flip every lane to always-trigger with in-workflow gates - #712
Conversation
Add a nextest config with two profiles: profile.default for local runs (surface slow tests, never kill), and profile.ci for the core-tests composite (fail-fast off for full failure reports, hard-kill at 5 minutes via slow-timeout terminate-after, leak detection at 1s). No retries anywhere: the workspace suite has no recorded flakes, and a same-invocation retry of a VM boot test would run against the leaked __krun-vmm/gvproxy processes of the failed attempt (#588) - the lane reap steps plus terminate-after remain the mitigation. core-tests also gains --no-tests=fail: an empty selection is a broken filter or a dropped target, not a pass. Refs: #687 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A PR touching only .config/nextest.toml (e.g. a future profile edit) must re-run the tests that consume it; this PR's own run only triggered because it also edited the core-tests composite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four changes, one review story (nextest archive, unified session e2e, scripts over YAML, composite verify): - nextest archive replaces the hand-rolled testbins.json + run-testbin.sh: build-linux ships testbed/nextest-archive.tar.zst and the test job selects harnesses with filtersets under the new profile.vm (one VM at a time, hung boots hard-killed at 6 min, no retries - a same-invocation retry runs against the leaked vmm/gvproxy of the failed attempt, #588). - the unified session e2e (scripts/session-e2e.sh) joins the lane, covering Deployment Model 3 (native Linux + VM): the minimal CLI is built in build-linux (separate invocation - a combined build unifies libkrun into the CLI, the #694 regression class), ships in the testbed, and drives activate/exec/destroy from PATH, with the #588 reap (sudo: relay leftovers are root-owned) before it and cli-e2e-boot.log uploaded. - the daemon-lifecycle shell blob becomes scripts/lifecycle-e2e.sh: PATH-resolved minvmd (no cargo, macOS-reusable), temp workdir, and an EXIT-trap teardown so a failed assert cannot strand a daemon. - the krun_add_disk3 export assert moves into setup-libkrun-linux, mirroring the macOS composite and covering release.yml (which uses the composite but had no check); an early actionable failure - minvmd's compile-time link is the real backstop. Also per owner direction: workflow comments drop PR/issue and R-numbers (history belongs in commits, not YAML), and the DM labels are corrected against docs/specs/03-spec-networking (DM1 is the macOS model; this lane is DM3, session-e2e.sh header fixed accordingly). Refs: #687 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The lifecycle step spawns a fresh daemon just like the CLI session e2e, so it was equally exposed to leaked __krun-vmm/gvproxy children from a failed harness run. Move the reap to run immediately after the harness e2es, ahead of both fresh-daemon steps. Addresses CodeRabbit review on #702. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three cross-lane changes that must fork every cache exactly once: - rust-cache gains shared-key (one per job class that compiles the same artifacts: workspace/tests/clippy/native-e2e/mac-unit) and save-if restricted to main, via new setup-rust inputs forwarded from core-tests. PR branches restore but never write, so PR churn stops LRU-evicting the main caches every PR restores from (the 10 GB pool is shared repo-wide). - the two multi-GB raw actions/cache users (KVM build-linux, macOS artifacts stage2) split into cache/restore + main-only cache/save guarded on cache-hit. - CARGO_PROFILE_DEV_DEBUG=line-tables-only in CI (setup-rust env step; explicit job env on the mac and KVM jobs that bypass the composite): usable backtraces, smaller target trees and caches, faster links - without a named Cargo profile, which would move output out of target/debug and break codesign/testbed/justfile paths. - --locked on every workflow cargo invocation (build/test/clippy/ archive), so a stale Cargo.lock fails loudly instead of silently re-resolving; previously only core-tests' cargo fetch enforced it. Refs: #687 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The last two trigger-level path filters (ci.yml's paths-ignore + ci-macos.yml/ci-shell-installer.yml's paths) become dorny/paths-filter changes jobs with if: always() aggregators, completing the lane pattern the native and KVM lanes established. This unblocks the ruleset flip: a required context a trigger filter skips stays Expected forever and wedges the PR; a skipped in-workflow job reports skipped, which the aggregators treat as pass. - ci.yml: changes job (code = everything minus md/docs/LICENSE, the old paths-ignore inverted); all six jobs gated; ci-success skip-tolerant. - ci-macos.yml: trigger paths move into a changes filter (keeping the rust-toolchain.toml entry); artifacts/e2e/unit gated (the e2e keeps the RUN_MACOS_CI kill-switch, ANDed); NEW ci-macos-success aggregator - the fifth and final required-check context. - ci-shell-installer.yml: same flip; aggregator now skip-tolerant. - ci-docs-skip.yml DELETED: the inverse-path hack (and its wedge and silent-ungate failure modes) is obsolete - docs-only changes now skip jobs inside always-running workflows. - ci-gvproxy.yml DELETED: pin verification lives at point-of-use (fetch-gvproxy.sh SHA-checks in the KVM lane and release); vendor/gvproxy/** joins the KVM changes filter so a pin bump re-runs the lane that consumes it. - workflow comments drop remaining PR/issue and R-numbers (owner direction: history belongs in commits, not YAML). The ruleset itself still requires only ci-success; flipping it to the five aggregator contexts happens after the soak. Refs: #687 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # .github/workflows/ci-linux-native.yml
# Conflicts: # .github/workflows/ci-macos.yml
# Conflicts: # .github/actions/core-tests/action.yml # .github/workflows/ci-linux-kvm.yml
# Conflicts: # .github/workflows/ci-macos.yml # .github/workflows/ci.yml
Same fix as the lane hotfix: dorny cannot diff push events against our shallow, credential-free checkouts; pushes to main and dispatches run everything, PRs keep path economy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesCI workflow gating
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant ChangesFilter
participant CIJobs
participant StatusAggregator
GitHub->>ChangesFilter: start workflow for main event
ChangesFilter->>CIJobs: provide path-change output
CIJobs-->>StatusAggregator: return success, skipped, failure, or cancelled
StatusAggregator-->>GitHub: publish required aggregate status
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.yml:
- Around line 138-140: Add persist-credentials: false to every remaining
actions/checkout step in the workflow, including the checkout associated with
the cargo-deny-action job, matching the existing hardened changes checkout
configuration.
🪄 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: 05bd249a-18d0-45fc-9752-55246cfa66ba
📒 Files selected for processing (7)
.github/workflows/ci-docs-skip.yml.github/workflows/ci-gvproxy.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
💤 Files with no reviewable changes (2)
- .github/workflows/ci-docs-skip.yml
- .github/workflows/ci-gvproxy.yml
The changes jobs hardened their checkouts but the pre-existing job checkouts still left the token in git config; make it uniform across ci.yml and ci-shell-installer.yml. No job in either workflow needs git credentials after checkout. Addresses CodeRabbit review on #712. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CodeRabbit finding addressed in the latest commit: |
PR7 of the CI end-state migration (#687) — the change that unblocks the ruleset flip.
What
The last two trigger-level path filters become in-workflow
changesgates, completing the lane pattern across all five workflows:paths-ignoreinverted into acodefilter (everything minus**.md/docs/**/LICENSE); all six jobs gated;ci-successskip-tolerant.changesfilter;artifacts/unitgated;e2ekeeps theRUN_MACOS_CIkill-switch ANDed with the gate; newci-macos-successaggregator — the fifth and final required-check context.ci-docs-skip.ymldeleted — the inverse-path hack is obsolete: a required context that a trigger filter skips stays "Expected" forever (PR wedge), and the no-op twin could silently green-wash mixed PRs. Docs-only changes now skip jobs inside always-running workflows, which aggregators report as pass.ci-gvproxy.ymldeleted — pin verification lives at point-of-use (fetch-gvproxy.shSHA-checks in the KVM lane and release);vendor/gvproxy/**joins the KVM changes filter so a pin bump re-runs the lane that consumes it.pull_request; pushes to main and dispatches run everything.timeout-minutes.The ruleset still requires only
ci-success— flipping it to the five aggregator contexts happens after the soak, which starts when this merges.Verification
Saved cachelines (PRs are restore-only now).codefilter's negation semantics; its failure mode is benign over-running) and one code PR exercising all contexts.🤖 Generated with Claude Code
Summary by CodeRabbit
CI Improvements
Maintenance