fix: resolve fork pull requests for publishing - #1
Conversation
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
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.
Summary
workflow_run.pull_requestsfor fork PRsValidation
npm run checkgit diff --check