You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The || true unconditionally swallows a non-zero exit from npm run control-plane:coverage
(scripts/control-plane-coverage.mjs, which re-runs control-plane/test/**/*.test.ts under c8 and
writes control-plane/coverage/lcov.info). The next step, "Verify control-plane coverage report
exists", only checks the lcov file is non-empty — it never checks the coverage run itself exited
cleanly.
The identical || true shape exists on the "REES coverage" step immediately above it in the same file,
but that one carries an explicit justifying comment explaining why a non-zero exit there is expected
and safe: c8's instrumentation inflates review-enrichment's timing-sensitive linear-time/ReDoS-guard
assertions past their (uninstrumented) budgets, so a non-zero child exit is a known false alarm — the
real pass/fail already happened in the uninstrumented npm --prefix review-enrichment test step just
before it.
The "Control-plane coverage" step has no such comment, and it needs none: a repo-wide search
(grep -rln "budget\|timing-sensitive\|instrumentation\|c8" control-plane/) confirms control-plane's
suite (14 files under control-plane/test/**) has no timing-budget or ReDoS-guard assertions that
would explain an instrumentation-induced false failure. This reads as the REES step's || true having
been copied over without its justification, not a verified decision for control-plane. As written,
if the c8-instrumented re-run crashes or exits non-zero for a real reason (a genuine bug or flake
under instrumentation, a c8 misconfiguration) after still managing to write a non-empty but
incomplete/wrong lcov.info, CI stays green and silently uploads bad coverage data to Codecov under
the control-plane flag — exactly what "Verify control-plane coverage report exists" is meant to catch
but, as written, cannot.
npm run control-plane:test (the step immediately before this one, uninstrumented) remains the real
correctness gate for the suite itself — this issue is only about the coverage-harvest step's own
unjustified error-swallowing, not about control-plane's test correctness.
Requirements
Remove || true from the "Control-plane coverage" step in .github/workflows/ci.yml, so a genuine
coverage-harvest failure fails the validate job instead of silently passing through.
Do not touch the "REES coverage" step's own || true (that one is justified and documented — leave
it as-is) or scripts/control-plane-coverage.mjs itself.
Confirm npm run control-plane:coverage currently exits 0 against a clean main checkout (run it
locally from the repo root) before removing the swallow, so this change doesn't itself immediately
break CI on an unrelated pre-existing issue. If it does NOT currently exit 0, stop and report that in
the PR instead of force-fixing an unrelated failure — this issue is scoped to the swallow itself.
Deliverables
.github/workflows/ci.yml's "Control-plane coverage" step has || true removed.
Test Coverage Requirements
This is a .github/workflows/** change; neither .github/** nor scripts/** is in Codecov's coverage.include (codecov.yml), so there is no codecov/patch obligation. Verification is: the
workflow file still passes npm run actionlint, and a local run of npm run control-plane:coverage
from the repo root exits 0 and produces a valid control-plane/coverage/lcov.info (confirming the
swallow was safe to remove).
Expected Outcome
The "Control-plane coverage" CI step fails the build on a genuine coverage-harvest failure instead of
silently swallowing it, closing the gap where broken coverage data could upload to Codecov undetected.
Links & Resources
.github/workflows/ci.yml — "REES coverage" step (the justified || true to leave untouched, for
contrast) and "Control-plane coverage" step (the one to fix), both in the same job.
scripts/control-plane-coverage.mjs (what the step runs; no change expected here).
Context
.github/workflows/ci.yml's "Control-plane coverage" step (in thecontrol-plane-scoped section ofthe
validatejob) runs:The
|| trueunconditionally swallows a non-zero exit fromnpm run control-plane:coverage(
scripts/control-plane-coverage.mjs, which re-runscontrol-plane/test/**/*.test.tsunderc8andwrites
control-plane/coverage/lcov.info). The next step, "Verify control-plane coverage reportexists", only checks the lcov file is non-empty — it never checks the coverage run itself exited
cleanly.
The identical
|| trueshape exists on the "REES coverage" step immediately above it in the same file,but that one carries an explicit justifying comment explaining why a non-zero exit there is expected
and safe:
c8's instrumentation inflates review-enrichment's timing-sensitive linear-time/ReDoS-guardassertions past their (uninstrumented) budgets, so a non-zero child exit is a known false alarm — the
real pass/fail already happened in the uninstrumented
npm --prefix review-enrichment teststep justbefore it.
The "Control-plane coverage" step has no such comment, and it needs none: a repo-wide search
(
grep -rln "budget\|timing-sensitive\|instrumentation\|c8" control-plane/) confirmscontrol-plane'ssuite (14 files under
control-plane/test/**) has no timing-budget or ReDoS-guard assertions thatwould explain an instrumentation-induced false failure. This reads as the REES step's
|| truehavingbeen copied over without its justification, not a verified decision for
control-plane. As written,if the
c8-instrumented re-run crashes or exits non-zero for a real reason (a genuine bug or flakeunder instrumentation, a
c8misconfiguration) after still managing to write a non-empty butincomplete/wrong
lcov.info, CI stays green and silently uploads bad coverage data to Codecov underthe
control-planeflag — exactly what "Verify control-plane coverage report exists" is meant to catchbut, as written, cannot.
npm run control-plane:test(the step immediately before this one, uninstrumented) remains the realcorrectness gate for the suite itself — this issue is only about the coverage-harvest step's own
unjustified error-swallowing, not about control-plane's test correctness.
Requirements
|| truefrom the "Control-plane coverage" step in.github/workflows/ci.yml, so a genuinecoverage-harvest failure fails the
validatejob instead of silently passing through.|| true(that one is justified and documented — leaveit as-is) or
scripts/control-plane-coverage.mjsitself.npm run control-plane:coveragecurrently exits 0 against a cleanmaincheckout (run itlocally from the repo root) before removing the swallow, so this change doesn't itself immediately
break CI on an unrelated pre-existing issue. If it does NOT currently exit 0, stop and report that in
the PR instead of force-fixing an unrelated failure — this issue is scoped to the swallow itself.
Deliverables
.github/workflows/ci.yml's "Control-plane coverage" step has|| trueremoved.Test Coverage Requirements
This is a
.github/workflows/**change; neither.github/**norscripts/**is in Codecov'scoverage.include(codecov.yml), so there is nocodecov/patchobligation. Verification is: theworkflow file still passes
npm run actionlint, and a local run ofnpm run control-plane:coveragefrom the repo root exits 0 and produces a valid
control-plane/coverage/lcov.info(confirming theswallow was safe to remove).
Expected Outcome
The "Control-plane coverage" CI step fails the build on a genuine coverage-harvest failure instead of
silently swallowing it, closing the gap where broken coverage data could upload to Codecov undetected.
Links & Resources
.github/workflows/ci.yml— "REES coverage" step (the justified|| trueto leave untouched, forcontrast) and "Control-plane coverage" step (the one to fix), both in the same job.
scripts/control-plane-coverage.mjs(what the step runs; no change expected here).in that wiring, not a duplicate.