Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Sources/Rownd/Rownd.swift
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,28 @@ public class Rownd: NSObject {
inst.displayHub(.signIn, jsFnOptions: jsFnOptions)
}

@MainActor
internal static func requestSignInForcedConversion(_ signInOptions: RowndSignInOptions?) {
inst.bottomSheetController.isUserDismissalDisabled = true
requestSignIn(signInOptions)
}

@MainActor
internal static func releaseForcedConversionLock() {
let wasLocked = inst.bottomSheetController.isUserDismissalDisabled
inst.bottomSheetController.isUserDismissalDisabled = false
guard wasLocked else { return }
// The post-auth auto-close may have fired and been blocked while the lock was held
// (Hub schedules hide() 1.5s after `.authentication` but `authLevel` propagates from a
// separate UserData.fetch — the timer can lose the race). Retry the close now.
(inst.bottomSheetController.controller as? HubViewProtocol)?.hide()
}
Comment on lines +188 to +196
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Auto-close can be skipped 🐞 Bug ☼ Reliability

Rownd.releaseForcedConversionLock() retries closing the Hub by calling hide() on
bottomSheetController.controller, but that controller is assigned asynchronously via Task in
displayHub/displayViewControllerOnTop. If authLevel transitions out of .instant before controller is
assigned, the cast to HubViewProtocol fails and the Hub may stay open after conversion.
Agent Prompt
## Issue description
`releaseForcedConversionLock()` attempts a single immediate close by calling `hide()` on `inst.bottomSheetController.controller`, but `controller` is only assigned later inside `Task { @MainActor in ... }` in `displayHub()`/`displayViewControllerOnTop()`. If forced-conversion unlock happens before that assignment, the optional cast to `HubViewProtocol` yields `nil` and the auto-close retry becomes a no-op.

## Issue Context
This PR’s goal includes reliably auto-closing after `authLevel` transitions out of `.instant`. The current implementation depends on `bottomSheetController.controller` being set at the time of unlock, which is not guaranteed due to the async Task-based presentation.

## Fix Focus Areas
- Sources/Rownd/Rownd.swift[181-196]
- Sources/Rownd/Rownd.swift[416-455]

### Implementation guidance
Pick one of these robust patterns:
1. **Two-phase hide:** keep the current immediate hide, and also schedule a retry on the next MainActor turn (e.g., `Task { @MainActor in await Task.yield(); (controller as? HubViewProtocol)?.hide() }`).
2. **Pending-hide flag:** store a `pendingForcedConversionHide` flag when unlock happens and `controller` is nil; then consume it in `displayHub()` after `bottomSheetController.controller` is assigned.
3. **Dismiss via presenter state:** if `inst.bottomSheetController.presentingViewController != nil`, dismiss the presented sheet controller directly instead of relying on `controller` casting.

Whichever approach you choose, ensure the Hub closes even when unlock occurs before `displayHub()` has assigned/presented the controller.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


@MainActor
internal static var _bottomSheetIsLocked: Bool {
inst.bottomSheetController.isUserDismissalDisabled
}

