Skip to content

feat: Introduce reset password session task#322

Draft
octoper wants to merge 3 commits intomike/clerk-v1from
vaggelis/session-tasks
Draft

feat: Introduce reset password session task#322
octoper wants to merge 3 commits intomike/clerk-v1from
vaggelis/session-tasks

Conversation

@octoper
Copy link
Member

@octoper octoper commented Feb 3, 2026

Summary by CodeRabbit

  • New Features
    • Added multi-factor authentication (MFA) setup flows with support for SMS codes and authenticator apps
    • Added backup codes generation and display for MFA
    • Added password reset capability for sessions requiring action
    • Added session task routing and management for pending actions
    • Expanded localization strings for MFA setup, verification, and authentication workflows

@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

Walkthrough

This pull request adds session task and multi-factor authentication (MFA) setup infrastructure to ClerkKit and ClerkKitUI. The changes introduce a typed task key system for pending sessions, new MFA configuration views (phone and TOTP-based), a session task reset password flow, navigation routing for these flows, client-server synchronization in the auth event middleware, and corresponding localization strings.

Changes

Cohort / File(s) Summary
Session Task Infrastructure
Sources/ClerkKit/Domains/Auth/Session/Session.swift
Added TaskKey enum with cases for resetPassword, setupMfa, and unknown; exposed currentTask and currentTaskKey computed properties on Session.
Auth Event Synchronization
Sources/ClerkKit/Networking/Middleware/ClerkAuthEventEmitterResponseMiddleware.swift
Refactored sign-in/sign-up response decoding to synchronize Clerk.shared.client with server response before emitting completion events.
Auth Navigation & Routing
Sources/ClerkKitUI/Components/Auth/AuthNavigation.swift, Sources/ClerkKitUI/Components/Auth/AuthView.swift
Added dismissAuthFlow property to AuthNavigation; extended AuthView.Destination enum with sessionTask and setupMfa\* cases, each mapped to corresponding view components.
Session Task Views
Sources/ClerkKitUI/Components/Auth/SessionTasks/SessionTaskView.swift, Sources/ClerkKitUI/Components/Auth/SessionTasks/SessionTaskResetPasswordView.swift
Created SessionTaskView to route pending session tasks and SessionTaskResetPasswordView to handle password reset flow with validation and API integration.
MFA Phone Setup Views
Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaStartView.swift, Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaPhoneSelectionView.swift, Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaAddPhoneView.swift, Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaVerifyPhoneView.swift
Added views for MFA setup entry point, phone number selection, new phone addition with async validation, and OTP-based phone verification with cooldown-managed code resending.
MFA TOTP Setup Views
Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaTotpSecretView.swift, Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaVerifyTotpView.swift
Created views for displaying TOTP secret and URI with copy-to-clipboard functionality, and verifying TOTP code during setup with error handling.
MFA Completion Views
Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaBackupCodesView.swift, Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaSuccessView.swift
Added views for displaying backup codes with copy functionality and confirming successful MFA setup with flow completion action.
Localization Strings
Sources/ClerkKitUI/Resources/Localizable.xcstrings
Added 15 new localization keys for MFA setup, password reset, and session task flows, including instructions and confirmation messages.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 A hop through tasks and codes so new,
Two-factor flows to see us through,
Phone numbers verified with care,
TOTP secrets floating in the air,
Sessions reset, auth anew—
What grand security we pursue! 🔐

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'feat: Introduce reset password session task' is partially related to the changeset. While password reset is a key feature, the PR encompasses much broader functionality including MFA setup (phone and TOTP), session task routing, phone number management, and backup codes—not just password reset. Consider a more comprehensive title such as 'feat: Add session tasks and MFA setup flows' or 'feat: Implement password reset and MFA setup session tasks' to accurately reflect all major changes in this PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vaggelis/session-tasks

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

@octoper
Copy link
Member Author

octoper commented Feb 3, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/ClerkKit/Networking/Middleware/ClerkAuthEventEmitterResponseMiddleware.swift (1)

33-37: ⚠️ Potential issue | 🟡 Minor

Inconsistent pattern: session removal block doesn't synchronize client state.

The SignIn and SignUp blocks now update Clerk.shared.client before emitting events, but this block directly accesses .response during decoding, making it impossible to access clientResponse.client. For consistency and to ensure event handlers receive synchronized state, consider applying the same pattern here.

🔧 Proposed fix to align with the new pattern
-      if let session = try? JSONDecoder.clerkDecoder.decode(ClientResponse<Session>.self, from: data).response,
-         session.status == .removed
+      if let clientResponse = try? JSONDecoder.clerkDecoder.decode(ClientResponse<Session>.self, from: data),
+         clientResponse.response.status == .removed
       {
-        Clerk.shared.auth.send(.signedOut(session: session))
+        // Update client first to ensure state is synchronized
+        if let client = clientResponse.client {
+          Clerk.shared.client = client
+        }
+        Clerk.shared.auth.send(.signedOut(session: clientResponse.response))
       }
🤖 Fix all issues with AI agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SessionTaskResetPasswordView.swift`:
- Around line 155-158: The preview for SessionTaskResetPasswordView is missing
required environment objects (Clerk and AuthNavigation) and will crash; update
the `#Preview` block to supply mock or preview instances for these environments
(e.g., provide a mock Clerk object and a mock AuthNavigation) alongside the
existing environment(\.clerkTheme, .clerk) so that SessionTaskResetPasswordView
has all required environment dependencies at preview time.

In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaAddPhoneView.swift`:
- Around line 57-104: The addPhoneNumber() routine allows empty submissions and
silently ignores a missing user; update it to validate input and surface errors:
in addPhoneNumber() check that phoneNumber.trimmed is non-empty and if empty set
self.error to a user-facing error (e.g., a localized NSError or custom Error
with a message like "Please enter a phone number") and return; also replace the
current guard let user else { return } with a guard that sets self.error to a
clear "User not available" error before returning so missing-user states are
shown; keep the existing do/catch behavior for server errors and append the
navigation path only after successful creation.

In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaPhoneSelectionView.swift`:
- Around line 103-110: In SetupMfaPhoneSelectionView update the back Button
action in the .toolbar so it guards against an empty navigation.path before
calling removeLast(); e.g., check navigation.path.isEmpty (or use guard/if) and
only call navigation.path.removeLast() when there is at least one element to
avoid a crash.
- Around line 22-28: The current computed property session uses
clerk.client?.sessions.first { $0.status == .pending && $0.currentTask != nil }
which can pick the wrong pending session; change it to explicitly select the
pending session whose currentTask matches the MFA task (e.g., check
currentTask?.type == .mfa or currentTask?.identifier == "mfa" depending on your
task model) or, better, accept a Session instance as an initializer/input to
this view and use that instead; update the session var (and any code referencing
clerk.client?.sessions) to filter by the MFA-specific predicate or rely on an
injected session to ensure the correct session is used.

In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaSuccessView.swift`:
- Around line 90-93: The preview for SetupMfaSuccessView is missing required
environment objects; update the preview block that instantiates
SetupMfaSuccessView(mfaType: .authenticatorApp) to inject the same environment
values used elsewhere by calling .environment(\.clerk, <a Clerk instance>) and
.environmentObject(<an AuthNavigation instance>) (or the equivalent
.environmentObject initializer used in the project) so SetupMfaSuccessView has
the Clerk and AuthNavigation dependencies available during preview.

In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaVerifyTotpView.swift`:
- Around line 64-121: verifyCode can be invoked concurrently (from OTP
auto-submit and the AsyncButton), causing duplicate requests and duplicate
navigation.path.append pushes; add an early gate in verifyCode (e.g., guard
!isVerifying else { return }) and set isVerifying = true immediately before
doing any async work (keep the existing defer { isVerifying = false }), and
ensure navigation.path.append is executed on the main actor (use await
MainActor.run { navigation.path.append(...) } or DispatchQueue.main.async) so
only one navigation push occurs; reference verifyCode, isVerifying, and
navigation.path.append when making the changes.

In `@Sources/ClerkKitUI/Resources/Localizable.xcstrings`:
- Around line 429-432: The new localization key "A text message containing a
verification code will be sent to this phone number" was added with an empty
map; update the Localizable.xcstrings entry by populating the translation values
for all target locales (add the appropriate translated strings under that key)
or, if this copy is intentionally not translatable, mark the entry as
non‑translatable/intentional (e.g., add a clear comment or a consistent
notranslate marker) so it doesn't silently fall back to English.
🧹 Nitpick comments (12)
Sources/ClerkKit/Domains/Auth/Session/Session.swift (1)

222-231: Confirm task ordering before using tasks?.first.

Line 225 assumes the server orders tasks by priority. If ordering isn’t guaranteed or multiple tasks can exist, consider selecting by explicit priority or by key to avoid nondeterministic routing.

