-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add PR preview deployments via Cloudflare Pages #302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 17 commits
841fb13
ddad993
a7ddb81
532bcbf
8fd59de
0785644
c42cbea
e809aeb
bafe318
7aee04b
208a9a5
c28baab
78c5219
35222d5
632f175
8e41d35
f3e43ba
c05a276
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,276 @@ | ||||||||||||||||||||||||||
| name: PR Preview | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||
| branches: [main] | ||||||||||||||||||||||||||
| types: [opened, synchronize, reopened, closed] | ||||||||||||||||||||||||||
| paths: | ||||||||||||||||||||||||||
| - "docs/**" | ||||||||||||||||||||||||||
| - "site/**" | ||||||||||||||||||||||||||
| - "mkdocs.yml" | ||||||||||||||||||||||||||
| - "src/ai_company/**" | ||||||||||||||||||||||||||
| - ".github/workflows/pages-preview.yml" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| permissions: {} | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| concurrency: | ||||||||||||||||||||||||||
| group: "pages-preview-${{ github.event.pull_request.number }}" | ||||||||||||||||||||||||||
| cancel-in-progress: true | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||
| build: | ||||||||||||||||||||||||||
| name: Build Preview | ||||||||||||||||||||||||||
| if: github.event.action != 'closed' | ||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||
|
greptile-apps[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 | ||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||
| persist-credentials: false | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| persist-credentials: false | |
| persist-credentials: false | |
| ref: ${{ github.event.pull_request.head.sha }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm dependency cache not configured in either Node.js setup step
Both setup-node calls in the workflow (here in build and again in deploy-preview at line 151) omit the cache input, so npm ci downloads Astro's entire dependency tree fresh on every PR push. For a preview workflow triggered on every synchronize event, this adds avoidable latency and bandwidth.
Adding cache: 'npm' (which keys on package-lock.json) is a one-liner:
| - name: Set up Node.js | |
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 | |
| with: | |
| node-version: "22" | |
| - name: Set up Node.js | |
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 | |
| with: | |
| node-version: "22" | |
| cache: 'npm' | |
| cache-dependency-path: site/package-lock.json |
The same change applies to the deploy-preview job's setup-node step at line 151. astral-sh/setup-uv similarly supports an enable-cache option that would cache Python package downloads between runs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pages-preview.yml
Line: 50-54
Comment:
**npm dependency cache not configured in either Node.js setup step**
Both `setup-node` calls in the workflow (here in `build` and again in `deploy-preview` at line 151) omit the `cache` input, so `npm ci` downloads Astro's entire dependency tree fresh on every PR push. For a preview workflow triggered on every `synchronize` event, this adds avoidable latency and bandwidth.
Adding `cache: 'npm'` (which keys on `package-lock.json`) is a one-liner:
```suggestion
- name: Set up Node.js
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
with:
node-version: "22"
cache: 'npm'
cache-dependency-path: site/package-lock.json
```
The same change applies to the `deploy-preview` job's `setup-node` step at line 151. `astral-sh/setup-uv` similarly supports an `enable-cache` option that would cache Python package downloads between runs.
How can I resolve this? If you propose a fix, please make it concise.
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The banner injection step logs how many HTML files were updated, but it never fails the build if count ends up as 0 (e.g., if the <body> regex doesn’t match). Since the banner is a key safety/UX signal for previews, consider failing the job when no files were modified (and/or make the <body> match case-insensitive) so a regression doesn’t silently ship an un-bannered preview.
| updated = re.sub(r"(<body[^>]*>)", lambda m: m.group(1) + banner, text, count=1) | |
| if updated != text: | |
| f.write_text(updated, encoding="utf-8") | |
| count += 1 | |
| print(f"Injected preview banner into {count} HTML files") | |
| updated = re.sub(r"(?i)(<body[^>]*>)", lambda m: m.group(1) + banner, text, count=1) | |
| if updated != text: | |
| f.write_text(updated, encoding="utf-8") | |
| count += 1 | |
| print(f"Injected preview banner into {count} HTML files") | |
| if count == 0: | |
| raise SystemExit("No HTML files were modified with the preview banner; failing build to avoid un-bannered preview.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Artifact upload/download major-version mismatch
actions/upload-artifact is pinned to # v7 but actions/download-artifact (line 121) is pinned to # v8. GitHub's artifact service uses a different internal format per major version — uploading with v7 and downloading with v8 (or vice versa) will result in the download step failing with "Artifact not found" or a format incompatibility error.
Both actions should reference the same major version. Pin both to the same version (typically the latest: both at v4, or both at whichever major version you intend):
| - name: Upload preview artifact | |
| uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7 | |
| - name: Upload preview artifact | |
| uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7 |
Fix: Update the
download-artifactpin on line 121 to the same v7 SHA/tag used here, or update both to a consistent version.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pages-preview.yml
Line: 104-105
Comment:
**Artifact upload/download major-version mismatch**
`actions/upload-artifact` is pinned to `# v7` but `actions/download-artifact` (line 121) is pinned to `# v8`. GitHub's artifact service uses a different internal format per major version — uploading with v7 and downloading with v8 (or vice versa) will result in the download step failing with "Artifact not found" or a format incompatibility error.
Both actions should reference the same major version. Pin both to the same version (typically the latest: both at v4, or both at whichever major version you intend):
```suggestion
- name: Upload preview artifact
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7
```
> **Fix:** Update the `download-artifact` pin on line 121 to the same v7 SHA/tag used here, or update both to a consistent version.
How can I resolve this? If you propose a fix, please make it concise.
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment preview URL uses github.rest.issues.* endpoints to list/update/create an issue comment, but this job only requests pull-requests: write. Grant issues: write (or replace pull-requests: write with issues: write) so the GITHUB_TOKEN can manage PR comments reliably.
| contents: read | |
| contents: read | |
| issues: write |
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This job uses the Issues API (issues.listComments, issues.updateComment, issues.createComment) to manage the PR comment, but the job permissions only grant pull-requests: write. With fine-grained workflow permissions, comment endpoints generally require issues: write; consider adding it here to avoid 403s when posting/updating the preview URL comment.
| pull-requests: write | |
| pull-requests: write | |
| issues: write |
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deploy-preview uses github.rest.issues.* (listComments/updateComment/createComment), but the job permissions only grant pull-requests: write. With top-level permissions: {}, this can lead to 403s because issue comments typically require issues: write. Add issues: write (and drop pull-requests: write if it’s no longer needed) so the preview URL comment can be created/updated reliably.
| pull-requests: write | |
| issues: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm install stdout suppression hides progress, consider redirecting to a log
npm i --no-save wrangler@3.114.17 > /dev/null discards stdout entirely, so if Wrangler takes an unexpectedly long time to install (large dependency tree, slow npm mirror, etc.) the step produces no visible output during that wait window. Stderr is still visible so genuine errors will surface, but a hung npm install would produce a silent, blank step log with no clue about what it is doing.
Consider redirecting to a step debug log file instead:
| npm i --no-save wrangler@3.114.17 > /dev/null | |
| npx wrangler pages deploy _site --project-name=synthorg-pr-preview --branch="pr-${PR_NUMBER}" | |
| npm i --no-save wrangler@3.114.17 2>&1 | grep -v "^npm warn" || true | |
| npx wrangler pages deploy _site --project-name=synthorg-pr-preview --branch="pr-${PR_NUMBER}" |
Or at minimum, only suppress the verbose npm progress lines:
npm i --no-save wrangler@3.114.17 --loglevel=errorThis keeps error output but hides the noisy download progress.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pages-preview.yml
Line: 165-166
Comment:
**`npm install` stdout suppression hides progress, consider redirecting to a log**
`npm i --no-save wrangler@3.114.17 > /dev/null` discards stdout entirely, so if Wrangler takes an unexpectedly long time to install (large dependency tree, slow npm mirror, etc.) the step produces no visible output during that wait window. Stderr is still visible so genuine errors will surface, but a hung `npm install` would produce a silent, blank step log with no clue about what it is doing.
Consider redirecting to a step debug log file instead:
```suggestion
npm i --no-save wrangler@3.114.17 2>&1 | grep -v "^npm warn" || true
npx wrangler pages deploy _site --project-name=synthorg-pr-preview --branch="pr-${PR_NUMBER}"
```
Or at minimum, only suppress the verbose npm progress lines:
```bash
npm i --no-save wrangler@3.114.17 --loglevel=error
```
This keeps error output but hides the noisy download progress.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
What is the latest version of Cloudflare wrangler npm package?
💡 Result:
The latest version of the Cloudflare wrangler npm package is 4.71.0. [1]
Update wrangler to the latest major version or document the reason for staying on 3.x.
Deploy job addresses all previous review feedback:
- Fork PR guard prevents secret exposure
- Stable branch alias URL construction (not scraped from output)
- Unique marker + bot-author check for comment matching
- Paginated comment listing
However, wrangler is pinned to 3.114.17 while the latest available version is 4.71.0. Using an outdated major version may miss security updates, bug fixes, and new features. Either upgrade to version 4.x or add a comment explaining the reason for pinning to 3.x.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pages-preview.yml around lines 136 - 209, The Deploy to
Cloudflare Pages step currently installs wrangler@3.114.17 (see the run block
that runs "npm i ... wrangler@3.114.17" and then "npx wrangler pages deploy");
update that install to a current 4.x release (e.g., wrangler@4.71.0) and verify
the CLI invocation (npx wrangler pages deploy) still works with v4, or if you
intentionally must stay on 3.x, add an inline comment in the same Deploy to
Cloudflare Pages step explaining the reason for pinning to 3.x
(compatibility/security/regression test) and reference the chosen 3.x version
and a plan/tracker issue for when it will be upgraded.
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleanup-preview deletes the PR comment via github.rest.issues.deleteComment, but the job permissions only grant pull-requests: write. With top-level permissions: {}, this may fail due to missing issues: write. Grant issues: write here as well to ensure cleanup can remove the preview comment.
| pull-requests: write | |
| pull-requests: write | |
| issues: write |
Copilot
AI
Mar 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step deletes the preview PR comment via the Issues API (issues.listComments / issues.deleteComment), but the job permissions only include pull-requests: write. Add issues: write at the job level to ensure the token can delete issue comments on PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|| true silently swallows Cloudflare API authentication and network errors
The || true on the DEPLOYMENTS=$(curl -sf ... | jq ...) pipeline causes any API-level failure to look identical to "no deployments found." Specifically:
curl -sfexits non-zero if the HTTP status is 4xx or 5xx (wrong token, wrong account ID, project not found, rate-limited) or if the connection fails entirely- When that happens, the pipeline returns non-zero,
|| trueconverts it to success,$DEPLOYMENTSis empty, and the nextif [ -z "$DEPLOYMENTS" ]branch prints"No deployments found for branch pr-N"and exits 0
A Cloudflare API call with invalid credentials would appear as a successful cleanup with no deployments — the stale preview branch would silently remain, and there is no indication in the workflow logs that anything went wrong.
To distinguish "no deployments for this branch" from "API call failed":
RESPONSE=$(curl -sf \
"https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_ACCOUNT_ID}/pages/projects/synthorg-pr-preview/deployments?per_page=100" \
-H "Authorization: Bearer ${CLOUDFLARE_API_TOKEN}") || {
echo "::error::Cloudflare API request failed — check CLOUDFLARE_API_TOKEN and CLOUDFLARE_ACCOUNT_ID"
exit 1
}
DEPLOYMENTS=$(echo "$RESPONSE" | jq -r ".result[] | select(.deployment_trigger.metadata.branch == \"${BRANCH}\") | .id")Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pages-preview.yml
Line: 265-271
Comment:
**`|| true` silently swallows Cloudflare API authentication and network errors**
The `|| true` on the `DEPLOYMENTS=$(curl -sf ... | jq ...)` pipeline causes any API-level failure to look identical to "no deployments found." Specifically:
- `curl -sf` exits non-zero if the HTTP status is 4xx or 5xx (wrong token, wrong account ID, project not found, rate-limited) **or** if the connection fails entirely
- When that happens, the pipeline returns non-zero, `|| true` converts it to success, `$DEPLOYMENTS` is empty, and the next `if [ -z "$DEPLOYMENTS" ]` branch prints `"No deployments found for branch pr-N"` and exits 0
A Cloudflare API call with invalid credentials would appear as a successful cleanup with no deployments — the stale preview branch would silently remain, and there is no indication in the workflow logs that anything went wrong.
To distinguish "no deployments for this branch" from "API call failed":
```bash
RESPONSE=$(curl -sf \
"https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_ACCOUNT_ID}/pages/projects/synthorg-pr-preview/deployments?per_page=100" \
-H "Authorization: Bearer ${CLOUDFLARE_API_TOKEN}") || {
echo "::error::Cloudflare API request failed — check CLOUDFLARE_API_TOKEN and CLOUDFLARE_ACCOUNT_ID"
exit 1
}
DEPLOYMENTS=$(echo "$RESPONSE" | jq -r ".result[] | select(.deployment_trigger.metadata.branch == \"${BRANCH}\") | .id")
```
How can I resolve this? If you propose a fix, please make it concise.
Uh oh!
There was an error while loading. Please reload this page.