Skip to content

Conversation

@kasperpeulen
Copy link
Contributor

This PR deploys to staging in my own fork of the epic stack to test out deployments

Copilot AI review requested due to automatic review settings December 24, 2025 12:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR transitions from a single shared staging environment to per-PR staging deployments using Fly.io's PR review apps feature. The change simplifies initial setup by removing the need to configure a separate staging app, enables parallel testing of multiple PRs, and automatically provisions/destroys staging environments.

Key Changes:

  • Removed dev branch deployment workflow and staging app setup from initialization script
  • Implemented per-PR staging environments using fly-pr-review-apps GitHub Action with automatic resource provisioning
  • Updated documentation to reflect GitHub Actions-based staging secrets management and new deployment workflow

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
.github/workflows/deploy.yml Replaced dev branch deployment with per-PR staging job; added automatic app creation with volume, secrets, and Tigris storage provisioning
remix.init/index.mjs Removed staging app setup prompts and configuration; simplified to single production app creation
fly.toml Updated app name and region to fork-specific values
other/litefs.yml Added staging database seeding command that runs on PR environments
prisma/seed.staging.sql New SQL file with test data for staging environments
docs/deployment.md Updated setup instructions to remove staging app creation and use GitHub CLI for staging secrets
docs/secrets.md Changed from fly secrets to GitHub environment secrets for staging configuration
docs/monitoring.md Updated Sentry setup to use GitHub secrets for staging
docs/email.md Updated Resend API key setup to use GitHub secrets for staging
docs/database.md Added explanation of staging seed process; minor formatting cleanup
docs/decisions/047-pr-staging-environments.md New ADR documenting the rationale and consequences of per-PR staging environments
prisma/migrations/migration_lock.toml Formatting-only change (whitespace)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

name: 🚀 Deploy
deploy-staging:
name: 🚁 Deploy staging app for PR
runs-on: ubuntu-24.04
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deploy-staging job runs independently without waiting for the test jobs (lint, typecheck, vitest, playwright) to pass. This means a staging deployment could occur even if tests are failing. Consider adding these jobs to the needs array to ensure code quality checks pass before deploying to staging.

Suggested change
runs-on: ubuntu-24.04
runs-on: ubuntu-24.04
needs:
- lint
- typecheck
- vitest
- playwright

Copilot uses AI. Check for mistakes.
Comment on lines +220 to +221
# Don't log the created tigris secrets!
flyctl storage create --app $FLY_APP_NAME --name epic-stack-$FLY_APP_NAME --yes > /dev/null 2>&1
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error output from the tigris storage creation is being completely suppressed with '> /dev/null 2>&1'. While the comment explains this is to avoid logging secrets, this also hides legitimate error messages that could help debug provisioning failures. Consider using a more targeted approach that logs errors while filtering sensitive information, or at least check the exit code and provide a generic error message if the command fails.

Suggested change
# Don't log the created tigris secrets!
flyctl storage create --app $FLY_APP_NAME --name epic-stack-$FLY_APP_NAME --yes > /dev/null 2>&1
# Don't log the created tigris secrets! Capture errors privately and surface a generic message.
if ! flyctl storage create --app "$FLY_APP_NAME" --name "epic-stack-$FLY_APP_NAME" --yes 2>storage-create-error.log; then
echo "Error: failed to create Tigris storage for app '${FLY_APP_NAME}'. See the Fly.io dashboard or rerun 'flyctl storage create' locally for details."
exit 1
fi

Copilot uses AI. Check for mistakes.
if-candidate: true

# Seed staging database with test data
- cmd: sh -c '[ "$APP_ENV" = "staging" ] && npx prisma db execute --file ./prisma/seed.staging.sql --url $DATABASE_URL'
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command uses a shell test '[' without proper error handling. If the APP_ENV variable is not set or the file doesn't exist, the command will fail silently or with unclear errors. Consider adding error handling or at least using '||true' to prevent the initialization from failing if this is an optional step.

Suggested change
- cmd: sh -c '[ "$APP_ENV" = "staging" ] && npx prisma db execute --file ./prisma/seed.staging.sql --url $DATABASE_URL'
- cmd: sh -c 'if [ "${APP_ENV:-}" = "staging" ]; then npx prisma db execute --file ./prisma/seed.staging.sql --url "$DATABASE_URL" || true; fi'

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +20
# Change this if you want to deploy to a different org
FLY_ORG: personal
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FLY_ORG environment variable is hardcoded to 'personal' at the workflow level. This creates an issue where all PRs from forks or contributors would attempt to deploy to your personal Fly.io organization, which would fail for external contributors. Consider making this configurable through a repository secret or variable, or document that this needs to be changed per repository.