Sources/ClerkKitUI/Resources/Localizable.xcstrings (1)

2119-2121: Avoid adding a punctuation-only key variant.

There’s already a localized key with the same text plus a trailing period (Line 2122). Reuse the existing key or trim punctuation in UI to avoid duplicated translations and missing localizations here.

Sources/ClerkKitUI/Components/Auth/SessionTasks/SessionTaskResetPasswordView.swift (1)

146-148: Consider logging or handling refreshClient failure.

Using try? silently swallows errors from refreshClient(). If the password update succeeds but the client refresh fails, the user may see stale session state. Consider at minimum logging the error for debugging purposes.

Proposed improvement
       // Refresh the client to get the updated session
       // The view will automatically update when the session changes
-      _ = try? await clerk.refreshClient()
+      do {
+        _ = try await clerk.refreshClient()
+      } catch {
+        ClerkLogger.error("Failed to refresh client after password reset", error: error)
+      }
     } catch {
       fieldError = error
     }
Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaSuccessView.swift (1)

20-41: Consider extracting MfaType enum to a shared location.

The MfaType enum is duplicated in SetupMfaBackupCodesView with different computed properties. Consider extracting to a shared file and consolidating the title, message, and instructions properties into a single enum definition to improve maintainability.

Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaStartView.swift (1)

87-98: Silent failure when user is nil may cause confusion.

If user is nil, createTotp() returns silently without feedback to the user. Consider showing an error or logging this unexpected state.

Proposed improvement
   func createTotp() async {
-    guard let user else { return }
+    guard let user else {
+      self.error = ClerkClientError(message: "User not found.")
+      return
+    }

     do {
       let totp = try await user.createTOTP()
       navigation.path.append(AuthView.Destination.setupMfaTotp(totp))
     } catch {
       self.error = error
     }
   }
Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaTotpSecretView.swift (2)

18-18: Unused error state variable.

The error state is declared but never assigned. The clerkErrorPresenting($error) modifier on line 101 has no effect. Either remove the dead code or implement error handling for potential future errors.

Remove unused error handling
-  `@State` private var error: Error?

And remove from the view body:

     .background(theme.colors.background)
-    .clerkErrorPresenting($error)
     .navigationBarBackButtonHidden()

131-138: Unreachable macOS branch and code duplication.

The #elseif os(macOS) branch is unreachable since the entire file is gated by #if os(iOS). Additionally, this function duplicates copyToClipboard in SetupMfaBackupCodesView.swift (lines 84-91). Consider extracting to a shared utility.

Simplified implementation
   func copyToClipboard(_ text: String) {
-    `#if` os(iOS)
     UIPasteboard.general.string = text
-    `#elseif` os(macOS)
-    NSPasteboard.general.clearContents()
-    NSPasteboard.general.setString(text, forType: .string)
-    `#endif`
   }

Or extract to a shared utility for reuse across views.

Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaBackupCodesView.swift (3)

14-14: Unused clerk environment dependency.

The clerk environment is injected but never used in this view. Consider removing if not needed.

-  `@Environment`(Clerk.self) private var clerk

21-33: Duplicate MfaType enum.

This enum duplicates the one in SetupMfaSuccessView.swift with different computed properties. Consider consolidating into a shared type with all necessary properties.


84-91: Unreachable macOS branch.

Same issue as in SetupMfaTotpSecretView.swift - the #elseif os(macOS) branch is unreachable since the file is gated by #if os(iOS).

Simplified implementation
 private func copyToClipboard(_ text: String) {
-  `#if` os(iOS)
   UIPasteboard.general.string = text
-  `#elseif` os(macOS)
-  NSPasteboard.general.clearContents()
-  NSPasteboard.general.setString(text, forType: .string)
-  `#endif`
 }
Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaVerifyPhoneView.swift (2)

21-21: Unused isVerifying state variable.

The isVerifying state is set in verifyCode() but never read in the UI. The AsyncButton already handles its own loading state via isRunning. Consider removing this unused state.

Remove unused state
-  `@State` private var isVerifying = false

