fix(ci): stop swallowing control-plane coverage failures with an unjustified || true#8497
Conversation
…stified || true The REES coverage step's identical || true is documented and justified (c8 instrumentation inflates its timing-sensitive assertions past their uninstrumented budgets). Control-plane's suite has no such timing-budget or ReDoS-guard assertions, so this looks like the shape being copied without its justification, not a verified decision. A genuine coverage-harvest failure that still writes a non-empty but incomplete/wrong lcov.info currently stays green and uploads bad data to Codecov -- exactly what the next step's existence check is meant to catch but, with the swallow in place, cannot. Verified locally: after rebuilding control-plane (stale dist), npm run control-plane:coverage exits 0 with all 201 tests passing and a clean coverage summary, confirming the swallow is safe to remove. Closes JSONbored#8392
|
Superagent did not find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8497 +/- ##
=======================================
Coverage 92.50% 92.51%
=======================================
Files 791 791
Lines 79333 79343 +10
Branches 23960 23967 +7
=======================================
+ Hits 73386 73403 +17
Misses 4807 4807
+ Partials 1140 1133 -7
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-24 16:14:45 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
.github/workflows/ci.yml's "Control-plane coverage" step rannpm run control-plane:coverage || true, unconditionally swallowing a non-zero exit. The next step ("Verify control-plane coverage report exists") only checks thatcontrol-plane/coverage/lcov.infois non-empty — it never checks the coverage run itself exited cleanly. So a genuine coverage-harvest failure that still manages to write a non-empty but incomplete/wronglcov.infocurrently stays green and silently uploads bad data to Codecov under thecontrol-planeflag.The identical
|| trueshape on the "REES coverage" step immediately above carries an explicit justifying comment:c8instrumentation inflates review-enrichment's timing-sensitive linear-time/ReDoS-guard assertions past their uninstrumented budgets, so a non-zero exit there is a known, documented false alarm (the real pass/fail already happened in the uninstrumented test step just before it). The "Control-plane coverage" step has no such comment, and a repo-wide search (grep -rln "budget|timing-sensitive|instrumentation|c8" control-plane/) confirmscontrol-plane's suite has no timing-budget or ReDoS-guard assertions that would explain an instrumentation-induced false failure — this reads as the REES step's shape copied without its justification, not a verified decision for control-plane.|| truefrom the "Control-plane coverage" step only. The REES coverage step andscripts/control-plane-coverage.mjsare untouched.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #8392
Validation
npm run actionlintgit diff --checkcontrol-plane(stale dist from an earlier main sync), then rannpm run control-plane:coveragefrom the repo root per the issue's explicit instruction: exits 0, all 201 tests pass, coverage summary is clean (99.95% statements, 99.54% branches, 99.04% functions, 99.95% lines) — confirming the swallow is safe to remove and this change doesn't itself introduce a CI failure..github/**andscripts/**are outside Codecov'scoverage.include, so this change owes nocodecov/patchobligation.