fix(ci): post the archive link on pull requests from forks - #857
Conversation
workflow_run.pull_requests is empty for pull requests opened from forks, so EVENT_PR_NUMBER was blank and the job exited before commenting. Fall back to matching workflow_run.head_sha against the open pull requests. The artifact name also encodes the number, but it comes from the untrusted pull_request workflow, so the trusted head SHA is used instead.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 988b8e6154
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # workflow, so it is not trusted to select the comment target here. | ||
| if [[ ! "$EVENT_PR_NUMBER" =~ ^[0-9]+$ ]]; then | ||
| EVENT_PR_NUMBER=$( | ||
| gh api "repos/$GITHUB_REPOSITORY/pulls?state=open&per_page=100" --paginate \ |
There was a problem hiding this comment.
When a fork PR is merged or closed before its archive run completes, workflow_run.pull_requests is still empty but this query can no longer find the PR, so the following guard exits without posting the archive link. This is plausible because the archive job in pr-archive.yml may run for up to 45 minutes and closing a PR does not cancel its existing run; the fallback should also resolve matching closed/merged PRs while avoiding ambiguous SHA matches.
Useful? React with 👍 / 👎.
Greptile SummaryThe PR adds a head-SHA-based fallback intended to recover pull-request numbers for successful archive workflows originating from forks.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if [[ ! "$EVENT_PR_NUMBER" =~ ^[0-9]+$ ]]; then | ||
| EVENT_PR_NUMBER=$( | ||
| gh api "repos/$GITHUB_REPOSITORY/pulls?state=open&per_page=100" --paginate \ | ||
| --jq '.[] | select(.head.sha == env.HEAD_SHA) | .number' \ |
There was a problem hiding this comment.
When a fork run has no event PR number, gh api --jq does not expose the shell's HEAD_SHA through env.HEAD_SHA, so the filter returns no PR number and the guard exits without posting the archive link.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pr-archive-comment.yml
Line: 41
Comment:
**Environment lookup returns no match**
When a fork run has no event PR number, `gh api --jq` does not expose the shell's `HEAD_SHA` through `env.HEAD_SHA`, so the filter returns no PR number and the guard exits without posting the archive link.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| if [[ ! "$EVENT_PR_NUMBER" =~ ^[0-9]+$ ]]; then | ||
| EVENT_PR_NUMBER=$( | ||
| gh api "repos/$GITHUB_REPOSITORY/pulls?state=open&per_page=100" --paginate \ | ||
| --jq '.[] | select(.head.sha == env.HEAD_SHA) | .number' \ |
There was a problem hiding this comment.
For the upstream pull_request workflow, workflow_run.head_sha identifies the synthetic merge commit while .head.sha identifies the PR's actual head commit, so the fallback finds no PR and fork contributors still receive no archive link.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pr-archive-comment.yml
Line: 41
Comment:
**Merge SHA cannot match head**
For the upstream `pull_request` workflow, `workflow_run.head_sha` identifies the synthetic merge commit while `.head.sha` identifies the PR's actual head commit, so the fallback finds no PR and fork contributors still receive no archive link.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
Both findings look incorrect to me. Environment lookup returns no match. Merge SHA cannot match head. That SHA is PR #387's No change made. One adjacent limitation in the fallback worth noting for the record: a head SHA does not uniquely identify an open PR, and two open PRs have briefly shared one in this repo before (#510/#511 for a few seconds). If that ever happens the |
|
Since my last comment, the workflow has failed eight more times with the same guard error. The first maps to #867: PR Archive run That case also checks the two Greptile findings directly. The archive run's This addresses the two Greptile findings. The separate case where a PR leaves the open list before its archive finishes is a different question. |
|
One more data point that isolates this to fork PRs specifically. I checked which recent PRs actually received the archive comment:
Four different contributors' fork PRs get nothing, and the one non-fork PR gets the comment. That lines up with Happy to rebase or add a workflow test if either would help. |
1 participant
Description
The PR Archive Comment workflow fails on every pull request opened from a fork, so outside contributors never get the build download link.
workflow_run.pull_requestsis populated only for same-repository pull requests and is empty for pull requests from forks. The job readspull_requests[0].number, so for a fork PR the number is blank, the guard rejects it, and the job exits 1 with "The completed workflow run is not associated with a pull request."Across the last 40 runs of this workflow: 10 failures, all fork PRs; 16 successes, all same-repo PRs; no exceptions either way. The 14 skipped runs are the job's own
if: workflow_run.conclusion == 'success'condition and are unrelated.Worth noting the archive itself is fine. The "PR Archive" job builds and uploads the artifact correctly for fork PRs, so the only thing lost is the comment linking to it. That is also why this has stayed invisible: it works for branches pushed to this repo, which is the path you would normally see.
This falls back to matching the head SHA from the
workflow_runpayload against the open pull requests when the event does not carry a number. Same-repo PRs keep taking the existing path.One thing I deliberately did not do: the artifact name already encodes the PR number, which would have been a shorter fix. But that name is produced by the untrusted
pull_requestworkflow, and this workflow is trusted and hasissues: write, so a fork could name an artifact to steer the comment onto an unrelated PR.workflow_run.head_shacomes from the event payload instead.Type of Change
Related Issue or Discussion
No existing issue. #847 is a concrete instance: it merged, and the contributor never received an archive comment.
Testing
swiftlint --strict --config .swiftlint.yml Sourcesswiftformat --config .swiftformat SourcesNo Swift changed, so the Swift toolchain checks do not apply here. I ran the resolution logic against this repository's live API instead:
I first tried
repos/{repo}/commits/{sha}/pulls, which is the more obvious fallback, and it does not work here. It resolves merged fork PRs (#847) but returns empty for open ones, because the head commit lives in the fork rather than this repository. That is the case the fix is for, so it is matched against the open pull requests instead.The paginated call is one request at the current volume (48 open PRs at
per_page=100). YAML validated;sed -n '1p'matches the idiom already used further down and avoids a SIGPIPE underpipefail.Screenshots / Video
Notes
Only the fork path changes. Same-repo PRs never enter the new branch, since
pull_requests[0].numberis already set for them.Happy to add a debug line logging which path resolved the number if that would help when this is running in anger.