fix: Start Free Trial prompts login when signed out and redirects same-tab - #1903
fix: Start Free Trial prompts login when signed out and redirects same-tab#1903sweetmantech wants to merge 1 commit into
Conversation
…e-tab The sidebar Start Free Trial button (and Manage Subscription) had three conversion-killing bugs in the subscribe click path: - hooks/useSubscribeClick.ts silently no-oped for signed-out users; it now opens the Privy login modal via login(), including when no access token is available. - lib/stripe/createClientCheckoutSession.ts and lib/stripe/createClientPortalSession.ts opened Stripe via window.open after an async fetch, which real-browser popup blockers kill; both now navigate same-tab with window.location.href. - Session-creation errors were returned but ignored; the hook now surfaces them as sonner error toasts. Part of #1902 (item C2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe subscription click flow now prompts authentication when required credentials are missing, surfaces Stripe session errors through toasts, and redirects the current window to checkout or billing portal URLs. ChangesSubscription and billing flow
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant useSubscribeClick
participant Privy
participant StripeSession
participant BrowserWindow
participant Toast
User->>useSubscribeClick: click subscription action
useSubscribeClick->>Privy: login when credentials are missing
useSubscribeClick->>StripeSession: create checkout or portal session
StripeSession-->>useSubscribeClick: return URL or error
useSubscribeClick->>Toast: show error when session creation fails
useSubscribeClick->>BrowserWindow: redirect current window to Stripe URL
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1209e16d73
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| vi.clearAllMocks(); | ||
| Object.defineProperty(window, "location", { | ||
| value: { href: "https://chat.test/current" }, | ||
| writable: true, |
There was a problem hiding this comment.
In the Vitest jsdom environment used by these tests, window.location is an unforgeable/non-configurable property, so this Object.defineProperty(window, "location", ...) setup throws in beforeEach before any redirect assertions run. The same setup was added in createClientPortalSession.test.ts, so the new Stripe tests will fail in CI instead of validating same-tab navigation; use jsdom URL reconfiguration for the current URL and mock an injectable navigation helper rather than replacing window.location.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@hooks/useSubscribeClick.ts`:
- Around line 19-22: Update the access-token handling in useSubscribeClick so
getAccessToken rejection is caught and triggers login before exiting, while
preserving the existing missing-token fallback. Match the error-handling
behavior used around user.data.accessToken in useChatTransport and allow other
Stripe-session errors to continue through their existing handling.
- Around line 9-17: Update useSubscribeClick’s usePrivy state handling to
include ready and authenticated. Make handleClick return while Privy is not
ready, invoke login and return when authenticated is false, and only use missing
userData.account_id to handle an incomplete profile after authentication; ensure
getAccessToken and all checkout/portal flows occur only for ready, authenticated
users.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56b215e2-82ed-40f7-aa18-ab28b51d859a
⛔ Files ignored due to path filters (3)
hooks/__tests__/useSubscribeClick.test.tsxis excluded by!**/*.test.*and included byhooks/**lib/stripe/__tests__/createClientCheckoutSession.test.tsis excluded by!**/*.test.*and included bylib/**lib/stripe/__tests__/createClientPortalSession.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (3)
hooks/useSubscribeClick.tslib/stripe/createClientCheckoutSession.tslib/stripe/createClientPortalSession.ts
| const { getAccessToken, login } = usePrivy(); | ||
| const { userData } = useUserProvider(); | ||
| const { isSubscribed } = usePaymentProvider(); | ||
|
|
||
| const handleClick = async () => { | ||
| if (!userData?.account_id) return; | ||
| if (!userData?.account_id) { | ||
| login(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)hooks/useSubscribeClick\.ts$|privy|payment|user|PaymentProvider|useUserProvider' || true
echo "== target file =="
if [ -f hooks/useSubscribeClick.ts ]; then
cat -n hooks/useSubscribeClick.ts
fi
echo "== related provider hook excerpts =="
for f in $(git ls-files | rg 'use(User|Payment).*Provider|UserProvider|PaymentProvider|privy.*provider|provider.*privy' | head -50); do
echo "--- $f"
sed -n '1,220p' "$f"
done
echo "== usages of subscribe click hooks =="
rg -n "useSubscribeClick|useSubscribe" -S . || trueRepository: recoupable/chat
Length of output: 4078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate UserProvider files =="
git ls-files | rg '(^|/)UserProvder\.tsx$|UserProvider' || true
echo "== payment hook =="
if [ -f hooks/usePayment.ts ]; then
cat -n hooks/usePayment.ts
fi
echo "== UserProvider =="
if [ -f providers/UserProvder.tsx ]; then
cat -n providers/UserProvder.tsx
fi
echo "== test file =="
cat -n hooks/__tests__/useSubscribeClick.test.tsx
echo "== public package/dependency versions =="
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
if [ -f "$f" ]; then
echo "--- $f"
if [ "$f" = "package.json" ]; then jq '.dependencies, .devDependencies' "$f"; else rg -n "`@privy-io/react-auth`|privy" "$f" | head -40 || true; fi
fi
doneRepository: recoupable/chat
Length of output: 13078
🌐 Web query:
Privy React authentication hook ready authenticated state usePrivy docs
💡 Result:
In the Privy React SDK, the usePrivy hook provides both ready and authenticated states to manage user authentication in your application [1][2]. Key Concepts: 1. ready: This is a boolean indicating whether the Privy SDK has finished initializing [1][3]. Because the SDK must perform tasks such as checking for existing user sessions, refreshing authentication tokens, and fetching user data upon loading, you must ensure ready is true before consuming other Privy state or interfaces to avoid using stale or inaccurate information [3]. 2. authenticated: This is a boolean indicating whether the current user is logged in [1]. Best Practice: Always check the ready state before relying on the authenticated status or the user object [1][3]. A common pattern is to prevent UI rendering or perform redirects until ready is true [1][3]. Example Usage: import { usePrivy } from '@privy-io/react-auth'; import { useRouter } from 'next/router'; function MyComponent { const { ready, authenticated, user } = usePrivy; const router = useRouter; if (!ready) { // Optionally return a loading state return
Citations:
- 1: https://docs.privy.io/authentication/user-authentication/authentication-state
- 2: https://www.npmjs.com/package/@privy-io/react-auth
- 3: https://docs.privy.io/basics/react/setup
Gate subscription actions behind Privy readiness and authentication state.
userData?.account_id is application data that may be unavailable while Privy finishes restoring an existing session. In hooks/useSubscribeClick.ts, wait on ready, skip getAccessToken()/checkout/portal flows when !authenticated, and only treat a missing account_id as an incomplete user profile rather than an unauthenticated request.
🤖 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 `@hooks/useSubscribeClick.ts` around lines 9 - 17, Update useSubscribeClick’s
usePrivy state handling to include ready and authenticated. Make handleClick
return while Privy is not ready, invoke login and return when authenticated is
false, and only use missing userData.account_id to handle an incomplete profile
after authentication; ensure getAccessToken and all checkout/portal flows occur
only for ready, authenticated users.
| const accessToken = await getAccessToken(); | ||
| if (!accessToken) return; | ||
| if (!accessToken) { | ||
| login(); | ||
| return; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked hook files:"
git ls-files | rg '(^|/)useSubscribeClick\.tsx?$|getAccessToken|login\(' || true
echo
echo "File outline/read:"
if [ -f hooks/useSubscribeClick.ts ]; then
wc -l hooks/useSubscribeClick.ts
cat -n hooks/useSubscribeClick.ts
elif [ -f hooks/useSubscribeClick.tsx ]; then
wc -l hooks/useSubscribeClick.tsx
cat -n hooks/useSubscribeClick.tsx
fi
echo
echo "Search getAccessToken implementation/usages:"
rg -n "function getAccessToken|const getAccessToken|getAccessToken\s*=|getAccessToken\(" .Repository: recoupable/chat
Length of output: 6796
🌐 Web query:
Privy React usePrivy getAccessToken throws refresh token error documentation
💡 Result:
The Privy React SDK's getAccessToken method is designed to automatically handle token refreshing [1][2][3]. If it fails to refresh the access token—for example, because the underlying refresh token has expired or the user has revoked access—the SDK typically logs the user out [2][4]. There is no specific "refresh token error" that developers are expected to catch directly from getAccessToken to manually perform a refresh, as the SDK manages refresh tokens internally and does not expose them to your application [1][5]. If you encounter authentication issues, the recommended practices are: 1. Handle Invalid Tokens: If your backend returns an "invalid auth token" error or a 401, you should call getAccessToken on the client side [2][4]. Because getAccessToken automatically triggers a refresh attempt, this often resolves temporary expiration issues [2][5]. 2. Handle Failed Refreshes: If getAccessToken fails (e.g., returns null or an error, and the user is effectively logged out), you must redirect the user to your login flow to re-authenticate [2][5]. 3. Server-Side Constraints: Privy does not permit applications to refresh access tokens from the server-side using a refresh token [5]. All refresh operations must occur via the Privy SDK on the client side [2][5]. If you are seeing an error related to refresh tokens, ensure that you are using getAccessToken correctly from the usePrivy hook and that your user's session has not exceeded the maximum lifetime (default is 30 days, configurable in the Privy Dashboard) [1][2].
Citations:
- 1: https://docs.privy.io/authentication/user-authentication/tokens
- 2: https://docs.privy.io/authentication/user-authentication/access-tokens
- 3: https://docs.privy.io/recipes/core-js
- 4: https://docs.privy.io/authentication/user-authentication/access-tokens.md
- 5: https://docs.privy.io/recipes/react/cookies
Handle access-token failure consistently.
getAccessToken() can reject when Privy can’t refresh the token, which currently cancels the Stripe-session flow before the existing error handling runs. Catch it here and re-trigger the login flow, similar to the fallback around user.data.accessToken in useChatTransport.
🤖 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 `@hooks/useSubscribeClick.ts` around lines 19 - 22, Update the access-token
handling in useSubscribeClick so getAccessToken rejection is caught and triggers
login before exiting, while preserving the existing missing-token fallback.
Match the error-handling behavior used around user.data.accessToken in
useChatTransport and allow other Stripe-session errors to continue through their
existing handling.
Source: Path instructions
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 files
Confidence score: 2/5
- In
lib/stripe/createClientPortalSession.ts, assigning unvalidated portal URLs towindow.location.hrefcould allow a malformed/compromisedjavascript:URL to execute in-tab, creating a direct client-side security risk—only allow expected HTTPS Stripe portal URLs and preserve the current error return shape when validation fails. - In
hooks/useSubscribeClick.ts, the!userData?.account_idguard can run before Privy finishes restoring session state, so valid users may be incorrectly sent through login again, causing subscription flow interruptions—gate this check on Privy readiness before prompting auth. - In
lib/stripe/__tests__/createClientCheckoutSession.test.ts, redefiningwindow.locationviaObject.definePropertycan fail in modern jsdom and breakbeforeEach, which can make the suite fail for environment reasons instead of product behavior—switch to a jsdom-safe location mocking approach. - In
lib/stripe/__tests__/createClientPortalSession.test.ts, the fetch-rejection/network-failure path increateClientPortalSessionis untested, leaving the main error-handling branch unverified and increasing regression risk during refactors—add a rejected-fetch test mirroring the checkout-session coverage.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/stripe/createClientPortalSession.ts">
<violation number="1" location="lib/stripe/createClientPortalSession.ts:28">
P1: A malformed or compromised portal response can now execute a `javascript:` URL in the app's current tab. Validate an HTTPS Stripe portal URL before assigning `window.location.href`, and return the existing error shape when it is invalid.</violation>
</file>
<file name="lib/stripe/__tests__/createClientPortalSession.test.ts">
<violation number="1" location="lib/stripe/__tests__/createClientPortalSession.test.ts:3">
P2: Missing test for network failure / fetch rejection path. The `catch` block in `createClientPortalSession` handles rejected fetches and JSON errors, but this test file doesn't cover it. The sibling `createClientCheckoutSession.test.ts` includes an identical test case (`"returns an error when fetch rejects"`) that you can mirror here.</violation>
</file>
<file name="hooks/useSubscribeClick.ts">
<violation number="1" location="hooks/useSubscribeClick.ts:14">
P2: The `!userData?.account_id` check may fire while Privy is still restoring an existing session (before `ready` is true), incorrectly prompting an already-authenticated user to log in again. Consider checking Privy's `ready`/`authenticated` state before treating a missing `account_id` as unauthenticated.</violation>
</file>
<file name="lib/stripe/__tests__/createClientCheckoutSession.test.ts">
<violation number="1" location="lib/stripe/__tests__/createClientCheckoutSession.test.ts:15">
P2: `Object.defineProperty(window, "location", ...)` may throw in this jsdom test environment since `window.location` is treated as non-configurable in current jsdom versions, which would cause `beforeEach` to fail before any redirect assertions run and break this test suite in CI. Consider mocking an injectable navigation helper or using jsdom's URL reconfiguration instead of replacing `window.location` directly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| window.open(data.url, "_blank", "noopener,noreferrer"); | ||
| window.location.href = data.url; |
There was a problem hiding this comment.
P1: A malformed or compromised portal response can now execute a javascript: URL in the app's current tab. Validate an HTTPS Stripe portal URL before assigning window.location.href, and return the existing error shape when it is invalid.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/stripe/createClientPortalSession.ts, line 28:
<comment>A malformed or compromised portal response can now execute a `javascript:` URL in the app's current tab. Validate an HTTPS Stripe portal URL before assigning `window.location.href`, and return the existing error shape when it is invalid.</comment>
<file context>
@@ -25,7 +25,7 @@ const createClientPortalSession = async (accessToken: string) => {
}
- window.open(data.url, "_blank", "noopener,noreferrer");
+ window.location.href = data.url;
} catch (error) {
return { error };
</file context>
| @@ -0,0 +1,49 @@ | |||
| // @vitest-environment jsdom | |||
There was a problem hiding this comment.
P2: Missing test for network failure / fetch rejection path. The catch block in createClientPortalSession handles rejected fetches and JSON errors, but this test file doesn't cover it. The sibling createClientCheckoutSession.test.ts includes an identical test case ("returns an error when fetch rejects") that you can mirror here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/stripe/__tests__/createClientPortalSession.test.ts, line 3:
<comment>Missing test for network failure / fetch rejection path. The `catch` block in `createClientPortalSession` handles rejected fetches and JSON errors, but this test file doesn't cover it. The sibling `createClientCheckoutSession.test.ts` includes an identical test case (`"returns an error when fetch rejects"`) that you can mirror here.</comment>
<file context>
@@ -0,0 +1,49 @@
+// @vitest-environment jsdom
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import createClientPortalSession from "@/lib/stripe/createClientPortalSession";
+
+vi.mock("@/lib/api/getClientApiBaseUrl", () => ({
</file context>
|
|
||
| const handleClick = async () => { | ||
| if (!userData?.account_id) return; | ||
| if (!userData?.account_id) { |
There was a problem hiding this comment.
P2: The !userData?.account_id check may fire while Privy is still restoring an existing session (before ready is true), incorrectly prompting an already-authenticated user to log in again. Consider checking Privy's ready/authenticated state before treating a missing account_id as unauthenticated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useSubscribeClick.ts, line 14:
<comment>The `!userData?.account_id` check may fire while Privy is still restoring an existing session (before `ready` is true), incorrectly prompting an already-authenticated user to log in again. Consider checking Privy's `ready`/`authenticated` state before treating a missing `account_id` as unauthenticated.</comment>
<file context>
@@ -1,26 +1,39 @@
const handleClick = async () => {
- if (!userData?.account_id) return;
+ if (!userData?.account_id) {
+ login();
+ return;
</file context>
| describe("createClientCheckoutSession", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| Object.defineProperty(window, "location", { |
There was a problem hiding this comment.
P2: Object.defineProperty(window, "location", ...) may throw in this jsdom test environment since window.location is treated as non-configurable in current jsdom versions, which would cause beforeEach to fail before any redirect assertions run and break this test suite in CI. Consider mocking an injectable navigation helper or using jsdom's URL reconfiguration instead of replacing window.location directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/stripe/__tests__/createClientCheckoutSession.test.ts, line 15:
<comment>`Object.defineProperty(window, "location", ...)` may throw in this jsdom test environment since `window.location` is treated as non-configurable in current jsdom versions, which would cause `beforeEach` to fail before any redirect assertions run and break this test suite in CI. Consider mocking an injectable navigation helper or using jsdom's URL reconfiguration instead of replacing `window.location` directly.</comment>
<file context>
@@ -0,0 +1,57 @@
+describe("createClientCheckoutSession", () => {
+ beforeEach(() => {
+ vi.clearAllMocks();
+ Object.defineProperty(window, "location", {
+ value: { href: "https://chat.test/current" },
+ writable: true,
</file context>
1 participant
What
Fixes three bugs in the sidebar "Start Free Trial" / "Manage Subscription" click path (
components/Sidebar/UnlockProCard.tsxandcomponents/Sidebar/UserProfileDropdown/ManageSubscriptionButton.tsx, both viahooks/useSubscribeClick.ts):hooks/useSubscribeClick.ts:13returned early onif (!userData?.account_id) return;, so a signed-out visitor clicking Start Free Trial saw nothing happen. The hook now opens the Privy login modal vialogin()fromusePrivy, both when there is noaccount_idand whengetAccessToken()yields no token.lib/stripe/createClientCheckoutSession.ts:28andlib/stripe/createClientPortalSession.ts:28calledwindow.open(url, "_blank")after an async fetch; real-browser popup blockers block window.open outside the synchronous click handler. Both now navigate same-tab withwindow.location.href = data.url.{ error }on failure and the hook discarded it. Failures now surface as a visiblesonnererror toast ("Could not start checkout. Please try again." / "Could not open billing. Please try again."), matching the toast pattern used across the newer hooks (hooks/useCreateTask.ts,hooks/usePulseToggle.ts, etc.).Part of #1902 (item C2).
Why
Start Free Trial is a primary conversion CTA. A silent no-op for signed-out users and a popup-blocked checkout tab for signed-in users both read as "the button is broken" and drop the user on the floor with no feedback.
How to test
POST /api/subscriptions/sessionsin devtools and click the button. Expected: an error toast appears (previously: silent failure).pnpm exec vitest run hooks/__tests__/useSubscribeClick.test.tsx lib/stripe/__tests__(13 tests, all passing;pnpm exec tsc --noEmitclean for the changed files).Preview verification to follow in a PR comment.
🤖 Generated with Claude Code
Summary by cubic
Fixes the Start Free Trial / Manage Subscription click path: signed-out users are prompted to log in, Stripe opens in the same tab, and failures show a toast. This removes silent no-ops and popup-blocked windows to improve conversion.
login()from@privy-io/react-authwhen no account or access token.window.location.hrefinstead ofwindow.open.sonnererror toasts for failed checkout/portal session creation.Written for commit 1209e16. Summary will update on new commits.
Summary by CodeRabbit