fix(#312): show email confirmation pending state after signup#317
Conversation
Web: remove the else-navigate bug that sent users to /dashboard before their email was confirmed. All no-session signups now show the awaitingEmail pending UI. Billing-specific copy is conditionalized. Mobile: check getSession() after signUp(); navigate to new SignupPendingScreen when no session, Dashboard when session exists. Also adds SignupPendingScreen, registers it in AppNavigator, and updates RootStackParamList and tests on both platforms.
CI Coverage & Test Summary
Suites: 43 passed, 0 failed (43 total) · Tests: 505 passed, 0 failed (505 total) ✅ All reported test suites passed. Coverage artifacts: Updated at: May 21, 2026 at 2:47 PM PDT |
ZappoMan
left a comment
ZappoMan
left a comment
There was a problem hiding this comment.
Review summary
Solid fix for #312. Diego's implementation matches the plan we discussed and correctly addresses the root cause on both platforms.
What works well
- Web: Consolidated no-session branch — all email signups without a session show the existing
awaitingEmailUI; billing redirect stash is preserved only for paid intent. - Web copy: Conditional billing sentence avoids the awkward em-dash-only fragment for plain signup.
- Mobile:
getSession()check + newSignupPendingScreenmirrors web behavior. - Tests: Web test rewritten correctly (18/18 pass); mobile tests split for session/no-session paths (10/10 pass). Mock strategy for
supabase.auth.getSessionis correct.
Suggested before/after merge (non-blocking)
- Add
AppHeadertoSignupPendingScreen— every other auth screen has it; web pending view does too. - Drop unnecessary
route?.params?.optional chaining — types already guarantee params. - Import shared
RootStackParamListinSignupScreeninstead of local duplicate. - Minor: smart apostrophe consistency in billing copy string.
Follow-up (out of scope)
- Mobile
emailRedirectTo+ native confirm deep link (confirmation emails currently land on web/auth/confirm) - Short docs note in
EMAIL_TEMPLATES.mdper issue AC
Verdict: Approve — nothing blocking merge from my side. Left inline comments on the specific lines.
mei-artificerinnovations
left a comment
mei-artificerinnovations
left a comment
There was a problem hiding this comment.
Core logic is correct and the test split (session vs no-session) is clean. Two things to resolve before merge.
⚠️ Conflict with #316 (SignupPage.tsx)
PR #316 (my kit-sync plan_id pipeline fix) also modifies apps/web/src/pages/SignupPage.tsx — specifically handleSignup. It adds plan_id to the auth.signUp call but did not pick up this PR's fix (it still has the silent navigate('/dashboard') for the no-session case). The two PRs need to land with a merged handleSignup:
const planId = searchParams.get('plan');
await auth.signUp(email, password, planId ? { data: { plan_id: planId } } : undefined);
const { data: { session } } = await supabase.auth.getSession();
if (session) {
clearPostAuthRedirectKeys();
navigate(postAuthPath, { replace: true });
} else {
if (paidIntent && postAuthPath !== '/dashboard') {
localStorage.setItem(POST_AUTH_REDIRECT_KEY, serializePostAuthRedirectPayload(postAuthPath));
}
setAwaitingEmail(true);
}I'll update #316 with this merged version once we know merge order. Either way one of us needs to pick up the other's change before Brad merges.
Nit: SignupPending needs a header title in AppNavigator
Without options, RN Stack will show the literal string "SignupPending" as the header title. Either add options={{ title: 'Confirm your email' }} or options={{ headerShown: false }} if the screen handles its own header.
Last deployed: 14:47 PDT 📱 Mobile preview: Channel 📱 Mobile Preview (OTA Updates)No native code changes detected - using OTA updates only. Step 1: Install Development Build (one-time setup)
Step 2: Load PR Preview Update
Note: You must use the full URL format - just entering the channel name ( Alternative: For local development, use: 📖 See Mobile Build Testing Guide for detailed instructions. |
DB & Integration Test Summary
Full logs: Updated at: May 21, 2026 at 2:49 PM PDT |
There was a problem hiding this comment.
Pull request overview
This PR fixes post-signup UX when Supabase email confirmations are enabled and auth.signUp() does not immediately create a session, by showing an explicit “check your email” pending state instead of navigating to authenticated areas.
Changes:
- Web: Treat all “no session after signup” outcomes as an email-confirmation-pending state (and only stash billing redirect keys when a paid intent exists).
- Mobile: After signup, check session and route either to
Dashboard(session present) or a newSignupPendingscreen (no session). - Tests: Update/add web + mobile tests to cover both session/no-session branches and verify the pending UI behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/pages/SignupPage.tsx | Removes dashboard redirect on no-session signup and adjusts pending UI copy to be conditional for paid intent. |
| apps/web/src/pages/tests/SignupPage.test.tsx | Updates expectations to assert pending UI for no-session signup and adds a test for non-billing copy on plain signup. |
| apps/mobile/src/screens/SignupScreen.tsx | Adds session check after signup and navigates to SignupPending when no session exists. |
| apps/mobile/src/screens/SignupPendingScreen.tsx | New screen to display email confirmation pending instructions with a link to sign in. |
| apps/mobile/src/navigation/types.ts | Adds SignupPending to the root stack param list typing. |
| apps/mobile/src/navigation/AppNavigator.tsx | Registers the new SignupPending screen in the navigator. |
| apps/mobile/tests/screens/SignupScreen.test.tsx | Splits “successful signup” coverage into session vs no-session navigation paths by mocking getSession(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Web: split billing copy into own <p>, use ' entities (no smart quotes)
- Web test: merge two pending tests into one; add localStorage null assertion
- Mobile: import shared RootStackParamList in SignupScreen (remove local dup)
- Mobile: add AppHeader to SignupPendingScreen for parity with other screens
- Mobile: use route.params.email (remove defensive optional chaining)
- Mobile: add options={{ title: 'Confirm your email' }} to SignupPending screen
- Mobile: new SignupPendingScreen.test.tsx (heading, email, Login navigation)
- Mobile: mock SignupPendingScreen in App.test.tsx and AppNavigator.test.tsx
ZappoMan
left a comment
ZappoMan
left a comment
There was a problem hiding this comment.
Re-review (post feedback commit e4da5c8)
Diego addressed all review feedback from the first pass. Re-reviewed the latest diff and ran tests locally.
Verified fixes
- Web: consolidated no-session branch + split billing copy into separate
<p>with'entities - Web tests: merged pending test +
localStoragenull assertion (17/17 pass) - Mobile: shared
RootStackParamListimport,AppHeaderon pending screen, screen title option,SignupPendingScreentests, navigator mocks - CI: all checks green; PR is mergeable
Follow-up issues filed
| Thread | Issue |
|---|---|
| RootStackParamList cleanup (other screens) | #318 |
| SignupScreen useEffect double navigation | #319 |
Native mobile confirm deep link + emailRedirectTo |
#320 |
Auth context vs singleton getSession |
#321 |
| Docs for post-signup pending UX | #322 |
Linked #318–#321 on the relevant inline threads.
Blockers
None for this PR. Ready to merge.
Coordination note (not a git conflict): PR #316 also touches SignupPage.tsx (adds plan_id to signup metadata). Whichever lands second should rebase — functionally both changes belong in the same handleSignup. Git merge state is currently clean.
|
Additional follow-ups from the re-review (no specific inline thread):
|
…onfig guards (#316) Clean rebase onto develop (b8c48d9), incorporating #317 and #298 without regressions. - deploy: gate KIT_API_KEY / KIT_CRON_SECRET with [[ -n ]] guards; preserve all #298 Resend sync step and develop/main branch if-conditions - SignupPage: signupPlanMetadata() validates plan before auth.signUp; waitlistCaptureMetadata restored to include plan_id alongside plan_interest - useAuth.signUp: forward options.data to supabase auth.signUp - waitlist-capture: runtime typeof string guard on plan_id before hoisting - kit-sync: typeof string guards on all payload.plan_id reads; namespace error message updated (removes nonexistent script reference); user.tier_changed dead-letters when tierTagNames is empty
Summary
Fixes #312. After
auth.signUp(), Supabase may not create a session if email confirmation is required. The old code silently redirected users to/dashboard(web) orDashboard(mobile) as if signup succeeded — which looked like a broken login.SignupPage.tsx): Remove theelse { navigate('/dashboard') }branch. All no-session signups now show the existingawaitingEmailpending UI. Billing-specific copy ("we'll take you to billing") is conditionalized onpaidIntent.SignupScreen.tsx): Callsupabase.auth.getSession()after signup; navigate to newSignupPendingScreenwhen no session,Dashboardwhen session is present.SignupPendingScreen.tsx: Displays email, instruction copy, and "Already confirmed? Sign in" link. Registered inAppNavigatorand typed inRootStackParamList.Test plan
npm run testinapps/web(18/18 SignupPage tests) andapps/mobile(231/231 tests)