fix(auth): handle non-browser auth redirects and add auto-retry #137
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces the default auth middleware with a per-request Next.js middleware that calls authkit(request, ...), adds runtime unauthenticated-path handling (including /share/*), merges/strips auth headers, returns 401 JSON for non-browser API clients, redirects browsers to the auth flow, adds an AutoRetryButton component and integrates it into the auth-error page, and removes callback response-body parsing/logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant AuthKit
participant Browser
participant APIClient
participant ErrorPage
participant AutoRetry
participant Storage
Client->>Middleware: HTTP request
Middleware->>AuthKit: authkit(request, options)
AuthKit-->>Middleware: { session?, authorizationUrl?, headers? }
alt session present or unauthenticated path
Middleware->>Client: NextResponse.next (merged request headers)
else no session and unauthenticated?=false
alt Browser (Accept includes "text/html")
Middleware->>Browser: Redirect to authorizationUrl (or /auth-error on missing URL)
Browser->>ErrorPage: GET /auth-error
ErrorPage->>ErrorPage: Lookup error entry (autoRetry?)
alt autoRetry true
ErrorPage->>AutoRetry: Render AutoRetryButton(loginUrl)
AutoRetry->>Storage: read RetryState (localStorage)
Storage-->>AutoRetry: RetryState
AutoRetry->>AutoRetry: start countdown
AutoRetry->>Storage: persist incremented RetryState
AutoRetry->>Browser: navigate to loginUrl when countdown ends
else
ErrorPage->>Browser: render static login link/button
end
else Non-browser (API client)
Middleware->>APIClient: Return 401 JSON { error: "unauthorized" }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (12)
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: 0
🧹 Nitpick comments (5)
middleware.ts (2)
56-63: Truncating user-agent may lose useful debugging info.The
.slice(0, 50)truncation cuts off details like browser version and OS that can help diagnose environment-specific auth issues. Consider increasing to ~100 characters or logging the full value.- userAgent: request.headers.get("user-agent")?.slice(0, 50), + userAgent: request.headers.get("user-agent")?.slice(0, 100),
69-71: Silent catch may hide debugging-relevant errors.While preserving the original redirect on URL parsing failure is safe, silently swallowing the error makes debugging harder if malformed
locationheaders occur in production.} catch { - // If URL parsing fails, let original redirect through + // If URL parsing fails, let original redirect through + console.warn("[Auth Middleware] Failed to parse redirect location", { location }); }lib/auth/use-auth-from-authkit.ts (3)
28-58: Consider handling URL-safe base64 encoding in JWT parsing.JWTs may use URL-safe base64 encoding (replacing
+with-and/with_). Theatob()function doesn't handle this variant, which could cause parsing failures for some tokens.- const payload = JSON.parse(atob(parts[1])); + // Handle URL-safe base64 (replace - with + and _ with /) + const base64 = parts[1].replace(/-/g, '+').replace(/_/g, '/'); + const payload = JSON.parse(atob(base64));
60-81: Module-level mutable counters are not safe for concurrent React renders.
lastCallTime,callCount, andserverCallCountare module-scoped mutable variables. In React 18+ with concurrent features or during SSR hydration mismatches, multiple renders can race on these values, producing misleading debug output.Since this is debug-only code (guarded by
DEBUG_TOKEN_FLOW), the impact is limited to debugging sessions. However, if you plan to use this instrumentation more broadly, consider using a ref-based approach per hook instance.
208-231: Clarify whether getAccessToken always hits the server.The comment says "potential Server Action" and increments
serverCallCount, butgetAccessTokenfrom AuthKit may return a cached token without a network call. This could make the debug output misleading since not every call is actually a server round-trip.Consider renaming
serverCallCounttogetAccessTokenCallCountor adding a note that it tracks API calls, not necessarily network requests.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/auth/use-auth-from-authkit.ts(4 hunks)middleware.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/auth/use-auth-from-authkit.ts (1)
lib/auth/shared-token.ts (1)
getFreshSharedTokenWithFallback(110-123)
🔇 Additional comments (6)
middleware.ts (3)
11-30: LGTM - middleware configuration looks correct.The addition of
/auth-errortounauthenticatedPathsis necessary to ensure users can access the error page without a valid session. The baseMiddleware pattern cleanly separates the AuthKit configuration from the custom redirect interception logic.
32-41: LGTM - pass-through logic is correct.Early return for non-redirect responses and the null check are appropriate. The status 307 check correctly targets temporary redirects issued by AuthKit during session failures.
49-68: The current logic is sound and doesn't risk false positives.The middleware only intercepts 307 redirects to
/loginwhen accessing protected routes without a valid session. Since/privacy-policyand/terms-of-serviceare in theunauthenticatedPathsconfiguration, AuthKit won't redirect to login from those paths. User-initiated login redirects (viawindow.location.href) are client-side navigation and never generate 307 responses, so there's no collision with server-side session failure redirects. The exclusions for/loginand/logoutpaths correctly prevent redirect loops.lib/auth/use-auth-from-authkit.ts (3)
16-18: LGTM - debug flag is properly disabled by default.The
DEBUG_TOKEN_FLOW = falseconstant ensures the debug instrumentation has zero runtime overhead in production. The buffer constant aligns with typical JWT refresh patterns.
124-139: LGTM - state change tracking is properly guarded.The debug useEffect correctly tracks
isLoadinganduserchanges using refs to avoid stale closures. The conditional check ensures no runtime cost when debugging is disabled.
154-242: LGTM - fetchAccessToken logic is well-structured.The debug instrumentation is cleanly integrated without changing the core flow. The cross-tab coordination path and fallback behaviors remain intact. Error handling appropriately falls back to cached tokens with debug logging.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
middleware.ts(3 hunks)
🔇 Additional comments (6)
middleware.ts (6)
2-2: LGTM!The added imports are necessary for the new middleware wrapper and are used correctly throughout the file.
11-30: LGTM!The conversion to
baseMiddlewareconstant and the addition of/auth-errortounauthenticatedPathsare both necessary for the new redirect handling. The/auth-errorpath must be accessible without authentication to display the error page properly.
32-41: LGTM!The middleware wrapper correctly delegates to
baseMiddlewareand passes through all non-307 responses without modification. The async/await pattern is properly implemented.
49-68: LGTM!The redirect interception logic is well-implemented:
- Correctly parses both relative and absolute redirect URLs
- Includes a defensive check for WorkOS OAuth redirects via hostname
- Excludes normal auth flow paths (
/login,/logout) from interception- Provides comprehensive logging with relevant debug information
- Truncates user agent to prevent log bloat
69-80: LGTM!The API route handling is correct:
- Returns 401 JSON instead of redirecting (prevents CORS issues with fetch)
- Error format matches the application's error handling convention
- Semantically appropriate 401 status code
82-86: LGTM!The page route handling correctly redirects to
/auth-error?code=401, providing a clear user experience for session failures while passing the error code for appropriate messaging.
Wrap AuthKit middleware to intercept auth redirects (3xx to WorkOS). For non-browser requests (API, RSC, Server Actions, fetch), return 401 JSON instead of redirect to prevent CORS errors. Browser navigations (Accept: text/html) still redirect to WorkOS.
fkesheh
force-pushed
the
fix/auth-failure-redirect
branch
from
e268646 to
bcb90fd
Compare
fkesheh
changed the title
fkesheh
force-pushed
the
fix/auth-failure-redirect
branch
from
35b2e95 to
cb559ac
Compare
fkesheh
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
middleware.ts (1)
81-83: Empty catch block still lacks error logging.The past review comment on these lines was marked "✅ Addressed in commits e76e98a to 747f5c8," but the current code still contains an empty catch block without logging. Logging parsing failures aids debugging when URL parsing unexpectedly fails.
🔎 Apply this diff to add error logging:
- } catch { + } catch (error) { // URL parsing failed, let original redirect through + console.error("[Auth Middleware] Failed to parse redirect URL:", error, { + location, + path: request.nextUrl.pathname + }); }
🧹 Nitpick comments (1)
app/auth-error/auto-retry-button.tsx (1)
56-60: Consider usinguseSyncExternalStorefor localStorage reads.Setting state directly in a
useEffectfor external store synchronization is not the React 19 recommended pattern. While the eslint-disable acknowledges this is intentional, consider refactoring to useuseSyncExternalStorefor better alignment with React's concurrent features.Alternative approach with useSyncExternalStore:
import { useSyncExternalStore, useEffect, useState } from "react"; function subscribe(callback: () => void) { window.addEventListener("storage", callback); return () => window.removeEventListener("storage", callback); } function getSnapshot() { return getRetryState().count; } function getServerSnapshot() { return 0; } export function AutoRetryButton({ loginUrl }: AutoRetryButtonProps) { const retryCount = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); const [countdown, setCountdown] = useState(() => calculateDelay(retryCount)); const [cancelled, setCancelled] = useState(false); // ... rest of the component }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/auth-error/auto-retry-button.tsx(1 hunks)app/auth-error/page.tsx(4 hunks)app/callback/route.ts(3 hunks)middleware.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/auth-error/page.tsx (1)
app/auth-error/auto-retry-button.tsx (1)
AutoRetryButton(51-106)
🔇 Additional comments (8)
app/callback/route.ts (2)
14-23: LGTM: Helpful debugging additions.The observational logging captures key callback parameters without exposing sensitive data. Truncating the search string is a good practice to prevent log spam.
33-37: LGTM: Response logging aids troubleshooting.Logging the AuthHandler response status and Location header (truncated) provides visibility into the authentication flow, complementing the middleware's new redirect interception.
app/auth-error/auto-retry-button.tsx (2)
62-70: LGTM: Countdown timer logic is correct.The interval-based countdown with proper cleanup and null checks is well-implemented.
72-78: LGTM: Navigation trigger handles state correctly.The effect properly checks conditions before navigating and updates the retry count in storage before redirecting.
app/auth-error/page.tsx (1)
94-103: LGTM: Clean integration of auto-retry for 500 errors.The conditional rendering correctly applies
AutoRetryButtononly to errors marked withautoRetry: true(currently only 500), while preserving the existing manual retry button for other error codes. The UI consistency is maintained.middleware.ts (3)
48-56: LGTM: Auth redirect detection logic is sound.The URL parsing with base handles relative redirects correctly, and checking both
/loginpathname andapi.workos.comhostname appropriately identifies WorkOS authentication redirects.
58-68: Browser detection heuristic is reasonable for common cases.Using
Accept: text/htmlto distinguish browser navigations from API/fetch/RSC requests is a pragmatic heuristic that works for the vast majority of cases:
- Browser navigations typically send
Accept: text/html,application/xhtml+xml,...- fetch/API calls typically send
Accept: application/jsonor*/*- RSC and Server Actions use special headers without
text/htmlEdge cases (e.g., fetch with explicit
Accept: text/html) are rare and likely acceptable tradeoffs.
71-79: LGTM: 401 JSON response structure is clear and actionable.The response includes helpful
code,message, andcausefields that client code can use to handle authentication failures gracefully.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
middleware.ts (1)
72-74: Consider logging URL parsing failures for debuggability.The empty catch block silently swallows parsing errors. While the fallback behavior (letting the redirect through) is correct, logging would aid debugging when unexpected failures occur.
This was flagged in a previous review and marked as addressed, but the logging appears to not have been added. If this was an intentional decision to keep the code minimal, this can be ignored.
app/auth-error/auto-retry-button.tsx (1)
33-39: Add error handling for localStorage writes.
getRetryStatehas error handling, butsetRetryStatedoes not. If localStorage is blocked (private browsing, quota exceeded), this will throw and break the retry flow.This was flagged in a previous review and appears unaddressed.
🔎 Apply this diff to add error handling:
function setRetryState(count: number): void { + try { const state: RetryState = { count, expiresAt: Date.now() + EXPIRY_HOURS * 60 * 60 * 1000, }; localStorage.setItem(STORAGE_KEY, JSON.stringify(state)); + } catch { + // Storage blocked - retry will still work but won't persist count + } }
🧹 Nitpick comments (1)
app/auth-error/auto-retry-button.tsx (1)
62-70: Consider removingcountdownfrom the dependency array.Including
countdownin the dependency array causes the interval to be cleared and recreated every second. While functionally correct due to the cleanup function, this is inefficient.🔎 Apply this diff to optimize the interval:
useEffect(() => { - if (cancelled || countdown === null || countdown <= 0) return; + if (cancelled || countdown === null) return; const timer = setInterval(() => { - setCountdown((prev) => (prev !== null ? prev - 1 : null)); + setCountdown((prev) => { + if (prev === null || prev <= 1) { + clearInterval(timer); + return prev === null ? null : 0; + } + return prev - 1; + }); }, 1000); return () => clearInterval(timer); - }, [cancelled, countdown]); + }, [cancelled, countdown === null]); // Only re-run when countdown becomes null/non-nullAlternatively, keep the current implementation if simplicity is preferred over micro-optimization.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/auth-error/auto-retry-button.tsx(1 hunks)app/auth-error/page.tsx(4 hunks)app/callback/route.ts(0 hunks)middleware.ts(3 hunks)
💤 Files with no reviewable changes (1)
- app/callback/route.ts
🧰 Additional context used
🧬 Code graph analysis (2)
app/auth-error/page.tsx (2)
lib/errors.ts (1)
ErrorCode(20-20)app/auth-error/auto-retry-button.tsx (1)
AutoRetryButton(51-106)
app/auth-error/auto-retry-button.tsx (2)
components/ui/button.tsx (1)
Button(59-59)app/components/MemoizedMarkdown.tsx (1)
a(14-26)
🔇 Additional comments (6)
middleware.ts (1)
32-77: LGTM! Clean implementation of the redirect interception pattern.The middleware correctly:
- Delegates to
baseMiddlewarefirst, preserving all existing auth logic- Detects auth redirects by checking both
/loginpath andapi.workos.comhostname- Uses the
Acceptheader to distinguish browser navigations from API/fetch requests- Returns a structured 401 JSON response for non-browser requests
This approach effectively solves the CORS issue for non-browser requests while maintaining the expected redirect behavior for browser navigations.
app/auth-error/auto-retry-button.tsx (2)
72-78: LGTM! Redirect logic is correct.The effect properly:
- Guards against cancelled state
- Persists the incremented retry count before redirecting
- Uses
window.location.hrefappropriately for full-page navigation to the login flow
80-105: LGTM! Clean state-based UI rendering.The component handles three distinct states well:
- Cancelled: Shows manual retry link
- Loading (null countdown): Shows disabled spinner while initializing
- Active countdown: Shows cancel-able countdown with clear feedback
app/auth-error/page.tsx (3)
12-12: LGTM! Clean import for the new component.
35-40: LGTM! Auto-retry for 500 errors makes sense.The description correctly identifies the multi-tab authentication race condition scenario, and auto-retry is appropriate for this transient failure case.
94-103: LGTM! Clean conditional rendering integration.The conditional properly switches between
AutoRetryButtonfor auto-retry errors and the standard button for others, maintaining consistent styling.
- Add AutoRetryButton client component with 3-second countdown - Auto-retry on 500 errors (common with multi-tab OAuth code races) - Track retry count via URL params to prevent infinite loops (max 2) - Allow users to cancel auto-retry and manually retry - Updated 500 error description to explain multi-tab scenario - Add debug logging to callback and middleware for production troubleshooting
fkesheh
force-pushed
the
fix/auth-failure-redirect
branch
from
cb559ac to
3927046
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
middleware.ts (1)
72-74: [Duplicate] Empty catch block should log parsing errors.This issue was previously flagged and marked as addressed, but the current code still contains an empty catch block. While the fallback behavior (letting the original redirect through) is correct, the catch block should log the error for debuggability.
🔎 Apply this diff to log the error:
- } catch { + } catch (error) { // URL parsing failed, let original redirect through + console.error("[Auth Middleware] Failed to parse redirect URL:", error, { + location, + requestUrl: request.url, + }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/auth-error/auto-retry-button.tsx(1 hunks)app/auth-error/page.tsx(4 hunks)app/callback/route.ts(0 hunks)middleware.ts(3 hunks)
💤 Files with no reviewable changes (1)
- app/callback/route.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/auth-error/auto-retry-button.tsx
- app/auth-error/page.tsx
🔇 Additional comments (4)
middleware.ts (4)
11-30: LGTM: Configuration and unauthenticated paths.The renaming to
baseMiddlewareenables the wrapper pattern, and the addition of/auth-errorcorrectly allows unauthenticated access to the error page as described in the PR objectives.
32-41: LGTM: Middleware delegation and redirect detection.The function signature is correct for Next.js middleware, and the redirect detection logic (checking for 3xx status codes) properly filters responses before applying custom behavior.
43-71: LGTM: Auth redirect handling logic.The logic correctly:
- Validates the Location header exists
- Distinguishes auth redirects (to
/loginorapi.workos.com)- Preserves browser navigation redirects while returning 401 JSON for non-browser requests
- Uses a structured error response that clients can handle programmatically
2-2: No action needed on the imports. The middleware signature withNextMiddleware = (request: NextRequest, event: NextFetchEvent)is the documented pattern for Next.js middleware, and NextResponse can return JSON responses with custom status codes as shown in the middleware wrapper. The imports and pattern are compatible across Next.js versions.
…trol Switch from authkitMiddleware wrapper to low-level authkit() composable for cleaner control flow. This eliminates response interception by checking session state before deciding the response type. - Use authkit() for direct access to session and authorizationUrl - Handle header forwarding explicitly (x-* to request, strip session from response) - Simplify path matching with Set + prefix check 🤖 Generated with [Claude Code](https://claude.com/claude-code)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
middleware.ts (1)
42-44: Consider the necessity ofeagerAuth: true.With
eagerAuth: true, the authkit library performs authentication checks on all paths, including those explicitly listed inUNAUTHENTICATED_PATHS. Since you're manually handling unauthenticated paths at line 51, this setting creates some redundancy. However, it may be intentional if you want authenticated users to have session information available even on public pages.If you don't need session data on unauthenticated pages, consider setting
eagerAuth: falseor using the authkit library's built-in unauthenticated paths handling to avoid the extra work.Based on learnings about the
@workos-inc/authkit-nextjslibrary, if present.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.cursor/rules/convex_rules.mdc(5 hunks)middleware.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .cursor/rules/convex_rules.mdc
🔇 Additional comments (5)
middleware.ts (5)
1-15: LGTM! Well-structured constants and imports.The migration from
authkitMiddlewaretoauthkitcomposable aligns with best practices for custom middleware logic. Using aSetfor unauthenticated paths provides O(1) lookup performance, and the inclusion of/auth-errorfulfills the PR requirement for an accessible auth error page.
24-37: Good helper functions for request classification.The functions correctly identify unauthenticated paths (including the new
/share/*public access pattern) and distinguish browser requests from API/RSC requests based on the Accept header.
58-67: Excellent implementation of non-browser auth handling.The 401 JSON response correctly addresses the PR requirement to avoid redirecting API/RSC requests to WorkOS. The error structure with
code,message, andcauseprovides clear debugging information for client applications and Server Actions.
72-86: Well-implemented header management utilities.The functions correctly:
- Merge only
x-prefixed headers from authkit into requests (avoiding header pollution)- Strip the session header from responses (preventing sensitive data exposure)
42-45: Verify error handling strategy for the composableauthkit()function.When using the composable
authkitmethod in middleware, you are responsible for handling what to do when there's no session on a protected route. The code assumes session is always returned but should verify:
The
authkit()function returns eitherUserInfoorNoUserInfotypes, meaning the session object may not always contain a user. Defensive checks forsession.userare appropriate, but confirm this is the intended pattern rather than relying on guaranteed session presence.Wrapping calls that perform redirects (like
withAuthwithensureSignedIn: true) in try-catch blocks will cause NEXT_REDIRECT errors, since redirects in Next.js must be called outside try-catch. Verify if the currentauthkit()call on line 42 requires error handling or if exceptions are expected to propagate naturally.When
eagerAuth: trueis set, the middleware temporarily stores the access token in a short-lived cookie (30 seconds) only on initial page loads. Confirm this behavior meets security and functional requirements.
Move countdown checks inside setInterval callback using functional update pattern, removing countdown from useEffect dependencies. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/auth-error/auto-retry-button.tsx (2)
61-65: Consider useSyncExternalStore for localStorage sync.React 19 provides
useSyncExternalStorespecifically for syncing with external stores like localStorage. This would be more idiomatic thanuseEffect+setStateand handles SSR hydration and concurrent rendering edge cases better.Example refactor using useSyncExternalStore
- // Initialize countdown from localStorage on mount (intentional one-time sync from external store) - useEffect(() => { - const { count } = getRetryState(); - // eslint-disable-next-line react-hooks/set-state-in-effect - setCountdown(calculateDelay(count)); - }, []); + const initialCount = useSyncExternalStore( + (callback) => { + // Subscribe: listen for storage events from other tabs + window.addEventListener('storage', callback); + return () => window.removeEventListener('storage', callback); + }, + () => getRetryState().count, // Client snapshot + () => 0 // Server snapshot + ); + + // Initialize countdown from stored count + const [countdown, setCountdown] = useState<number | null>( + () => calculateDelay(initialCount) + );Note: You'd need to adjust the logic since the count changes over time, but this pattern is more aligned with React 19 best practices.
67-78: Minor efficiency: interval continues after countdown reaches zero.The interval continues running after
countdownreaches 0 (line 72 returnsprevunchanged when<= 0). While not a bug, clearing the interval when countdown hits zero would be slightly more efficient.Optional optimization
useEffect(() => { if (cancelled) return; const timer = setInterval(() => { setCountdown((prev) => { if (prev === null || prev <= 0) return prev; return prev - 1; }); }, 1000); return () => clearInterval(timer); - }, [cancelled]); + }, [cancelled, countdown]);And add logic to clear when countdown reaches 0, or restructure to stop the interval.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/auth-error/auto-retry-button.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/auth-error/auto-retry-button.tsx (2)
components/ui/button.tsx (1)
Button(59-59)app/components/MemoizedMarkdown.tsx (1)
a(14-26)
🔇 Additional comments (6)
app/auth-error/auto-retry-button.tsx (6)
7-10: Constants are well-chosen for the retry strategy.The exponential backoff progression (BASE_DELAY=5, MAX_DELAY=600) and 4-hour expiry align perfectly with the PR objectives and provide a good balance between responsiveness and server load.
17-31: Robust localStorage read with expiry handling.The error handling, expiry check, and cleanup logic are well-implemented. Returns a safe default when storage is unavailable or corrupted.
33-44: Previous localStorage error handling concern addressed.The try-catch wrapper correctly handles storage failures without breaking the retry flow. The inline comment clearly documents the fallback behavior.
46-50: Exponential backoff calculation is correct.The formula correctly implements the progression described in the PR objectives (5s → 10s → 20s → ... → 600s capped).
88-113: Render logic correctly handles all states.The three rendering branches (cancelled → link, loading → disabled button, active → cancellable countdown) are well-structured and provide clear user feedback. The use of
asChildfor the link (line 90) and appropriate button states is good practice.
80-86: loginUrl is safely hardcoded and not user-controllable.The component passes
loginUrl="/login"as a hardcoded relative path, not from user input or query parameters. No open redirect vulnerability exists here.
Prevents runtime crash if WorkOS auth service is unavailable. Redirects to /auth-error?code=503 with appropriate error logging. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
authkitMiddlewarewrapper to low-levelauthkit()composable for direct session controlx-*headers to request, stripx-workos-sessionfrom response (security)Set+ prefix check for/share/*Test plan
/auth-errorpage is accessible without authenticationwithAuth()still works in page handlers (session header forwarding)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.