Uh oh!
There was an error while loading. Please reload this page.
Enable ci for external collaborators - #325
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
89ee70c to
d6888f2Compared6888f2 to
ad93a6fCompare| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| result-encoding: string | ||
| script: | | ||
| try { |
There was a problem hiding this comment.
I'd prefer we pull these out into script files, like here.
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| await github.rest.issues.createComment({ |
There was a problem hiding this comment.
Not sure we need to post a comment on the PR. Maybe just no-op.
| const timestamp = new Date().getTime(); | ||
| const ciBranchName = `ci-test/${prDetails.number}-${timestamp}`; | ||
| // Add remote for the external fork if it's from a fork |
There was a problem hiding this comment.
It would always be from a fork, no?
| script: | | ||
| const prDetails = ${{ steps.pr-details.outputs.result }}; | ||
| const timestamp = new Date().getTime(); | ||
| const ciBranchName = `ci-test/${prDetails.number}-${timestamp}`; |
There was a problem hiding this comment.
timestamp seems unnecessary. PR numbers are unique.
Unless we are planning on creating a new branch each time this workflow is run, but my thinking was that we'd have a 1:1 relationship, and the test PR would update if the branch already exists and /run-ci is invoked for future updates on a given PR. Maybe this is fine for now as
* Initial plan * Extract inline scripts to separate files Co-authored-by: pranaygp <1797812+pranaygp@users.noreply.github.com> * Fix indentation in create-ci-pr.js Co-authored-by: pranaygp <1797812+pranaygp@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pranaygp <1797812+pranaygp@users.noreply.github.com>
📊 Benchmark Results
workflow with no steps
workflow with 1 step
workflow with 10 sequential steps
workflow with 10 parallel steps
Stream BenchmarksStream benchmarks include Time to First Byte (TTFB) metrics. workflow with stream
Summary: Fastest Framework by WorldWinner determined by most benchmark wins
Summary: Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
Check the workflow run for details. |
Co-authored-by: pranaygp <1797812+pranaygp@users.noreply.github.com>
| return { | ||
| head_ref: pr.data.head.ref, | ||
| head_sha: pr.data.head.sha, | ||
| head_repo_full_name: pr.data.head.repo.full_name, |
There was a problem hiding this comment.
The code will crash with a TypeError when a PR is from a deleted fork, because it tries to access full_name on a null object. When a fork is deleted, GitHub returns head.repo as null in the PR data.
View Details
📝 Patch Details
diff --git a/.github/scripts/get-pr-details.js b/.github/scripts/get-pr-details.js
index 88a0ce5..3aad14c 100644
--- a/.github/scripts/get-pr-details.js+++ b/.github/scripts/get-pr-details.js@@ -5,6 +5,16 @@ module.exports = async ({ github, context }) => {
pull_number: context.issue.number
});
+ if (!pr.data.head.repo) {+ await github.rest.issues.createComment({+ owner: context.repo.owner,+ repo: context.repo.repo,+ issue_number: context.issue.number,+ body: '❌ Cannot run CI: The source fork for this PR has been deleted. The fork must exist in order to fetch the changes and run CI tests.'+ });+ throw new Error('Source fork has been deleted');+ }+
return {
head_ref: pr.data.head.ref,
head_sha: pr.data.head.sha,
Analysis
Unhandled null reference crash when PR is from deleted fork
What fails: The script .github/scripts/get-pr-details.js at line 11 crashes with TypeError: Cannot read properties of null (reading 'full_name') when attempting to access pr.data.head.repo.full_name without checking if head.repo is null.
How to reproduce:
- Create a pull request from an external fork
- Delete the external fork
- Comment
/run-cion the original PR - The workflow "Trigger CI for External PRs" will crash in the "Get PR details" step
Result: Crashes with: Error: Cannot read properties of null (reading 'full_name')
Expected behavior: Should gracefully handle the deleted fork scenario and post a helpful error message to the PR, as documented in the octokit/rest.js issue #31. When a PR originates from an "unknown repository" (i.e., deleted fork), the GitHub API returns head.repo as null.
Fix implemented:
- Added null check for
pr.data.head.repobefore accessing its properties - Posts a user-friendly error comment to the PR explaining why CI cannot run
- Throws an error to fail the workflow gracefully instead of crashing
Add new
/run-cicommand to allow repo admins to trigger a CI run for external collaborator PRsLeaving in draft just so we remove the claude generated md files before actually merging :)