fix: stop the test suite wiping the simulator's wallet

[jvsena42]

Stack — review in order; each targets the one before it.

1. fix: keep transfer unit tests out of the app's UserDefaults #756 — keep transfer unit tests out of the app's UserDefaults (fixes fix: hardware transfer unit tests write mock transfers into the app's UserDefaults #733)
2. fix: stop the test suite wiping the simulator's wallet #757 — stop the test suite wiping the simulator's wallet
3. test: isolate remaining suites from the app's state #758 — isolate remaining suites from the app's state

This PR stops the unit test suite destroying the wallet on the simulator it runs against.

Running the suite deletes the seed, the PIN and the whole Pubky and Paykit identity: the app reopens at Terms of Use and the regtest ledger on disk is orphaned, because without the mnemonic the app will not adopt it. Eight suites call the keychain wipe from setUp or tearDown with no guard, and four of those are in the default unit lane rather than the integration one. The test bundle is hosted in the app, and the network resolves to regtest for both a test run and a Debug build, so they share one keychain access group.

Entries now carry a test-only account prefix when the process is running tests, and the wipe skips anything un-prefixed while under test. This is the same lever the app already uses to keep its LDK and core storage out of the real directories, it needs no entitlement change, and it leaves the accessibility, synchronisability and access-group pinning alone. It only ever narrows what a process can reach. A compile-time flag would not have worked here: the flag is defined for the test target only, so the copy of the keychain helper that the services under test actually call would have kept writing real accounts.

The installation marker has to move with it, and this is the part worth a second look. Two suites delete the marker, which lives in the real sandbox Documents directory rather than the app group, so it is outside the existing redirect. A missing marker is how the app detects a keychain orphaned by a reinstall: on the next launch it finds no marker, finds a mnemonic, and wipes the keychain. So a test run arms a wipe that fires later. Today that is invisible because the keychain is already empty by then — fixing the keychain alone would have made it worse, with the wallet surviving the run only to be deleted when the app next started.

Two other things came out of auditing the same area. The channel migration tests were nilling a real pending migration, which is a React-Native channel manager and its monitors, so that key is now snapshotted and restored. And the React-Native cleanup suite cannot be namespaced away at all — it deletes the real mmkv and ldk directories under Documents and removes keychain items by service with no access-group filter, which on a device mid-migration is the migration source. That one is now opt-in behind an environment variable, set only in the integration lane, which already erases its simulator between attempts. It moves into that lane's test list so it keeps running somewhere rather than being quietly dropped.

Also drops a skip for a test class that no longer exists.

No changelog fragment: the namespacing is inert unless the process is running tests, and no shipped behaviour changes.

Linked Issues/Tasks

Follows #756. Found while auditing the suite for #733.

Design

N/A — no UI changes.

QA Notes

Manual Tests

• 1a. On a simulator carrying a wallet and a Pubky identity, run the full unit lane → launch the app: still opens to the wallet, not Terms of Use, with the identity and its contact links intact.
  • 1b. launch a second time: wallet still there. This is the launch where the orphan check would fire if the marker fix were missing.
• 2. regression: Settings → Security: PIN and biometrics settings are as they were before the run.

Automated Checks

• Unit tests modified: BitkitTests/KeychainTests.swift addresses entries through the same helper the app uses, since it builds its own keychain queries.
• Test gating added: BitkitTests/RNMigrationCleanupTests.swift skips unless BITKIT_DESTRUCTIVE_TESTS=1; the guard covers tearDown too, because XCTest runs it even when setUp skips and the cleanup is the destructive part.
• CI: the integration workflow sets that variable and gains the suite; the unit workflow skips it.
• Verified locally on iPhone 17 against a live wallet: the suites that previously wiped it now leave it intact, the marker keeps its original timestamp, and the tests create and delete their own marker in a separate directory.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; the changed isolation boundaries consistently protect simulator wallet and migration state while retaining destructive coverage in the designated integration lane.

Summary

This PR isolates test-owned keychain entries and installation markers from simulator wallet state, preserves pending channel-migration preferences across tests, and confines destructive React Native migration cleanup to the integration lane.

• Prefixes native keychain accounts only in hosted test processes and prevents test wipes from deleting unprefixed entries.
• Redirects the installation marker to a test-only Documents subdirectory.
• Snapshots and restores the pending channel-migration preference.
• Gates destructive legacy migration cleanup behind an explicit environment variable and updates CI lane selection.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  U[Unit test process] --> T{Env.isUnitTest}
  T -->|true| K[Test-prefixed keychain accounts]
  T -->|true| M[Documents/unit-tests marker]
  K --> W[Test wipe deletes only prefixed accounts]
  D[RN migration cleanup suite] --> G{BITKIT_DESTRUCTIVE_TESTS=1}
  G -->|false| S[Skip without destructive teardown]
  G -->|true in integration lane| C[Clean legacy mmkv, ldk, and RN keychain data]

Loading

_{Reviews (1) · Last reviewed commit: "test: gate the RN migration cleanup suit..."}

[jvsena42]

One MEDIUM (test-only): the destructive RN cleanup suite now runs in no lane. Details inline.

Checked and clean:

• Env.isUnitTest is XCTestConfigurationFilePath != nil in a static let. It is never true in Release/TestFlight, in a plain Debug run, in the XCUIApplication-launched app used by E2E and the Trezor UI tests, or in BitkitNotification, so no shipped path reads or writes prefixed accounts.
• Every Keychain entry point (save, update, delete, upsert, load, plus the helpers that delegate to them) goes through account(for:). The direct SecItem* calls in MigrationsService key RN items by service and TrezorCredentialStorage uses its own service, so neither is inconsistent with the prefix.
• InstallationMarker: when not under test the path is unchanged Documents, and createDirectory(withIntermediateDirectories:) is a no-op there. The handleOrphanedKeychain logic is untouched. The marker-deleting suites now only touch Documents/unit-tests/.
• RNMigrationCleanupTests throws its skip before any deletion, and tearDown is gated on didRunDestructiveSetUp, so the skipped path runs nothing destructive.
• The -only-testing/-skip-testing names match real classes. TxBumpingTests no longer exists.
• KeychainTests asserts through Keychain.account(for:) and the access group, so the tests are not vacuous. The ChannelMigrationPersistenceTests snapshot restores rnPendingChannelMigration in a teardown block.

[ovi-reviewer]

Verdict: ✅ Approve

---

Reaudit: diff 2 files.

Findings:
N/A

Audit:
Audited - no findings.

Coverage:
QA: Manual Tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test

---

Reviewed by gpt-6-astra-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer test · retest · audit (author or owner)

[piotr-iohk]

QA reviewed on cc4ff06.

Reviewed the keychain account prefix, installation-marker redirect, RN cleanup gate, and the two tests that fail if those namespaces are removed; no device run this round, test-only / no UI.

No findings.

Checked and clean

• Env.isUnitTest is XCTestConfigurationFilePath; prefixed accounts and Documents/unit-tests only apply in the XCTest-hosted process, not Debug / E2E / notifications.
• wipeEntireKeychain under test skips un-prefixed accounts; KeychainTests plants a throwaway probe so removing that skip goes red.
• InstallationMarker.delete() is asserted not to remove Documents/.bitkit_installed; a missing real marker is still what handleOrphanedKeychain uses on a normal launch.
• RNMigrationCleanupTests is skipped in the unit lane, opted in via TEST_RUNNER_BITKIT_DESTRUCTIVE_TESTS on integration, and tearDown does nothing if setUp skipped.
• Channel migration tests snapshot rnPendingChannelMigration instead of nilling it.

QA LGTM

[ovitrif]

utAck