-
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 5 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,183 @@ | ||||||||||||||||||||||||||
| name: PR Preview | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||
| branches: [main] | ||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||
| 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 |
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 to Cloudflare Pages uses npm/npx but the deploy-preview job doesn’t set up/pin Node like the build job does. Add an explicit actions/setup-node step (matching the Node version used in build) to keep deployments reproducible and avoid failures if the runner image changes.
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.
URL extraction uses grep ... | tail -1 under the default bash -eo pipefail runner shell; if grep finds no match, it exits 1 and the whole deploy step fails before the later “skip comment if no URL” logic can run. Make the extraction tolerant (e.g., || true) or avoid parsing by constructing the deterministic Pages URL from PR_NUMBER/project name or using a structured/JSON output flag if available.
| DEPLOY_URL=$(echo "$DEPLOY_OUTPUT" | grep -oP 'https://[^\s]+\.pages\.dev' | tail -1) | |
| DEPLOY_URL="https://pr-${PR_NUMBER}.synthorg-pr-preview.pages.dev" |
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.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -173,6 +173,7 @@ src/ai_company/ | |||||||||||||||
|
|
||||||||||||||||
| - **Jobs**: lint (ruff) + type-check (mypy src/ tests/) + test (pytest + coverage) run in parallel → ci-pass (gate) | ||||||||||||||||
| - **Pages**: `.github/workflows/pages.yml` — builds Astro landing + MkDocs docs, merges, deploys to GitHub Pages on push to main | ||||||||||||||||
| - **PR Preview**: `.github/workflows/pages-preview.yml` — builds site on PRs (same path triggers as Pages), injects "Development Preview" banner, deploys to Cloudflare Pages (`synthorg-pr-preview` project) via wrangler CLI. Each PR gets a unique preview URL at `pr-<number>.synthorg-pr-preview.pages.dev`. Requires `CLOUDFLARE_API_TOKEN` + `CLOUDFLARE_ACCOUNT_ID` secrets. Build job runs regardless (catches build failures); deploy job skips on fork PRs (no secrets access). Concurrency group cancels stale builds on rapid pushes. | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is very long (over 450 characters), which makes it difficult to read and maintain. For better readability, consider breaking this down into a nested list that details the different aspects of the PR Preview workflow.
Suggested change
|
||||||||||||||||
| - **Docker**: `.github/workflows/docker.yml` — builds backend + web images, pushes to GHCR, signs with cosign. Scans: Trivy (CRITICAL = hard fail, HIGH = warn-only) + Grype (critical cutoff). CVE triage via `.github/.trivyignore.yaml` and `.github/.grype.yaml`. Images only pushed after scans pass. Triggers on push to main and version tags (`v*`). | ||||||||||||||||
| - **Matrix**: Python 3.14 | ||||||||||||||||
| - **Dependabot**: daily uv + github-actions + docker updates, grouped minor/patch, no auto-merge | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -150,6 +150,6 @@ VS Code should auto-detect the `.venv` directory. If not, use **Python: Select I | |||||||||||||
|
|
||||||||||||||
| ## Next Steps | ||||||||||||||
|
|
||||||||||||||
| - [CONTRIBUTING.md](../.github/CONTRIBUTING.md) — branch, commit, and PR workflow | ||||||||||||||
| - [CLAUDE.md](../CLAUDE.md) — code conventions and quick command reference | ||||||||||||||
| - [DESIGN_SPEC.md](../DESIGN_SPEC.md) — full high-level design specification | ||||||||||||||
| - [CONTRIBUTING.md](https://github.com/Aureliolo/synthorg/blob/main/.github/CONTRIBUTING.md) — branch, commit, and PR workflow | ||||||||||||||
| - [CLAUDE.md](https://github.com/Aureliolo/synthorg/blob/main/CLAUDE.md) — code conventions and quick command reference | ||||||||||||||
| - [DESIGN_SPEC.md](https://github.com/Aureliolo/synthorg/blob/main/DESIGN_SPEC.md) — full high-level design specification | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using absolute URLs pointing to the A more robust approach is to make these documentation assets part of the For example, you could add a step in your - name: Copy root markdown files to docs
run: |
cp .github/CONTRIBUTING.md docs/
cp CLAUDE.md docs/
cp DESIGN_SPEC.md docs/With that change, you can update the links in this file to be relative, as suggested.
Suggested change
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -291,7 +291,7 @@ async def receive( | |||||
| - *timeout* expires without a message arriving. | ||||||
| - The bus is shut down while waiting. | ||||||
| - The subscription is cancelled via :meth:`unsubscribe` | ||||||
| while a ``receive()`` call is in flight. | ||||||
| while a ``receive()`` call is in flight. | ||||||
|
||||||
| while a ``receive()`` call is in flight. | |
| while a ``receive()`` call is in flight. |
Uh oh!
There was an error while loading. Please reload this page.