Add token refresh coordinator - #4087
Merged
Merged
Conversation
bbirman
commented
Jun 26, 2026
Generated by 🚫 Danger |
bbirman
commented
Jun 26, 2026
Clang Static Analysis Issues
Generated by 🚫 Danger |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #4087 +/- ##
==========================================
- Coverage 70.79% 68.47% -2.33%
==========================================
Files 246 247 +1
Lines 21541 21614 +73
==========================================
- Hits 15250 14800 -450
- Misses 6291 6814 +523
🚀 New features to boost your workflow:
|
|
||||||||||||||||||
|
||||||||||||||||
wmathurin
reviewed
Jun 26, 2026
wmathurin
reviewed
Jun 26, 2026
wmathurin
reviewed
Jun 26, 2026
wmathurin
reviewed
Jun 26, 2026
wmathurin
reviewed
Jun 26, 2026
wmathurin
reviewed
Jun 26, 2026
wmathurin
reviewed
Jun 26, 2026
|
||||||||||||||
Member
Author
|
Updated based on feedback and after some more deliberating, moved the completion blocks back to the main thread |
sfdctaka
reviewed
Jul 9, 2026
sfdctaka
reviewed
Jul 9, 2026
wmathurin
approved these changes
Jul 9, 2026
JohnsonEricAtSalesforce
added a commit
to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS
that referenced
this pull request
Jul 16, 2026
…baseline Two test-only resolutions plus a baseline decision, all triaged against the pre-migration oracle (unmigrated ObjC at merge-base 6ed0ab4). - SFUserAccountPhotoTests.testPhotoWithoutCompletionBlock: the migration changed the assertion from the ObjC original's XCTAssertNotNil(user.photo) to XCTAssertTrue over a reference-equality poll (user.photo == testPhoto). The `photo` getter re-decodes from disk into a NEW UIImage when _photo is nil (byte-faithful to ObjC SFUserAccount.m:170-185), so ref-equality never converges and the poll times out. Restored the ObjC assertion semantics (wait for the async setPhoto to settle, then assert final state). Both photo tests pass. Test-only; production unchanged. - SalesforceOAuthUnitTests/testCredentialsCoding: already green (resolved by the cluster forcedotcom#1 init?(coder:) decode fix, deae6b0); confirmed by run. No change. - SFSDKAuthUtilTests.testOpenIDToken: baselined as pre-existing/old-refresh-flow, NOT a migration regression. A 3-way oracle comparison proved it: the same intermittent setUp auth-refresh hang (listener status stuck 'waiting' -> 30s timeout, callback never fires) reproduces in the UNMIGRATED ObjC at our merge-base 6ed0ab4, while current dev -- which carries upstream's new token refresh coordinator (PR forcedotcom#4087 / 8f597c9) ~155 commits ahead -- runs 9/9 green. So it is an old-refresh-flow defect upstream already fixed, independent of the ObjC->Swift migration. Likely refresh-token rotation. Will be superseded when the refresh-coordinator work is pulled via the port queue. Baseline entries added with rationale (anti-laundering: not migration-caused).
JohnsonEricAtSalesforce
added a commit
to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS
that referenced
this pull request
Jul 17, 2026
…0.2h) Five live-org test classes (RestClientPublisherTests, RestClientTest, SalesforceRestAPITests, SFSDKAuthUtilTests, MobileSync SyncManagerTestCase) run TestSetupUtils.synchronousAuthRefresh() in class func setUp(). The pre-token-refresh-coordinator OAuth flow hangs in the simulator (the refresh callback never fires, 30s timeout, returnStatus stays 'waiting'), and the old fatal assert(returnStatus == didLoad) then trapped the test host before any test ran. xcodebuild restarts, re-traps, exceeds max-restart-count, and aborts the ENTIRE run -- silently masking every class alphabetically after the first live-org class. That masking is what hid the 3 migration regressions found in the 2026-07-17 oracle revalidation. A fresh, independently-verified-valid refresh token does NOT fix the hang: the defect is the old refresh flow itself (fixed upstream by the token refresh coordinator, 997c4e0 / PR forcedotcom#4087 / 8f597c9), not token staleness. So the only local remedy is to degrade the abort into a clean per-class skip. Fix (test-harness only): - TestSetupUtils: record `authRefreshDidSucceed` instead of asserting; log a warning when the live refresh doesn't complete. - The 5 live classes: XCTSkipUnless(authRefreshDidSucceed) in setUpWithError so they skip cleanly instead of trapping the host. - SalesforceRestAPITests.tearDown: guard on authRefreshDidSucceed -- tearDown runs even after a skip, and its cleanup() sent a live REST request with no session, tripping a separate assert (SFRestAPI.swift:262) and restart-looping. This is a DELIBERATE divergence from the merge-base oracle. It is intended to conflict with the eventual token-refresh-coordinator port as a detectable "revisit me" marker; see the comment block in TestSetupUtils.swift. Verified: the full SalesforceSDKCore suite now runs to the alphabetical end (WebViewStateManagerTests executes) with only 2 self-recovering restarts and no run-ending abort -- 481 passed / 95 skipped / 4 distinct failing / 0 aborts. Removing the mask surfaced one deterministic failure, SFNetworkTests.testSessionSharing, which is pending merge-base-oracle triage. Docs: .claude/test-baseline.md (Live-auth abort hardening section; also tightened the two SmartStore baseline entries) + tracker row P0.2h.
JohnsonEricAtSalesforce
added a commit
to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS
that referenced
this pull request
Jul 20, 2026
…cedotcom/dev b5d37d8 - Add forcedotcom remote; target b5d37d8 = forcedotcom/dev HEAD, marker bac0171 ancestor. - Re-seed .claude/upstream-sync-backlog.md with all 49 first-parent units in strict upstream order (21 libs-prod / 15 test-proj / 13 non-libs / 9 flaky-stabilize); archive drained-12 pass. - Re-derive .claude/test-baseline-ids.txt at HEAD 5a9d307: SDKCore 497p/3f, SmartStore 176p/1f; drop stale live-gated entries (testRedirect/testOpenIDToken now SKIP) + testGetGlobalStoreNames. - Record live-org-skip-ledger REVALIDATION 2026-07-19 (deferrals re-checked; forcedotcom#4087 = unit 44). Analysis only; no product code, no marker move.
JohnsonEricAtSalesforce
added a commit
to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS
that referenced
this pull request
Jul 20, 2026
… 18/49) Test-only (live-gated class). Bump all 4 publisher-test timeouts (5/5/10/10 -> 60) and route the shared evaluateResults sink through .receive(on: DispatchQueue.main) for deterministic main-thread delivery; inline generateRecordName. Preserved our migration deltas: the live-gate setUpWithError XCTSkipUnless(authRefreshDidSucceed) and the RestClient.sharedInstance/CompositeRequestBuilder.addRequest API names. LIVE-gated class: SDKCore TEST BUILD ✓ (0 new warnings) is the gate; tests SKIP at runtime until the token-refresh coordinator lands (unit 44, forcedotcom#4087). The known pre-coordinator auth-refresh hang is in class func setUp() (before the instance skip); oracle-identical, not a regression. Baseline unchanged.
JohnsonEricAtSalesforce
added a commit
to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS
that referenced
this pull request
Jul 20, 2026
… (unit 44) Semantic re-implementation of forcedotcom/dev 6e09678 (merge of bbirman/refreshCoordinator) onto the ObjC→Swift migration branch. WHAT: introduces SFSDKTokenRefreshCoordinator — a process-wide singleton that coalesces concurrent token-refresh requests per credential (keyed by credentials.identifier) so at most one refresh is in-flight at a time. This prevents the double-spend race with single-use (rotating) refresh tokens, where concurrent refreshes would invalidate each other's tokens. Callbacks are delivered on the main queue; background-task protection wraps the refresh. MIGRATION NOTES (no new ObjC; compiled path is Swift): - NEW SFSDKTokenRefreshCoordinator.swift (upstream added .h/.m ObjC) — @objc singleton, wired into pbxproj (framework Sources + Headers-free). NEW SFSDKTokenRefreshCoordinatorTests.swift (upstream .m, +602) ported to Swift and wired to the test target. - SFRestAPI.swift / SFIdentityCoordinator.swift / SFUserAccountManager.swift / UserAccountManager.swift (async refresh) / SFSDKOAuth2.swift / SFSDKTestRequestListener.swift: the compiled Swift twins carry the behavior change (route through the coordinator; SFRestAPI: sessionRefreshInProgress + pendingRequestsBeingProcessed + oauthSessionRefresher collapse to a single refreshCycleActive flag; cleanup now delivers "User logged out" and cancels in-flight tasks; OAuth2 completion no longer double-hops main queue; TestRequestListener spins the run loop instead of a semaphore to avoid a main-thread deadlock now that the coordinator delivers on main). - WebSocketClient.swift: TokenRefreshCoordinator actor renamed WebSocketReconnectCoordinator (it only gates reconnection; token dedup now lives in SFSDKTokenRefreshCoordinator). - Public-API deprecation of SFOAuthSessionRefresher (14.0→15.0): expressed on the Swift members consumers call (@available deprecated), with non-deprecated internal seams (init(internalCredentials:) / refreshSessionInternal) that the coordinator, tests, and mock use so the SDK's own paths stay warning-free — mirrors the unit-39 forceAdvancedAuthenticationInternal precedent. Upstream's ObjC SFSDK_DEPRECATED lives on tombstoned headers, so it has no compiled home. - De-referenced .m/.h mirrors (SFRestAPI.m, SFIdentityCoordinator.m, SFSDKOAuth2.m, SFSDKTestRequestListener.m, SFOAuthSessionRefresher.m/.h and the two test .m) ref-synced to the upstream post-image on top of the migrated pre-image (keeping the migration's @import/-Swift.h deltas); tombstone headers skipped; SFUserAccountManager.m had nothing to sync (refreshCredentials lives in the .swift twin). ESCALATION (flag for human PR review): OAuth/token-refresh control flow + public-API deprecation (SFOAuthSessionRefresher). This is the live-auth unblocker for Phase 2. Gate: SDKCore/SmartStore/MobileSync build-for-testing all GREEN, 0 new warnings; 22 targeted tests pass (11 coordinator + 7 data-task-race + 4 refresher). Live-org auth-util end-to-end tests remain XCTSkip-gated (Phase 2). Upstream: 6e09678 (PR forcedotcom#4087) · marker unit 44 · __U44__
JohnsonEricAtSalesforce
added a commit
to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS
that referenced
this pull request
Jul 20, 2026
… (unit 44) Semantic re-implementation of forcedotcom/dev 6e09678 (merge of bbirman/refreshCoordinator) onto the ObjC→Swift migration branch. WHAT: introduces SFSDKTokenRefreshCoordinator — a process-wide singleton that coalesces concurrent token-refresh requests per credential (keyed by credentials.identifier) so at most one refresh is in-flight at a time. This prevents the double-spend race with single-use (rotating) refresh tokens, where concurrent refreshes would invalidate each other's tokens. Callbacks are delivered on the main queue; background-task protection wraps the refresh. MIGRATION NOTES (no new ObjC; compiled path is Swift): - NEW SFSDKTokenRefreshCoordinator.swift (upstream added .h/.m ObjC) — @objc singleton, wired into pbxproj (framework Sources + Headers-free). NEW SFSDKTokenRefreshCoordinatorTests.swift (upstream .m, +602) ported to Swift and wired to the test target. - SFRestAPI.swift / SFIdentityCoordinator.swift / SFUserAccountManager.swift / UserAccountManager.swift (async refresh) / SFSDKOAuth2.swift / SFSDKTestRequestListener.swift: the compiled Swift twins carry the behavior change (route through the coordinator; SFRestAPI: sessionRefreshInProgress + pendingRequestsBeingProcessed + oauthSessionRefresher collapse to a single refreshCycleActive flag; cleanup now delivers "User logged out" and cancels in-flight tasks; OAuth2 completion no longer double-hops main queue; TestRequestListener spins the run loop instead of a semaphore to avoid a main-thread deadlock now that the coordinator delivers on main). - WebSocketClient.swift: TokenRefreshCoordinator actor renamed WebSocketReconnectCoordinator (it only gates reconnection; token dedup now lives in SFSDKTokenRefreshCoordinator). - Public-API deprecation of SFOAuthSessionRefresher (14.0→15.0): expressed on the Swift members consumers call (@available deprecated), with non-deprecated internal seams (init(internalCredentials:) / refreshSessionInternal) that the coordinator, tests, and mock use so the SDK's own paths stay warning-free — mirrors the unit-39 forceAdvancedAuthenticationInternal precedent. Upstream's ObjC SFSDK_DEPRECATED lives on tombstoned headers, so it has no compiled home. - De-referenced .m/.h mirrors (SFRestAPI.m, SFIdentityCoordinator.m, SFSDKOAuth2.m, SFSDKTestRequestListener.m, SFOAuthSessionRefresher.m/.h and the two test .m) ref-synced to the upstream post-image on top of the migrated pre-image (keeping the migration's @import/-Swift.h deltas); tombstone headers skipped; SFUserAccountManager.m had nothing to sync (refreshCredentials lives in the .swift twin). ESCALATION (flag for human PR review): OAuth/token-refresh control flow + public-API deprecation (SFOAuthSessionRefresher). This is the live-auth unblocker for Phase 2. Gate: SDKCore/SmartStore/MobileSync build-for-testing all GREEN, 0 new warnings; 22 targeted tests pass (11 coordinator + 7 data-task-race + 4 refresher). Live-org auth-util end-to-end tests remain XCTSkip-gated (Phase 2). Upstream: 6e09678 (PR forcedotcom#4087) · marker unit 44 · __U44__
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
3 participants
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.
SFRestAPI,SFIdentityCoordinatorandSFUserAccountManagerSFUserAccountManagerSFOAuthSessionRefresherso that it will be internal only in the future