Suggested change
# Change this if you want to deploy to a different org
FLY_ORG: personal
# Fly organization to deploy to; override via repository variable FLY_ORG
FLY_ORG: ${{ vars.FLY_ORG || 'personal' }}

Copilot uses AI. Check for mistakes.
await $I`fly secrets set SESSION_SECRET=${getRandomString32()} INTERNAL_COMMAND_TOKEN=${getRandomString32()} HONEYPOT_SECRET=${getRandomString32()} ALLOW_INDEXING=false --app ${APP_NAME}-staging`
}
await $I`fly secrets set SESSION_SECRET=${getRandomString32()} INTERNAL_COMMAND_TOKEN=${getRandomString32()} HONEYPOT_SECRET=${getRandomString32()} --app ${APP_NAME}`
await $I`fly secrets set SESSION_SECRET=${getRandomString32()} HONEYPOT_SECRET=${getRandomString32()} --app ${APP_NAME}`
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The INTERNAL_COMMAND_TOKEN secret has been removed from the secrets configuration. If this token is still required by the application, its removal could break functionality. Verify that INTERNAL_COMMAND_TOKEN is no longer needed, or if it is still required, it should be added back to the secrets set command.

Suggested change
await $I`fly secrets set SESSION_SECRET=${getRandomString32()} HONEYPOT_SECRET=${getRandomString32()} --app ${APP_NAME}`
await $I`fly secrets set SESSION_SECRET=${getRandomString32()} HONEYPOT_SECRET=${getRandomString32()} INTERNAL_COMMAND_TOKEN=${getRandomString32()} --app ${APP_NAME}`

Copilot uses AI. Check for mistakes.
by deleting the particular migration folder from `prisma/migrations` and
re-generating the migration after fixing the error.
3. If you do care about the data and don't have a backup, you can follow these
steps:
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An extra blank line was removed. While this change doesn't affect functionality, it's not clear why this formatting change was included in a PR focused on deployment changes. Consider whether this whitespace change adds value to the PR or creates unnecessary noise in the diff.

Suggested change
steps:
steps:

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,48 @@
# Per-PR Staging Environments

Date: 2025-12-24
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decision document is dated 2025-12-24, which is today's date (December 24, 2025). Ensure this date is accurate and intentional. If this document was drafted earlier, the date should reflect when the decision was actually made rather than when the PR was created.

Copilot uses AI. Check for mistakes.
FLY_REGION=$(flyctl config show | jq -r '.primary_region')
# Create app if it doesn't exist
if ! flyctl status --app "$FLY_APP_NAME"; then
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'flyctl status' command is used to check if an app exists, but this command will exit with a non-zero status code if the app doesn't exist, potentially causing the workflow to fail. The conditional check may not work as expected without proper error handling. Consider using 'flyctl apps list' with grep or adding '|| true' to handle the expected failure case gracefully.

Copilot uses AI. Check for mistakes.
Comment on lines 73 to 75
```sh
fly secrets set ALLOW_INDEXING=false --app [YOUR_APP_NAME]-staging
fly secrets set SESSION_SECRET=$(openssl rand -hex 32) HONEYPOT_SECRET=$(openssl rand -hex 32)
```
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ALLOW_INDEXING secret has been removed from production setup but is still set for staging environments (line 231). Production environments typically want to allow search engine indexing, while staging should not. However, the documentation previously mentioned setting ALLOW_INDEXING=false for non-production environments. Verify that production's default behavior (when ALLOW_INDEXING is not set) is to allow indexing, or explicitly set ALLOW_INDEXING=true for production to make the intent clear.

Copilot uses AI. Check for mistakes.
env:
FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
FLY_APP_NAME="${{ steps.app_name.outputs.value }}-pr-${{ github.event.number }}"
FLY_REGION=$(flyctl config show | jq -r '.primary_region')
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command uses 'flyctl config show' piped to jq to extract the primary_region, but this assumes the current directory context has a fly.toml file. If the checkout step or working directory is different than expected, this could fail. Consider explicitly specifying the config file path or using a more robust method to get the region value, such as reading it directly from the fly.toml file using the same toml-action used elsewhere in the workflow.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant