Device serial - #3800
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLogin redirects now wait for transitions to finish. NextAuth uses a preconfigured route handler, and environment detection plus URL normalization handle staging indicators and cross-environment domain values. ChangesAuthentication and environment handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/beacon/app/login/page.tsx (1)
139-152: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
isTransitioningis never reset, which can trap users on the login page.The
!isTransitioningguard correctly prevents double-redirects whenhandleFormSubmitalready scheduled navigation. However,isTransitioningis set totrueat line 221 and never reset tofalse. If therouter.push(redirectTarget)at line 243 fails or the user lands back on/login(e.g., middleware redirect, expired session per thesessioncallback insrc/beacon/lib/auth.ts:301), the transition overlay stays visible and the useEffect's redirect branch is permanently blocked — the user is stuck with no recovery path.Consider resetting
isTransitioningin thesetTimeoutcallback after navigation is initiated, or adding a fallback timeout that clears it.🛡️ Proposed fix: reset isTransitioning after redirect
// Small delay to show transition, then redirect setTimeout(() => { router.push(redirectTarget) router.refresh() + // Reset transition state in case navigation fails or user returns + setTimeout(() => setIsTransitioning(false), 100) }, 1500)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/beacon/app/login/page.tsx` around lines 139 - 152, Reset isTransitioning after the delayed navigation initiated by handleFormSubmit completes, ensuring the state cannot permanently block the authenticated redirect branch in the login useEffect. Update the setTimeout callback around router.push(redirectTarget) to clear the transition state after navigation is initiated, while preserving the existing double-redirect guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/beacon/lib/config.js`:
- Around line 43-48: Update the SERVICE_NAME/HOSTNAME staging detection in the
configuration environment check to recognize both “stage” and “staging”
substrings, matching the existing NEXTAUTH_URL and client-side detection
behavior. Preserve the current precedence of this branch before the localhost
handling.
---
Outside diff comments:
In `@src/beacon/app/login/page.tsx`:
- Around line 139-152: Reset isTransitioning after the delayed navigation
initiated by handleFormSubmit completes, ensuring the state cannot permanently
block the authenticated redirect branch in the login useEffect. Update the
setTimeout callback around router.push(redirectTarget) to clear the transition
state after navigation is initiated, while preserving the existing
double-redirect guard.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8acc141-881b-4c3e-8f98-4de520a24e95
📒 Files selected for processing (2)
src/beacon/app/login/page.tsxsrc/beacon/lib/config.js
|
New azure beacon changes available for preview here |
|
New azure beacon changes available for preview here |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/beacon/lib/auth.ts`:
- Around line 15-24: Normalize rawUrl by trimming surrounding whitespace and
trailing slashes before deriving isLocalhostUrl, isProdUrlOnStaging, and the
production staging-domain comparison in the NEXTAUTH_URL correction logic. Use
the normalized value for all canonical URL comparisons while preserving the
existing environment-specific assignments.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db64042f-241b-46b5-ac07-dcf8a930f222
📒 Files selected for processing (2)
src/beacon/app/api/auth/[...nextauth]/route.tssrc/beacon/lib/auth.ts
There was a problem hiding this comment.
Pull request overview
This PR updates the Beacon app’s authentication and environment handling to behave correctly across staging/production deployments and during client-side auth transitions.
Changes:
- Adjust Beacon server-side environment detection to correctly classify staging based on service/pod identifiers (not only
NEXTAUTH_URL). - Make
NEXTAUTH_URLself-correct when it’s missing/localhost or mispointing between staging and production. - Prevent login page redirects while a post-login transition screen is active, and simplify the NextAuth app-route handler.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/beacon/lib/config.js | Expands staging detection on the server using SERVICE_NAME/HOSTNAME. |
| src/beacon/lib/auth.ts | Updates NEXTAUTH_URL normalization rules for staging/prod correctness. |
| src/beacon/app/login/page.tsx | Avoids redirecting while isTransitioning is true during login flow. |
| src/beacon/app/api/auth/[...nextauth]/route.ts | Switches to the App Router-style NextAuth(authOptions) handler export. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
New azure beacon changes available for preview here |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit