Skip to content

Resolve PR by head SHA in coverage-comment workflow#847

Merged
mjp41 merged 1 commit intomicrosoft:mainfrom
mjp41:coverage-ci-fix
May 8, 2026
Merged

Resolve PR by head SHA in coverage-comment workflow#847
mjp41 merged 1 commit intomicrosoft:mainfrom
mjp41:coverage-ci-fix

Conversation

@mjp41
Copy link
Copy Markdown
Member

@mjp41 mjp41 commented May 8, 2026

workflow_run.pull_requests[] is always empty for PRs whose head ref lives in a fork. By GitHub design, the default-branch workflow_run handler (which holds the write token) does not receive cross-repo PR linkage. The first PR through the new coverage workflow happened to come from a fork, so the comment was posted to the tracking issue instead of the PR.

Replace the bash step with a github-script step:

  • reads github.event via an environment variable, immune to payload-quoting issues;
  • first tries workflow_run.pull_requests[0] for the same-repo branch case;
  • falls back to GET /repos/{owner}/{repo}/commits/{sha}/pulls (the "pull requests associated with commit" API), which is the only way to recover a PR number from a fork workflow_run payload;
  • prefers an open PR but tolerates a closed one if there's a squash-merge race between the build and this comment job;
  • falls through to the tracking-issue path only when neither source produces a PR.

The downstream "Comment on PR" step is unchanged — it consumes the same steps.pr.outputs.pr it always did.

Two issues with the previous "Resolve PR number" step:

1. Inline `${{ toJson(github.event) }}` interpolation inside `'...'`
   shell quoting could be broken by any single quote in the payload
   (e.g. an apostrophe in a commit message), producing the
   notorious `syntax error near unexpected token "else"`.

2. `workflow_run.pull_requests[]` is *always empty* for PRs whose
   head ref lives in a fork. By GitHub design, the default-branch
   `workflow_run` handler (which holds the write token) does not
   receive cross-repo PR linkage. The first PR through the new
   coverage workflow happened to come from a fork, so the comment
   was posted to the tracking issue instead of the PR.

Replace the bash step with a github-script step:

  - reads `github.event` via an environment variable, immune to
    payload-quoting issues;
  - first tries `workflow_run.pull_requests[0]` for the same-repo
    branch case;
  - falls back to
    `GET /repos/{owner}/{repo}/commits/{sha}/pulls` (the
    "pull requests associated with commit" API), which is the only
    way to recover a PR number from a fork `workflow_run` payload;
  - prefers an open PR but tolerates a closed one if there's a
    squash-merge race between the build and this comment job;
  - falls through to the tracking-issue path only when neither
    source produces a PR.

The downstream "Comment on PR" step is unchanged — it consumes the
same `steps.pr.outputs.pr` it always did.
@mjp41 mjp41 merged commit 6e0989c into microsoft:main May 8, 2026
195 checks passed
@mjp41 mjp41 deleted the coverage-ci-fix branch May 8, 2026 08:24
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Coverage report (cross-platform merged)

Lines covered (src/snmalloc/**): 2788 / 3178 (87.73%)

Merged line coverage is the per-line union across all platforms. Region coverage is reported per-platform only; no cross-platform region total is computed.

Per-directory breakdown

Directory Lines covered Lines executable %
src/snmalloc/mitigations 8 18 44.44%
src/snmalloc/stl 2 4 50.00%
src/snmalloc/override 94 156 60.26%
src/snmalloc/pal 330 449 73.50%
src/snmalloc/ds_core 340 399 85.21%
src/snmalloc/aal 51 59 86.44%
src/snmalloc/global 264 303 87.13%
src/snmalloc/ds 328 357 91.88%
src/snmalloc/backend_helpers 336 356 94.38%
src/snmalloc/mem 837 872 95.99%
src/snmalloc/ds_aal 108 112 96.43%
src/snmalloc/backend 90 93 96.77%
Per-platform contributions (advisory)
Platform Lines covered Lines executable Lines % Regions covered Regions executable Regions %
freebsd-14 4177 4680 89.25% 3782 5609 67.43%
linux-self-host-shim-checks 4261 4736 89.97% 3881 5909 65.68%
linux-self-host-shim-checks-selfhost 1640 2323 70.60% 1498 2911 51.46%
macos-14 4213 4664 90.33% 3841 5628 68.25%
windows-2022 4119 4684 87.94% 3774 5590 67.51%

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.

1 participant