fix(ci): derive Railway preview URLs from env name#52
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe workflow removes the "Extract PR number" step and updates the "Derive service URLs" step to parse, lowercase, and validate the deployment environment slug from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
🚅 Deployed to the ePDS-pr-52 environment in ePDS
|
Coverage Report for CI Build 24111163068Coverage remained the same at 29.583%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-pr.yml:
- Around line 39-44: The env parsing is too permissive: before deriving
ENV_NAME/ENV_SLUG from FULL_ENV ensure FULL_ENV contains the literal separator "
/ " and validate the parsed ENV_NAME against an allowed charset (e.g.,
alphanumeric, dash, underscore, dot) with a regexp; if either check fails, emit
a clear ::error:: message including the offending FULL_ENV and exit 1. Update
the block that sets ENV_NAME, ENV_SLUG and the error branch to first test for
the separator and then test ENV_NAME with the regexp (use the ENV_NAME variable
and ENV_SLUG creation via lowercasing as currently implemented) so malformed
payloads fail immediately rather than producing bad URLs later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4463b2d3-259a-44b3-b97e-a5b289981db7
📒 Files selected for processing (1)
.github/workflows/e2e-pr.yml
The previous approach hardcoded a URL template "certified-app<svc>-epds-pr-<N>.up.railway.app", which only works when Railway gives the env its normal "ePDS-pr-<N>" name. When a PR is force-pushed (or closed/reopened) during Railway's 1-2 minute env cleanup window, Railway falls back to a collision-avoidance name like "pr-3897f1-47" (per Railway's own explanation: https://station.railway.com/questions/pr-environment-name-format-change-causin-9aaa904f). The resulting service domains then look like "certified-app<svc>-pr-3897f1-47.up.railway.app", and the templated URLs become unreachable, causing the wait-for-services step to time out after 5 minutes. Read the live env name out of the deployment_status webhook payload instead, lowercase it, and build URLs as <service>-<env-name>. This matches Railway's actual domain-generation rule and works for both the normal and the fallback naming formats with no Railway API lookup (which would require a Railway token in repo secrets). Also validate the incoming env string: require the "<project> / <env>" separator and constrain the parsed env name to a DNS-slug charset so malformed payloads fail immediately with a clear error rather than producing bad URLs that only fail later in the polling step. Verified empirically against current envs ePDS-pr-48 and pr-3897f1-47: both produce URLs matching what Railway has actually generated.
aspiers
force-pushed
the
fix/e2e-preview-url-derivation
branch
from
47f81e1 to
3c4b50f
Compare
|
1 participant
Summary
Fixes the e2e-pr workflow's URL derivation so it works when Railway gives a PR environment its collision-avoidance fallback name (e.g.
pr-3897f1-47) instead of the usualePDS-pr-<N>.Observed on PR #47: a force-push hit Railway's 1–2 minute env-cleanup window, Railway created the new env as
pr-3897f1-47, the service domains becamecertified-app<svc>-pr-3897f1-47.up.railway.app, and the workflow's hardcodedcertified-app<svc>-epds-pr-<N>.up.railway.apptemplate pointed at nothing. The wait-for-services step then timed out after 5 minutes. Railway's explanation of the fallback naming: https://station.railway.com/questions/pr-environment-name-format-change-causin-9aaa904fThis PR makes the workflow read the live env name out of
github.event.deployment.environment, lowercase it, and build URLs as<service>-<env-name>.up.railway.app. That matches Railway's actual domain-generation rule for both the normal and the fallback naming formats — no Railway API token needed.Why this is a separate PR
GitHub runs
deployment_status-triggered workflows from the default branch's copy of the workflow file, not the PR branch's. So this fix has to land onmainbefore it can be exercised on #47 (which is where it was discovered). Kept minimal and workflow-only to make review trivial.Test plan
ePDS-pr-48(normal) → producescertified-apppds-core-epds-pr-48.up.railway.app✓ ;pr-3897f1-47(fallback) → producescertified-apppds-core-pr-3897f1-47.up.railway.app✓. Both match whatenvironment { serviceInstances { domains { serviceDomains } } }reports from Railway's GraphQL API."ePDS / ePDS-pr-48","ePDS / ePDS-pr-47","ePDS / pr-3897f1-47".deployment_statuson expand e2e coverage to account-settings, consent and auto-account-creation #47 and confirm the e2e suite reaches the test phase (not a wait-for-services timeout).Summary by CodeRabbit