Uh oh!
There was an error while loading. Please reload this page.
fix(auth): Remove redundant black sign-in page, redirect to Google - #58
Conversation
- Middleware sends unauthenticated users to /api/auth/google instead of /signin - /signin only for OAuth errors; light UI, no black Welcome back screen - Fix auth checks to use /api/auth/me (matches FastAPI router) - Avoid redirect loop when API URL missing (signin?error=google_not_configured) Made-with: Cursor
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR standardizes the live-approval endpoint from Changes
Sequence DiagramsequenceDiagram
participant User
participant Middleware
participant SignInPage as Sign-In Page
participant Backend as Backend API
User->>Middleware: Request /signin
activate Middleware
Middleware->>Middleware: Check `sapling_session` cookie
alt cookie exists
Middleware->>Backend: GET ${API_URL}/api/auth/me (with timeout)
activate Backend
Backend-->>Middleware: { ok / 403 / error }
deactivate Backend
alt ok (approved)
Middleware-->>User: Redirect to /dashboard
else 403 (pending)
Middleware-->>User: Redirect to /pending
else error/timeout
Middleware-->>User: Redirect to Google or /signin?error=google_not_configured
end
else no cookie
Middleware-->>User: NextResponse.next() (allow /signin)
end
deactivate Middleware
User->>SignInPage: Mounts /signin
activate SignInPage
SignInPage->>SignInPage: Read `localStorage.sapling_user`
alt user id found
SignInPage->>Backend: POST /api/auth/session (with user_id)
activate Backend
Backend-->>SignInPage: { ok / 403 / error }
deactivate Backend
alt ok (approved)
SignInPage-->>User: Redirect to /dashboard
else 403 (pending)
SignInPage-->>User: Redirect to /pending
else error
SignInPage-->>User: Show error UI with "Try again with Google"
end
else no user data
SignInPage-->>User: Show fallback message / sign-in prompt
end
deactivate SignInPage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying web with |
| Latest commit: | dcef2f2 |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7a6f124a.web-75h.pages.dev |
| Branch Preview URL: | https://fix-signin-skip-black-page-g.web-75h.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/middleware.ts (1)
25-50: Consider extracting common fetch-with-timeout logic.The approval-check fetch pattern (3-second abort timeout,
/api/auth/mecall,is_approvedparsing) appears twice. A shared helper could reduce duplication, though the slightly different return behaviors make this optional.Also applies to: 82-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/middleware.ts` around lines 25 - 50, Extract the duplicated "approval-check" fetch into a shared helper (e.g., fetchUserApproval or fetchWithTimeout) and use it from redirectIfSignedIn and the other middleware function that performs the same `/api/auth/me` check; the helper should accept userId (and optional timeout), create an AbortController with a 3000ms timeout, call `${API_URL}/api/auth/me?user_id=${encodeURIComponent(userId)}`, clear the timeout in a finally, return the parsed JSON (or null/throw on non-ok/failure) so callers can decide redirect behavior, and preserve existing error-swallowing behavior in redirectIfSignedIn by returning null on helper failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/signin/page.tsx`:
- Around line 27-43: The fetch call in the signin page uses fetch(...).then(...)
without handling rejections, causing unhandled promise rejections on network
errors; update the logic in frontend/src/app/signin/page.tsx (the fetch block
that references cancelled and router.replace) to handle fetch failures by either
appending a .catch(...) to the promise chain that checks cancelled and
handles/logs the error and routes appropriately, or convert the call to
async/await inside the surrounding try/catch so network errors are caught and
handled before using router.replace('/dashboard') or router.replace('/pending').
---
Nitpick comments:
In `@frontend/src/middleware.ts`:
- Around line 25-50: Extract the duplicated "approval-check" fetch into a shared
helper (e.g., fetchUserApproval or fetchWithTimeout) and use it from
redirectIfSignedIn and the other middleware function that performs the same
`/api/auth/me` check; the helper should accept userId (and optional timeout),
create an AbortController with a 3000ms timeout, call
`${API_URL}/api/auth/me?user_id=${encodeURIComponent(userId)}`, clear the
timeout in a finally, return the parsed JSON (or null/throw on non-ok/failure)
so callers can decide redirect behavior, and preserve existing error-swallowing
behavior in redirectIfSignedIn by returning null on helper failure.
🪄 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 Plus
Run ID: f50d0ac0-c75b-4c30-8b29-dbfb1e73eb3f
📒 Files selected for processing (3)
frontend/src/app/api/auth/session/route.tsfrontend/src/app/signin/page.tsxfrontend/src/middleware.ts
Uh oh!
There was an error while loading. Please reload this page.
Add .catch() to the fetch chain so network errors don't produce unhandled promise rejections. Clarify the existing catch comment to distinguish parse errors from fetch errors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Problem
Users saw an extra black "Welcome back" / Continue with Google screen after or instead of a smooth login, and session checks could hit the wrong path.
Changes
/signinor protected routes redirect straight to{API}/api/auth/google(Google OAuth) instead of rendering the old sign-in page./signinroute: Only used when OAuth fails or config is wrong; shows a light error screen with "Try again with Google" (no black full-screen card for the normal path)./api/auth/meconsistently (matches FastAPIprefix="/api/auth").NEXT_PUBLIC_API_URLis missing, redirect to/signin?error=google_not_configuredto avoid a redirect loop.Testing
npm run buildandnpm testpass locally.Notes
Signed-in users hitting
/signinare still redirected to dashboard or pending (unchanged behavior).Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Style