Keep a wrong dashboard bundle ID from locking customers in test mode

[jakemor]

Changes in this pull request

Three fixes for the test mode incident on Natural Camera (app 1128), one per commit so any of them can be dropped.

What happened

On Oct 6 the app's bundle ID was changed on the dashboard. With testModeBehavior at its default, every App Store install went into test mode: 46 of 54 active users over the next 17 hours.

1. The test mode sheet was presented from the root view controller, which was already presenting the app's main screen, so UIKit dropped it. Open the test mode sheet above any sheet already showing #534 fixes this, but it isn't in 4.17.0.
2. Config is held until the sheet closes, so configState never published and every register call waited for the rest of the launch. The app saves photos inside register, so no photo saved.
3. The bundle ID was corrected 17 hours later. Non-subscribers recovered on their next launch. Subscribers did not: they launch from the saved config with no network call, the saved config still turned test mode on, and the refresh that would replace it is only scheduled after processConfig returns. All 13 affected subscribers were stuck until the app shipped a build with test mode set to .never.

It isn't only this app

I went through testModeModal_open events for every app over the last 30 days. Two more apps have App Store users in test mode. Their names are left out because this repo is public; Jake has them.

• A second app, wrong since it was created. Its dashboard bundle ID never matched the app. Every day for two months, between 33 and 100 App Store users were put in test mode: 1,658 users in total, 1,563 of them flipped to inactive. The sheet does present in that app, so this is test mode in production, not the hang. Commit 3 would have prevented it. The dashboard value was corrected on Oct 8.
• A third app, the same failure as Natural Camera, three weeks earlier. On Sept 17, 682 users entered test mode within one hour and 655 were flipped to inactive. It was corrected within the hour. Since then 70 users have kept logging the sheet opening with no close and no config loaded, 48 of them on more than one day, and 13 were still doing so in the last two days. The dashboard and device bundle IDs match, so these launches come from the saved config. Commit 1 is what un-sticks them.
• About 100 apps a day log the sheet in total. Nearly all are development, staging or QA builds with one to five users, which is the feature working as intended.

How to tell the cases apart in the events:

Sign	Meaning
testModeModal_open from a device whose launch device_attributes have $isSandbox: "false"	An App Store install is in test mode
testModeModal_open with no testModeModal_close, and staticConfigBuildId blank on every event of that launch	The sheet never showed and config never loaded
The same users showing that pattern on later days	They are launching from a saved config that turns test mode on
subscriptionStatus_didChange to INACTIVE right after testModeModal_close	A real subscription was replaced with the simulated one

1. Never save or launch from a config that turns test mode on

• A config whose own contents put the user in test mode (test user match or bundle ID mismatch) is not written to LatestConfig, and the config saved before it is deleted.
• A config like that saved by an earlier SDK version is deleted at launch and a fresh one is fetched. This is what un-sticks devices already in the field.
• Test mode asked for by testModeBehavior = .always or the dev server doesn't depend on the config, so those configs are still saved.
• TestModeManager.evaluateTestMode is split so the reason can be worked out without applying it. Behaviour is unchanged.

The cost: a user in test mode fetches config from the network on every launch. Test mode already needs the network for its products.

2. Stop waiting on a test mode sheet that never appears

#534 handles UIKit refusing the presentation on the spot. If UIKit accepts the sheet but it still hasn't started appearing after 3 seconds, the sheet is taken back and the saved settings are used, the same as the refusal path. This is a backstop; I don't have a reproduction for this path.

3. Ignore a bundle ID mismatch in App Store builds

A bundle ID mismatch now only turns test mode on where a developer runs the app (simulator, development, TestFlight), using the same DeviceHelper.isSandboxEnvironment check as the dev server. In an App Store build it is logged and ignored. Test users marked on the dashboard still get test mode in any build, and .always is unchanged.

This is a behaviour change and the online docs still describe the old behaviour. One known gap: before the first AppTransaction result, a Mac Catalyst development build reads as production, so test mode there turns on at the first config refresh instead of at launch.

On #534 and #540

#534 is the right fix for the layout that broke here: topPresenter puts the sheet above the controller the root is presenting instead of having it dropped. #540 covers the multi-scene and multi-window case and this PR doesn't touch it.

One case neither closes: if the SDK presents from the app's root before the app presents its own main UI from that root, the app's presentation is the one UIKit refuses. Paywalls avoid this by presenting from the SDK's own window (createPresentingWindowIfNeeded). Presenting the sheet the same way would close it. Not done here.

Testing

• Commit 1: new tests in ConfigManagerTests and TestModeManagerTests. I ran ConfigManagerTests, ConfigManagerEarlyPublishTests, TestModeManagerTests and TestModeModalTests on an iOS simulator: 60 tests passed. With the ConfigManager change reverted, the two cache tests fail (the subscriber test fails on getConfigCalled == false, which is the bug).
• Commit 2: no test. It compiles.
• Commit 3: two tests added in TestModeManagerTests, not run. It compiles.
• The full suite was not run after commits 2 and 3. Nothing was run on a device.
• SwiftLint is clean on the changed source files.

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

🌸 Shipped with Kanna — an open-source workspace for all your coding agents. Written by claude/opus[1m].

[greptile-apps]

This PR does not match any of the 1 configured review trigger rule.

[maple-review-bot]

Maple review

🟡 Confidence 5/10 · needs attention
Cache invalidation and App Store behavior have tests, but the new delayed dismissal and appearance guard are untested.
_{quality 90/100 · 1 warning · tests partial · risk high · 1/1 new units observable}

Prevents stale dashboard settings from trapping launches in test mode and ignores bundle ID mismatches in App Store builds. The delayed sheet fallback needs regression coverage before merging.

• cachedConfigForLaunch rejects configs that activate dashboard-driven test mode.
• storeAndApply removes test-mode configs instead of caching them.
• giveUpIfNeverShown releases config loading after three seconds.

Findings

🟠 Warning · F1 · giveUpIfNeverShown has no timeout-path regression test

tests · Sources/SuperwallKit/TestMode/Alert/TestModeModal.swift:93-107

The new timeout decides whether config loading resumes or a presented sheet is dismissed, but no test exercises it. TestModeModalTests covers immediate presentation refusal and finish() idempotency, not the delayed fallback or the hasStartedAppearing guard.

Add Swift Testing cases using the injectable `after` delay: an unseen sheet returns saved settings once, and a sheet receiving `viewWillAppear` is neither finished nor dismissed.

🤖 Prompt to fix this finding with an AI agent

Findings from an automated review of commit 1bbac73f84e381c63e43d76449e118f643d9cd19. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F1 · Warning · tests · Sources/SuperwallKit/TestMode/Alert/TestModeModal.swift:93-107
`giveUpIfNeverShown` has no timeout-path regression test
The new timeout decides whether config loading resumes or a presented sheet is dismissed, but no test exercises it. `TestModeModalTests` covers immediate presentation refusal and `finish()` idempotency, not the delayed fallback or the `hasStartedAppearing` guard.
Suggested fix: Add Swift Testing cases using the injectable `after` delay: an unseen sheet returns saved settings once, and a sheet receiving `viewWillAppear` is neither finished nor dismissed.

What was checked

• Cache.delete removes both memory and disk copies of LatestConfig.
• configMatchReason takes precedence over the App Store bundle-mismatch guard.
• finish clears its callback before resuming, preventing duplicate continuation completion.

Observability coverage: 1 of 1 changes observable

Change	Kind	Observable	Evidence
TestModeModal.giveUpIfNeverShown	delayed UI fallback	yes	Uses the existing Logger warning convention; completion flows through ConfigManager's existing TestModeModalClose event.

_{1bbac73 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

[maple-review-bot]

1 inline note from Maple's review. The score and summary are in the review comment above.

[pullfrog]

ℹ️ No critical issues. One small rough edge inline and one nitpick.

Reviewed changes

I reviewed all three commits. Each fix stands on its own, and the evaluateTestMode → reasonForTestMode refactor keeps the existing behaviour.

• Saved configs that turn test mode on are dropped: cachedConfigForLaunch() deletes a saved LatestConfig whose own contents (test user match or bundle ID mismatch) would turn test mode on. storeAndApply(_:keepingForNextLaunch:) deletes instead of saving those configs. .always and DevMode configs are still saved because TestModeReason.comesFromConfig excludes .testModeOption.
• Backstop for an accepted sheet that never shows: giveUpIfNeverShown waits 3s for viewWillAppear on TestModeNavigationController. If it never comes, it takes the sheet back and calls finish(), which is idempotent, so a late double resume is safe.
• App Store builds ignore a bundle ID mismatch: bundleIdMismatchReason only returns a reason when isSandboxEnvironment() is true. That is an instance closure, so overriding it in tests doesn't leak across tests the way the DevMode static can. The existing mismatch tests still pass on the simulator because detectSandbox() returns "true" there.

I checked the identity timing: IdentityManager loads appUserId and aliasId from storage in init, so cachedConfigForLaunch() sees the same identity as the later evaluateTestMode call, even though fetchConfiguration and identityManager.configure() run concurrently.

ℹ️ Nitpicks

• Commit 2 has no test, but giveUpIfNeverShown(_:modal:after:) already takes the timeout as a parameter. A Swift Testing case could call it with after: 0 on a TestModeNavigationController that was never presented, then check that onDismiss fires once. That would pin the fallback cheaply. CLAUDE.md asks for a test with each SDK change.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using claude-opus-5.5 ｜ 𝕏}

[maple-review-bot]

Maple review

🟢 Confidence 8/10 · likely safe to merge
Timeout branches and cache retention are covered; warning frequency has no direct regression assertion.
_{quality 100/100 · no findings · tests partial · risk medium}

The revision confines ignored-mismatch warnings to config application and reuses the evaluated reason for cache retention. New timeout and appearance-guard tests address F1; no new defects found in the three files changed since the previous review.

Fixed since the last review

• ✅ F1 · giveUpIfNeverShown has no timeout-path regression test

What was checked

• Compared all three files against previously reviewed head 1bbac73.
• Verified processConfig uses the applied test-mode reason for cache retention.
• Read timeout implementation and tests covering fallback and the appearance guard.

_{e9cb8a2 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

[maple-review-bot]

Maple review

🟢 Confidence 9/10 · safe to merge
The incremental update contains no additional SDK changes; its source and tests are identical to the base.
_{quality 100/100 · no findings · tests not needed · risk low}

Prevents cached test-mode configurations and dashboard bundle-ID mismatches from trapping production users. The latest commit only merges develop; the newly listed source and test files match the base exactly, introducing no additional SDK changes.

What was checked

• Verified all six newly listed non-documentation files match base commit 91072048c exactly.
• Confirmed b533a54 merges the previously reviewed head with the current base.
• Checked config publication still follows test-mode evaluation and cache handling.

_{b533a54 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

[pullfrog]

✅ No new issues found. Both points from the last review are fixed.

Reviewed changes

I reviewed e9cb8a2 and the merge from develop. The merge only moves CHANGELOG entries.

• The ignored-mismatch warning now logs once per config: reasonForTestMode(config:options:) no longer has side effects. Only evaluateTestMode passes warnAboutIgnoredMismatch: true, and it now returns the reason. processConfig works out the reason once and uses that one value both to apply test mode and for keepingForNextLaunch.
• evaluateTestMode now runs before storeAndApply in processConfig: I checked that this order is safe. evaluateTestMode and clearTestModeState don't read anything storeAndApply sets (triggers, paywall variants, IP collection). Both run synchronously before the first await, so nothing can see the state between them.
• Added tests for giveUpIfNeverShown: one test checks that a sheet that never appears reports back exactly once. The other checks that a sheet whose viewWillAppear already ran is left alone. Both pass after: 0 and run on the @MainActor, .serialized suite.

  ^{｜ View workflow run ｜ Using claude-opus-5.5 ｜ 𝕏}