Sean/trusted device sign in - #486
Open
seanperez29 wants to merge 33 commits into
Open
Conversation
seanperez29
force-pushed
the
sean/Trusted-device-sign-in
branch
3 times, most recently
from
July 1, 2026 15:55
93212da to
d39d20a
Compare
seanperez29
marked this pull request as ready for review
July 1, 2026 15:56
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughAdds end-to-end trusted-device enrollment, biometric key storage, credential reconciliation, sign-in, revocation, availability APIs, authentication-flow routing, profile controls, localization, platform configuration, and extensive unit and integration tests. Auth-flow completion tracking now coordinates completed client responses with post-auth navigation and optional trusted-device enrollment prompts. Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swift (2)
184-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated fields between
PrepareEnrollmentParamsandAttemptEnrollmentParams.
platform,appIdentifier,name,algorithm,publicKeyJWKare identical in both structs;AttemptEnrollmentParamsonly addsclientData/signature. Consider composingAttemptEnrollmentParamsfrom a shared base to avoid drift if fields change.🤖 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 `@Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swift` around lines 184 - 234, `PrepareEnrollmentParams` and `AttemptEnrollmentParams` duplicate the same enrollment fields, so changes can drift over time. Introduce a shared base type for the common properties used by `TrustedDevice.PrepareEnrollmentParams` and `TrustedDevice.AttemptEnrollmentParams`, and have `AttemptEnrollmentParams` compose or reuse that base while keeping only `clientData` and `signature` as its extra fields. Update the initializers and `Encodable` conformance so the shared fields are defined in one place.
70-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated boilerplate across
Platform,Algorithm,Status.All three nested enums duplicate the same rawValue/Codable/unknown-case pattern. Consider a shared protocol with default
Codableconformance to reduce duplication as more string-backed enums are added.🤖 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 `@Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swift` around lines 70 - 182, The nested enums TrustedDevice.Platform, TrustedDevice.Algorithm, and TrustedDevice.Status all repeat the same string-backed rawValue plus Codable/unknown-case logic. Refactor this shared behavior into a reusable protocol or helper with default Codable implementations, then have these enums conform to it and keep only their case definitions and any enum-specific raw string mappings. Make sure the new abstraction preserves the current unknown(String) behavior and matches the existing init(rawValue:), init(from:), and encode(to:) semantics.
🤖 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 `@Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceKeyManager.swift`:
- Around line 255-259: Update
TrustedDeviceKeyManager.localAuthenticationPolicy(for:) so
.biometryOrDevicePasscode maps to .deviceOwnerAuthentication instead of
.deviceOwnerAuthenticationWithBiometrics. Keep .biometryCurrentSet and
.biometryAny on biometrics-only, and make the mapping consistent with
accessControlFlags(for:) so preflight allows passcode fallback for key creation.
In
`@Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStore.swift`:
- Around line 136-154: `deleteAllLocalCredentials(keyManager:)` is removing all
credential metadata even when some
`TrustedDeviceKeyManagerProtocol.deleteKey(localKeyId:)` calls fail, which
prevents retries for the failed keys. Update the flow so metadata is only
deleted for credentials whose key removal succeeded, and preserve the remaining
entries when any deletion throws. Use the existing `all()`, `deleteAll()`, and
`keyDeletionError` logic in `TrustedDeviceLocalCredentialStore` to identify and
delete only successful credentials, then throw after processing failures.
In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift`:
- Around line 435-437: The trusted-device sign-in path in AuthStartView starts
authentication without cancelling any in-flight automatic passkey task, unlike
the social sign-in handlers. Update the TrustedDeviceSignInButton action to
mirror the cancellation pattern used by the social buttons by cancelling the
existing passkey task before calling signInWithTrustedDevice(), and apply the
same fix in the duplicated trusted-device flow around the later related block as
well.
- Around line 180-182: The trusted-device sign-in visibility in AuthStartView is
still driven only by trustedDeviceAvailability, so stale availability can keep
the button visible after trustedDeviceFeatureIsEnabled turns false. Update the
shouldShowTrustedDeviceSignIn computed property to also require the feature flag
to be enabled, alongside the existing authState.mode and
trustedDeviceAvailability checks. Keep the logic synchronous in this property so
visibility updates immediately when the flag changes, without waiting for the
.task cleanup.
In `@Sources/ClerkKitUI/Components/Auth/AuthView.swift`:
- Around line 316-341: The trusted-device enrollment branch in AuthView should
also be gated by the native trusted-device feature flag, not just
session.status.allowsTrustedDeviceEnrollment. Update the logic around
result.shouldOfferTrustedDeviceEnrollmentPrompt and the availability check to
short-circuit before routing to TrustedDeviceEnrollmentView or appending
.trustedDeviceEnrollment when the native settings flag is off, using the
existing trusted-devices feature flag check already used by
TrustedDevices.enroll(...).
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift`:
- Around line 186-205: Clear the cached trusted-device availability when refresh
fails. In `refreshTrustedDeviceAvailability()` inside `UserProfileSecurityView`,
the `catch` path currently returns nil without updating
`trustedDeviceAvailability`, so `trustedDeviceIsEnabled` can keep using stale
state. Make the non-cancellation failure branch explicitly reset
`trustedDeviceAvailability` to nil before logging and returning nil, so
`UserProfileTrustedDeviceSection` no longer shows the old availability after a
failed refresh.
In
`@Sources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swift`:
- Around line 32-45: The toggle flow in toggleBinding (and the related
setTrustedDeviceSignInEnabled path mentioned in the comment) allows a second
toggle to queue another Task before isLoading is updated, causing overlapping
enroll/revoke operations. Set the loading state immediately in the Binding
setter before launching the async Task, then invoke
setTrustedDeviceSignInEnabled so subsequent taps are blocked while the operation
is in flight. Make the same adjustment anywhere else in
UserProfileTrustedDeviceSection that starts the trusted-device enable/disable
task.
In `@Tests/Domains/Auth/TrustedDevice/TrustedDeviceKeyManagerTests.swift`:
- Around line 104-105: The guard in TrustedDeviceKeyManagerTests around
privateKeyLookupError is pattern-matching the result of an optional cast, so it
needs to account for the KeychainError? returned by as? KeychainError. Update
the guard case to match the optional enum payload correctly in the test method
that asserts errSecNotAvailable handling, using the privateKeyLookupError(for:)
symbol and the .unexpectedStatus case with the optional suffix so the expression
type-checks.
---
Nitpick comments:
In `@Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swift`:
- Around line 184-234: `PrepareEnrollmentParams` and `AttemptEnrollmentParams`
duplicate the same enrollment fields, so changes can drift over time. Introduce
a shared base type for the common properties used by
`TrustedDevice.PrepareEnrollmentParams` and
`TrustedDevice.AttemptEnrollmentParams`, and have `AttemptEnrollmentParams`
compose or reuse that base while keeping only `clientData` and `signature` as
its extra fields. Update the initializers and `Encodable` conformance so the
shared fields are defined in one place.
- Around line 70-182: The nested enums TrustedDevice.Platform,
TrustedDevice.Algorithm, and TrustedDevice.Status all repeat the same
string-backed rawValue plus Codable/unknown-case logic. Refactor this shared
behavior into a reusable protocol or helper with default Codable
implementations, then have these enums conform to it and keep only their case
definitions and any enum-specific raw string mappings. Make sure the new
abstraction preserves the current unknown(String) behavior and matches the
existing init(rawValue:), init(from:), and encode(to:) semantics.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d22d8509-2079-4927-bdc5-5f26df4dd08d
📒 Files selected for processing (59)
Examples/Quickstart/Quickstart/Info.plistSources/ClerkKit/Core/Clerk+Installation.swiftSources/ClerkKit/Core/Clerk.swiftSources/ClerkKit/Core/TrustedDevices.swiftSources/ClerkKit/Dependencies/Dependencies.swiftSources/ClerkKit/Dependencies/DependencyContainer.swiftSources/ClerkKit/Domains/Auth/Factor.swiftSources/ClerkKit/Domains/Auth/FactorStrategy.swiftSources/ClerkKit/Domains/Auth/SignIn/SignIn+ServiceParams.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceAvailability.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceChallenge.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceKeyManager.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStore.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevicePolicy.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceService.swiftSources/ClerkKit/Domains/Auth/Verification.swiftSources/ClerkKit/Domains/Environment/AuthConfig.swiftSources/ClerkKit/Mocks/MockDependencyContainer.swiftSources/ClerkKit/Mocks/MockModels.swiftSources/ClerkKit/Mocks/MockServices/MockTrustedDeviceKeyManager.swiftSources/ClerkKit/Mocks/MockServices/MockTrustedDeviceService.swiftSources/ClerkKit/Storage/Keychain/Clerk+Keychain.swiftSources/ClerkKit/Storage/Keychain/ClerkKeychainKey.swiftSources/ClerkKitUI/Components/Auth/AuthNavigation.swiftSources/ClerkKitUI/Components/Auth/AuthStartView.swiftSources/ClerkKitUI/Components/Auth/AuthView.swiftSources/ClerkKitUI/Components/Auth/ClerkAccessibilityIdentifiers.swiftSources/ClerkKitUI/Components/Auth/SignUp/LegalConsentView.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentPrompt.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentStrings.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentView.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceSignInButton.swiftSources/ClerkKitUI/Components/Common/TrustedDeviceBiometryDisplayName.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSectionHeader.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileView.swiftSources/ClerkKitUI/Extensions/Error+Ext.swiftSources/ClerkKitUI/Extensions/TrustedDeviceAvailability+Enrollment.swiftSources/ClerkKitUI/Extensions/User+Ext.swiftSources/ClerkKitUI/Resources/Localizable.xcstringsSources/ClerkKitUI/Theme/ClerkColors.swiftTests/Core/ClerkTests.swiftTests/Domains/Auth/FactorTests.swiftTests/Domains/Auth/SignIn/SignInServiceTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceKeyManagerTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStoreTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceServiceTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDevicesTests.swiftTests/Domains/Environment/AuthConfigDecodingTests.swiftTests/Integration/IntegrationTestHelpers.swiftTests/Storage/ClerkKeychainKeyTests.swiftTests/TestSupport/TestHelpers.swiftTests/UI/AuthNavigationTests.swiftTests/UI/TrustedDeviceEnrollmentPromptTests.swiftTests/UI/TrustedDeviceSignInButtonTests.swift
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual) → reviewed against branchsean/Trusted-device-sign-ininstead of the default branch
💤 Files with no reviewable changes (1)
- Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift
Contributor
CI pausedCI does not run automatically on pull request updates. Before merging, manual CI must be run and pass for the latest commit. When this PR is ready for CI, the PR author or a repository maintainer can comment:
Or use the quick action below:
|
Collaborator
Author
|
@codex review |
![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: 3ec8d83322
ℹ️ 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".
seanperez29
force-pushed
the
sean/Trusted-device-sign-in
branch
from
July 1, 2026 21:00
1a76680 to
ac1b93d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift (1)
17-21: 📐 Maintainability & Code Quality | 🔵 TrivialAdd UI-level test coverage for the trusted-device refresh logic.
This file's user-scoped refresh key, local/remote refresh helpers, and error-clearing behavior have no corresponding unit tests in this PR —
TrustedDevicesTests.swiftonly coversClerkKitcore, not this view's logic (e.g.,trustedDeviceAvailabilityRefreshKeycomposition, cache-clearing on failure).As per path instructions, "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
Also applies to: 35-81, 161-202
🤖 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 `@Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift` around lines 17 - 21, Add UI-level tests for the trusted-device refresh flow in UserProfileSecurityView, since the PR changes to trusted-device availability state and refresh logic are currently untested. Cover the user-scoped trustedDeviceAvailabilityRefreshKey composition, the local/remote refresh helpers, and the error-clearing path when a refresh fails, using the relevant UserProfileSecurityView methods/properties so the behavior is protected even if the implementation shifts.Source: Path instructions
🤖 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 `@Tests/Domains/Auth/TrustedDevice/TrustedDevicesTests.swift`:
- Around line 533-562: The
revokeCurrentDeviceCredentialUsesUserIDWhenIdentifierHintChanged test only
covers a single local credential, so it does not catch cases where the wrong
record is revoked. Add a second credential for a different user into the
credentialStore setup in TrustedDevicesTests.swift, then assert
revokeCurrentDeviceCredential still targets the current user’s credential via
its userID and leaves the adversarial credential untouched. Use the existing
revokeCurrentDeviceCredential, makeTrustedDevices, and credentialStore helpers
to keep the test focused on the current-user lookup path.
---
Nitpick comments:
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift`:
- Around line 17-21: Add UI-level tests for the trusted-device refresh flow in
UserProfileSecurityView, since the PR changes to trusted-device availability
state and refresh logic are currently untested. Cover the user-scoped
trustedDeviceAvailabilityRefreshKey composition, the local/remote refresh
helpers, and the error-clearing path when a refresh fails, using the relevant
UserProfileSecurityView methods/properties so the behavior is protected even if
the implementation shifts.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0fa3524f-c842-4079-a295-3bd9f3e7b9bd
📒 Files selected for processing (6)
Sources/ClerkKit/Core/TrustedDevices.swiftSources/ClerkKitUI/Components/Auth/AuthView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swiftTests/Domains/Auth/TrustedDevice/TrustedDevicesTests.swift
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual) → reviewed against branchsean/Trusted-device-sign-ininstead of the default branch
🚧 Files skipped from review as they are similar to previous changes (4)
- Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swift
- Sources/ClerkKitUI/Components/Auth/AuthView.swift
- Sources/ClerkKit/Core/TrustedDevices.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Tests/Core/ClerkTests.swift (1)
638-651: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest duplicates internal key-format logic instead of reusing it.
This free function re-implements the exact same string-join logic as the private
trustedDeviceInstallationMarkerKeyoverloads inClerk+Installation.swift. If the production format changes but this copy isn't updated in lockstep, these tests would keep passing against a stale format and stop catching real regressions.Consider raising the production helper's visibility (e.g.
package) so tests call the real implementation instead of duplicating it.🤖 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 `@Tests/Core/ClerkTests.swift` around lines 638 - 651, The test helper trustedDeviceInstallationMarkerKey is duplicating the production key-format logic instead of using the real implementation from Clerk+Installation.swift. Update the tests to call the shared helper directly, and if access control blocks that, raise the production helper’s visibility (for example to package) rather than maintaining a separate copy. Keep the tests aligned with the existing trustedDeviceInstallationMarkerKey overloads so the format stays in one place.
🤖 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 `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift`:
- Around line 317-330: The trusted-device availability refresh is being
retriggered on every identifier keystroke because
`trustedDeviceAvailabilityRefreshState` changes with
`trustedDeviceIdentifierHint`. Update the `.task(id:
trustedDeviceAvailabilityRefreshState)` flow in `AuthStartView` to debounce
before calling `refreshTrustedDeviceAvailability()`, so rapid typing only
results in one fetch after input settles. Use the existing
`trustedDeviceAvailabilityRefreshState`, `trustedDeviceIdentifierHint`, and
`refreshTrustedDeviceAvailability()` symbols to locate the path, and add a test
that verifies typing does not cause repeated availability refreshes.
---
Nitpick comments:
In `@Tests/Core/ClerkTests.swift`:
- Around line 638-651: The test helper trustedDeviceInstallationMarkerKey is
duplicating the production key-format logic instead of using the real
implementation from Clerk+Installation.swift. Update the tests to call the
shared helper directly, and if access control blocks that, raise the production
helper’s visibility (for example to package) rather than maintaining a separate
copy. Keep the tests aligned with the existing
trustedDeviceInstallationMarkerKey overloads so the format stays in one place.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0c3421d-402a-4817-b1ec-78d2e8445ebd
📒 Files selected for processing (23)
Sources/ClerkKit/Core/Clerk+Installation.swiftSources/ClerkKit/Core/TrustedDevices.swiftSources/ClerkKit/Dependencies/DependencyContainer.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceKeyManager.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStore.swiftSources/ClerkKitUI/Components/Auth/AuthNavigation.swiftSources/ClerkKitUI/Components/Auth/AuthStartView.swiftSources/ClerkKitUI/Components/Auth/AuthView.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentStrings.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentView.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceSignInButton.swiftSources/ClerkKitUI/Components/Common/TrustedDeviceBiometryDisplayName.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swiftSources/ClerkKitUI/Extensions/TrustedDeviceAvailability+Enrollment.swiftTests/Core/ClerkTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceKeyManagerTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStoreTests.swiftTests/Networking/ClerkAPIClientTests.swiftTests/TestSupport/TestHelpers.swiftTests/UI/AuthNavigationTests.swiftTests/UI/TrustedDeviceEnrollmentPromptTests.swiftTests/UI/TrustedDeviceSignInButtonTests.swift
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual) → reviewed against branchsean/Trusted-device-sign-ininstead of the default branch
💤 Files with no reviewable changes (4)
- Tests/UI/AuthNavigationTests.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift
- Sources/ClerkKitUI/Components/Auth/AuthNavigation.swift
- Sources/ClerkKitUI/Components/Auth/AuthView.swift
🚧 Files skipped from review as they are similar to previous changes (10)
- Tests/UI/TrustedDeviceSignInButtonTests.swift
- Sources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentStrings.swift
- Tests/TestSupport/TestHelpers.swift
- Tests/UI/TrustedDeviceEnrollmentPromptTests.swift
- Sources/ClerkKitUI/Components/Auth/TrustedDeviceSignInButton.swift
- Sources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentView.swift
- Tests/Domains/Auth/TrustedDevice/TrustedDeviceKeyManagerTests.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStore.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceKeyManager.swift
- Sources/ClerkKit/Core/TrustedDevices.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ClerkKitUI/Components/Auth/AuthStartView.swift (1)
451-459: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering an empty
socialButtonsSectionwhen there are no social providers.
alternativeAuthMethodsSectionalways includessocialButtonsSection, even whenhasSocialProvidersisfalse. In a trusted-device-only configuration (no social providers, which is exactly the scenario this PR targets), this leaves an effectively emptyVStackas a sibling oftrustedDeviceSignInButton, adding an unwanted 16pt gap before the trailing content.🎨 Proposed fix
private var alternativeAuthMethodsSection: some View { VStack(spacing: 16) { if shouldShowTrustedDeviceSignIn { trustedDeviceSignInButton } - socialButtonsSection + if hasSocialProviders { + socialButtonsSection + } } }🤖 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 `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift` around lines 451 - 459, The alternativeAuthMethodsSection in AuthStartView always appends socialButtonsSection, which leaves an empty sibling view and extra spacing when hasSocialProviders is false. Update alternativeAuthMethodsSection to conditionally include socialButtonsSection only when social providers exist, using the existing hasSocialProviders and shouldShowTrustedDeviceSignIn checks so trustedDeviceSignInButton is the only content in trusted-device-only configurations.
🤖 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.
Outside diff comments:
In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift`:
- Around line 451-459: The alternativeAuthMethodsSection in AuthStartView always
appends socialButtonsSection, which leaves an empty sibling view and extra
spacing when hasSocialProviders is false. Update alternativeAuthMethodsSection
to conditionally include socialButtonsSection only when social providers exist,
using the existing hasSocialProviders and shouldShowTrustedDeviceSignIn checks
so trustedDeviceSignInButton is the only content in trusted-device-only
configurations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d44fd6fc-2d44-4686-ab82-d4f219ed18d0
📒 Files selected for processing (15)
Sources/ClerkKit/Core/TrustedDevices.swiftSources/ClerkKit/Domains/Environment/AuthConfig.swiftSources/ClerkKitUI/Components/Auth/AuthConfig.swiftSources/ClerkKitUI/Components/Auth/AuthStartView.swiftSources/ClerkKitUI/Components/Auth/AuthState.swiftSources/ClerkKitUI/Components/Auth/AuthView.swiftSources/ClerkKitUI/Components/Auth/LastUsedAuth.swiftSources/ClerkKitUI/Components/Auth/TrustedDeviceEnrollmentPrompt.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileView.swiftTests/Domains/Auth/TrustedDevice/TrustedDevicesTests.swiftTests/Domains/Environment/AuthConfigDecodingTests.swiftTests/UI/AuthStartViewTrustedDeviceTests.swiftTests/UI/AuthStateConfigurationTests.swiftTests/UI/LastUsedAuthTests.swiftTests/UI/TrustedDeviceEnrollmentPromptTests.swift
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual) → reviewed against branchsean/Trusted-device-sign-ininstead of the default branch
🚧 Files skipped from review as they are similar to previous changes (5)
- Tests/Domains/Environment/AuthConfigDecodingTests.swift
- Sources/ClerkKitUI/Components/Auth/AuthView.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift
- Tests/Domains/Auth/TrustedDevice/TrustedDevicesTests.swift
- Sources/ClerkKit/Core/TrustedDevices.swift
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/Core/ClerkTests.swift (1)
258-300: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider asserting legacy marker cleanup post-migration.
The test verifies the new app-scoped marker key gets set, but doesn't assert whether the legacy (non-scoped) marker key is removed/reset after migration. Confirming this would make the migration behavior test more complete.
🤖 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 `@Tests/Core/ClerkTests.swift` around lines 258 - 300, In configureMigratesLegacyTrustedDeviceInstallationMarkerWithoutDeletingCredentials, the migration test only checks that the new app-scoped installation marker is set; update it to also verify the legacy non-scoped marker is cleared or removed after Clerk.performConfiguration runs. Use Clerk.trustedDeviceInstallationMarkerKey(for:) and Clerk.trustedDeviceInstallationMarkerKey(for:appIdentifier:) to assert both the migrated target key and the old key’s post-migration state.
🤖 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.
Nitpick comments:
In `@Tests/Core/ClerkTests.swift`:
- Around line 258-300: In
configureMigratesLegacyTrustedDeviceInstallationMarkerWithoutDeletingCredentials,
the migration test only checks that the new app-scoped installation marker is
set; update it to also verify the legacy non-scoped marker is cleared or removed
after Clerk.performConfiguration runs. Use
Clerk.trustedDeviceInstallationMarkerKey(for:) and
Clerk.trustedDeviceInstallationMarkerKey(for:appIdentifier:) to assert both the
migrated target key and the old key’s post-migration state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 954399fd-2bce-4524-b1f8-9f6029609661
📒 Files selected for processing (3)
Sources/ClerkKit/Core/Clerk+Installation.swiftSources/ClerkKitUI/Components/Auth/AuthStartView.swiftTests/Core/ClerkTests.swift
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual) → reviewed against branchsean/Trusted-device-sign-ininstead of the default branch
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/ClerkKit/Core/Clerk+Installation.swift
- Sources/ClerkKitUI/Components/Auth/AuthStartView.swift
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
seanperez29
force-pushed
the
sean/Trusted-device-sign-in
branch
from
July 20, 2026 02:44
db742c4 to
1635b0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/ClerkKit/Core/TrustedDevices.swift (1)
559-582: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHardcoded backend error codes duplicated as magic strings.
isMissingTrustedDeviceCredentialandtrustedDeviceValidationUnavailableReasonhardcode"form_resource_not_found","trusted_device_not_registered","native_api_disabled","feature_not_enabled"inline. If these drift from the actual clerk_go error codes, failures silently fall through to a generic error path instead of the intended recovery/messaging.Please confirm these exact codes/param names against the
clerk/clerk_gotrusted-device endpoints, and consider centralizing them as named constants shared with otherClerkAPIError.codechecks in the codebase.🤖 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 `@Sources/ClerkKit/Core/TrustedDevices.swift` around lines 559 - 582, Verify the trusted-device error codes and the “trusted_device_id” parameter name against the clerk_go endpoint definitions, then replace the inline literals in Error.isMissingTrustedDeviceCredential and Error.trustedDeviceValidationUnavailableReason with centralized named constants shared by related ClerkAPIError.code checks. Preserve the existing recovery and availability-reason mapping after confirming the exact backend values.
🤖 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 `@Sources/ClerkKit/Core/TrustedDevices.swift`:
- Line 147: Update removeOtherLocalCredentialsForCurrentApp and its call from
the trusted-device enrollment flow to accept and use the current userID when
selecting credentials to remove. Ensure cleanup deletes only other credentials
belonging to that same user, preserving biometric credentials for different
users on the shared installation.
In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift`:
- Around line 728-740: In the trusted-device refresh flow, capture the result of
clerk.trustedDevices.validateLocalCredentialIfPossible() and immediately check
task cancellation before updating trustedDeviceAvailability. Return without
entering the switch when the task is cancelled, preserving the existing result
handling for active tasks.
---
Nitpick comments:
In `@Sources/ClerkKit/Core/TrustedDevices.swift`:
- Around line 559-582: Verify the trusted-device error codes and the
“trusted_device_id” parameter name against the clerk_go endpoint definitions,
then replace the inline literals in Error.isMissingTrustedDeviceCredential and
Error.trustedDeviceValidationUnavailableReason with centralized named constants
shared by related ClerkAPIError.code checks. Preserve the existing recovery and
availability-reason mapping after confirming the exact backend values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d9aa0ee-e350-4119-a236-a8e55c457239
📒 Files selected for processing (74)
Examples/Quickstart/Quickstart/Info.plistSources/ClerkKit/Core/Auth.swiftSources/ClerkKit/Core/AuthFlowRegistration.swiftSources/ClerkKit/Core/Clerk+Installation.swiftSources/ClerkKit/Core/Clerk.swiftSources/ClerkKit/Core/TrustedDevices.swiftSources/ClerkKit/Dependencies/Dependencies.swiftSources/ClerkKit/Dependencies/DependencyContainer.swiftSources/ClerkKit/Domains/Auth/Factor.swiftSources/ClerkKit/Domains/Auth/FactorStrategy.swiftSources/ClerkKit/Domains/Auth/SignIn/SignIn+ServiceParams.swiftSources/ClerkKit/Domains/Auth/TransferFlowResult.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceAvailability.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceChallenge.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceKeyManager.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStore.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevicePolicy.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceService.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceValidation.swiftSources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceValidationResult.swiftSources/ClerkKit/Domains/Auth/Verification.swiftSources/ClerkKit/Domains/Environment/AuthConfig.swiftSources/ClerkKit/Mocks/MockDependencyContainer.swiftSources/ClerkKit/Mocks/MockModels.swiftSources/ClerkKit/Mocks/MockServices/MockTrustedDeviceKeyManager.swiftSources/ClerkKit/Mocks/MockServices/MockTrustedDeviceService.swiftSources/ClerkKit/Networking/Middleware/ClerkAuthEventEmitterResponseMiddleware.swiftSources/ClerkKit/Networking/Middleware/ClerkAuthResponseDecoder.swiftSources/ClerkKit/Networking/Middleware/ClerkClientSyncResponseMiddleware.swiftSources/ClerkKit/Storage/Keychain/Clerk+Keychain.swiftSources/ClerkKit/Storage/Keychain/ClerkKeychainKey.swiftSources/ClerkKitUI/Components/Auth/AuthNavigation.swiftSources/ClerkKitUI/Components/Auth/AuthStartView.swiftSources/ClerkKitUI/Components/Auth/AuthView.swiftSources/ClerkKitUI/Components/Auth/ClerkAccessibilityIdentifiers.swiftSources/ClerkKitUI/Components/Auth/LastUsedAuth.swiftSources/ClerkKitUI/Components/Auth/SignUp/LegalConsentView.swiftSources/ClerkKitUI/Components/Auth/TrustedDevice/TrustedDeviceEnrollmentPrompt.swiftSources/ClerkKitUI/Components/Auth/TrustedDevice/TrustedDeviceEnrollmentStrings.swiftSources/ClerkKitUI/Components/Auth/TrustedDevice/TrustedDeviceEnrollmentView.swiftSources/ClerkKitUI/Components/Auth/TrustedDevice/TrustedDeviceSignInButton.swiftSources/ClerkKitUI/Components/Common/TrustedDeviceBiometryDisplayName.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSectionHeader.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileView.swiftSources/ClerkKitUI/Extensions/Error+Ext.swiftSources/ClerkKitUI/Extensions/TrustedDeviceAvailability+Enrollment.swiftSources/ClerkKitUI/Extensions/User+Ext.swiftSources/ClerkKitUI/Resources/Localizable.xcstringsSources/ClerkKitUI/Theme/ClerkColors.swiftTests/Core/ClerkReconfigureTests.swiftTests/Core/ClerkTests.swiftTests/Domains/Auth/AuthTests.swiftTests/Domains/Auth/FactorTests.swiftTests/Domains/Auth/SignIn/SignInServiceTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceKeyManagerTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStoreTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceServiceTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDeviceTests.swiftTests/Domains/Auth/TrustedDevice/TrustedDevicesTests.swiftTests/Domains/Environment/AuthConfigDecodingTests.swiftTests/Integration/IntegrationTestHelpers.swiftTests/Middleware/ClerkClientSyncResponseMiddlewareTests.swiftTests/Networking/ClerkAPIClientTests.swiftTests/Storage/ClerkKeychainKeyTests.swiftTests/TestSupport/TestHelpers.swiftTests/UI/AuthNavigationTests.swiftTests/UI/AuthStartViewTrustedDeviceTests.swiftTests/UI/LastUsedAuthTests.swiftTests/UI/TrustedDeviceEnrollmentPromptTests.swiftTests/UI/TrustedDeviceSignInButtonTests.swift
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual) → reviewed against branchsean/Trusted-device-sign-ininstead of the default branchclerk/dashboard(manual) → reviewed against open PR#9624sean/Trusted-device-sign-ininstead of the default branchclerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (38)
- Tests/Domains/Environment/AuthConfigDecodingTests.swift
- Sources/ClerkKit/Domains/Auth/Verification.swift
- Sources/ClerkKitUI/Extensions/TrustedDeviceAvailability+Enrollment.swift
- Sources/ClerkKit/Domains/Auth/Factor.swift
- Tests/UI/TrustedDeviceSignInButtonTests.swift
- Sources/ClerkKitUI/Extensions/User+Ext.swift
- Sources/ClerkKit/Dependencies/Dependencies.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevicePolicy.swift
- Tests/Storage/ClerkKeychainKeyTests.swift
- Tests/Integration/IntegrationTestHelpers.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceAvailability.swift
- Tests/Domains/Auth/SignIn/SignInServiceTests.swift
- Sources/ClerkKitUI/Components/Auth/SignUp/LegalConsentView.swift
- Tests/TestSupport/TestHelpers.swift
- Tests/Domains/Auth/FactorTests.swift
- Examples/Quickstart/Quickstart/Info.plist
- Sources/ClerkKit/Storage/Keychain/ClerkKeychainKey.swift
- Sources/ClerkKitUI/Theme/ClerkColors.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceChallenge.swift
- Tests/Domains/Auth/TrustedDevice/TrustedDeviceTests.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceService.swift
- Tests/UI/TrustedDeviceEnrollmentPromptTests.swift
- Sources/ClerkKitUI/Extensions/Error+Ext.swift
- Sources/ClerkKit/Storage/Keychain/Clerk+Keychain.swift
- Sources/ClerkKitUI/Components/Common/TrustedDeviceBiometryDisplayName.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceLocalCredentialStore.swift
- Sources/ClerkKit/Dependencies/DependencyContainer.swift
- Sources/ClerkKit/Mocks/MockModels.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDevice.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileTrustedDeviceSection.swift
- Sources/ClerkKit/Domains/Environment/AuthConfig.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift
- Sources/ClerkKit/Mocks/MockDependencyContainer.swift
- Sources/ClerkKitUI/Resources/Localizable.xcstrings
- Sources/ClerkKitUI/Components/Auth/LastUsedAuth.swift
- Sources/ClerkKit/Mocks/MockServices/MockTrustedDeviceKeyManager.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift
- Sources/ClerkKit/Domains/Auth/TrustedDevice/TrustedDeviceKeyManager.swift
This was referenced Jul 20, 2026
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add trusted device biometric sign-in enrollment and authentication flow
TrustedDevice,TrustedDeviceChallenge,TrustedDeviceAvailability, and related model types represent server credentials and local state.TrustedDeviceKeyManagerto create, sign with, and delete ES256 Secure Enclave keys protected by LocalAuthentication policies (Face ID, Touch ID, device passcode).TrustedDeviceLocalCredentialStoreto persist per-device credentials in keychain andTrustedDeviceServicefor CRUD and validation against backend endpoints.Clerk.trustedDevicesandAuth.signInWithTrustedDevice()as the primary API surface;AuthConfig.NativeSettingscarries feature flags decoded from the environment.AuthView,AuthStartView) conditionally presentsTrustedDeviceEnrollmentViewafter sign-in/sign-up and shows a biometric sign-in button on the start screen when a local credential is available.UserProfileSecurityViewgains aUserProfileTrustedDeviceSectiontoggle to enroll or revoke the current device.reconcileTrustedDeviceCredentialsForCurrentInstallation()deletes stale local credentials once and persists an installation marker to avoid repeating the operation.clearAllKeychainItemswill leave thetrustedDeviceCredentialskey intact.Macroscope summarized 1026c9d.