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

`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

Co-authored-by: JSONbored <aetherealdev@gmail.com>

Author: JSONbored

.github/workflows/ci.yml

@@ -1275,3 +1275,60 @@ jobs:
           echo "::error title=CI::A required CI job failed or was cancelled."
           echo "Job results: ${{ join(needs.*.result, ', ') }}"
           exit 1
+
+  # Codecov sends nothing until it is told the uploads are done (codecov.yml's notify.manual_trigger).
+  # This job is the one place that can truthfully say "that's all of them": it needs every uploading job
+  # (validate-code carries rees / control-plane / engine, validate-tests carries backend), so it cannot
+  # run before they have finished.
+  #
+  # Before this existed, notify.after_n_builds fired the verdict on the first upload to ARRIVE, and
+  # arrival order does not match importance -- validate-code finishes in ~2 minutes while validate-tests
+  # is still producing the backend lcov. codecov/patch therefore posted a conclusion computed from a
+  # partial report (#9801).
+  #
+  # Deliberately NOT in `validate`'s needs, and deliberately non-fatal. The bug being fixed is a phantom
+  # red closing a good PR, so the fix must not add a new way to do that: a Codecov delivery hiccup must
+  # never fail the required check. If the trigger does not run, no status posts, the gate sees an
+  # incomplete check set and waits -- it cannot merge unreviewed and cannot false-close.
+  codecov-notify:
+    name: codecov-notify
+    needs: [changes, validate-code, validate-tests]
+    # Skip only when neither uploading job ran: a fully path-filtered PR uploads nothing, and asking
+    # Codecov to notify on a commit it has no reports for is an error rather than a verdict. Uses
+    # !cancelled() rather than always() so a run superseded by a newer push does not notify on a stale
+    # sha -- the replacement run posts the real verdict.
+    if: ${{ !cancelled() && (needs.validate-code.result != 'skipped' || needs.validate-tests.result != 'skipped') }}
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      contents: read
+    steps:
+      # Every failure path below is swallowed on purpose. A red check from THIS job would be closed by
+      # the gate, which is precisely the bug being fixed -- the notifier must never be able to fail a PR.
+      # Worst case is that no verdict posts, leaving the check set incomplete, and the gate waits.
+      - name: Release the Codecov verdict
+        env:
+          # Empty on fork PRs, where Actions withholds secrets. That matches the tokenless upload steps
+          # in validate-code / validate-tests, so pass --token only when there is one to pass.
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+          # MUST match the uploads' override_commit exactly (head sha on a PR, github.sha on push) or the
+          # notification targets a commit that has no reports attached to it.
+          COMMIT_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
+          SLUG: ${{ github.repository }}
+          # Pinned like every action SHA in this file -- the CLI reaches the same Codecov API the uploads do.
+          CODECOV_CLI_VERSION: "11.3.1"
+        run: |
+          if ! pipx install "codecov-cli==${CODECOV_CLI_VERSION}"; then
+            echo "::warning title=Codecov::could not install codecov-cli==${CODECOV_CLI_VERSION}; no verdict will post for ${COMMIT_SHA}."
+            exit 0
+          fi
+          args=(--commit-sha "$COMMIT_SHA" --slug "$SLUG" --git-service github --fail-on-error)
+          # `[ -n "$X" ] && args+=(...)` would abort the step under `bash -e` when the token is absent.
+          if [ -n "$CODECOV_TOKEN" ]; then
+            args+=(--token "$CODECOV_TOKEN")
+          fi
+          # --fail-on-error makes the CLI exit non-zero so a real failure is visible in the log; catching
+          # it keeps that visibility without ever turning this check red.
+          if ! codecovcli send-notifications "${args[@]}"; then
+            echo "::warning title=Codecov::send-notifications failed for ${COMMIT_SHA} -- codecov/patch will not post for this run."
+          fi

codecov.yml

@@ -11,14 +11,32 @@ codecov:
   # Don't post a verdict until the CI run that produced the report has finished.
   require_ci_to_pass: true
   notify:
-    # The count that ALWAYS lands whenever validate-tests runs: since the 2026-07-24 unsharding, the
-    # backend suite produces ONE whole-suite lcov upload (flags: backend) instead of the former 3 shard
-    # uploads this threshold was sized for. It must stay 1, not 2, even though review-enrichment /
-    # control-plane add their own flag uploads on PRs that touch them -- 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. The narrow premature-window this reintroduces (a rees/control-plane upload landing
-    # and posting before the ~25-minute backend run finishes) self-corrects on the backend upload, and
-    # require_ci_to_pass above still holds the final verdict to the run's own conclusion.
+    # Hold every status and comment until CI says the uploads are done, rather than guessing from a
+    # count (#9801). `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 while
+    # validate-tests is still running the backend lcov that covers most of src/**. So the verdict was
+    # computed from a partial report -- codecov/patch 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 here: 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.
+    #
+    # Raising the count cannot fix it -- see the reasoning preserved on after_n_builds below -- because
+    # a count cannot express "wait for the backend upload specifically". manual_trigger can: Codecov
+    # sends nothing until `codecovcli send-notifications` runs, and the codecov-notify job in
+    # .github/workflows/ci.yml runs that once, after every uploading job has finished.
+    #
+    # Failure mode if the trigger never runs: codecov/patch simply never posts, so the gate sees an
+    # incomplete check set and waits. That is fail-safe -- it cannot merge unreviewed and cannot
+    # false-close -- which is the whole point of the change.
+    manual_trigger: true
+    # Inert while manual_trigger is true, and kept deliberately so removing manual_trigger restores the
+    # correct value rather than the default. The count that ALWAYS lands whenever validate-tests runs:
+    # since the 2026-07-24 unsharding, the backend suite produces ONE whole-suite lcov upload
+    # (flags: backend) instead of the former 3 shard uploads this threshold was sized for. It must stay
+    # 1, not 2, even though review-enrichment / control-plane add their own flag uploads on PRs that
+    # touch them -- 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.
     after_n_builds: 1
 
 coverage:

