Skip to content

fix(ci): hold the Codecov verdict until every upload has landed - #9807

Merged
JSONbored merged 2 commits into
mainfrom
fix/codecov-premature-notify
Jul 29, 2026
Merged

fix(ci): hold the Codecov verdict until every upload has landed#9807
JSONbored merged 2 commits into
mainfrom
fix/codecov-premature-notify

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

codecov/patch posts a red conclusion computed from a partial coverage report, while
validate-tests is still running.

notify.after_n_builds: 1 fires the verdict on the first upload to arrive, and arrival order does
not match importance:

upload flag job timing
review-enrichment rees validate-code ~2 min
control-plane control-plane validate-code ~2 min
engine engine validate-code ~2 min
root vitest backend validate-tests much longer

Observed on #9799: codecov/patch went pass (after rees), then fail (after engine), while
validate-tests was still pending throughout. Codecov's own comment printed it plainly:

HEAD has 1 upload less than BASEbackend | BASE 1 | HEAD 0

This is not cosmetic. The gate closes a contributor PR on any red check, codecov/patch
included. A PR can be closed on a status computed from an incomplete report — one that would have
resolved green minutes later. Same class as #9798: a CI signal corresponding to no real defect.
require_ci_to_pass: true gates the final verdict but does not suppress the interim status, and
wait_for_ci (already true by default) did not prevent it either.

Why not just raise after_n_builds

The existing comment already rules it out, correctly:

It must stay 1, not 2 [...] those uploads only exist on such PRs, and a floor above the guaranteed
minimum would leave codecov/patch permanently un-posted on every other PR.

A count cannot express "wait for the backend upload specifically." Any floor high enough to
guarantee it would hang the check on PRs producing fewer uploads. That reasoning is preserved on the
key rather than deleted.

The fix

codecov.notify.manual_trigger: true — Codecov sends nothing until told. A new codecov-notify job
runs codecovcli send-notifications once, and it needs every uploading job, so it is the only place
that can truthfully say "that's all of them."

The notifier is deliberately unable to fail a PR. The bug being fixed is a phantom red closing a
good PR, so the fix must not add a new way to do that:

  • it is not in validate's needs, so it can never fail the required check;
  • every failure path — missing pipx, failed install, rejected notification — warns and exits 0;
  • if it never runs, no status posts, the check set stays incomplete, and the gate waits. It cannot
    merge unreviewed and cannot false-close.

!cancelled() rather than always(), so a run superseded by a newer push does not notify on a stale
sha. The job skips entirely when neither upload job ran, because notifying on a commit Codecov has no
reports for is an error, not a verdict.

Validation

Shell logic exercised against stubs for every path, all exiting 0:

scenario result
token present, CLI succeeds exit=0, notification sent
fork PR, token empty exit=0[ -n "$X" ] && args+=(…) would have aborted under bash -e
CLI notification fails exit=0 + ::warning
pipx install fails exit=0 + ::warning
pipx missing entirely exit=0 + ::warning

test/unit/ci-codecov-notify.test.ts (9 cases) pins the invariants that are silent when broken — the
commit sha matching the uploads' override_commit, the needs list, the non-fatal shape, the
bash -e token trap. Mutation-checked: each of always() for !cancelled(), dropping the failure
guard, restoring the && token trap, and removing validate-tests from needs fails the guard.

  • npx vitest run test/unit/ci-*.test.ts71 passed (12 files, incl. the 9 new)
  • npm run actionlint — clean · npm run lint:composite-actions — clean · git diff --check — clean

Closes #9801

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • The diff is one workflow job, one codecov.yml key, and one test file. I ran what can regress:
    npm run actionlint, npm run lint:composite-actions, and all 12 test/unit/ci-*.test.ts workflow
    guards (71 passing), plus the stubbed shell matrix above. No src/** line changes, so codecov/patch
    has nothing to measure here; the MCP/UI/workers/audit paths are untouched and CI runs them anyway.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

CODECOV_TOKEN is read from secrets into an env var and passed as a CLI flag only when non-empty; it
is never echoed, and the ::warning messages contain only the commit sha. Auth/CORS/session,
OpenAPI/MCP and UI are not applicable — this diff adds no runtime code. No changelog edit.

UI Evidence

Not applicable — CI configuration only, no visible UI, frontend, docs, or extension change.

Notes

  • This is a guarded path (CI config), so I expect it to be held for manual review rather than
    auto-merged. That seems right for a change to how the coverage gate reports.
  • Found while validating fix(ci): normalize committed CRLF to LF and name the failure for what it is #9799. That PR is unaffected in the end — it is fully green — but the
    premature red sent the investigation down a false path for a while, which is the cost this fixes.
  • pipx ships on the GitHub-hosted Ubuntu images, but the step does not assume it: a missing or failing
    pipx warns and exits 0 rather than reddening the check.

`notify.after_n_builds` fires on the Nth upload to ARRIVE, and arrival order does not match
importance. validate-code's rees / control-plane / engine reports land in ~2 minutes; validate-tests
is still producing the backend lcov that covers most of src/**. So codecov/patch posted a conclusion
computed from a partial report — on #9799 it went pass, then fail, while validate-tests was still
pending, with Codecov's own comment printing `backend | BASE 1 | HEAD 0`.

That is not cosmetic: the gate closes a contributor PR on any red check, codecov/patch included, so a
transient partial-report red can close a PR that has no defect. `require_ci_to_pass` gates the final
verdict but does not suppress the interim status, and `wait_for_ci` (already on by default) did not
prevent it either.

Raising the count cannot fix this — the reasoning preserved on `after_n_builds` explains why a floor
above the guaranteed minimum leaves codecov/patch permanently un-posted on PRs that produce fewer
uploads — because a count cannot express "wait for the backend upload specifically". `manual_trigger`
can: Codecov sends nothing until `codecovcli send-notifications` runs, and the new codecov-notify job
runs it once, after every uploading job has finished.

The notifier is deliberately unable to fail a PR. It is not in `validate`'s needs, and every failure
path — a missing pipx, a failed install, a rejected notification — warns and exits 0. If it never
runs, no status posts, the check set stays incomplete and the gate waits: it cannot merge unreviewed
and cannot false-close, which is the whole point.

`!cancelled()` rather than `always()` so a run superseded by a newer push does not notify on a stale
sha, and the job skips entirely when neither upload job ran, since notifying on a commit Codecov has
no reports for is an error rather than a verdict.

Closes #9801
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Important

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏳ LoopOver is waiting…

LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting

@JSONbored JSONbored self-assigned this Jul 29, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
loopover-ui 3537439 Commit Preview URL

Branch Preview URL
Jul 29 2026, 10:29 AM

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit 7ad87d3 into main Jul 29, 2026
5 checks passed
@JSONbored
JSONbored deleted the fix/codecov-premature-notify branch July 29, 2026 10:44
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.46%. Comparing base (ee98373) to head (3537439).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9807   +/-   ##
=======================================
  Coverage   90.46%   90.46%           
=======================================
  Files         918      918           
  Lines      113935   113935           
  Branches    27049    27049           
=======================================
  Hits       103073   103073           
  Misses       9533     9533           
  Partials     1329     1329           
Flag Coverage Δ
backend 95.57% <ø> (ø)
control-plane 100.00% <ø> (ø)
rees 89.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codecov/patch posts a red verdict from a partial report while validate-tests is still running

1 participant