And in verifyCode():

   func verifyCode() async {
     error = nil
     fieldState = .default
     isFocused = false
-    isVerifying = true
-    defer { isVerifying = false }

130-137: Initial code sent without checking cooldown.

sendInitialCode() is called on view appearance without checking if a code was recently sent. If the user navigates away and back, this could send duplicate codes. Consider checking the cooldown first.

Add cooldown check
   func sendInitialCode() async {
+    // Skip if code was recently sent
+    guard codeLimiter.remainingCooldown(for: phoneNumber.phoneNumber) == 0 else {
+      return
+    }
+
     do {
       _ = try await phoneNumber.sendCode()
       codeLimiter.recordCodeSent(for: phoneNumber.phoneNumber)
     } catch {
       self.error = error
     }
   }

Comment on lines +155 to +158
#Preview {
SessionTaskResetPasswordView()
.environment(\.clerkTheme, .clerk)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Preview missing required environment objects.

The preview is missing Clerk and AuthNavigation environment objects, which will cause it to crash. Consider adding mock environments for preview to work correctly.

🤖 Prompt for AI Agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SessionTaskResetPasswordView.swift`
around lines 155 - 158, The preview for SessionTaskResetPasswordView is missing
required environment objects (Clerk and AuthNavigation) and will crash; update
the `#Preview` block to supply mock or preview instances for these environments
(e.g., provide a mock Clerk object and a mock AuthNavigation) alongside the
existing environment(\.clerkTheme, .clerk) so that SessionTaskResetPasswordView
has all required environment dependencies at preview time.

Comment on lines +57 to +104
AsyncButton {
await addPhoneNumber()
} label: { isRunning in
HStack(spacing: 4) {
Text("Continue", bundle: .module)
Image("icon-triangle-right", bundle: .module)
.foregroundStyle(theme.colors.primaryForeground)
.opacity(0.6)
}
.frame(maxWidth: .infinity)
.overlayProgressView(isActive: isRunning) {
SpinnerView(color: theme.colors.primaryForeground)
}
}
.buttonStyle(.primary())
.padding(.bottom, 32)

SecuredByClerkView()
}
.padding(16)
}
.background(theme.colors.background)
.clerkErrorPresenting($error)
.navigationBarBackButtonHidden()
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
navigation.path.removeLast()
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
}
}
}

func addPhoneNumber() async {
error = nil
isFocused = false

guard let user else { return }

do {
let newPhone = try await user.createPhoneNumber(phoneNumber)
navigation.path.append(AuthView.Destination.setupMfaPhoneVerify(newPhone))
} catch {
self.error = error
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Block empty submissions and surface missing-user errors.

Right now a blank phone number (or a missing user) silently does nothing or relies on server errors. That leaves the user stuck in this view with no actionable feedback.

🔧 Proposed fix
         AsyncButton {
           await addPhoneNumber()
         } label: { isRunning in
@@
         }
         .buttonStyle(.primary())
+        .disabled(phoneNumber.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty)
         .padding(.bottom, 32)
@@
   func addPhoneNumber() async {
     error = nil
     isFocused = false
-
-    guard let user else { return }
+    let trimmed = phoneNumber.trimmingCharacters(in: .whitespacesAndNewlines)
+    guard !trimmed.isEmpty else {
+      error = ClerkClientError(message: "Please enter a phone number.")
+      isFocused = true
+      return
+    }
+
+    guard let user else {
+      error = ClerkClientError(message: "User not found.")
+      return
+    }
 
     do {
-      let newPhone = try await user.createPhoneNumber(phoneNumber)
+      let newPhone = try await user.createPhoneNumber(trimmed)
       navigation.path.append(AuthView.Destination.setupMfaPhoneVerify(newPhone))
     } catch {
       self.error = error
     }
   }
🤖 Prompt for AI Agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaAddPhoneView.swift`
around lines 57 - 104, The addPhoneNumber() routine allows empty submissions and
silently ignores a missing user; update it to validate input and surface errors:
in addPhoneNumber() check that phoneNumber.trimmed is non-empty and if empty set
self.error to a user-facing error (e.g., a localized NSError or custom Error
with a message like "Please enter a phone number") and return; also replace the
current guard let user else { return } with a guard that sets self.error to a
clear "User not available" error before returning so missing-user states are
shown; keep the existing do/catch behavior for server errors and append the
navigation path only after successful creation.

Comment on lines +22 to +28
var session: Session? {
clerk.client?.sessions.first { $0.status == .pending && $0.currentTask != nil }
}

var user: User? {
session?.user
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid selecting the wrong pending session.

Line 23 picks the first pending session with any task, which can misroute if multiple pending sessions exist. Consider filtering for the MFA task explicitly (or pass the session into this view).

Suggested change
 var session: Session? {
-  clerk.client?.sessions.first { $0.status == .pending && $0.currentTask != nil }
+  clerk.client?.sessions.first { $0.status == .pending && $0.currentTaskKey == .setupMfa }
 }
🤖 Prompt for AI Agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaPhoneSelectionView.swift`
around lines 22 - 28, The current computed property session uses
clerk.client?.sessions.first { $0.status == .pending && $0.currentTask != nil }
which can pick the wrong pending session; change it to explicitly select the
pending session whose currentTask matches the MFA task (e.g., check
currentTask?.type == .mfa or currentTask?.identifier == "mfa" depending on your
task model) or, better, accept a Session instance as an initializer/input to
this view and use that instead; update the session var (and any code referencing
clerk.client?.sessions) to filter by the MFA-specific predicate or rely on an
injected session to ensure the correct session is used.

Comment on lines +103 to +110
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
navigation.path.removeLast()
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard against empty navigation path before removeLast().

Line 106 will crash if the path is empty. Add a guard to keep this safe.

Suggested change
 Button {
-  navigation.path.removeLast()
+  if !navigation.path.isEmpty {
+    navigation.path.removeLast()
+  }
 } label: {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
navigation.path.removeLast()
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
if !navigation.path.isEmpty {
navigation.path.removeLast()
}
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
🤖 Prompt for AI Agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaPhoneSelectionView.swift`
around lines 103 - 110, In SetupMfaPhoneSelectionView update the back Button
action in the .toolbar so it guards against an empty navigation.path before
calling removeLast(); e.g., check navigation.path.isEmpty (or use guard/if) and
only call navigation.path.removeLast() when there is at least one element to
avoid a crash.

Comment on lines +90 to +93
#Preview {
SetupMfaSuccessView(mfaType: .authenticatorApp)
.environment(\.clerkTheme, .clerk)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Preview missing required environment objects.

Similar to other previews, this is missing Clerk and AuthNavigation environment objects.

🤖 Prompt for AI Agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaSuccessView.swift`
around lines 90 - 93, The preview for SetupMfaSuccessView is missing required
environment objects; update the preview block that instantiates
SetupMfaSuccessView(mfaType: .authenticatorApp) to inject the same environment
values used elsewhere by calling .environment(\.clerk, <a Clerk instance>) and
.environmentObject(<an AuthNavigation instance>) (or the equivalent
.environmentObject initializer used in the project) so SetupMfaSuccessView has
the Clerk and AuthNavigation dependencies available during preview.

Comment on lines +64 to +121
AsyncButton {
await verifyCode()
} label: { isRunning in
HStack(spacing: 4) {
Text("Continue", bundle: .module)
Image("icon-triangle-right", bundle: .module)
.foregroundStyle(theme.colors.primaryForeground)
.opacity(0.6)
}
.frame(maxWidth: .infinity)
.overlayProgressView(isActive: isRunning) {
SpinnerView(color: theme.colors.primaryForeground)
}
}
.buttonStyle(.primary())
.disabled(code.count != 6)
.padding(.bottom, 32)

SecuredByClerkView()
}
.padding(16)
}
.background(theme.colors.background)
.clerkErrorPresenting($error)
.navigationBarBackButtonHidden()
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
navigation.path.removeLast()
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
}
}
}

