Skip to content

fix: resolve fork pull requests for publishing - #1

Merged
cpunion merged 1 commit into
mainfrom
fix/fork-pr-resolution
Jul 30, 2026
Merged

fix: resolve fork pull requests for publishing#1
cpunion merged 1 commit into
mainfrom
fix/fork-pr-resolution

Conversation

@cpunion

@cpunion cpunion commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • fall back to the workflow run head owner and branch when GitHub omits workflow_run.pull_requests for fork PRs
  • verify the matched PR head repository, branch, and SHA before publishing
  • keep benchmark data repository, branch, and site-path handling unchanged

Validation

  • npm run check
  • YAML parse and git diff --check
  • verified the fallback resolves xgo-dev/llgo PRs #2219, #2229, and #2231 from their fork heads

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

The change correctly fixes fork PR resolution: it derives the head owner from HEAD_REPOSITORY (${HEAD_REPOSITORY%%/*}), uses --method GET so -f fields become query params, and filters via env.* jq lookups (not string interpolation, so no jq/shell injection). The three-field jq match on head.repo.full_name, head.ref, and head.sha also correctly prevents binding to a PR the fork author doesn't own.

One concern (below), inline. Other subagent-raised items (owner derivation, --method GET, jq injection, missing three-field match) were checked against the actual diff and are already handled correctly — no action needed.

gh api --method GET "repos/${GITHUB_REPOSITORY}/pulls" \
-f state=open \
-f "head=${head_owner}:${HEAD_BRANCH}" \
--jq '[.[] | select(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jq filter requires head.sha == env.HEAD_SHA to match a PR. HEAD_SHA is workflow_run.head_sha (the benchmarked commit), while the live PR's head.sha advances if the contributor pushes again before this publish job runs. On that race the filter returns empty, PR_NUMBER fails the ^[1-9][0-9]*$ check, and the step exits 1 with "cannot resolve pull request for $HEAD_SHA" — a hard workflow failure.

The downstream guards ("Commit benchmark data" and "Create or update PR comment") already re-check head.sha == SOURCE_SHA and skip gracefully on a stale head. Consider resolving the PR on head.repo.full_name + head.ref only (the open PR for that fork branch) and letting those existing guards handle staleness — that avoids turning a benign fast-follow-up push into a failed run.

@cpunion
cpunion merged commit b2ee436 into main Jul 30, 2026
0 of 2 checks passed
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