Split pr-preview-artifact into separate build and deploy jobs with pull_request_target#185
Conversation
…oudflare environment Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
|
@copilot Fix the comment |
Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
Done! The workflow is now split into two jobs:
Commit: be6aeba
Fixed the PR comment script to:
Commit: be6aeba |
There was a problem hiding this comment.
Pull request overview
This PR refactors the preview deployment workflow to improve security by separating the build and deployment stages. The CI workflow now builds and uploads the website artifact on pull requests with minimal permissions, while the new deployment workflow downloads this artifact and deploys it to Cloudflare Pages using pull_request_target trigger with environment protection.
Key changes:
- CI workflow uploads website preview artifacts for all pull requests
- Preview deployment workflow uses
pull_request_targetwithcloudflareenvironment for secure secret access - Deployment workflow discovers and downloads artifacts from the CI workflow run
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
.github/workflows/ci.yml |
Added artifact upload step to build website previews for PRs with 30-day retention |
.github/workflows/pr-preview-artifact.yml |
Converted from build-and-deploy to deploy-only workflow using pull_request_target trigger, discovering CI runs, downloading artifacts, and deploying to Cloudflare |
| - name: Upload preview artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: website-preview-${{ github.event.pull_request.number || format('{0}-{1}', github.ref_name, github.run_number) }} | ||
| path: website/dist | ||
| retention-days: 30 |
There was a problem hiding this comment.
The artifact upload runs unconditionally on both push and pull_request events. This wastes storage and CI time for pushes to main branch since the preview workflow only consumes artifacts from PRs. Add a condition if: github.event_name == 'pull_request' to this step.
There was a problem hiding this comment.
Added if: github.event_name == 'pull_request' condition to the artifact upload step. Also simplified the artifact name since it will only run on pull requests now. Commit: 4486232
| while (status !== 'completed') { | ||
| await new Promise(resolve => setTimeout(resolve, 30000)); // Wait 30 seconds | ||
|
|
||
| const { data: run } = await github.rest.actions.getWorkflowRun({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| run_id: runId | ||
| }); | ||
|
|
||
| status = run.status; | ||
| console.log(`CI run status: ${status}`); | ||
|
|
||
| if (status === 'completed' && run.conclusion !== 'success') { | ||
| throw new Error(`CI workflow failed with conclusion: ${run.conclusion}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
The while loop polling for CI completion has no timeout or maximum iteration limit. If the CI workflow hangs or takes unexpectedly long, this step will run until the GitHub Actions job timeout (default 360 minutes), wasting resources. Add a maximum iteration counter (e.g., 40 iterations = 20 minutes) and throw an error if exceeded.
| // Look for CI runs that match this PR's head SHA or are associated with this PR | ||
| const ciRun = runs.workflow_runs.find(run => | ||
| run.head_sha === headSha || | ||
| (run.pull_requests && run.pull_requests.some(pr => pr.number == prNumber)) |
There was a problem hiding this comment.
Using loose equality (==) to compare PR numbers can lead to type coercion issues. The prNumber variable is a string (from step output), while pr.number is a number. Use strict equality (===) with explicit type conversion: run.pull_requests.some(pr => pr.number === parseInt(prNumber, 10)).
| (run.pull_requests && run.pull_requests.some(pr => pr.number == prNumber)) | |
| (run.pull_requests && run.pull_requests.some(pr => pr.number === parseInt(prNumber, 10))) |
| const ciRun = runs.workflow_runs.find(run => | ||
| run.head_sha === headSha || | ||
| (run.pull_requests && run.pull_requests.some(pr => pr.number == prNumber)) | ||
| ); |
There was a problem hiding this comment.
The find() method returns the first matching workflow run without ensuring it's the most recent. If multiple CI runs exist for the same PR (e.g., from re-runs or multiple commits), this could select an older run. Sort runs.workflow_runs by created_at descending before finding: runs.workflow_runs.sort((a, b) => new Date(b.created_at) - new Date(a.created_at)).find(...).
| const ciRun = runs.workflow_runs.find(run => | |
| run.head_sha === headSha || | |
| (run.pull_requests && run.pull_requests.some(pr => pr.number == prNumber)) | |
| ); | |
| const ciRun = runs.workflow_runs | |
| .sort((a, b) => new Date(b.created_at) - new Date(a.created_at)) | |
| .find(run => | |
| run.head_sha === headSha || | |
| (run.pull_requests && run.pull_requests.some(pr => pr.number == prNumber)) | |
| ); |
| if (!ciRun) { | ||
| throw new Error(`No CI workflow run found for PR #${prNumber} with SHA ${headSha}`); | ||
| } |
There was a problem hiding this comment.
The error message references both PR number and SHA, but when using workflow_dispatch, headSha will be github.sha which may not match the PR's actual head SHA. This makes debugging confusing. Consider enhancing the error message to clarify the search criteria used or fetch the actual PR head SHA from the GitHub API in workflow_dispatch mode.
| let comment = `## 🚀 Preview Deployment | ||
|
|
||
| A preview build has been generated for this PR! | ||
| A preview build has been generated for this PR from CI run [#${ciRunId}](${artifactUrl})! |
There was a problem hiding this comment.
[nitpick] The link text '#${ciRunId}' is misleading because it suggests a PR or issue number format, but workflow run IDs are not prefixed with '#' in GitHub's UI. Use descriptive text like 'CI run ${ciRunId}' or 'workflow run ${ciRunId}' instead.
| A preview build has been generated for this PR from CI run [#${ciRunId}](${artifactUrl})! | |
| A preview build has been generated for this PR from CI run [CI run ${ciRunId}](${artifactUrl})! |
Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
Co-authored-by: LittleLittleCloud <16876986+LittleLittleCloud@users.noreply.github.com>
Refactors the preview deployment workflow into two separate jobs for better security and control: a build job using
pull_requesttrigger and a deploy job usingpull_request_targettrigger with Cloudflare environment protection.Changes
Build Job (CI workflow)
if: github.event_name == 'pull_request'condition to artifact upload step to avoid wasting storage on main branch pushesDeploy Job (pull_request_target trigger)
pull_request_targetwithcloudflareenvironment for secure secret accessContext
This separation provides:
Better security: Build runs in PR context without secrets; deploy runs in base context with environment protection
Environment controls: Cloudflare environment can have approval workflows and environment-specific secrets
Resource efficiency: Preview artifacts only uploaded for pull requests, not pushes to main
Fixes update pr-preview-artifact's trigger to pull-request-target and consume environment: cloudflare #184
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.