@MainActor
public static func connectAuthenticator(
with: RowndConnectSignInHint, completion: (() -> Void)? = nil
Expand Down
33 changes: 33 additions & 0 deletions Sources/Rownd/Views/BottomSheetViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ class BottomSheetViewController: UIViewController {
var latestTargetHeight: CGFloat = 0.9
var isKeyboardOpen = false

// When true, swipe-to-dismiss and tap-outside-to-dismiss are disabled for this presentation,
// and the Hub's `can_touch_background_to_dismiss` message cannot re-enable them. Programmatic
// dismissal (e.g. after a successful sign-in) is unaffected. Reset on viewDidDisappear.
// The didSet applies the change to a live sheetController, so toggling the lock after
// presentation still takes effect (and unlock restores the Hub's most recent preference).
var isUserDismissalDisabled: Bool = false {
didSet {
guard isUserDismissalDisabled != oldValue, let sheetController = sheetController else { return }
if isUserDismissalDisabled {
sheetController.setCanTouchDimmingBackgroundToDismiss(false)
Comment on lines +27 to +36
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): The lock currently also blocks programmatic dismissals, which contradicts the comment and may be too strict.

The docstring says programmatic dismissal is unaffected, but hideBottomSheet now returns early when the lock is active:

def hideBottomSheet(_ completion: (() -> Void)? = nil) {
    if isUserDismissalDisabled {
        logger.debug("Ignoring hideBottomSheet: forced-conversion lock is active")
        return
    }
    sheetController?.dismiss(completion)
}

So all calls into hideBottomSheet are blocked while locked, and HubViewController.hide() already checks isUserDismissalDisabled, making this double enforcement. If the goal is to block only user-driven dismissals, consider restricting the lock to user-interaction paths (e.g. canTouchDimmingBackgroundToDismiss / gesture logic) and let hideBottomSheet always dismiss, or provide a separate API that bypasses the lock for trusted, SDK-initiated closes.

} else {
sheetController.setCanTouchDimmingBackgroundToDismiss(hubRequestedCanTouchToDismiss)
}
}
}

// Tracks the most recent Hub-requested dismissibility state so that releasing the lock
// restores whatever the Hub last asked for instead of unconditionally re-enabling.
private var hubRequestedCanTouchToDismiss: Bool = true

override func viewWillAppear(_ animated: Bool) {
super.viewWillAppear(animated)
guard let controller = controller else {
Expand Down Expand Up @@ -54,6 +74,9 @@ class BottomSheetViewController: UIViewController {

sheetController = presentAsBottomSheet(controller, theme: theme, behavior: behavior)

if isUserDismissalDisabled {
sheetController?.setCanTouchDimmingBackgroundToDismiss(false)
}
}

override func viewDidLoad() {
Expand All @@ -69,6 +92,8 @@ class BottomSheetViewController: UIViewController {
override func viewDidDisappear(_ animated: Bool) {
super.viewDidDisappear(animated)
self.controller = nil
self.isUserDismissalDisabled = false
self.hubRequestedCanTouchToDismiss = true
}

func updateBottomSheetHeight(_ number: CGFloat) {
Expand All @@ -77,6 +102,10 @@ class BottomSheetViewController: UIViewController {
}

public func hideBottomSheet(_ completion: (() -> Void)? = nil) {
if isUserDismissalDisabled {
logger.debug("Ignoring hideBottomSheet: forced-conversion lock is active")
return
}
sheetController?.dismiss(completion)
}

Expand All @@ -99,6 +128,10 @@ class BottomSheetViewController: UIViewController {
}

func canTouchDimmingBackgroundToDismiss(_ enable: Bool) {
hubRequestedCanTouchToDismiss = enable
if isUserDismissalDisabled && enable {
return
}
if let sheetController = sheetController {
sheetController.setCanTouchDimmingBackgroundToDismiss(enable)
}
Expand Down
9 changes: 7 additions & 2 deletions Sources/Rownd/Views/HubViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,16 @@ public class HubViewController: UIViewController, HubViewProtocol, BottomSheetHo
self.dismiss(animated: true)
return
}


if bottomSheetController.isUserDismissalDisabled {
logger.debug("Ignoring HubViewController.hide(): forced-conversion lock is active")
return
}

if (isBottomSheetDismissing) {
return
}

isBottomSheetDismissing = true
bottomSheetController.hideBottomSheet({
self.dismiss(animated: true)
Expand Down
39 changes: 21 additions & 18 deletions Sources/Rownd/framework/InstantUsers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Combine
class InstantUsers {
private let context: Context
private var cancellables = Set<AnyCancellable>()
private var hasTriggeredConversion: Bool = false

init(
context: Context
Expand All @@ -36,24 +37,26 @@ class InstantUsers {
.removeDuplicates(
by: ==
)
.first {
isAuthenticated,
authLevel in
isAuthenticated && authLevel == .instant
}
.sink {
isAuthenticated,
authLevel in

var signInOptions = RowndSignInOptions()
signInOptions.title = "Add a sign-in method"
signInOptions.subtitle = "To make sure you can always access your account, please add a sign-in method."
signInOptions.intent = .signUp
Rownd
.requestSignIn(signInOptions)

subscriber
.unsubscribe()
.sink { [weak self] isAuthenticated, authLevel in
guard let self = self, isAuthenticated else { return }

if !self.hasTriggeredConversion && authLevel == .instant {
self.hasTriggeredConversion = true

var signInOptions = RowndSignInOptions()
signInOptions.title = "Add a sign-in method"
signInOptions.subtitle = "To make sure you can always access your account, please add a sign-in method."
signInOptions.intent = .signUp
Rownd.requestSignInForcedConversion(signInOptions)
return
}

// User has converted to a non-instant auth level (verified, unverified, guest).
// Release the lock so the Hub's post-success auto-close can proceed.
if self.hasTriggeredConversion && authLevel != .instant && authLevel != .unknown {
Rownd.releaseForcedConversionLock()
subscriber.unsubscribe()
}
}.store(
in: &self.cancellables
)
Expand Down
123 changes: 123 additions & 0 deletions Tests/RowndTests/InstantUsersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class InstantUsersTests: XCTestCase {

override func tearDown() {
Rownd.config = originalConfig
// Reset the singleton lock so it does not leak between tests.
Rownd.releaseForcedConversionLock()
super.tearDown()
}

Expand Down Expand Up @@ -197,4 +199,125 @@ class InstantUsersTests: XCTestCase {

_ = instantUsers
}

/// Verifies that the forced-conversion lock is engaged on the singleton bottom
/// sheet when the conversion subscription fires for an instant user.
func testLockIsEngagedWhenConversionTriggers() async throws {
let store = createStore()
_ = Context(store)

Rownd.config.forceInstantUserConversion = true
XCTAssertFalse(Rownd._bottomSheetIsLocked, "Pre-condition: lock should start cleared")

let instantUsers = InstantUsers(context: Context.currentContext)
instantUsers.tmpForceInstantUserConversionIfRequested()

try await Task.sleep(nanoseconds: 50_000_000)

store.dispatch(SetAuthState(payload: AuthState(
accessToken: generateJwt(expires: Date(timeIntervalSinceNow: 3600).timeIntervalSince1970),
refreshToken: generateJwt(expires: Date(timeIntervalSinceNow: 36000).timeIntervalSince1970)
)))
store.dispatch(SetClockSync(clockSyncState: .synced))
store.dispatch(SetUserState(payload: UserState(
data: ["user_id": "test_instant_user"],
authLevel: .instant
)))

try await waitUntil(timeout: 2.0) { Rownd._bottomSheetIsLocked }
XCTAssertTrue(Rownd._bottomSheetIsLocked, "Lock should engage when authLevel transitions to .instant")

_ = instantUsers
}

/// Verifies that the lock releases once the user transitions from `.instant`
/// to a non-instant identifier auth level (e.g. `.verified`).
func testLockReleasesAfterVerifiedConversion() async throws {
let store = createStore()
_ = Context(store)

Rownd.config.forceInstantUserConversion = true

let instantUsers = InstantUsers(context: Context.currentContext)
instantUsers.tmpForceInstantUserConversionIfRequested()

try await Task.sleep(nanoseconds: 50_000_000)

store.dispatch(SetAuthState(payload: AuthState(
accessToken: generateJwt(expires: Date(timeIntervalSinceNow: 3600).timeIntervalSince1970),
refreshToken: generateJwt(expires: Date(timeIntervalSinceNow: 36000).timeIntervalSince1970)
)))
store.dispatch(SetClockSync(clockSyncState: .synced))
store.dispatch(SetUserState(payload: UserState(
data: ["user_id": "test_instant_user"],
authLevel: .instant
)))

try await waitUntil(timeout: 2.0) { Rownd._bottomSheetIsLocked }

// Simulate successful conversion: user-data fetch returns with verified level.
store.dispatch(SetUserState(payload: UserState(
data: ["user_id": "test_verified_user", "email": "user@example.com"],
authLevel: .verified
)))

try await waitUntil(timeout: 2.0) { !Rownd._bottomSheetIsLocked }
XCTAssertFalse(Rownd._bottomSheetIsLocked, "Lock should release once authLevel becomes .verified")

_ = instantUsers
}

/// Verifies that the `hasTriggeredConversion` gate prevents the conversion
/// flow from re-triggering after a successful conversion + lock release.
/// (The customer-confirmed behavior is once-per-session.)
func testConversionDoesNotRetriggerAfterRelease() async throws {
let store = createStore()
_ = Context(store)

Rownd.config.forceInstantUserConversion = true

let instantUsers = InstantUsers(context: Context.currentContext)
instantUsers.tmpForceInstantUserConversionIfRequested()

try await Task.sleep(nanoseconds: 50_000_000)

// First .instant → lock should engage.
store.dispatch(SetAuthState(payload: AuthState(
accessToken: generateJwt(expires: Date(timeIntervalSinceNow: 3600).timeIntervalSince1970),
refreshToken: generateJwt(expires: Date(timeIntervalSinceNow: 36000).timeIntervalSince1970)
)))
store.dispatch(SetClockSync(clockSyncState: .synced))
store.dispatch(SetUserState(payload: UserState(
data: ["user_id": "test_instant_user"],
authLevel: .instant
)))
try await waitUntil(timeout: 2.0) { Rownd._bottomSheetIsLocked }

// Convert and release.
store.dispatch(SetUserState(payload: UserState(
data: ["user_id": "test_user", "email": "user@example.com"],
authLevel: .verified
)))
try await waitUntil(timeout: 2.0) { !Rownd._bottomSheetIsLocked }

// Drop back to .instant — should NOT re-engage the lock (once-per-session).
store.dispatch(SetUserState(payload: UserState(
data: ["user_id": "test_user"],
authLevel: .instant
)))
try await Task.sleep(nanoseconds: 200_000_000)
XCTAssertFalse(Rownd._bottomSheetIsLocked, "Conversion must not re-trigger after a successful release")

_ = instantUsers
}

// MARK: - helpers

private func waitUntil(timeout: TimeInterval, condition: @escaping () -> Bool) async throws {
let deadline = Date().addingTimeInterval(timeout)
while Date() < deadline {
if condition() { return }
try await Task.sleep(nanoseconds: 25_000_000)
}
}
}