func verifyCode() async {
error = nil
fieldState = .default
isFocused = false
isVerifying = true
defer { isVerifying = false }

guard let user else { return }

do {
let verifiedTotp = try await user.verifyTOTP(code: code)
if let backupCodes = verifiedTotp.backupCodes {
navigation.path.append(AuthView.Destination.setupMfaTotpBackupCodes(backupCodes))
} else {
navigation.path.append(AuthView.Destination.setupMfaTotpSuccess)
}
} catch {
self.error = error
fieldState = .error
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prevent concurrent verification and duplicate navigation pushes.

verifyCode() can fire from both the OTP auto-submit and the button tap. Without gating, you can end up with multiple in-flight requests and duplicate path appends.

🔧 Proposed fix
         AsyncButton {
           await verifyCode()
         } label: { isRunning in
@@
         }
         .buttonStyle(.primary())
-        .disabled(code.count != 6)
+        .disabled(code.count != 6 || isVerifying)
         .padding(.bottom, 32)
@@
   func verifyCode() async {
+    guard !isVerifying else { return }
     error = nil
     fieldState = .default
     isFocused = false
     isVerifying = true
     defer { isVerifying = false }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
AsyncButton {
await verifyCode()
} label: { isRunning in
HStack(spacing: 4) {
Text("Continue", bundle: .module)
Image("icon-triangle-right", bundle: .module)
.foregroundStyle(theme.colors.primaryForeground)
.opacity(0.6)
}
.frame(maxWidth: .infinity)
.overlayProgressView(isActive: isRunning) {
SpinnerView(color: theme.colors.primaryForeground)
}
}
.buttonStyle(.primary())
.disabled(code.count != 6)
.padding(.bottom, 32)
SecuredByClerkView()
}
.padding(16)
}
.background(theme.colors.background)
.clerkErrorPresenting($error)
.navigationBarBackButtonHidden()
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
navigation.path.removeLast()
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
}
}
}
func verifyCode() async {
error = nil
fieldState = .default
isFocused = false
isVerifying = true
defer { isVerifying = false }
guard let user else { return }
do {
let verifiedTotp = try await user.verifyTOTP(code: code)
if let backupCodes = verifiedTotp.backupCodes {
navigation.path.append(AuthView.Destination.setupMfaTotpBackupCodes(backupCodes))
} else {
navigation.path.append(AuthView.Destination.setupMfaTotpSuccess)
}
} catch {
self.error = error
fieldState = .error
}
}
AsyncButton {
await verifyCode()
} label: { isRunning in
HStack(spacing: 4) {
Text("Continue", bundle: .module)
Image("icon-triangle-right", bundle: .module)
.foregroundStyle(theme.colors.primaryForeground)
.opacity(0.6)
}
.frame(maxWidth: .infinity)
.overlayProgressView(isActive: isRunning) {
SpinnerView(color: theme.colors.primaryForeground)
}
}
.buttonStyle(.primary())
.disabled(code.count != 6 || isVerifying)
.padding(.bottom, 32)
SecuredByClerkView()
}
.padding(16)
}
.background(theme.colors.background)
.clerkErrorPresenting($error)
.navigationBarBackButtonHidden()
.toolbar {
ToolbarItem(placement: .topBarLeading) {
Button {
navigation.path.removeLast()
} label: {
Image("icon-caret-left", bundle: .module)
.foregroundStyle(theme.colors.foreground)
}
}
}
}
func verifyCode() async {
guard !isVerifying else { return }
error = nil
fieldState = .default
isFocused = false
isVerifying = true
defer { isVerifying = false }
guard let user else { return }
do {
let verifiedTotp = try await user.verifyTOTP(code: code)
if let backupCodes = verifiedTotp.backupCodes {
navigation.path.append(AuthView.Destination.setupMfaTotpBackupCodes(backupCodes))
} else {
navigation.path.append(AuthView.Destination.setupMfaTotpSuccess)
}
} catch {
self.error = error
fieldState = .error
}
}
🤖 Prompt for AI Agents
In
`@Sources/ClerkKitUI/Components/Auth/SessionTasks/SetupMfa/SetupMfaVerifyTotpView.swift`
around lines 64 - 121, verifyCode can be invoked concurrently (from OTP
auto-submit and the AsyncButton), causing duplicate requests and duplicate
navigation.path.append pushes; add an early gate in verifyCode (e.g., guard
!isVerifying else { return }) and set isVerifying = true immediately before
doing any async work (keep the existing defer { isVerifying = false }), and
ensure navigation.path.append is executed on the main actor (use await
MainActor.run { navigation.path.append(...) } or DispatchQueue.main.async) so
only one navigation push occurs; reference verifyCode, isVerifying, and
navigation.path.append when making the changes.

Comment on lines +429 to 432
},
"A text message containing a verification code will be sent to this phone number" : {

},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add translations for the new SMS verification copy.

Line 430 adds a new key with an empty localization map, which will likely fall back to English in non‑English locales. Please add translations (or mark it intentionally non‑translatable if that’s the goal).

🤖 Prompt for AI Agents
In `@Sources/ClerkKitUI/Resources/Localizable.xcstrings` around lines 429 - 432,
The new localization key "A text message containing a verification code will be
sent to this phone number" was added with an empty map; update the
Localizable.xcstrings entry by populating the translation values for all target
locales (add the appropriate translated strings under that key) or, if this copy
is intentionally not translatable, mark the entry as
non‑translatable/intentional (e.g., add a clear comment or a consistent
notranslate marker) so it doesn't silently fall back to English.

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