Repository navigation
Conversation
When a prebuilt sign-up returns from a social provider for an account that already exists, the OAuth callback transfers it to a sign-in. A verification challenge interrupts the callback before that transfer runs, and the challenge card then routed with `completeSignUpFlow`, which has no transfer step, so the user landed on "Fill in missing fields". The card now hands back to the callback router once the challenge clears, the same way the sign-in card does for a sign-in that becomes a sign-up.
🦋 Changeset detectedLatest commit: 52c8eec The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (8)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. 9 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📝 WalkthroughWalkthroughThe sign-up Protect Check flow now receives OAuth callback parameters and resumes callback handling when a cleared challenge reveals a transferable account that already belongs to a user. Tests cover challenge routing, transfer to sign-in, session activation, retries, and ordinary sign-up completion. Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to The returning-user challenge flow has no identified issue requiring a fix before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. (1 skipped: 1 unsupported.)
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-biometrics
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/mosaic
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Once the transfer commits, the client no longer holds a sign-up that looks like a transfer. If a later step failed, "Try again" re-read that sign-up, fell through to `completeSignUpFlow` and did nothing. The card now remembers that it handed off, so a retry goes back to the router, which routes on the sign-in the transfer created.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 52c8eeccd6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| oauthCallbackParams && | ||
| typeof __internal_resumeAfterProtectCheck === 'function' && | ||
| (didResumeOAuthCallbackRef.current || isSignUpPendingOAuthTransfer(updatedSignUp)) |
There was a problem hiding this comment.
When the gated sign-up initially carries external_account_exists but submitProtectCheck returns a resource without that verification marker, this first-run predicate is false because didResumeOAuthCallbackRef is also still false. SignUp.fromJSON replaces verifications wholesale, so the marker is not guaranteed to survive the challenge response; the code then falls through to completeSignUpFlow and sends the returning user to fill the missing email/password fields—the original regression. Snapshot the transfer state before submitting, as SignInProtectCheck already does with startedAsOAuthTransfer, and include that snapshot in this condition.
AGENTS.md reference: AGENTS.md:L12-L12
Useful? React with 👍 / 👎.
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
This branch was successfully deployed
1 participant
Description
A returning user who picks a social provider on the prebuilt
<SignUp />is normally signed in to their existing account. The OAuth callback sees thetransferable/external_account_existsverification on the sign-up and completes it withsignIn.create({ transfer: true }).When that sign-up comes back gated by a verification challenge, the callback routes to the challenge card before it reaches the transfer. Once the challenge clears,
SignUpProtectCheckroutes withcompleteSignUpFlow, which has no transfer step. It seesemail_addressandpasswordstill missing and sends the user to "Fill in missing fields", where they are asked to create a new account.This is the mirror image of #9497, which resumed a challenged sign-in that has to become a sign-up. The sign-up card now does the same in the other direction: when the resolved sign-up is still an existing-account transfer, it hands back to the callback router through
__internal_resumeAfterProtectCheck, which skips the challenge short-circuits and runs the transfer.sso-callbackroute (buildSignUpOAuthCallbackParamsin<SignUp />,buildCombinedFlowOAuthCallbackParamsunder the combined flow'screate/), so relative sign-in step URLs resolve from the right depth.continue/protect-checkmounts are not given the params. They are reached from the Continue form rather than from a callback, and keep routing as before.@clerk/uican load against aclerk-jsthat predates__internal_resumeAfterProtectCheck, so the call is feature-detected and falls back to the previous routing.There are no
clerk-jssource changes. The additions toclerk.test.tspin the router behavior the card now relies on.To try it: on an instance that challenges every sign-up, use
<SignUp />with Google and an account that already exists. After the challenge you should be signed in to that account instead of landing on "Fill in missing fields".Fixes PROT-1201
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change