Skip to content

PR coverage run is cancelled by any PR comment, including comments the job skips #279

Description

Describe the bug

Any comment posted on a PR cancels that PR's in-flight PR Code Coverage run — including comments the coverage job explicitly declines to act on.

.github/workflows/pr-code-coverage.yml triggers on issue_comment: types: [created] with no filter at the trigger level (lines 40–42). The /coverage filter lives only in the job-level if: (line 68):

startsWith(github.event.comment.body, '/coverage')

But the workflow-level concurrency block (lines 48–50) is keyed on the PR number alone and evaluated at run creation, before any job if: runs:

concurrency:
  group: pr-coverage-${{ github.event.pull_request.number || github.event.issue.number }}
  cancel-in-progress: true

So an ordinary review comment creates a run, that run joins group pr-coverage-<n>, cancel-in-progress kills the real pull_request run, and only then does the job evaluate its if: and skip — having done no work but destroyed the run that was doing the work.

The guard is correct; it is just in the wrong place to prevent the cancellation.

Steps to reproduce

  1. Open a same-repo PR so the pull_request coverage run starts. It begins a long wait for the ADO Cobertura artifact (the workflow allows up to 1h15m; ADO builds take ~45 min).
  2. While that run is waiting, post any normal comment on the PR — a review reply, not /coverage.
  3. Observe the coverage run is cancelled within seconds, at the Download Cobertura coverage artifact step.

Observed on PR #264 (4c64fd2f):

Time (UTC) Event
19:32:15 pull_request run 31736332629 starts on dev/saurabh/row-decode-dispatch-perf @ 4c64fd2f
19:35:06 review reply comment 5285489791 posted — does not start with /coverage
19:35:09 issue_comment run 31736577259 created on main, joins group pr-coverage-264
19:35:26 run 31736332629 cancelled at step 6 of 9
19:35:27 run 31736577259 concludes skipped

The canceller's own conclusion is skipped, which is the proof that concurrency is resolved before the job if: — a run whose only job never executed still cancelled a sibling.

Note that issue_comment runs execute against main, not the PR head. Listing runs filtered to the PR branch shows only the cancelled victim and none of the cause, which makes this awkward to diagnose from the PR page:

gh run list --branch dev/saurabh/row-decode-dispatch-perf --workflow "PR Code Coverage"
# -> 31736332629  pull_request  cancelled  4c64fd2f     (1 run; canceller not shown)

Expected behavior

A comment that does not start with /coverage should have no effect on a running coverage report. Only a new push to the PR should cancel and supersede an in-flight run.

Actual behavior

The coverage run is cancelled mid-wait and reports as a failed check on the PR (gh pr checks renders the cancelled conclusion as fail), even though nothing about the code or the coverage changed. Re-running it is the only recovery, and it is cancelled again by the next comment.

Because coverage waits up to 1h15m and review rounds consist of posting comments, essentially any PR under active review loses its coverage run.

Suggested fix

Keep real runs sharing the group — a new push should supersede a stale wait — and shunt no-op comment runs into their own group so they cannot cancel anything:

concurrency:
  group: >-
    pr-coverage-${{ github.event.pull_request.number || github.event.issue.number }}${{
      github.event_name == 'issue_comment'
      && !startsWith(github.event.comment.body, '/coverage')
      && format('-noop-{0}', github.run_id) || '' }}
  cancel-in-progress: true

This duplicates the /coverage predicate in two places, so an alternative is to drop cancel-in-progress for issue_comment runs, or to gate the trigger itself. Either is fine; the requirement is only that a run which will skip must not share a cancelling group with a run that will work.

Version

main @ 445459bb; workflow file .github/workflows/pr-code-coverage.yml unchanged since its introduction.

Affected crate

N/A — this is CI infrastructure (.github/workflows/pr-code-coverage.yml), not a crate. It affects coverage reporting for every crate.

Environment

GitHub Actions, ubuntu-latest. Not environment-specific.

Additional context

Reporting-only: diff-cover runs without --fail-under, so this never blocks a merge. The cost is a red check that looks like a coverage regression, plus the loss of the diff-coverage number for the round.

Found while confirming CI on #264 — the cancellation there was caused by my own review replies.

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions