feat: implement read-only mode for demo organization and add error handling#35
Conversation
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-35
|
🚅 Deployed to the applirank-pr-35 environment in applirank
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a standardized preview read-only error and enforces demo-org write guards in middleware and the public job-apply endpoint, updates preview-detection and env preprocessing, renames a dashboard UI label from "Demo mode" to "Preview mode", and makes the auth handler async with structured error logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware as demo-guard
participant Handler as apply.post
participant Utils as previewReadOnly
participant DB
Client->>Middleware: POST /api/public/jobs/:slug/apply
Middleware->>Middleware: is write method? (POST)
Middleware->>DB: getDemoOrgId()
DB-->>Middleware: demoOrgId
Middleware->>DB: resolve job slug → jobOrgId
DB-->>Middleware: jobOrgId
Middleware->>Middleware: compare jobOrgId == demoOrgId
alt demo org matched
Middleware->>Utils: createPreviewReadOnlyError()
Utils-->>Middleware: 403 PREVIEW_READ_ONLY
Middleware-->>Client: 403 Forbidden
else not demo org
Middleware->>Handler: pass through
Handler->>DB: process application
Handler-->>Client: 200 Success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
server/middleware/demo-guard.ts (2)
20-22:throwDemoReadOnlyErrorwrapper adds no value overthrow createPreviewReadOnlyError()at the call site.The wrapper is a
never-typed function solely to perform athrow. TypeScript already narrows control flow correctly at athrowexpression, so callers gain nothing from the indirection. It also creates an inconsistency:apply.post.tscallsthrow createPreviewReadOnlyError()directly while this file goes through the wrapper.♻️ Proposed change
-function throwDemoReadOnlyError(): never { - throw createPreviewReadOnlyError() -} -Then at call sites (lines 76 and 90):
- throwDemoReadOnlyError() + throw createPreviewReadOnlyError()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/middleware/demo-guard.ts` around lines 20 - 22, Remove the unnecessary throwDemoReadOnlyError() wrapper and replace its uses with a direct throw createPreviewReadOnlyError() expression; locate the throwDemoReadOnlyError function and delete it, then update callers in this module (where throwDemoReadOnlyError() is invoked) to use throw createPreviewReadOnlyError() so behavior matches apply.post.ts and avoids the redundant indirection.
3-3: Remove explicit import —createPreviewReadOnlyErroris auto-imported by Nitro.Same violation as in
apply.post.ts: exports fromserver/utils/are scanned by Nitro's auto-import, making the explicit import redundant.♻️ Proposed change
import { eq } from 'drizzle-orm' import * as schema from '../database/schema' -import { createPreviewReadOnlyError } from '../utils/previewReadOnly'As per coding guidelines: "All utilities in
server/utils/are auto-imported by Nitro—do not use explicit imports fordb,auth,env,requireAuth, …"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/middleware/demo-guard.ts` at line 3, Remove the redundant explicit import of createPreviewReadOnlyError in server/middleware/demo-guard.ts because Nitro auto-imports utilities from server/utils; delete the import line "import { createPreviewReadOnlyError } from '../utils/previewReadOnly'" and rely on the auto-imported createPreviewReadOnlyError symbol where it's used in the demo-guard middleware.server/api/public/jobs/[slug]/apply.post.ts (2)
144-155: Demo-org check performs an uncached DB query on every application submission.The middleware's
getDemoOrgId()caches the demo org ID in a module-level variable after the first lookup. This route-level guard re-queriesorganizationby slug on every request wheneverDEMO_ORG_SLUGis configured — even for applications targeting non-demo orgs that were never going to be blocked.The cleanest fix is to move (or re-export)
getDemoOrgIdtoserver/utils/previewReadOnly.ts(or a dedicatedserver/utils/demoOrg.ts) so the cache is shared between the middleware and this route.♻️ Suggested refactor
In
server/utils/previewReadOnly.ts, export the cached lookup:+let _demoOrgId: string | null | undefined + +export async function getDemoOrgId(): Promise<string | null> { + if (_demoOrgId !== undefined) return _demoOrgId + const slug = env.DEMO_ORG_SLUG + if (!slug) return (_demoOrgId = null) + const [org] = await db + .select({ id: schema.organization.id }) + .from(schema.organization) + .where(eq(schema.organization.slug, slug)) + .limit(1) + return (_demoOrgId = org?.id ?? null) +}Then in
apply.post.ts, replace the inline block:- // Demo org is strictly read-only (defense in depth; middleware also blocks this route) - if (env.DEMO_ORG_SLUG) { - const [demoOrg] = await db - .select({ id: organization.id }) - .from(organization) - .where(eq(organization.slug, env.DEMO_ORG_SLUG)) - .limit(1) - - if (demoOrg?.id === orgId) { - throw createPreviewReadOnlyError() - } - } + // Demo org is strictly read-only (defense in depth; middleware also blocks this route) + const demoOrgId = await getDemoOrgId() + if (demoOrgId && demoOrgId === orgId) { + throw createPreviewReadOnlyError() + }And update
demo-guard.tsto remove its localdemoOrgId/getDemoOrgId(now auto-imported frompreviewReadOnly.ts).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/public/jobs/`[slug]/apply.post.ts around lines 144 - 155, The demo-org check in the apply.post.ts route re-queries the organization every request; move the cached lookup into a shared util and use it here: export the existing getDemoOrgId (module-level cached lookup) from server/utils/previewReadOnly.ts (or server/utils/demoOrg.ts), remove any duplicate local demoOrgId/getDemoOrgId from demo-guard.ts, then import and call that shared getDemoOrgId() from apply.post.ts and keep the existing guard logic (compare returned id to orgId and throw createPreviewReadOnlyError()) so the DB lookup is cached across middleware and this route.
5-5: Remove explicit import —createPreviewReadOnlyErroris auto-imported by Nitro.All exports from
server/utils/are auto-imported in Nitro server context. This file already usescreateRateLimiter,uploadToS3,deleteFromS3, andenvwithout explicit imports;createPreviewReadOnlyErrorshould follow the same pattern for consistency.♻️ Proposed change
import { job, candidate, application, jobQuestion, questionResponse, document, organization } from '../../../../database/schema' import { publicApplicationSchema, publicJobSlugSchema } from '../../../../utils/schemas/publicApplication' -import { createPreviewReadOnlyError } from '../../../../utils/previewReadOnly' import {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/public/jobs/`[slug]/apply.post.ts at line 5, Remove the explicit import of createPreviewReadOnlyError and rely on Nitro's auto-imports like the other utilities (createRateLimiter, uploadToS3, deleteFromS3, env) already used in this file; delete the import statement referencing createPreviewReadOnlyError and ensure any references to createPreviewReadOnlyError in the apply.post handler continue to work without the manual import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/api/public/jobs/`[slug]/apply.post.ts:
- Around line 144-155: The demo-org check in the apply.post.ts route re-queries
the organization every request; move the cached lookup into a shared util and
use it here: export the existing getDemoOrgId (module-level cached lookup) from
server/utils/previewReadOnly.ts (or server/utils/demoOrg.ts), remove any
duplicate local demoOrgId/getDemoOrgId from demo-guard.ts, then import and call
that shared getDemoOrgId() from apply.post.ts and keep the existing guard logic
(compare returned id to orgId and throw createPreviewReadOnlyError()) so the DB
lookup is cached across middleware and this route.
- Line 5: Remove the explicit import of createPreviewReadOnlyError and rely on
Nitro's auto-imports like the other utilities (createRateLimiter, uploadToS3,
deleteFromS3, env) already used in this file; delete the import statement
referencing createPreviewReadOnlyError and ensure any references to
createPreviewReadOnlyError in the apply.post handler continue to work without
the manual import.
In `@server/middleware/demo-guard.ts`:
- Around line 20-22: Remove the unnecessary throwDemoReadOnlyError() wrapper and
replace its uses with a direct throw createPreviewReadOnlyError() expression;
locate the throwDemoReadOnlyError function and delete it, then update callers in
this module (where throwDemoReadOnlyError() is invoked) to use throw
createPreviewReadOnlyError() so behavior matches apply.post.ts and avoids the
redundant indirection.
- Line 3: Remove the redundant explicit import of createPreviewReadOnlyError in
server/middleware/demo-guard.ts because Nitro auto-imports utilities from
server/utils; delete the import line "import { createPreviewReadOnlyError } from
'../utils/previewReadOnly'" and rely on the auto-imported
createPreviewReadOnlyError symbol where it's used in the demo-guard middleware.
…variable normalization
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-35
…ronment detection
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-35
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
server/api/auth/[...all].ts (1)
13-13:exposeDetailsuses a narrower preview signal thanresolveBetterAuthUrlinauth.ts.
exposeDetailsonly checksisRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME), butauth.tsnow also detects preview environments viaRAILWAY_GIT_PR_NUMBERandRAILWAY_PUBLIC_DOMAIN. In an edge-case preview deployment detected only through those signals,exposeDetailswould befalseand the response would fall back to the generic"Server Error"with no diagnostic data.✏️ Suggested alignment
- const exposeDetails = isRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME) || import.meta.dev + const hasPreviewDomain = env.RAILWAY_PUBLIC_DOMAIN?.toLowerCase().includes('-pr-') ?? false + const hasPrNumber = !!env.RAILWAY_GIT_PR_NUMBER + const exposeDetails = + isRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME) || + hasPreviewDomain || + hasPrNumber || + import.meta.dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/auth/`[...all].ts at line 13, The exposeDetails computation in server/api/auth/[...all].ts currently uses only isRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME) || import.meta.dev, which is narrower than the preview detection used by resolveBetterAuthUrl in auth.ts; update exposeDetails to use the same preview detection (e.g., include checks for env.RAILWAY_GIT_PR_NUMBER and env.RAILWAY_PUBLIC_DOMAIN or call the shared helper used by resolveBetterAuthUrl) so edge-case preview deployments are recognized and diagnostic error details are exposed consistently.server/middleware/demo-guard.ts (2)
63-79: Redundant DB lookup: the route handler already guards the public apply path.Per the AI summary,
server/api/public/jobs/[slug]/apply.post.tsindependently checks whether the target job belongs to the demo org and throwscreatePreviewReadOnlyError(). The middleware therefore duplicates that exact query (SELECT organizationId FROM job WHERE slug = ?) on every write to/api/public/jobs/*/apply. When the path is not the demo org (the common case), both queries still run — the middleware lookup is entirely wasted.Consider removing the
publicApplyMatchblock from the middleware and relying solely on the guard inside the route handler. If defense-in-depth is desired, document why the check is intentionally duplicated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/middleware/demo-guard.ts` around lines 63 - 79, The middleware contains a redundant DB lookup for PUBLIC_APPLY_PATH_REGEX that duplicates the check already performed in server/api/public/jobs/[slug]/apply.post.ts; remove the entire publicApplyMatch branch (the path.match(PUBLIC_APPLY_PATH_REGEX) block that decodes slug and runs db.select(...).where(eq(schema.job.slug, slug)) and throws throwDemoReadOnlyError()) from demo-guard.ts so the route handler’s createPreviewReadOnlyError() is the sole check, or if you want defense-in-depth keep the branch but add a clear comment documenting the intentional duplication and the performance tradeoff.
20-22:throwDemoReadOnlyErroris an unnecessary single-line wrapper.The function body is
throw createPreviewReadOnlyError()— callers can simply inline this. The extra indirection adds a stack frame and a name to learn without any encapsulation benefit.♻️ Proposed refactor — inline at both call sites
-function throwDemoReadOnlyError(): never { - throw createPreviewReadOnlyError() -} - ... if (targetJob?.organizationId === guardedOrgId) { - throwDemoReadOnlyError() + throw createPreviewReadOnlyError() } ... if (activeOrganizationId === guardedOrgId) { - throwDemoReadOnlyError() + throw createPreviewReadOnlyError() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/middleware/demo-guard.ts` around lines 20 - 22, Remove the unnecessary wrapper function throwDemoReadOnlyError and replace its call sites with a direct throw createPreviewReadOnlyError(); specifically delete the throwDemoReadOnlyError function declaration and update any usages to inline throw createPreviewReadOnlyError() so you avoid the extra stack frame and indirection; ensure createPreviewReadOnlyError remains imported/available in the module after the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/auth/`[...all].ts:
- Around line 1-27: You added a try/catch wrapper around the auth catch-all
handler (the defineEventHandler that calls auth.handler(toWebRequest(event))),
which violates the team's guideline to not modify the catch-all auth route;
revert this file to simply return auth.handler(toWebRequest(event)) (remove the
try/catch and any createError logic) and instead implement any structured error
handling in a server plugin or h3 middleware that globally catches/auth-related
errors; use defineEventHandler, auth.handler, and toWebRequest as the locating
symbols when restoring the original behavior and move logging/creation of errors
into the new middleware/plugin.
In `@server/middleware/demo-guard.ts`:
- Line 3: Remove the explicit import of createPreviewReadOnlyError (the line
importing from ../utils/previewReadOnly) and rely on Nitro's auto-imported
utilities from server/utils so the middleware uses createPreviewReadOnlyError
the same way it already uses env, auth, and db; simply delete the import
statement referencing createPreviewReadOnlyError to keep imports consistent and
avoid redundant explicit imports.
In `@server/utils/auth.ts`:
- Around line 11-14: The env.ts superRefine currently only exempts
BETTER_AUTH_URL when isRailwayPreviewEnvironment(RAILWAY_ENVIRONMENT_NAME) is
true; update superRefine to mirror resolveBetterAuthUrl by computing
railwayDomain = RAILWAY_PUBLIC_DOMAIN?.trim(), hasPreviewDomain = railwayDomain
? railwayDomain.toLowerCase().includes('-pr-') : false, hasPrNumber =
!!RAILWAY_GIT_PR_NUMBER?.trim(), and set isPreview =
isRailwayPreviewEnvironment(RAILWAY_ENVIRONMENT_NAME) || hasPreviewDomain ||
hasPrNumber so that BETTER_AUTH_URL validation is skipped for any of those three
signals (refer to symbols superRefine, isRailwayPreviewEnvironment,
resolveBetterAuthUrl, BETTER_AUTH_URL, RAILWAY_PUBLIC_DOMAIN,
RAILWAY_GIT_PR_NUMBER, RAILWAY_ENVIRONMENT_NAME).
In `@server/utils/env.ts`:
- Around line 25-26: The check name.startsWith('pr') in env detection is too
broad and causes false positives; update the logic in server/utils/env.ts (the
code that determines preview envs and the superRefine behavior) to only treat
names matching Railway PR patterns or the exact "pr" as preview — e.g., replace
the startsWith check with a regex test like /^pr(?:-|\d)/i OR an explicit
equality check for "pr" so names like "prod-v2", "proxy", "primary" are not
misclassified; ensure the updated condition is used wherever the
preview-environment branch (including the superRefine that skips
BETTER_AUTH_URL) runs.
---
Nitpick comments:
In `@server/api/auth/`[...all].ts:
- Line 13: The exposeDetails computation in server/api/auth/[...all].ts
currently uses only isRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME) ||
import.meta.dev, which is narrower than the preview detection used by
resolveBetterAuthUrl in auth.ts; update exposeDetails to use the same preview
detection (e.g., include checks for env.RAILWAY_GIT_PR_NUMBER and
env.RAILWAY_PUBLIC_DOMAIN or call the shared helper used by
resolveBetterAuthUrl) so edge-case preview deployments are recognized and
diagnostic error details are exposed consistently.
In `@server/middleware/demo-guard.ts`:
- Around line 63-79: The middleware contains a redundant DB lookup for
PUBLIC_APPLY_PATH_REGEX that duplicates the check already performed in
server/api/public/jobs/[slug]/apply.post.ts; remove the entire publicApplyMatch
branch (the path.match(PUBLIC_APPLY_PATH_REGEX) block that decodes slug and runs
db.select(...).where(eq(schema.job.slug, slug)) and throws
throwDemoReadOnlyError()) from demo-guard.ts so the route handler’s
createPreviewReadOnlyError() is the sole check, or if you want defense-in-depth
keep the branch but add a clear comment documenting the intentional duplication
and the performance tradeoff.
- Around line 20-22: Remove the unnecessary wrapper function
throwDemoReadOnlyError and replace its call sites with a direct throw
createPreviewReadOnlyError(); specifically delete the throwDemoReadOnlyError
function declaration and update any usages to inline throw
createPreviewReadOnlyError() so you avoid the extra stack frame and indirection;
ensure createPreviewReadOnlyError remains imported/available in the module after
the change.
| export default defineEventHandler(async (event) => { | ||
| try { | ||
| return await auth.handler(toWebRequest(event)) | ||
| } catch (error) { | ||
| const requestUrl = getRequestURL(event) | ||
| console.error('[Applirank] Auth handler error', { | ||
| method: event.method, | ||
| path: requestUrl.pathname, | ||
| message: error instanceof Error ? error.message : 'Unknown error', | ||
| stack: error instanceof Error ? error.stack : undefined, | ||
| }) | ||
|
|
||
| const exposeDetails = isRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME) || import.meta.dev | ||
| const details = error instanceof Error ? error.message : 'Unknown error' | ||
|
|
||
| throw createError({ | ||
| statusCode: 500, | ||
| statusMessage: exposeDetails ? `Auth handler failed: ${details}` : 'Server Error', | ||
| data: exposeDetails | ||
| ? { | ||
| code: 'AUTH_HANDLER_ERROR', | ||
| message: details, | ||
| } | ||
| : undefined, | ||
| }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Modifying this file conflicts with the stated policy for server/api/auth/[...all].ts.
The team's own guideline is: "Do not add auth guards to the Better Auth catch-all handler at server/api/auth/[...all].ts and do not modify this file unless changing the auth mount path." This PR adds a try/catch wrapper without changing the auth mount path.
If structured error handling for the auth handler is needed, consider wrapping it in a server plugin or h3 middleware that catches unhandled auth errors globally, rather than touching the catch-all route. Based on learnings, server/api/auth/[...all].ts should only be changed for auth mount path modifications.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/auth/`[...all].ts around lines 1 - 27, You added a try/catch
wrapper around the auth catch-all handler (the defineEventHandler that calls
auth.handler(toWebRequest(event))), which violates the team's guideline to not
modify the catch-all auth route; revert this file to simply return
auth.handler(toWebRequest(event)) (remove the try/catch and any createError
logic) and instead implement any structured error handling in a server plugin or
h3 middleware that globally catches/auth-related errors; use defineEventHandler,
auth.handler, and toWebRequest as the locating symbols when restoring the
original behavior and move logging/creation of errors into the new
middleware/plugin.
| @@ -1,5 +1,6 @@ | |||
| import { eq } from 'drizzle-orm' | |||
| import * as schema from '../database/schema' | |||
| import { createPreviewReadOnlyError } from '../utils/previewReadOnly' | |||
There was a problem hiding this comment.
Remove explicit import — createPreviewReadOnlyError is auto-imported from server/utils/.
Nitro auto-imports all utilities from server/utils/, so the explicit import { createPreviewReadOnlyError } from '../utils/previewReadOnly' is unnecessary and inconsistent with how the same file already uses env, auth, and db without imports. As per coding guidelines, "All utilities in server/utils/ are auto-imported by Nitro—do not use explicit imports for db, auth, env, … etc. in server code."
🛠️ Proposed fix
import { eq } from 'drizzle-orm'
import * as schema from '../database/schema'
-import { createPreviewReadOnlyError } from '../utils/previewReadOnly'📝 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.
| import { createPreviewReadOnlyError } from '../utils/previewReadOnly' | |
| import { eq } from 'drizzle-orm' | |
| import * as schema from '../database/schema' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/middleware/demo-guard.ts` at line 3, Remove the explicit import of
createPreviewReadOnlyError (the line importing from ../utils/previewReadOnly)
and rely on Nitro's auto-imported utilities from server/utils so the middleware
uses createPreviewReadOnlyError the same way it already uses env, auth, and db;
simply delete the import statement referencing createPreviewReadOnlyError to
keep imports consistent and avoid redundant explicit imports.
| const railwayDomain = env.RAILWAY_PUBLIC_DOMAIN?.trim() | ||
| const hasPreviewDomain = railwayDomain ? railwayDomain.toLowerCase().includes('-pr-') : false | ||
| const hasPrNumber = !!env.RAILWAY_GIT_PR_NUMBER?.trim() | ||
| const isPreview = isRailwayPreviewEnvironment(env.RAILWAY_ENVIRONMENT_NAME) || hasPreviewDomain || hasPrNumber |
There was a problem hiding this comment.
superRefine in env.ts doesn't account for the two new isPreview signals, risking a startup failure.
resolveBetterAuthUrl now treats an environment as preview if hasPrNumber or hasPreviewDomain is true, but env.ts's superRefine still only exempts BETTER_AUTH_URL when isRailwayPreviewEnvironment(RAILWAY_ENVIRONMENT_NAME) returns true. In a Railway deployment where RAILWAY_GIT_PR_NUMBER is set (or RAILWAY_PUBLIC_DOMAIN contains -pr-) but RAILWAY_ENVIRONMENT_NAME is empty or non-preview-like, env.ts validation will throw "BETTER_AUTH_URL is required …" and crash the server before resolveBetterAuthUrl ever runs.
🛠️ Proposed fix — extend `superRefine` to mirror the same three-signal logic
// server/utils/env.ts
.superRefine((data, ctx) => {
- const isPreview = isRailwayPreviewEnvironment(data.RAILWAY_ENVIRONMENT_NAME)
+ const hasPreviewDomain = data.RAILWAY_PUBLIC_DOMAIN?.toLowerCase().includes('-pr-') ?? false
+ const hasPrNumber = !!data.RAILWAY_GIT_PR_NUMBER
+ const isPreview =
+ isRailwayPreviewEnvironment(data.RAILWAY_ENVIRONMENT_NAME) ||
+ hasPreviewDomain ||
+ hasPrNumber
if (!isPreview && !data.BETTER_AUTH_URL) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/auth.ts` around lines 11 - 14, The env.ts superRefine currently
only exempts BETTER_AUTH_URL when
isRailwayPreviewEnvironment(RAILWAY_ENVIRONMENT_NAME) is true; update
superRefine to mirror resolveBetterAuthUrl by computing railwayDomain =
RAILWAY_PUBLIC_DOMAIN?.trim(), hasPreviewDomain = railwayDomain ?
railwayDomain.toLowerCase().includes('-pr-') : false, hasPrNumber =
!!RAILWAY_GIT_PR_NUMBER?.trim(), and set isPreview =
isRailwayPreviewEnvironment(RAILWAY_ENVIRONMENT_NAME) || hasPreviewDomain ||
hasPrNumber so that BETTER_AUTH_URL validation is skipped for any of those three
signals (refer to symbols superRefine, isRailwayPreviewEnvironment,
resolveBetterAuthUrl, BETTER_AUTH_URL, RAILWAY_PUBLIC_DOMAIN,
RAILWAY_GIT_PR_NUMBER, RAILWAY_ENVIRONMENT_NAME).
| return ( | ||
| name.startsWith('pr') |
There was a problem hiding this comment.
name.startsWith('pr') is too broad and creates false positives.
Any environment name that starts with "pr" (after lowercasing) — except the explicit "production"/"prod" carve-outs — will now be treated as a preview environment. That includes legitimate long-lived names such as "prod-v2", "proxy", "private", "primary", and "proto". In those environments:
env.tssuperRefinewould skip theBETTER_AUTH_URLrequirement.auth.tswould attempt to auto-generate the URL fromRAILWAY_GIT_PR_NUMBER/RAILWAY_PUBLIC_DOMAIN, both likely unset, ultimately throwing the opaque "Unable to resolve BETTER_AUTH_URL" error at init time.
The pre-existing /^pr(?:-|\d)/ regex already covers Railway's standard PR environment naming (pr-123, pr123). If the goal is to also catch a bare "pr" environment name, use an exact match instead:
🛠️ Proposed fix — replace startsWith with a precise check
return (
- name.startsWith('pr')
- ||
- /^pr(?:-|\d)/.test(name)
+ /^pr($|-|\d)/.test(name)
|| name.includes('pr-')
|| name.includes('pr ')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/env.ts` around lines 25 - 26, The check name.startsWith('pr') in
env detection is too broad and causes false positives; update the logic in
server/utils/env.ts (the code that determines preview envs and the superRefine
behavior) to only treat names matching Railway PR patterns or the exact "pr" as
preview — e.g., replace the startsWith check with a regex test like
/^pr(?:-|\d)/i OR an explicit equality check for "pr" so names like "prod-v2",
"proxy", "primary" are not misclassified; ensure the updated condition is used
wherever the preview-environment branch (including the superRefine that skips
BETTER_AUTH_URL) runs.
1 participant
Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit
New Features
UI Updates
Bug Fixes / Reliability
Chores