Set up temporary Fly deployments for PRs #546
Conversation
Co-authored-by: me <[email protected]>
Co-authored-by: me <[email protected]>
|
🚀 PR Preview Deployed Your pull request has been deployed to a temporary Fly machine: 🔗 Preview URL: https://kcd-pr-546.fly.dev This preview will automatically scale to zero when not in use to save costs. Updated: 2025-07-05T22:22:01.374Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/pr-preview.yml (2)
1-6: Add a concurrency block to avoid parallel preview clashesWithout
concurrency, two rapid pushes can kick off overlapping deployments that race to update the same app, often leaving it in an invalid state. A minimal safeguard:concurrency: group: pr-preview-${{ github.event.pull_request.number }} cancel-in-progress: truePlace it at the top-level (next to
on:).
15-159: YAMLlint reports multiple trailing-space and EOF newline issuesWhile not breaking functionality, fixing these keeps CI linters green and avoids noisy diffs later. Run
yarn dlx yamlfixor equivalent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-preview.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 60-60: trailing spaces
(trailing-spaces)
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 96-96: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 128-128: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 159-159: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ʦ TypeScript
- GitHub Check: ⬣ ESLint
- GitHub Check: 🎭 Playwright
🔇 Additional comments (1)
.github/workflows/pr-preview.yml (1)
42-56: AWK injection into[[services]]is brittleThe multiline AWK script:
- Assumes exactly one
[[services]]block.- Breaks if the block already contains any of the injected keys.
- May corrupt ordering/indentation (TOML is whitespace-insensitive but human readability matters).
Consider shipping a patched
fly.tomltemplate for previews or usetomlkit(see above) to append/update keys programmatically.
.github/workflows/pr-preview.yml
Outdated
| sed -i 's/^app = .*/app = "${{ steps.app-name.outputs.app_name }}"/' fly.toml | ||
|
|
||
| # Remove consul from experimental section (if present) | ||
| sed -i '/enable_consul = true/d' fly.toml | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
sed replacement risks false matches and TOML breakage
sed -i 's/^app = .*/app = "..."/' fly.toml relies on a very specific formatting of fly.toml.
• If the app key is indented or appears twice (common with multiple processes) the wrong line may be replaced.
• A safer, semantics-aware option is to use tomlq/yq or flyctl apps list --json to update the field, e.g.:
pip install tomlkit
python - <<'PY'
import tomlkit, sys, pathlib, os
d = tomlkit.parse(pathlib.Path("fly.toml").read_text())
d["app"] = os.environ["APP_NAME"]
pathlib.Path("fly.toml").write_text(tomlkit.dumps(d))
PY🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/pr-preview.yml around lines 37 to 41, the current sed
command to replace the app name in fly.toml risks incorrect replacements due to
formatting variations and multiple app entries. Replace the sed command with a
Python script using tomlkit to parse fly.toml, update the "app" key safely with
the environment variable APP_NAME, and write back the file. This ensures a
semantics-aware, reliable update without breaking the TOML structure.
.github/workflows/pr-preview.yml
Outdated
| # This removes everything from "lease:" to the next top-level section | ||
| awk ' | ||
| /^lease:/ { in_lease = 1; next } | ||
| /^[a-zA-Z]/ && in_lease && !/^ / { in_lease = 0 } | ||
| !in_lease { print } | ||
| ' other/litefs.yml > other/litefs.yml.tmp && mv other/litefs.yml.tmp other/litefs.yml |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Lease-section removal: YAML parsing with AWK is error-prone
The current pattern stops on the first top-level key that is not indented, which fails if nested objects appear before lease: ends, or if comments alter indentation.
Prefer yq for structural deletion:
yq -i 'del(.lease)' other/litefs.yml🤖 Prompt for AI Agents
In .github/workflows/pr-preview.yml around lines 62 to 67, the current AWK
script used to remove the "lease:" section from the YAML file is error-prone due
to its reliance on indentation and line patterns. Replace the AWK command with a
`yq` command that structurally deletes the "lease" key from the YAML file by
running `yq -i 'del(.lease)' other/litefs.yml` to ensure reliable and accurate
removal of the section.
Co-authored-by: me <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
.github/workflows/pr-preview.yml (4)
7-9: ScopeFLY_API_TOKENonly to the steps that need it (previous feedback still unresolved)
The secret is still declared at workflow level, exposing it to every step/job. Please move it into the Fly-CLI steps instead.
37-41:sedreplacement remains fragile (previous feedback still unresolved)
Line-orientedsedcan mis-fire ifapp =is indented or appears multiple times. Prefer a TOML-aware tool (tomlkit,yq, etc.) for reliability.
62-67: Removelease:block with a YAML parser, not AWK (previous feedback still unresolved)
Indentation-based AWK is brittle; useyq -i 'del(.lease)' other/litefs.ymlinstead.
143-145:flyctl apps destroywill fail while volumes exist (previous feedback still unresolved)
Insert a pre-cleanup loop to delete volumes before destroying the app.
🧹 Nitpick comments (1)
.github/workflows/pr-preview.yml (1)
14-16: Trailing spaces flagged by YAML-lint
Multiple lines (e.g. 15, 35, 38 …) contain trailing whitespace. Run a formatter or strip these to keep CI green.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-preview.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 60-60: trailing spaces
(trailing-spaces)
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 98-98: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 105-105: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 111-111: trailing spaces
(trailing-spaces)
[error] 132-132: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 163-163: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: ⬣ ESLint
- GitHub Check: 🎭 Playwright
- GitHub Check: ʦ TypeScript
- GitHub Check: 🚀 Deploy PR Preview
Co-authored-by: me <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
.github/workflows/pr-preview.yml (4)
7-9: ScopeFLY_API_TOKENto only the Fly steps
Exposing the secret at workflow level leaks it to every step (e.g. third-party actions,echo ${{ env.* }}debugging).
Move the variable into the individual Fly-CLI steps instead, exactly as suggested in the earlier review.
37-41:sed-basedfly.tomlrewrite is brittle
String substitution breaks if the file contains multipleappkeys, comments, or different indentation. Use a TOML parser (tomlkitvia a short Python script) for a semantics-aware edit; see previous review snippet.
61-70: Lease swap still uses raw text manipulation
Reliance onsedmeans YAML comments/indentation changes will break the match window.yq -i 'del(.lease) | .lease.type="static"' other/litefs.ymlis safer and clearer.
142-144: Volumes can block app deletion
flyctl apps destroyfails when persistent volumes exist. Add a pre-step to enumerate and remove volumes before the destroy call (see prior review).
🧹 Nitpick comments (1)
.github/workflows/pr-preview.yml (1)
1-162: Trailing-space noise and missing final newline
yamllintflags dozens of trailing spaces plus a missing newline at EOF. Configure an editor/CI formatter (prettier-plugin-yaml,yamlfmt) to auto-strip whitespace.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-preview.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr-preview.yml
64-64: could not parse as YAML: yaml: line 64: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 60-60: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 93-93: trailing spaces
(trailing-spaces)
[error] 65-65: syntax error: could not find expected ':'
(syntax)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 156-156: trailing spaces
(trailing-spaces)
[error] 162-162: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: 🎭 Playwright
- GitHub Check: ⬣ ESLint
- GitHub Check: ʦ TypeScript
Co-authored-by: me <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/pr-preview.yml (4)
37-41:sedupdate ofappkey is brittle – use a TOML parser
String replacement fails ifappis indented, appears twice, or comments shuffle order. Prefer a semantics-aware tool:pip install tomlkit python - <<'PY' import os, tomlkit, pathlib, sys p = pathlib.Path("fly.toml") doc = tomlkit.parse(p.read_text()) doc["app"] = os.environ["APP_NAME"] p.write_text(tomlkit.dumps(doc)) PYThis guarantees only the intended key is touched and preserves formatting.
42-56: Manualawkinjection into[[services]]is fragile
The heuristic stops after the firstinternal_port; a re-ordering or an extra key silently drops the autoscaling stanza. Usetomlkit(or forthcomingflyctl toml) to append the autoscaling keys to all[[services]]tables in a structurally safe way.
7-9: Limit secret exposure: moveFLY_API_TOKENfrom workflow-wideenvto the Fly-only steps
Declaring the secret globally leaks it to every step in all jobs (e.g.awk,sed,github-script) and widens the accidental-echo surface. Scope it only whereflyctlis invoked.- env: - FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}Add the secret to each
Setup Flystep instead:- name: 🎈 Setup Fly uses: superfly/flyctl-actions/[email protected] env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
145-147: Destroy attached volumes beforeflyctl apps destroy
flyctl apps destroyfails when volumes exist, leaving the preview app behind. Pre-clean volumes:vols=$(flyctl volumes list -a ${{ steps.app-name.outputs.app_name }} --json | jq -r '.[].ID') for v in $vols; do flyctl volumes destroy "$v" --yes done flyctl apps destroy ${{ steps.app-name.outputs.app_name }} --yes || echo "Already deleted"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-preview.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr-preview.yml
64-64: could not parse as YAML: yaml: line 64: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 57-57: trailing spaces
(trailing-spaces)
[error] 60-60: trailing spaces
(trailing-spaces)
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 73-73: trailing spaces
(trailing-spaces)
[error] 96-96: trailing spaces
(trailing-spaces)
[error] 65-65: syntax error: could not find expected ':'
(syntax)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 156-156: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 165-165: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ⬣ ESLint
- GitHub Check: ʦ TypeScript
- GitHub Check: 🎭 Playwright
.github/workflows/pr-preview.yml
Outdated
| sed -i '/^lease:/,/^exec:/c\ | ||
| lease:\ | ||
| type: static\ | ||
| \ | ||
| exec:' other/litefs.yml |
There was a problem hiding this comment.
YAML parse error: multi-line sed replacement breaks the workflow
actionlint reports could not find expected ':' at line 64 because the raw lease: block exits the run: scalar. Escape the newline or, better, switch to yq:
-sed -i '/^lease:/,/^exec:/c\
-lease:\
- type: static\
-\
-exec:' other/litefs.yml
+yq -i 'del(.lease) | .lease.type="static"' other/litefs.ymlThis eliminates the syntax error and avoids brittle text manipulation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sed -i '/^lease:/,/^exec:/c\ | |
| lease:\ | |
| type: static\ | |
| \ | |
| exec:' other/litefs.yml | |
| yq -i 'del(.lease) | .lease.type="static"' other/litefs.yml |
🧰 Tools
🪛 actionlint (1.7.7)
64-64: could not parse as YAML: yaml: line 64: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.37.1)
[error] 65-65: syntax error: could not find expected ':'
(syntax)
🤖 Prompt for AI Agents
In .github/workflows/pr-preview.yml around lines 63 to 67, the multi-line sed
replacement breaks the YAML syntax causing a parse error. To fix this, replace
the sed command with a yq command to perform the YAML modification safely. This
avoids breaking the run scalar block and prevents brittle text manipulation by
using proper YAML-aware editing.
…ion management Co-authored-by: me <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/pr-preview.yml (2)
7-8: Limit FLY_API_TOKEN scope to specific steps.Setting the secret at workflow level exposes it unnecessarily to all steps and jobs.
As noted in previous reviews, move the FLY_API_TOKEN to only the steps that need it:
-env: - FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}And add it to specific steps:
- name: 🎈 Setup Fly uses: superfly/flyctl-actions/[email protected] env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
108-111: Add volume cleanup to prevent app destruction failures.As noted in previous reviews, persistent volumes can block
flyctl apps destroy. Add volume cleanup before app destruction:- name: 🗑️ Delete PR app run: | + # Clean up volumes first + flyctl volumes list -a ${{ steps.app-name.outputs.app_name }} --json | jq -r '.[].id' | while read vol_id; do + flyctl volumes destroy "$vol_id" --yes || echo "Volume $vol_id already deleted" + done flyctl apps destroy ${{ steps.app-name.outputs.app_name }} --yes || echo "App doesn't exist or already deleted"
🧹 Nitpick comments (2)
other/setup-pr-preview.js (1)
44-54: Complex regex pattern may miss edge cases in TOML structure.The services section regex assumes a specific structure and formatting. If the TOML has different formatting, comments, or multiple services sections, this could fail silently or modify the wrong section.
Consider using a proper TOML parser for more robust configuration updates:
+import toml from '@iarna/toml'; // Replace the regex-based approach: -const servicesRegex = /(\[\[services\]\][\s\S]*?internal_port = \d+)/; -if (servicesRegex.test(flyToml)) { - flyToml = flyToml.replace( - servicesRegex, - `$1 - auto_stop_machines = true - auto_start_machines = true - min_machines_running = 0 - processes = ["app"]` - ); -} +const config = toml.parse(flyToml); +if (config.services && config.services[0]) { + config.services[0].auto_stop_machines = true; + config.services[0].auto_start_machines = true; + config.services[0].min_machines_running = 0; + config.services[0].processes = ["app"]; +} +flyToml = toml.stringify(config);.github/workflows/pr-preview.yml (1)
15-15: Clean up trailing spaces and add missing newline.Static analysis found trailing spaces on multiple lines and a missing newline at the end of the file.
Remove trailing spaces from the affected lines and add a newline at the end of the file:
if: github.event.action != 'closed' - +body: commentBody }); +Also applies to: 36-36, 59-59, 60-60, 63-63, 65-65, 67-67, 70-70, 73-73, 76-76, 97-97, 117-117, 119-119, 122-122
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-preview.yml(1 hunks)other/setup-pr-preview.js(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: 🔍 PR Preview Deploy
other/setup-pr-preview.js
[error] 3-3: ReferenceError: 'require' is not defined in ES module scope. The file is treated as an ES module due to 'type': 'module' in package.json. Use 'import' instead or rename the file to '.cjs' to treat it as CommonJS.
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 60-60: trailing spaces
(trailing-spaces)
[error] 63-63: trailing spaces
(trailing-spaces)
[error] 65-65: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 73-73: trailing spaces
(trailing-spaces)
[error] 76-76: trailing spaces
(trailing-spaces)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 119-119: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 128-128: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ⬣ ESLint
- GitHub Check: 🎭 Playwright
- GitHub Check: ʦ TypeScript
🔇 Additional comments (4)
other/setup-pr-preview.js (1)
6-22: Well-structured entry point with proper argument validation.The function provides clear error messages and follows a logical flow for setting up PR preview configurations.
.github/workflows/pr-preview.yml (3)
31-41: Good integration with the Node.js script approach.The move from inline sed/awk commands to a dedicated Node.js script is a significant improvement in maintainability and readability. The debug output helps with troubleshooting.
50-91: Excellent PR comment implementation with duplicate prevention.The logic to find and update existing bot comments instead of creating duplicates is well implemented. The comment provides comprehensive information about the preview deployment.
1-128: Well-designed workflow with clear separation of concerns.The workflow effectively separates deployment and cleanup responsibilities, uses appropriate conditions for job execution, and integrates well with the Node.js configuration script.
Co-authored-by: me <[email protected]>
Co-authored-by: me <[email protected]>
WalkthroughA new GitHub Actions workflow and a supporting Node.js script were added to automate the deployment and cleanup of preview environments for pull requests using Fly.io. The workflow manages environment setup, deployment, and teardown, while the script customizes configuration files for each PR-specific deployment. Additionally, the Dockerfile was updated to explicitly copy production dependencies into the final image stage. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant setup-pr-preview.js
participant Fly.io
participant PR
GitHub Actions->>setup-pr-preview.js: Run script with PR app name
setup-pr-preview.js->>setup-pr-preview.js: Modify fly.toml and litefs.yml
setup-pr-preview.js-->>GitHub Actions: Configuration ready
GitHub Actions->>Fly.io: Deploy PR app with updated config
Fly.io-->>GitHub Actions: Deployment result
GitHub Actions->>PR: Post/Update preview URL comment
alt On PR close
GitHub Actions->>Fly.io: Delete PR app and volumes
Fly.io-->>GitHub Actions: Cleanup complete
GitHub Actions->>PR: Post cleanup comment
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/pr-preview.yml (1)
7-8: FLY_API_TOKEN still exposed at workflow levelThe security concern from previous reviews remains unaddressed. The secret is exposed to all steps in both jobs unnecessarily.
Move the secret to only the steps that need it:
-env: - FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}And add it to the specific Fly CLI steps:
- name: 🎈 Setup Fly uses: superfly/flyctl-actions/[email protected] env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
🧹 Nitpick comments (1)
.github/workflows/pr-preview.yml (1)
15-156: Fix trailing spaces and add missing newlineMultiple trailing spaces and missing newline at end of file should be cleaned up for proper formatting.
Remove trailing spaces from lines 15, 46, 69, 70, 73, 75, 77, 80, 83, 86, 107, 123, 145, 147, 150 and add a newline at the end of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-preview.yml(1 hunks)other/setup-pr-preview.js(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 69-69: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 73-73: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 77-77: trailing spaces
(trailing-spaces)
[error] 80-80: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 86-86: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 156-156: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: 🎭 Playwright
- GitHub Check: ⬣ ESLint
- GitHub Check: ʦ TypeScript
- GitHub Check: 🚀 Deploy PR Preview
🔇 Additional comments (7)
.github/workflows/pr-preview.yml (2)
118-135: Robust volume cleanup implementationThe volume cleanup logic is well-implemented with proper error handling, JSON parsing, and graceful failure recovery. This addresses the previous concern about volumes blocking app destruction.
60-101: Well-designed PR comment managementThe comment logic properly handles both creating new comments and updating existing ones, avoiding spam and providing clear status updates to users.
other/setup-pr-preview.js (5)
3-5: ES module compatibility properly addressedThe script now correctly uses ES module imports instead of CommonJS require statements, resolving the previous pipeline failure.
32-71: Comprehensive error handling for file operationsThe script now includes proper error handling with try-catch blocks, file existence checks, and meaningful error messages, addressing the previous concern about unclear failures.
89-131: Robust YAML parsing replaces fragile regexThe script now uses the js-yaml library for proper YAML parsing and modification instead of fragile regex operations, making it much more reliable and maintainable.
99-115: Smart exec command handlingThe logic to find and update the startup command is well-designed, with fallback behavior to add a new command if none is found. This ensures the mocks mode is properly configured.
125-130: Helpful explanatory comments addedAdding explanatory comments to the generated configuration files helps future developers understand why the lease type was changed for PR previews.
Co-authored-by: me <[email protected]>
Co-authored-by: me <[email protected]>
… deployments Co-authored-by: me <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/pr-preview.yml (1)
7-8: Scope FLY_API_TOKEN to specific steps that need it.The secret is exposed at the workflow level to all jobs and steps, increasing the risk of accidental exposure. Limit it to only the steps that require Fly CLI access.
Remove from the global env and add to specific steps that need it:
-env: - FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}Then add to the Fly CLI steps:
- name: 🎈 Setup Fly uses: superfly/flyctl-actions/[email protected] env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
🧹 Nitpick comments (1)
.github/workflows/pr-preview.yml (1)
1-237: Clean up formatting issues.The file has multiple trailing spaces and is missing a newline at the end. These formatting issues should be addressed for consistency.
Fix the following formatting issues:
- Remove trailing spaces from lines 48, 79, 102, 103, 104, 107, 109, 111, 114, 117, 120, 142, 153, 154, 155, 158, 160, 162, 164, 167, 188, 204, 226, 228, 231
- Add a newline at the end of the file (line 237)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/pr-preview.yml(1 hunks)Dockerfile(1 hunks)other/setup-pr-preview.js(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr-preview.yml
35-35: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yml
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 111-111: trailing spaces
(trailing-spaces)
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 142-142: trailing spaces
(trailing-spaces)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 155-155: trailing spaces
(trailing-spaces)
[error] 158-158: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 162-162: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 167-167: trailing spaces
(trailing-spaces)
[error] 188-188: trailing spaces
(trailing-spaces)
[error] 204-204: trailing spaces
(trailing-spaces)
[error] 226-226: trailing spaces
(trailing-spaces)
[error] 228-228: trailing spaces
(trailing-spaces)
[error] 231-231: trailing spaces
(trailing-spaces)
[error] 237-237: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: 🎭 Playwright
- GitHub Check: ⬣ ESLint
- GitHub Check: ʦ TypeScript
- GitHub Check: 🚀 Deploy PR Preview
🔇 Additional comments (3)
Dockerfile (1)
84-85: LGTM! Production dependencies copy supports mocks mode.The explicit copy of production dependencies is well-documented and correctly supports the mocks mode operation used in PR previews. This ensures
cross-envand other necessary dependencies are available in the final image.other/setup-pr-preview.js (1)
1-156: LGTM! Script properly implements PR preview configuration.The script is well-structured with proper error handling, input validation, and uses appropriate libraries (js-yaml) for configuration modifications. The previous review comments have been addressed effectively:
- ✓ ES module syntax used correctly
- ✓ Error handling implemented for file operations
- ✓ YAML parsing done with proper library instead of regex
- ✓ Clear logging and backup creation
The integration with the GitHub Actions workflow and Dockerfile changes is well-designed.
.github/workflows/pr-preview.yml (1)
15-237: Confirm is-deployable.js Integration✓
other/is-deployable.jsexists
✓ Invoked vianode ./other/is-deployable.js, so the missing shebang does not impact the workflow
(You may add#!/usr/bin/env nodeif you ever run it directly, but it’s optional here.)No further changes required—workflow is correctly wired up.
| run: >- | ||
| echo ::set-output name=DEPLOYABLE::$(node ./other/is-deployable.js ${{ | ||
| github.sha }}) |
There was a problem hiding this comment.
Fix deprecated workflow command.
The set-output command is deprecated and should be replaced with the newer $GITHUB_OUTPUT format.
Apply this diff to use the modern workflow command:
- - name: 🔎 Determine deployable changes
- id: changes
- run: >-
- echo ::set-output name=DEPLOYABLE::$(node ./other/is-deployable.js ${{
- github.sha }})
+ - name: 🔎 Determine deployable changes
+ id: changes
+ run: >-
+ echo "DEPLOYABLE=$(node ./other/is-deployable.js ${{
+ github.sha }})" >> $GITHUB_OUTPUT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: >- | |
| echo ::set-output name=DEPLOYABLE::$(node ./other/is-deployable.js ${{ | |
| github.sha }}) | |
| - name: 🔎 Determine deployable changes | |
| id: changes | |
| run: >- | |
| echo "DEPLOYABLE=$(node ./other/is-deployable.js ${{ | |
| github.sha }})" >> $GITHUB_OUTPUT |
🧰 Tools
🪛 actionlint (1.7.7)
35-35: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🤖 Prompt for AI Agents
In .github/workflows/pr-preview.yml around lines 35 to 37, the workflow uses the
deprecated set-output command to set the DEPLOYABLE output. Replace this with
the new $GITHUB_OUTPUT method by echoing the output in the format
DEPLOYABLE=value >> $GITHUB_OUTPUT instead of using echo ::set-output. Update
the run command accordingly to write the output to the $GITHUB_OUTPUT file.
Add GitHub Action to deploy PR previews to temporary Fly.io machines with isolated databases and automatic cleanup.
The workflow programmatically modifies
fly.tomlandlitefs.ymlto ensure PR preview environments are completely isolated from production data. Specifically, it removes Consul integration and the LiteFSleasesection, making each preview a standalone LiteFS instance with its own clean database.Summary by CodeRabbit