fix: ensure PDS_INTERNAL_URL is propagated and documented for Railway#20
Open
fix: ensure PDS_INTERNAL_URL is propagated and documented for Railway#20
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughPDS_INTERNAL_URL is now treated as a required internal config: docs, .env examples, setup scripts, and auth-service runtime code were updated. A new helper enforces presence of PDS_INTERNAL_URL and EPDS_INTERNAL_SECRET at auth-service startup, and setup/deployment guidance (including Railway commands) was added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![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
🧹 Nitpick comments (1)
docs/configuration.md (1)
70-70: Optional: unify Railway placeholder naming across docs.Line 70 uses
http://<service>.railway.internal:3000, while other updated docs use<pds-core-service>/<private-domain>. Using one placeholder style will reduce copy/paste mistakes.Proposed doc tweak
-| `PDS_INTERNAL_URL` | **Required.** Internal URL for auth→PDS calls. Docker: `http://core:3000`; Railway: `http://<service>.railway.internal:3000`; local dev: `http://localhost:3000`. Auth service crashes at startup if unset. | +| `PDS_INTERNAL_URL` | **Required.** Internal URL for auth→PDS calls. Docker: `http://core:3000`; Railway: `http://<private-domain>:3000`; local dev: `http://localhost:3000`. Auth service crashes at startup if unset. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/configuration.md` at line 70, The docs inconsistency: the PDS_INTERNAL_URL example uses `http://<service>.railway.internal:3000` while other docs use `<pds-core-service>` / `<private-domain>`; update the example for the PDS_INTERNAL_URL entry to use the unified placeholder style (`<pds-core-service>` or `<private-domain>`) so all Railway placeholders match other docs and reduce copy/paste errors; locate the `PDS_INTERNAL_URL` table row in configuration.md and replace the Railway placeholder accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/deployment.md`:
- Around line 134-135: The sentence incorrectly states that "without a correct
PDS_INTERNAL_URL, the auth service will crash at startup"; change the wording to
clarify that the specific startup error "PDS_INTERNAL_URL and
EPDS_INTERNAL_SECRET must be set" is raised only when the environment variables
PDS_INTERNAL_URL and EPDS_INTERNAL_SECRET are missing/unset, whereas an
incorrect but non-empty PDS_INTERNAL_URL will typically not cause a startup
crash and will surface failures later at request/runtime; update the text to
mention both cases and include the exact error string "PDS_INTERNAL_URL and
EPDS_INTERNAL_SECRET must be set" to make the distinction clear.
---
Nitpick comments:
In `@docs/configuration.md`:
- Line 70: The docs inconsistency: the PDS_INTERNAL_URL example uses
`http://<service>.railway.internal:3000` while other docs use
`<pds-core-service>` / `<private-domain>`; update the example for the
PDS_INTERNAL_URL entry to use the unified placeholder style
(`<pds-core-service>` or `<private-domain>`) so all Railway placeholders match
other docs and reduce copy/paste errors; locate the `PDS_INTERNAL_URL` table row
in configuration.md and replace the Railway placeholder accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9db89ce-f964-412b-b10c-dd3ac9d8e06b
📒 Files selected for processing (4)
docs/configuration.mddocs/deployment.mdpackages/auth-service/.env.examplescripts/setup.sh
- Add PDS_INTERNAL_URL to inject_shared_vars list in setup.sh so it's propagated to per-package .env files - Uncomment PDS_INTERNAL_URL in auth-service .env.example (now required at startup since handle selection PR) - Add Railway warning to setup.sh print_next_steps about changing the Docker-specific value to the Railway internal URL - Expand deployment.md Railway section with steps to find and set the correct internal URL - Update configuration.md to mark PDS_INTERNAL_URL as required
aspiers
force-pushed
the
fix/pds-internal-url-setup
branch
from
521e070 to
ee3db18
Compare
Pull Request Test Coverage Report for Build 23095139064Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
…ars are missing Extract duplicated PDS_INTERNAL_URL/EPDS_INTERNAL_SECRET validation into a shared helper that names exactly which variable(s) are unset. Update deployment docs to distinguish missing vars (startup crash) from incorrect values (runtime failure).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/auth-service/src/lib/require-internal-env.ts (1)
8-21: Incomplete migration: other files still read env vars directly.Per the relevant code snippets, several files still read
PDS_INTERNAL_URLandEPDS_INTERNAL_SECRETdirectly fromprocess.envwith fallbacks instead of using this helper:
account-settings.ts(lines 50-51)auto-provision.ts(line 18)better-auth.ts(lines 167-170)login-page.ts(lines 130-132)These will silently fall back to empty strings or alternative URLs rather than failing fast at startup. Consider migrating these call sites in a follow-up PR to ensure consistent behavior.
Would you like me to open a new issue to track migrating the remaining direct env reads to use
requireInternalEnv()?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth-service/src/lib/require-internal-env.ts` around lines 8 - 21, Update the call sites that read PDS_INTERNAL_URL and EPDS_INTERNAL_SECRET directly to use the requireInternalEnv() helper: import requireInternalEnv and replace direct uses of process.env.PDS_INTERNAL_URL / process.env.EPDS_INTERNAL_SECRET (and any fallbacks to '' or alternate URLs) with the returned { pdsUrl, internalSecret } values so the app fails fast on missing vars; specifically change the usages in account-settings.ts, auto-provision.ts, better-auth.ts, and login-page.ts to call requireInternalEnv(), remove local fallbacks, and adjust references to use pdsUrl/internalSecret, and run tests/build to ensure imports/paths are correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/auth-service/src/lib/require-internal-env.ts`:
- Around line 8-21: Update the call sites that read PDS_INTERNAL_URL and
EPDS_INTERNAL_SECRET directly to use the requireInternalEnv() helper: import
requireInternalEnv and replace direct uses of process.env.PDS_INTERNAL_URL /
process.env.EPDS_INTERNAL_SECRET (and any fallbacks to '' or alternate URLs)
with the returned { pdsUrl, internalSecret } values so the app fails fast on
missing vars; specifically change the usages in account-settings.ts,
auto-provision.ts, better-auth.ts, and login-page.ts to call
requireInternalEnv(), remove local fallbacks, and adjust references to use
pdsUrl/internalSecret, and run tests/build to ensure imports/paths are correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e83cb4d7-e0cb-43b5-a12f-c190e45d1f7a
📒 Files selected for processing (7)
docs/configuration.mddocs/deployment.mdpackages/auth-service/.env.examplepackages/auth-service/src/lib/require-internal-env.tspackages/auth-service/src/routes/choose-handle.tspackages/auth-service/src/routes/complete.tsscripts/setup.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/setup.sh
- docs/configuration.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After #13 (handle selection), the auth service crashes at startup if
PDS_INTERNAL_URLis not set — it was changed from an optional fallback to a hard requirement. The setup script generates this variable in the top-level.envbut:.envfiles (which are what you paste into Railway).env.examplehad it commented out, soinject_shared_varsskipped ithttp://core:3000) is Docker-specific and wrong for RailwayThis caused a crash in Railway staging.
Changes
scripts/setup.sh: AddPDS_INTERNAL_URLto theinject_shared_varslist; add Railway warning toprint_next_stepsabout changing the Docker valuepackages/auth-service/.env.example: UncommentPDS_INTERNAL_URLand mark as requireddocs/deployment.md: Expand Railway section with specific steps to find and set the correct internal URLdocs/configuration.md: MarkPDS_INTERNAL_URLas required, add local dev valueSummary by CodeRabbit
Documentation
Configuration
Bug Fixes