ci(integrated-benchmark): list open PRs to find fork-PR head SHA by zkochan · Pull Request #399 · pnpm/pacquet

zkochan · 2026-05-07T16:40:22Z

Summary

Fix the comment-posting workflow's fork-PR fallback. Verified failing on the test rebase of #391: the Stage-2 workflow erred with no open PR matches head_sha=b965c5e... in pnpm/pacquet.

Why the previous fallback failed

The fallback in #398 used gh api repos/$REPO/commits/$HEAD_SHA/pulls — but that endpoint only returns PRs whose head commit lives on the base repo. For fork PRs, the head SHA only exists on the fork (Saturate/pacquet for #391), so the base-repo lookup returns []. The exact case the workflow exists to support hits the wrong endpoint.

I tested both endpoints directly against PR #391's head SHA b965c5ef03ee15caffb65bddcbe43510d5f47f8d:

$ gh api repos/pnpm/pacquet/commits/b965c5ef03ee15caffb65bddcbe43510d5f47f8d/pulls --jq '[.[].number]'
[]                                  # ← what the workflow saw

$ gh api 'repos/pnpm/pacquet/pulls?state=open&per_page=100' --paginate \
    --jq '[.[] | select(.head.sha == "b965c5ef03ee15caffb65bddcbe43510d5f47f8d") | .number]'
[391]                               # ← what we want

What changes

Switch the fork-PR fallback from commits/{sha}/pulls to pulls?state=open + jq filter on head.sha. The PRs API records each PR's head SHA on the base-repo side regardless of where the head branch lives, so one code path covers both same-repo and fork.

Same-repo PRs still bypass the API call entirely via workflow_run.pull_requests[0].number. The fallback only runs when that array is empty (fork PRs).

Trust model unchanged

HEAD_SHA still comes from the trusted workflow_run event payload, never from the artifact. The artifact still carries only SUMMARY.md. The collision-on-SHA check (count != 1 ⇒ error) still fires loudly if the impossible happens.

Test plan

actionlint clean.
Validates end-to-end on the next benchmark run of any fork PR. Will rebase feat: lifecycle script execution and allowBuilds policy #391 again once this lands to confirm.

Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

Bug Fixes
- Fixed benchmark workflow PR detection for forked contributions so benchmark comments are reliably posted to the correct pull request.
- Added stricter matching and safeguards to prevent ambiguous or incorrect PR attribution for benchmark artifacts.

The fallback `gh api repos/$REPO/commits/$HEAD_SHA/pulls` lookup only returns PRs whose head commit lives on the base repo — for fork PRs the head SHA exists on the fork (e.g. `Saturate/pacquet`), not the base, so that endpoint returns `[]`. The Stage-2 comment workflow on PR #391 failed at this step: > ##[error]no open PR matches head_sha=b965c5e... in pnpm/pacquet Switch to listing open PRs in the base repo and filtering by `head.sha`. The PRs API records each PR's head SHA on the base-repo side regardless of where the head branch lives, so this works for both same-repo PRs and fork PRs through one code path. Same-repo PRs still bypass the API call entirely via the trusted `workflow_run.pull_requests[0].number` shortcut. The fallback only runs when that array is empty (the fork-PR case).

coderabbitai · 2026-05-07T16:40:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8fa47271-c8a5-47b3-b601-0ffb5036ae8f

📥 Commits

Reviewing files that changed from the base of the PR and between 918feca and 9e10cae.

📒 Files selected for processing (1)

.github/workflows/integrated-benchmark-comment.yml

🚧 Files skipped from review as they are similar to previous changes (1)

.github/workflows/integrated-benchmark-comment.yml

📝 Walkthrough

Walkthrough

The workflow updates PR-number resolution for fork-triggered benchmark runs: when TRIGGER_PR is unset it lists open PRs in the base repo, filters by matching head.sha using the trusted workflow_run.head_sha, and returns a unique PR number; same-repo behavior is unchanged.

Changes

Fork PR Number Resolution

Layer / File(s)	Summary
Strategy Documentation `.github/workflows/integrated-benchmark-comment.yml`	Comments clarify that `workflow_run.pull_requests` is empty for forks and the fallback lists open PRs and filters by `head.sha` using `workflow_run.head_sha`.
PR Lookup Implementation `.github/workflows/integrated-benchmark-comment.yml`	Fork PR resolution replaces `repos/$REPO/commits/$HEAD_SHA/pulls` with `repos/$REPO/pulls`, filters results by `head.sha`, computes a unique set of matching PR numbers, and returns the single PR number.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

pnpm/pacquet#398: Updates PR-number resolution logic for fork-triggered benchmark workflow runs using similar open PR listing and head.sha filtering approach.

Suggested reviewers

KSXGitHub

Poem

🐰 A fork once lost its PR in the night,
The workflow listed open PRs to make it right.
It matched the head SHA, found the number true,
One unique PR — hop, bounce, woohoo! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: switching from a commit-based endpoint to listing open PRs to find the fork PR's head SHA.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ci/benchmark-comment-fork-pr-lookup

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/integrated-benchmark-comment.yml (1)
64-77: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

--paginate --jq applies the filter per page, breaking count/extraction when > 100 open PRs exist

