Skip to content

feat(api): add typed Strategy and migrate strategy comparisons - #1025

Merged
swolfand merged 3 commits into
mainfrom
mike/typed-strategies
Oct 6, 2026
Merged

swolfand merged 3 commits into
mainfrom
mike/typed-strategies

Conversation

@swolfand

@swolfand swolfand commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Architecture review finding #11: strategies are plain strings, compared as strings dozens of times across api and ui, with the constants duplicated in four places (Constants.Strategy, Constants.Passkey.PASSKEY_STRATEGY_VALUE, the SignIn.PrepareSecondFactorParams companion, and the UI's internal StrategyKeys). This PR adds the typed Strategy the review asked for and moves the SDK's own comparisons onto it.

This is PR 1 of 3 for #11:

  1. This PR: typed Strategy.
  2. One policy for unknown server enums (stacked on this one).
  3. A sealed outcome for OAuthResult (stacked on 2).

Scope

  • New public com.clerk.api.auth.types.Strategy, a sealed class:
    • one object per known strategy (PhoneCode, EmailCode, EmailLink, Totp, BackupCode, Password, Passkey, ResetPasswordEmailCode, ResetPasswordPhoneCode, Ticket, Transfer, EnterpriseSso, Saml, TrustedDevice, GoogleOneTap)
    • OAuth (oauth_*, exposes provider) and OAuthToken (oauth_token_*)
    • Unknown(raw) for anything else
    • Strategy.from(raw) never fails. Equality and toString() use the wire value. StrategySerializer round-trips the raw string.
  • New typed accessors next to the existing String fields, which are unchanged: Factor.strategyType, Verification.strategyType, Client.lastAuthenticationStrategyType, and an overload SignIn.identifyingFirstFactor(Strategy).
  • One source for the constants:
    • Deprecated: Constants.Strategy.*, Constants.Passkey.PASSKEY_STRATEGY_VALUE, and SignIn.PrepareSecondFactorParams.{PHONE_CODE, EMAIL_CODE, PASSKEY}. They have no ReplaceWith, because Strategy.X.value is not a compile-time constant and an automatic swap would break const val and annotation uses. The message names the replacement and says this.
    • Removed (internal): UI StrategyKeys, PasskeyHelper.passkeyStrategy, and the SignUp.CreateParams.{Ticket,GoogleOneTap} companion STRATEGY consts.
  • api and ui strategy comparisons (==, when, in, sort orders, LastUsedAuth) now use strategyType. Request parameter defaults use Strategy.X.value.
  • Rebased onto the auth-facade stack (fix(api): stop the two auth API layers drifting on enterprise sign-up, sendCode and error events #1026, refactor(api): route both auth API layers through one implementation per operation #1034, feat(api): fill the Clerk.auth gaps the prebuilt UI needs #1035, refactor(ui): move the prebuilt UI onto the Clerk.auth facade #1038, refactor(api): deprecate the old SignIn/SignUp auth entry points #1039) and fix(ui): keep the email code screen when email link is supported #1043. Main's function bodies are kept as they are, with only the strategy typing applied on top:
    • AuthFlows.kt (createSignIn/createSignUp): transfer, ticket and Google One Tap strategies come from Strategy.
    • SignInExtensions.kt: firstFactorFor takes List<Strategy> and noMatchingFactor takes a Strategy. The identifier-masking logic is unchanged.
    • SignIn.kt: prepareFirstFactorImpl, prepareSecondFactor and sendMfaPhoneCode/sendMfaEmailCode compare strategyType instead of the now-deprecated PrepareSecondFactorParams constants.
    • Clerk.kt identifyingFirstFactor and the UI SignInPrepareHandler.
    • SignInFactorCodeViewModel keeps main's removal of prefersEmailLinkOverEmailCode/shouldPreferEmailLink from fix(ui): keep the email code screen when email link is supported #1043.
    • The SDK's own main code produces no deprecation warnings from these deprecations.
  • Tests: StrategyTest covers known, OAuth, OAuth-token and unknown mapping, serializer round-trip, decoding a Factor with an unrecognized strategy, and the typed accessors.

Blast Radius

  • Consumers: additive only. No field types or signatures changed, so binary and source compatibility hold. Code that uses the old constants gets deprecation warnings that name the typed replacement.
  • Behavior: the same, with two deliberate exceptions:
    • alternativeFirstFactors() used contains("oauth"). It now excludes the oauth_* and oauth_token_* families explicitly. No real strategy is affected.
    • SignInFactorCodeViewModel.prepare now has an explicit Strategy.Totp -> Unit branch (TOTP has nothing to prepare) plus else -> Unit, because when over a sealed type must be exhaustive. Both are no-ops, matching the old fall-through.
  • Expected conflicts:
    • add renovate.json #6 (explicit API) touches the same public declarations; Strategy will need public modifiers.
    • The auth-facade stack has merged and this PR is rebased onto it.

Testing

After the rebase onto main (7ffc6d3):

  • ./gradlew spotlessApply :source:api:detekt :source:ui:detekt :source:api:testDebugUnitTest :source:ui:testDebugUnitTest :samples:quickstart:compileDebugKotlin :samples:custom-flows:compileDebugKotlin :e2e:compileDebugKotlin: all pass.

Before the rebase:

  • ./gradlew :source:api:spotlessApply :source:ui:spotlessApply :source:api:detekt :source:ui:detekt :source:api:testDebugUnitTest :source:ui:testDebugUnitTest :source:ui:verifyPaparazziDebug --continue
    • spotless, detekt and api tests: pass.
    • ui tests: 21 failures, all Paparazzi snapshot mismatches. The same 21 tests fail on origin/main (a2efe4f) in this environment. I ran verifyPaparazziDebug there and diffed the failing-test lists: they are identical. Several goldens are missing from the repo, and the rest are sub-0.1% font-rendering diffs. No snapshot changed because of this PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added typed authentication strategies for sign-in, sign-up, and verification, including OAuth and unrecognized strategies.
    • Added typed strategy information for authentication factors, verifications, and the last authentication method.
    • Added the ability to identify a sign-in’s first factor by typed strategy.
  • Improvements
    • Updated authentication flows and factor selection to use typed strategies while preserving existing behavior.
    • Marked legacy strategy constants as deprecated and provided guidance on typed alternatives.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: clerk/clerk-android/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 197409e9-c70c-422f-8645-20f75769342c
📥 Commits

Reviewing files that changed from the base of the PR and between 9279665 and 851854a.

📒 Files selected for processing (5)
  • source/api/src/main/kotlin/com/clerk/api/Clerk.kt
  • source/api/src/main/kotlin/com/clerk/api/restorecredentials/RestoreCredentials.kt
  • source/api/src/test/java/com/clerk/api/passkeys/PasskeyHelperTest.kt
  • source/ui/src/test/java/com/clerk/ui/signin/SignInFactorOneViewTest.kt
  • source/ui/src/test/java/com/clerk/ui/signin/code/SignInFactorCodeViewModelTest.kt

Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour.


📝 Walkthrough

Walkthrough

The API adds a typed Strategy model and typed accessors for authentication strategy values. API and UI authentication code now uses typed strategies for defaults, factor matching, verification, and routing. Legacy strategy constants remain available with deprecation guidance.

Changes

Typed authentication strategies

Layer / File(s) Summary
Strategy model and API accessors
source/api/src/main/kotlin/com/clerk/api/auth/types/Strategy.kt, source/api/src/main/kotlin/com/clerk/api/network/model/{client,factor,verification}/*, source/api/src/main/kotlin/com/clerk/api/network/model/factor/FactorComparators.kt, source/api/src/test/java/com/clerk/api/auth/types/StrategyTest.kt
Adds typed strategy values for known, OAuth, OAuth-token, and unknown strategies. API models expose typed strategy properties, factor comparators use typed values, and tests cover parsing, serialization, and accessors.
API defaults and integrations
source/api/src/main/kotlin/com/clerk/api/Constants.kt, source/api/src/main/kotlin/com/clerk/api/{auth,biometriccredential,emailaddress,magiclink,network,passkeys,phonenumber,restorecredentials,session,signup,sso}/*, source/api/src/test/java/com/clerk/api/{biometriccredential,passkeys,signup}/*
API strategy defaults and authentication integrations use typed strategy values. Legacy constants retain their values and gain deprecation guidance. Related API test fixtures use typed strategy values.
Sign-in factor flows
source/api/src/main/kotlin/com/clerk/api/Clerk.kt, source/api/src/main/kotlin/com/clerk/api/signin/*
Sign-in factor preparation, selection, verification, and reset-password helpers compare typed strategies. identifyingFirstFactor gains a Strategy overload, and its string overload delegates through Strategy.from.
UI strategy routing
source/ui/src/main/java/com/clerk/ui/{auth,core,signin,signup,util}/*, source/ui/src/test/java/com/clerk/ui/{auth,signin}/*
UI authentication routing, factor dispatch, code preparation, and display helpers use typed strategies. The internal StrategyKeys object is removed, and related test fixtures use typed values.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Feature

Suggested reviewers: mikepitre

Merge Risk: ⚪ Minimal · up to 85185

The reviewed changes preserve the checked strategy and authentication-flow behavior; no introduced issue requiring remediation was established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 149 functions across 53 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a typed Strategy and migrating strategy comparisons.
Description check ✅ Passed The description explains the motivation, scope, compatibility, behavior changes, and testing. It does not use the template’s exact headings or include an explicit “What to focus on” section or screens…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

Add com.clerk.api.auth.types.Strategy, a sealed type with the known
strategies, the OAuth/OAuthToken families and Unknown(raw), serialized as
the raw wire string. Expose typed accessors next to the existing String
fields (Factor.strategyType, Verification.strategyType,
Client.lastAuthenticationStrategyType, SignIn.identifyingFirstFactor(Strategy)).

Collapse the four strategy constant copies onto Strategy: the public
Constants.Strategy, Constants.Passkey.PASSKEY_STRATEGY_VALUE and
SignIn.PrepareSecondFactorParams constants are deprecated (without
ReplaceWith, since Strategy.X.value is not a compile-time constant), the
internal copies (UI StrategyKeys, PasskeyHelper.passkeyStrategy,
SignUp.CreateParams companions) are removed, and api/ui strategy
comparisons use the typed form, including the Clerk.auth facade paths
(AuthFlows, firstFactorFor/noMatchingFactor, prepareSecondFactor and the
sendMfa* helpers).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant