Skip to content

fix: accent color resetting to system blue when configuring from a SwiftUI App init - #501

Merged
yusuftor merged 6 commits into
developfrom
fix/accent-color-pre-launch-uikit-reads
Aug 12, 2026
Merged

yusuftor merged 6 commits into
developfrom
fix/accent-color-pre-launch-uikit-reads

Conversation

@yusuftor

@yusuftor yusuftor commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #493

The bug

Calling Superwall.configure from a SwiftUI App.init reset the host app's accent color to system blue (most visibly on TabView/tab bar selected items).

The cause is not the paywall presenting window suggested in the issue — the repro uses an invalid API key, so that code never runs. App.init executes before UIApplicationMain creates the application object, and touching UIScreen.main — or even just UIFontMetrics.default.scaledValue(for:) — in that window makes UIKit build its trait system before the app's asset-catalog accent color (NSAccentColorName) is registered. The global tint then permanently falls back to system blue. DeviceHelper.init, which runs synchronously under configure, did both reads. The same calls made from application(_:didFinishLaunchingWithOptions:) are harmless, which is why UIKit apps never saw this.

Each of these was verified in isolation in a sample app with Superwall removed: _ = UIScreen.main alone in App.init reproduces the blue tint; the full configure in the app delegate does not.

The fix

DeviceHelper now refuses to touch UIKit until it's safe, via a single gate (isUIKitReadSafe):

  • The screen metrics and UI trait readers return nil instead of touching UIKit when an app process hasn't created its application object yet. Screen metrics move from init-time lets to a self-healing cache using the same @DispatchQueueBacked pattern as the traits.
  • The caches fill on the first post-launch read or on app activation. Off-main readers (the MMP install match, getTemplateDevice(), network headers) block on the main-queue hop, which can't drain until launch completes — so they always see real values, never placeholders.
  • Processes that never create a UIApplication (host-less unit-test runners, app extensions) legitimately read UIScreen.main, so the gate also checks the bundle type: only .app bundles run UIApplicationMain. Without this, device attributes would report placeholders forever in those environments.

Verification

  • Sample app from the issue, built against this branch with configure in App.init: selected tab renders the accent color exactly (sampled (255, 0, 255) for a magenta asset), and getDeviceAttributes() reports the device's live trait values.
  • New tests pin the pre-launch nil behavior, the launched-state reads, the off-main screen-metrics path, and the test-runner/extension allowance.
  • Full suite: 903 tests in 93 suites pass on iOS 26.5.

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs on iOS.
  • Demo project builds and runs on Mac Catalyst.
  • Demo project builds and runs on visionOS.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run swiftlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

🤖 Generated with Claude Code

Greptile Summary

The PR prevents pre-launch UIKit trait initialization from resetting a SwiftUI host app’s accent color.

  • Defers screen and trait reads until UIKit is safe to access.
  • Adds self-healing caches populated after launch or activation.
  • Adds tests for pre-launch, post-launch, background-thread, and host-less process behavior.
  • Records the fix in the changelog without the previously reported conflict markers.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
CHANGELOG.md Adds a single release-note bullet, and the previously reported conflict markers are no longer present.
Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift Gates appearance-adjacent UIKit reads and lazily fills cached screen metrics and traits after application launch.
Tests/SuperwallKitTests/Network/DeviceHelperTests.swift Adds coverage for blocked pre-launch reads, cache recovery, lifecycle refresh, and off-main screen access.

Reviews (2): Last reviewed commit: "test(device): cover the instance-level s..." | Re-trigger Greptile

Context used:

Touching UIScreen or UIFontMetrics before UIApplicationMain has created
the application object — e.g. configure called from a SwiftUI App.init —
makes UIKit build its trait system before the app's asset-catalog accent
color is registered, permanently resetting the global tint to system
blue. DeviceHelper did both synchronously under configure.

Screen metrics and UI traits now return nil in that window instead of
touching UIKit, and the caches self-heal on first post-launch read or on
app activation. Off-main readers block on the main-queue hop, which
cannot drain until launch completes, so they always see real values.

Processes that never create an application (unit-test runners, app
extensions) read UIScreen.main legitimately, so the gate also checks the
bundle type: only app bundles run UIApplicationMain.

Fixes #493

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread CHANGELOG.md Outdated
The 4.16.2 entry was committed mid stash-pop, carrying conflict markers
that presented the ATT/permissions fixes and the accent-color fix as
competing alternatives. All three bullets belong.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

CHANGELOG.md ships with unresolved git conflict markers (<<<<<<< Updated upstream / ======= / >>>>>>> Stashed changes), and the "Stashed changes" side drops two existing 4.16.2 entries. The DeviceHelper change itself looks sound; the test seam is the one structural gap worth a second look.

Reviewed changes — the full diff of b2d2ad3 (3 files): the new pre-launch UIKit gate in DeviceHelper, its tests, and the changelog entry.

  • isUIKitReadSafe gate — new static var returning true when UIApplication.sharedApplication exists, or when the main bundle isn't an .app (hostless test runners and app extensions never create an application object but can read UIScreen.main).
  • Optional readers — makeScreenMetrics() / makeUITraits() became internal, return Optional, and take an injectable isUIKitReadSafe: closure evaluated inside the main-thread read closure so the KVC lookup never happens off-thread.
  • Screen metrics became a self-healing cache — screenWidth / screenHeight / devicePixelRatio moved from init-time lets to computed properties over a @DispatchQueueBacked private var screenMetrics: ScreenMetrics?, filled on first read or on didBecomeActiveNotification, mirroring the existing uiTraits pattern.
  • refreshUITraits() gained a blocking branch — an off-main read with an empty cache now calls makeUITraits() directly (which does its own DispatchQueue.main.sync) rather than scheduling an async refresh, so early readers block instead of reporting placeholders.
  • Fallbacks — ScreenMetrics.placeholder (0×0, scale 1.0) and the existing UITraits.unavailable now back the ?? sites.
  • Tests — three gate tests on the static makers, one live screen read, one off-main screen-metrics read pinned to exact UIScreen values.

Two things I chased down and confirmed are not problems, so they don't need re-litigating: the paywall-preload path's raw UIScreen.main reads (SWWebView.swift:106, PaywallViewController.swift:148) are unreachable in the pre-launch window because PaywallManager.getViewController and DependencyContainer.makePaywallViewController are both @MainActor and the main actor's executor can't run until the run loop services the main queue; and no deadlock exists, since configure() never awaits its bootstrap Task and @DispatchQueueBacked uses a private serial queue distinct from DispatchQueue.main.

⚠️ Nothing tests the self-healing caches the fix is built on

The isUIKitReadSafe: seam sits on the static makers only, so no test can construct a DeviceHelper in the pre-launch state. Every piece of new instance behavior is therefore uncovered: currentScreenMetrics filling on first post-launch read, refreshUITraits()'s new uiTraits == nil blocking branch, and the didBecomeActiveNotification screen-metrics fill. The three new tests pin only that the static functions return nil when handed { false } — which is the least likely part to regress.

Technical details
# Instance-level self-healing has no coverage

## Affected sites
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:162-171` — `currentScreenMetrics`' heal-on-read is untested; a regression that caches `.placeholder`, or never writes the cache, would report a 0×0 screen forever and no test would fail.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:341-350` — the new blocking `uiTraits == nil` branch is untested. Its whole purpose is "off-main readers never see placeholders"; if it regressed to scheduling an async refresh, `Network.matchMMPInstall` and `getTemplateDevice()` would silently send `UITraits.unavailable` to the backend.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:487-490` — the `didBecomeActive` screen-metrics fill is untested.
- `Tests/SuperwallKitTests/Network/DeviceHelperTests.swift:237-251` — the three gate tests only exercise the static functions.

## Required outcome
- A test can put a real `DeviceHelper` instance into the "reads not yet allowed" state and then release it, pinning that (a) reads before release don't touch UIKit, and (b) reads after release return the device's live values rather than `.placeholder` / `.unavailable`.

## Suggested approach (optional)
- Move the seam from the static functions' default argument onto the instance — e.g. a `let isUIKitReadSafe: () -> Bool` stored on `DeviceHelper` (defaulting to `{ DeviceHelper.isUIKitReadSafe }`) that `currentScreenMetrics` / `refreshUITraits` consult, with the statics taking it as a parameter. A test can then flip a captured `Bool` and assert the cache heals on the next read and on a posted `UIApplication.didBecomeActiveNotification`.
- If `DependencyContainer` makes injecting that awkward, an internal `DeviceHelper` init parameter used only by tests is still a strict improvement over no coverage.

ℹ️ Pre-launch device reads now block a pool thread rather than returning a cached value

Before this change, an off-main read of the screen metrics or traits during configure returned an already-populated cache instantly. Now, when configure runs pre-launch, the first off-main read parks a real OS thread in DispatchQueue.main.sync until the main run loop starts servicing the main queue. That's the deliberate design (better than shipping placeholders) and it's bounded in a normal launch, but it lands on two paths that aren't covered by the SDK's timeout budget, and the "bounded" part rests on an assumption worth stating explicitly.

Technical details
# Blocking pre-launch reads sit outside the timeout budget

## Affected sites
- `Sources/SuperwallKit/Network/Network.swift:486-488` — `matchMMPInstall` reads `deviceHelper.screenWidth` / `screenHeight` / `devicePixelRatio` off-main with no `appInForeground()` gate. It runs from the `configure` bootstrap `Task` (`Superwall.swift:500-517`) on a fresh install, i.e. exactly the pre-launch window, and now blocks there.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:889` — `getEnrichment()` snapshots `currentUITraits` via `getTemplateDevice()` *before* `network.getEnrichment(...)` at `:894`, so the block happens outside the `timeout:` that `ConfigManager.swift:250,304` sets (0.5–1s).
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:341-350` — the branch that introduces the block.

## Required outcome
- Confirmation that holding cooperative-pool threads in `DispatchQueue.main.sync` for the duration of the pre-launch window is acceptable, or a bound on the wait with a documented fallback.

## Open questions for the human
- iOS prewarming runs an app's launch sequence up to, but not including, `main()`'s call to `UIApplicationMain`, then suspends the process. Since `App.init` runs before `UIApplicationMain`, a prewarmed process can execute `configure` and then sit suspended indefinitely — in which case "blocks only until launch completes" isn't bounded by launch at all. The process being suspended makes the practical impact small, but was this case considered? (I could not verify whether `App.init` actually executes during prewarm; treating it as an open question, not a claim.)
- `DeviceHelper` currently reports `ScreenMetrics.placeholder` (0×0) and `UITraits.unavailable` ("Unknown", 16, 1.0, "unspecified") for a *main-thread* read in the pre-launch window, since blocking there is impossible. Is silently sending those to audience filters preferable to no read at all, or should those attributes be omitted from the payload when the cache is empty?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment thread CHANGELOG.md Outdated
Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift Outdated
Comment thread Tests/SuperwallKitTests/Network/DeviceHelperTests.swift Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ The blocking changelog issue is fixed. Nothing new to flag in this commit.

Reviewed changes — the incremental range-diff from b2d2ad3 to 875d8d6. 875d8d6 touches CHANGELOG.md only.

  • Conflict markers removed — <<<<<<< Updated upstream / ======= / >>>>>>> Stashed changes are gone, and all three 4.16.2 fix entries survive the resolution (CHANGELOG.md:18-20): the App Tracking Transparency entry, the microphone/location/contacts symbol entry, and the new accent-color entry. Verified against the file on disk, not just the range-diff.

No source or test changes in this commit, so the three non-blocking points from my previous review still stand — chiefly that the isUIKitReadSafe: seam lives only on the static makers, so none of the new instance-level self-healing behaviour is covered by a test.

Pullfrog  | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏

yusuftor and others added 2 commits August 12, 2026 16:25
The gate's doc claimed every UIKit read in the file sits behind it, but
the UIDevice reads at init (model, vendorId, interfaceType) are
unguarded and safe — they don't touch the trait system. Narrow the claim
and say why they're exempt.

The test ran in a host-less runner where no UIApplication exists, so
whenTheApplicationExists described the wrong clause; reads are allowed
there via the bundle check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The isUIKitReadSafe seam lived only on the static makers, so no test
could hold a real DeviceHelper in the pre-launch state. The gate is now
injected at init and consulted by every instance-level cache read and
refresh, with the statics keeping their parameter.

Three tests pin the behaviors the fix is built on: placeholders served
without touching UIKit while reads are disallowed, both caches healing
with live values on the first allowed read (traits off-main, through
the blocking empty-cache branch), and the notification observer filling
the caches — gated shut again before reading so heal-on-read can't mask
a broken observer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yusuftor

Copy link
Copy Markdown
Collaborator Author

Addressing the two review-body points from the initial review (the inline threads were fixed earlier):

⚠️ Instance-level self-healing coverage — fixed in f417e0c. The seam moved onto the instance as suggested: DeviceHelper.init now takes isUIKitReadSafe: @escaping () -> Bool = { DeviceHelper.isUIKitReadSafe }, stored and consulted by currentScreenMetrics, refreshUITraits() (all branches), the notification observer, and the trait-change registration callback; the statics keep their parameter. Three new tests construct a real DeviceHelper behind a flippable gate and pin: (1) placeholders served without touching UIKit while the gate is closed, (2) both caches healing with live UIScreen values on the first allowed read — traits read from off-main so the blocking empty-cache branch in refreshUITraits() is the path exercised, and (3) the notification observer filling both caches, with the gate closed again before reading so a heal-on-read can't mask a broken observer (posted as UIContentSizeCategory.didChangeNotification, which shares the handler with didBecomeActive but doesn't wake the app-session machinery of containers other tests hold).

ℹ️ Blocking pre-launch reads / prewarming — confirming the design is intentional, with the prewarm case considered:

  • Prewarmed processes: agreed that App.init can run during prewarm, so a bootstrap-Task thread can park in DispatchQueue.main.sync while the process sits suspended — "until launch completes" is wall-clock-unbounded there. But a parked thread in a suspended process consumes no CPU, and it unblocks the moment the user actually launches and the run loop drains. Bounding the wait with a timeout would trade that harmless park for placeholder data reaching the backend on every prewarmed fresh install, which is the worse outcome. The block also only exists on the fresh-install pre-launch path; every subsequent launch reads the cache.
  • Main-thread placeholder reads: no SDK request path builds a payload on the main thread pre-launch — the MMP match, enrichment, and getTemplateDevice() all read from async contexts that take the blocking path. The placeholder tokens ("Unknown", "unspecified", 0×0 @1.0) are the exact values the SDK has always shipped on visionOS, so the backend already tolerates them; omitting keys instead would be a new payload shape for no observed producer. If a developer synchronously reads attributes inside App.init they get placeholders for one frame — acceptable for an API being called before the app exists.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ No blocking issues — the coverage gap from my earlier review is properly closed. Two nits inline.

Reviewed changes — the incremental range-diff from 875d8d6 to f417e0c. e086280 was a doc-comment and test-rename commit only; f417e0c moves the isUIKitReadSafe test seam from the static makers onto the DeviceHelper instance and adds coverage for the self-healing caches.

  • Instance-level gate — DeviceHelper.init gained isUIKitReadSafe: @escaping () -> Bool = { DeviceHelper.isUIKitReadSafe }, stored as a private let (DeviceHelper.swift:203). Every instance site threads it through: currentScreenMetrics:166, currentUITraits:335, refreshUITraits:346,356,368, the iOS 17 trait-change handler :424, and the activation handler :500,504. Grepped Sources — no bare makeUITraits() / makeScreenMetrics() call sites remain, and the default argument keeps production behaviour identical while still re-evaluating the static on every call, which is what makes the caches heal.
  • guard let self in three closures — the self?.uiTraits = … writes in refreshUITraits' async refresh, the trait-change registration, and the notification observer became guard let self else { return }. Behaviourally identical, including the isRefreshingUITraits latch reset.
  • Three instance tests — instance_whileReadsAreDisallowed_servesPlaceholders (DeviceHelperTests.swift:348) pins the pre-launch placeholders; instance_healsOnFirstAllowedRead (:366) reads off the main thread so it genuinely exercises refreshUITraits' new uiTraits == nil blocking branch; instance_fillsCachesOnNotification (:400) closes the gate again before asserting, so the values can only have come from the observer's fill. All three assert exact live values, so each one fails against .placeholder / .unavailable.

Two things I chased down and won't flag: the non-@Sendable () -> Bool stored on a non-Sendable class and captured in DispatchQueue.main.async raises no diagnostics, because the package is Swift 5 language mode (swift-tools-version:5.5, no swiftLanguageMode); and the new test's global notification post is benign in both directions against the rest of the suite (AppSessionManagerTests posts didBecomeActiveNotification), since a stray fill can only ever write live values — which is what every gate-open assertion expects — and the gate-closed test keeps its gate shut for its whole body.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment thread Tests/SuperwallKitTests/Network/DeviceHelperTests.swift
Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift Outdated
…ne delivery

The observer registers with queue: .main, so delivery is scheduled onto
OperationQueue.main rather than run inline on the posting thread — the
test only passed because an await happened to yield the main thread
first. Poll the observable condition with a bounded wait; polling with
the gate closed can't fill the caches, so the proof is intact.

Also flips a misattached 'which' in the uiTraits doc that read as the
opposite of the gate's behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ No blocking issues, and no production behaviour changed in this delta — but the new bounded wait can't observe the thing it waits for. One nit inline.

Reviewed changes — the incremental range-diff from f417e0c to b0a9b3e: a doc-comment rewording and the notification test's synchronisation.

  • uiTraits doc polarity fixed — DeviceHelper.swift:297-298 now reads "nil while a read isn't yet allowed to touch UIKit … which only happens when the SDK is configured before UIApplicationMain runs", so the relative clause attaches to the nil state instead of to reads being allowed. Consistent with the screenMetrics doc at :131-135.
  • instance_fillsCachesOnNotification waits for the fill — DeviceHelperTests.swift:411-423 replaced the implicit reliance on await expectedLiveValues() yielding the main thread with a bounded poll (200 × 10 ms) on screenWidth != 0 && interfaceStyle != "Unknown".

DeviceHelper.swift's only edit in this range is the doc comment, so the shipped fix is byte-for-byte what was reviewed at f417e0c.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment thread Tests/SuperwallKitTests/Network/DeviceHelperTests.swift Outdated
The bounded poll ran after the gate closed, but the fill consults the
gate when its block runs — under deferred delivery nothing could ever
fill and the loop was dead code under inline delivery. Await a barrier
operation on OperationQueue.main instead, enqueued after the post so it
can't run before the observer's block, then close the gate. Nothing
reads the helper while the gate is open, so the fill remains the only
possible writer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found. The barrier fix closes the last open thread, and production code is untouched in this delta.

Reviewed changes — the incremental range-diff from b0a9b3e to bba3660: a single test-synchronisation change in instance_fillsCachesOnNotification.

  • Replaced the inert poll with a same-queue barrier — DeviceHelperTests.swift:411-423 now keeps the gate open across await withCheckedContinuation { OperationQueue.main.addOperation { continuation.resume() } } and closes it immediately after, so the observer's fill is the only possible writer under both inline and deferred delivery. Verified: nothing reads the helper between the post and gate.isOpen = false, and all five assertions still compare against exact live values, so the test fails against .placeholder / .unavailable.
  • No production change — DeviceHelper.swift and CHANGELOG.md are byte-for-byte what was reviewed at f417e0c / 875d8d6.

Two things I chased down and won't flag. Ordering: Apple documents OperationQueue.main as serial ("executes one operation at a time on the app's main thread") and equal-priority ready operations as running in add order, with a general "don't rely on queue semantics" hedge that doesn't bite for two dependency-free, default-priority closures — and if Darwin delivers inline instead, the barrier is a harmless no-op. Hang risk: the barrier needs the main queue serviced in the hostless runner, which is already the load-bearing assumption behind the pre-existing await MainActor.run {} drain at DeviceHelperTests.swift:162 and behind DeviceHelper.swift:234,474's DispatchQueue.main.sync off-main path — no new exposure, and this barrier is the stronger form since it sits on the same queue as the observer rather than being a plain main-actor hop.

The two open questions from my initial review still stand unchanged (the pre-launch DispatchQueue.main.sync block on matchMMPInstall / getEnrichment sitting outside the config timeout budget, and whether main-thread pre-launch reads should send placeholders or omit those attributes) — both are judgement calls for you, not blockers.

Pullfrog  | View workflow run | Using Claude Opus | 𝕏

@yusuftor
yusuftor merged commit a8dd696 into develop Aug 12, 2026
4 checks passed
@yusuftor
yusuftor deleted the fix/accent-color-pre-launch-uikit-reads branch August 12, 2026 15:52
@pullfrog pullfrog Bot mentioned this pull request Aug 12, 2026
8 of 10 tasks
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.

[BUG] Presenting UIWindow resets app tintColor to system blue (tab bar / AccentColor)

1 participant