test/unit/ci-codecov-notify.test.ts

@@ -0,0 +1,114 @@
+import { readFileSync } from "node:fs";
+import { parse } from "yaml";
+import { describe, expect, it } from "vitest";
+
+function readYaml(path: string): Record<string, unknown> {
+  return record(parse(readFileSync(path, "utf8")), path);
+}
+
+function record(value: unknown, label: string): Record<string, unknown> {
+  if (!value || typeof value !== "object" || Array.isArray(value)) {
+    throw new Error(`${label} must be an object`);
+  }
+  return value as Record<string, unknown>;
+}
+
+function steps(job: Record<string, unknown>): Array<Record<string, unknown>> {
+  return (job.steps as Array<Record<string, unknown>>) ?? [];
+}
+
+// Regression guard for #9801: codecov/patch posted a red conclusion computed from a PARTIAL report.
+// 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 while validate-tests is still
+// producing the backend lcov that covers most of src/**. Codecov's own comment printed
+// `backend | BASE 1 | HEAD 0` while it was posting a failure. Because the gate closes a contributor PR on
+// ANY red check, codecov/patch included, that transient red could close a PR with no defect.
+//
+// The fix holds all notifications (notify.manual_trigger) until the codecov-notify job releases them once
+// every uploading job has finished. These assertions pin the parts that are load-bearing and silent when
+// broken -- a wrong commit sha or a resurrected count would fail open, not loudly.
+describe("codecov notifications wait for every upload (#9801)", () => {
+  const workflow = readYaml(".github/workflows/ci.yml");
+  const jobs = record(workflow.jobs, "workflow.jobs");
+  const codecov = readYaml("codecov.yml");
+
+  it("codecov.yml holds every status until the CLI releases it", () => {
+    const notify = record(record(codecov.codecov, "codecov").notify, "codecov.notify");
+    expect(notify.manual_trigger).toBe(true);
+    // Kept deliberately so dropping manual_trigger restores the correct floor rather than the default.
+    expect(notify.after_n_builds).toBe(1);
+  });
+
+  it("codecov-notify waits on every job that uploads a report", () => {
+    const job = record(jobs["codecov-notify"], "jobs.codecov-notify");
+    // validate-code carries rees/control-plane/engine; validate-tests carries backend. Missing either
+    // reintroduces the partial-report verdict this job exists to prevent.
+    expect(job.needs).toEqual(["changes", "validate-code", "validate-tests"]);
+  });
+
+  it("codecov-notify still runs when an upload job fails, but not on a superseded run", () => {
+    const condition = String(record(jobs["codecov-notify"], "jobs.codecov-notify").if);
+    // A failed validate-tests must still release the verdict, otherwise codecov/patch never posts at all.
+    expect(condition).toContain("!cancelled()");
+    // always() would notify on a sha the replacement run has already superseded.
+    expect(condition).not.toContain("always()");
+    // A fully path-filtered PR uploads nothing; notifying on a commit Codecov has no reports for errors.
+    expect(condition).toContain("needs.validate-code.result != 'skipped'");
+    expect(condition).toContain("needs.validate-tests.result != 'skipped'");
+  });
+
+  it("codecov-notify is NOT a dependency of the required validate check", () => {
+    // The bug being fixed is a phantom red closing a good PR. Wiring the notifier into the required
+    // check would let a Codecov delivery hiccup do exactly that.
+    expect(record(jobs.validate, "jobs.validate").needs).not.toContain("codecov-notify");
+  });
+
+  it("a failed send-notifications warns instead of failing the job", () => {
+    const run = String(steps(record(jobs["codecov-notify"], "jobs.codecov-notify")).at(-1)?.run ?? "");
+    // The CLI exits non-zero (--fail-on-error) so a real failure is visible, and the `if !` converts it
+    // to a warning. Without the guard the step would go red and re-create the false-close class.
+    expect(run).toContain("--fail-on-error");
+    expect(run).toContain("if ! codecovcli send-notifications");
+    expect(run).toContain("::warning title=Codecov::");
+  });
+
+  it("the token flag is only added when a token exists (fork PRs run without secrets)", () => {
+    const run = String(steps(record(jobs["codecov-notify"], "jobs.codecov-notify")).at(-1)?.run ?? "");
+    // `[ -n "$X" ] && args+=(...)` aborts the whole step under `bash -e` when the token is empty, which
+    // is exactly the fork-PR case -- it must stay an if-block.
+    expect(run).toContain('if [ -n "$CODECOV_TOKEN" ]; then');
+    expect(run).not.toMatch(/\[ -n "\$CODECOV_TOKEN" \] &&/);
+  });
+
+  it("codecov-notify targets the same commit the uploads used", () => {
+    const notifyStep = steps(record(jobs["codecov-notify"], "jobs.codecov-notify")).at(-1);
+    const sha = String(record(notifyStep?.env, "codecov-notify.env").COMMIT_SHA);
+    // The uploads all pass override_commit: head sha on a PR, github.sha on push. A mismatch here points
+    // the notification at a commit with no reports attached, so nothing ever posts.
+    const expected = "${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}";
+    expect(sha).toBe(expected);
+
+    const uploadOverrides = steps(record(jobs["validate-tests"], "jobs.validate-tests"))
+      .filter((step) => String(step.name ?? "").startsWith("Upload coverage to Codecov") && !String(step.name).includes("fork"))
+      .map((step) => String(record(step.with, "step.with").override_commit));
+    expect(uploadOverrides.length).toBeGreaterThan(0);
+    for (const override of uploadOverrides) expect(override).toBe(expected);
+  });
+
+  it("the Codecov CLI is pinned like every other third-party dependency in the workflow", () => {
+    const env = record(steps(record(jobs["codecov-notify"], "jobs.codecov-notify")).at(-1)?.env, "codecov-notify.env");
+    expect(String(env.CODECOV_CLI_VERSION)).toMatch(/^\d+\.\d+\.\d+$/);
+  });
+
+  it("no step in codecov-notify can turn the check red", () => {
+    // The gate closes a contributor PR on ANY red check, so a failed notifier would re-create the very
+    // bug it fixes. Every failure path -- install included -- has to be swallowed.
+    const job = record(jobs["codecov-notify"], "jobs.codecov-notify");
+    const runs = steps(job).map((step) => String(step.run ?? ""));
+    expect(runs).toHaveLength(1);
+    // A bare `pipx install` would fail the step (and the check) when the runner image lacks the CLI.
+    expect(runs[0]).toContain("if ! pipx install");
+    expect(runs[0]).toContain("exit 0");
+    for (const step of steps(job)) expect(step.uses).toBeUndefined();
+  });
+});