fix(ci): collapse Runtime CI onto one hosted runner - #347
seonghobae wants to merge 2 commits into
Conversation
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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. Comment |
There was a problem hiding this comment.
📝 Info: Coverage diagnostics fire on unrelated step failures
Collapsing the three jobs into one changes the two bare if: failure() diagnostics (ci.yml and ci.yml) from job-scoped to job-wide. Any earlier failure (cargo fmt, cargo clippy, cargo test) now triggers them, and they crash reading a coverage.json/coverage-branches.json that was never generated. The job still fails correctly, but the real cause gets buried under spurious tracebacks. Scoping each diagnostic to a specific step id (as the lockfile gate does at ci.yml) would restore the original behavior.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
RCA
Fresh exact-head runs across multiple open PRs are currently stalling before any repository step executes: Runtime CI, Security, SAST, SBOM, and provenance runs remain
queued, and Runtime CI alone requests three independentubuntu-latestallocations per pull request. The first failing boundary is therefore hosted-runner allocation, not checkout, PostgreSQL startup, compilation, tests, coverage, credentials, or repository code.PR #285 already removed an avoidable second SBOM runner allocation while preserving its artifact handoff. Runtime CI still duplicates exact-head checkout, the pinned PostgreSQL service, and ephemeral database setup across three jobs solely to parallelize quality, line coverage, and branch coverage. Under the current allocation pressure that parallelism increases scheduling boundaries and blocks evidence arrival.
Falsifiable hypothesis: reducing Runtime CI from three hosted-runner allocations to one will reduce allocation pressure while preserving every existing quality and exact-coverage gate. Acceptance is observable on this exact head: Runtime CI must expose one job, execute all existing formatting/compile/lockfile/Clippy/test/rustdoc/line-coverage/branch-coverage checks against the same exact PR head, and complete without weakening permissions or evidence semantics.
TDD
4e2a8fb14d57b63de3b7010f7c75c7d5f27fc476changes the executable CI contract to require a singleubuntu-latestallocation, a single exact-head checkout/PostgreSQL service, and one pinnedcargo-llvm-covinstall while retaining both coverage diagnostics.8a53ed1c72243246dbe2dea394003cc238db67afcollapses the three Runtime CI jobs into one sequential evidence job. The branch was kept private from PR-triggered CI until GREEN, so the RED/GREEN history is preserved without amplifying the already-congested runner queue.Preserved gates
persist-credentials: false;contents: readonly;cargo-llvm-covremain pinned;The only intentional tradeoff is wall-clock serialization inside one runner. Timeout is raised from the former per-job 15 minutes to 45 minutes so the same checks can complete sequentially; this does not relax any assertion or coverage threshold.
Superseded
Closed without merge after fresh live-state reconciliation found that PR #288 already carries the safer allocation remedy and preserves the long-lived Runtime check identities (
Format, lint, test, and rustdoc,Production line coverage, andProduction branch coverage). The available connector cannot prove that no organization ruleset still binds those identities, so replacing three check identities with this PR's single renamed job would introduce an avoidable governance risk. PR #288 has now been non-destructively reconciled with current protectedmainand remains the sole landing lane.A fresh Devin finding on this branch also correctly notes that the collapsed job's bare
if: failure()coverage diagnostics can fire after unrelated earlier failures and obscure the primary failure. That defect is not patched here because this branch is superseded; the thread is intentionally left unresolved rather than falsely marked addressed.Acceptance
Do not revive this PR as a competing writer lane. Continue the root-cause work in #288, preserving live check identities and exact-head evidence.