In --paginate mode, each page is a separate JSON array or object; --slurp is needed to wrap all pages into a single outer JSON array before applying a --jq filter.

Without --slurp, when the repo has > 100 open PRs (triggering a second page), $pr is a newline-separated stream of one JSON array per page, e.g.:
[200]
[]
[]
count=$(echo "$pr" | jq 'length') then outputs 1\n0\n0 — a multi-line string. The [[ "$count" != "1" ]] check compares that multi-line string to "1" and is always true, causing a spurious "ambiguous head_sha" error even when exactly one PR matches. jq '.[0]' on the same input would similarly yield 200\nnull\nnull, which fails the integer guard.

Add --slurp and flatten with .[][]:
🐛 Proposed fix
             pr=$(
-              gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate \
-                --jq "[.[] | select(.head.sha == \"$HEAD_SHA\") | .number] | unique"
+              gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate --slurp \
+                --jq "[.[][] | select(.head.sha == \"$HEAD_SHA\") | .number] | unique"
             )
Alternatively, drop --jq entirely and pipe to jq with -rs, which also avoids shell-interpolating $HEAD_SHA into the jq expression (using env.HEAD_SHA instead):
             pr=$(
-              gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate \
-                --jq "[.[] | select(.head.sha == \"$HEAD_SHA\") | .number] | unique"
+              gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate \
+                | jq -rs "[.[][] | select(.head.sha == env.HEAD_SHA) | .number] | unique"
             )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/integrated-benchmark-comment.yml around lines 64 - 77, The
gh api call using --paginate --jq returns per-page JSON chunks which makes the
pr and count variables multiline when >100 PRs exist; update the invocation so
pagination is slurped and flattened before filtering (e.g., add --slurp and use
a jq filter like .[][] | select(.head.sha == env.HEAD_SHA) | .number or remove
--jq and pipe output to jq -rs with env.HEAD_SHA), then compute count and
extract pr with a single-array result so the existing checks on count and pr
(variables named pr and count and the jq '.[0]' extraction) work correctly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/workflows/integrated-benchmark-comment.yml:
- Around line 64-77: The gh api call using --paginate --jq returns per-page JSON
chunks which makes the pr and count variables multiline when >100 PRs exist;
update the invocation so pagination is slurped and flattened before filtering
(e.g., add --slurp and use a jq filter like .[][] | select(.head.sha ==
env.HEAD_SHA) | .number or remove --jq and pipe output to jq -rs with
env.HEAD_SHA), then compute count and extract pr with a single-array result so
the existing checks on count and pr (variables named pr and count and the jq
'.[0]' extraction) work correctly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fdaca5a4-90d8-4db3-9315-e76faebc5124

📥 Commits

Reviewing files that changed from the base of the PR and between 8416e38 and 918feca.

📒 Files selected for processing (1)

.github/workflows/integrated-benchmark-comment.yml

Copilot

Pull request overview

Updates the Stage-2 integrated benchmark comment workflow to correctly resolve fork PR numbers by querying open PRs and matching on workflow_run.head_sha, avoiding the base-repo-only commits/{sha}/pulls endpoint limitation.

Changes:

Replace fork-PR fallback from commits/{sha}/pulls to pulls?state=open + head.sha filter.
Expand inline documentation explaining why the previous approach fails for fork PRs and reaffirm the trust model for HEAD_SHA.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`gh api ... --paginate --jq` runs the jq filter per page, so a query that returns multiple pages emits one JSON array per page on stdout, not a single combined array. Reproduced locally with per_page=2: $ gh api '.../pulls?state=open&per_page=2' --paginate \ --jq '[.[] | .number]' [399,391] [385,337] [333,332] ... Today pacquet has under 100 open PRs so a single page suffices and the bug is dormant; past that threshold the downstream `jq 'length'` would read each line as a separate input and produce multiple values like `2\n2\n2\n1`, which fails both the `count == 0` and `count == 1` checks and bottoms out in the "ambiguous" error. Fix: emit one PR per line with `--paginate --jq '.[]'` so the stream spans pages cleanly, then `jq -s` slurps the whole thing back into a single array. `--arg sha` keeps the head SHA out of the jq program text. Verified with per_page=2 against PR #391's head SHA: returns `[391]` as expected. Reported by Copilot on #399.

Copilot AI review requested due to automatic review settings May 7, 2026 16:40

Copilot started reviewing on behalf of zkochan May 7, 2026 16:41 View session

coderabbitai Bot reviewed May 7, 2026

View reviewed changes

Copilot AI reviewed May 7, 2026

View reviewed changes

Comment thread .github/workflows/integrated-benchmark-comment.yml Outdated

zkochan merged commit 03154e6 into main May 7, 2026
8 checks passed

zkochan deleted the ci/benchmark-comment-fork-pr-lookup branch May 7, 2026 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(integrated-benchmark): list open PRs to find fork-PR head SHA#399

ci(integrated-benchmark): list open PRs to find fork-PR head SHA#399
zkochan merged 2 commits into
mainfrom
ci/benchmark-comment-fork-pr-lookup

zkochan commented May 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 7, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zkochan commented May 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why the previous fallback failed

What changes

Trust model unchanged

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented May 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 7, 2026 •

edited

Loading