Skip to content

refactor: delete legacy migration code from AppState (Stage 2) - #32

Merged
DocNR merged 2 commits into
mainfrom
chore/delete-legacy-migrations
May 7, 2026
Merged

refactor: delete legacy migration code from AppState (Stage 2)#32
DocNR merged 2 commits into
mainfrom
chore/delete-legacy-migrations

Conversation

@DocNR

@DocNRDocNR commented May 7, 2026

Copy link
Copy Markdown
Owner

Summary

Stage 2 of the AppState god-object split. Deletes the build-31 → multi-account migration code that's no longer needed.

The data: App Store Connect shows 4/40 users on legacy versions, none active. The worst case for those inactive users is OnboardingView + nsec re-import on next launch — equivalent to a normal reinstall. Given the build 31 → 64 gap (33+ builds, weeks/months), their proxy-side state has likely already decayed beyond what migration could preserve.

The original BACKLOG entry ("move as-is, do NOT delete") was based on the assumption that legacy users were active. The data refutes that. Cleaner to delete than to extract dead code into a separate file with a TODO-delete comment.

Numbers

FileBeforeAfterΔ
Clave/AppState.swift2,261 LOC1,934 LOC−327
ClaveTests/AppStateMultiAccountTests.swift487 LOC291 LOC−196
Total−540 / +16

AppState.swift is −14.5% in this PR alone.

What was deleted

From Clave/AppState.swift:

  • bootstrapFromLegacyKeychainIfNeeded (one-shot build-31 → multi-account)
  • legacyCachedProfile (helper for bootstrap)
  • migrateLegacyUserDefaultsKeys (UserDefaults shape change)
  • migrateLegacyProfileImageFile (disk filename change)
  • migrateRemainingLegacyKeysIfNeeded (cross-version upgrade catch-up)
  • cleanupEmptySignerRowsIfSafe (build-32 regression backfill, also dead — build 32 was 30+ builds ago)
  • backfillSignerPubkeyHex (helper for the above)
  • 3 call sites in loadState() + their step comments
  • // MARK: - Multi-account loading + bootstrap (Task 5) header
  • Stale doc-comment reference in cachedImageURL to a deleted method

From ClaveTests/AppStateMultiAccountTests.swift:

  • 8 tests that exclusively exercised the migration paths (set up legacy state → call loadState() → assert migrated)
  • 3 stale MARK section headers
  • Class doc comment updated to reflect the new surface

What was kept

  • recoverAccountsFromKeychainIfNeeded — NOT legacy migration code. Handles the Apple-documented "iOS Storage settings UserDefaults wipe" scenario where Keychain persists but UserDefaults is empty (e.g., user clears "App Data" via iOS Settings → Storage).
  • cleanupOrphanLegacyKeychainEntry — sunset candidate. Kept for one more cycle as a defensive sweep for any pre-existing build-31-era bootstrap orphan that was never cleaned. Doc-comment updated to flag this. Safe to delete in a few build cycles.
  • The 3 corresponding tests for the kept methods.

Test plan

  • xcodebuild test -scheme Clave -destination 'platform=iOS Simulator,name=iPhone 17,OS=26.4' -skip-testing:ClaveUITests on main (pre-baseline): 240 passed / 0 failed
  • Same command on this branch: 232 passed / 0 failed (240 − 8 deleted tests) ✅
  • ** TEST SUCCEEDED ** in both runs
  • Zero remaining references to deleted method names in any .swift file (grep -rnE "bootstrapFromLegacy|migrateLegacy*|backfillSignerPubkeyHex|legacyCachedProfile|cleanupEmptySigner" --include="*.swift" → empty)
  • After merge: separate chore/pbxproj-build-65 PR bumps CURRENT_PROJECT_VERSION 64 → 65; archive + on-device verify (fresh install + multi-account flow exercises live loadState() path)

Worst-case impact for the 4 inactive legacy users

If any of them auto-updates and opens the app:

  • They see OnboardingView
  • They re-import their nsec (paper backup, password manager, iCloud Keychain — wherever they stored it)
  • They re-pair their NIP-46 clients
  • Profile cache, activity log, prior pairings = gone

This is the same UX as any normal reinstall scenario. Given the build 31 → 64 gap, their server-side proxy state was likely going to need re-pairing anyway.

Stages 3-4

ProfileFetcher, NostrConnectCoordinator, ProxyClient, AccountManager, PendingApprovalCoordinator extractions remain in BACKLOG. Each gets its own brainstorm + plan + PR cycle.

🤖 Generated with Claude Code

DocNRand others added 2 commits May 7, 2026 16:46
Stage 2 of the AppState god-object split. Deletes the build-31 →
multi-account migration code that's no longer needed: App Store
Connect data shows 4/40 users on legacy versions, none active. The
worst case for those inactive users is OnboardingView + nsec re-import
on next launch, equivalent to a normal reinstall — and given the
build 31 → 64 gap, their proxy-side state has likely already decayed
beyond what migration could preserve.
Deleted from AppState.swift (-330 LOC):
- bootstrapFromLegacyKeychainIfNeeded
- legacyCachedProfile
- migrateLegacyUserDefaultsKeys
- migrateLegacyProfileImageFile
- migrateRemainingLegacyKeysIfNeeded
- cleanupEmptySignerRowsIfSafe (build-32 regression backfill, also dead)
- backfillSignerPubkeyHex
- 3 call sites in loadState() + their step comments
- "// MARK: - Multi-account loading + bootstrap (Task 5)" header
- Stale doc-comment reference in cachedImageURL
Deleted from ClaveTests/AppStateMultiAccountTests.swift (-200 LOC):
- testBootstrap_legacyKeychainEntryPresent_seedsAccountsKey
- testBootstrap_freshInstall_noLegacyState_isNoOp
- testBootstrap_backfillsSignerPubkeyHexOnLegacyRecords
- testBootstrap_migratesLegacyCachedProfile_intoAccountProfile
- testBootstrap_migratesLegacyBunkerSecret_intoPerSignerDict
- testBootstrap_migratesLegacyLastContactSet_intoPerSignerDict
- testBootstrap_migratesLegacyRegisterTimestamps
- testMigrateRemainingLegacyKeys_idempotentlyCleansUpAfterPriorBootstrap
- 3 stale MARK section headers
- Class doc comment updated to reflect the new surface
Kept:
- recoverAccountsFromKeychainIfNeeded — NOT legacy migration code.
Handles the Apple-documented "iOS Storage settings UserDefaults
wipe" scenario where Keychain persists but UserDefaults is empty
(e.g., user deletes "App Data" via iOS Settings → Storage).
- cleanupOrphanLegacyKeychainEntry — sunset candidate, kept for one
more cycle as a defensive sweep for any pre-existing build-31-era
bootstrap orphan that was never cleaned. Doc updated to flag this.
- testReinstallRecovery_seedsAccountsFromKeychain (×2)
- testCleanupOrphanLegacyKeychainEntry_idempotent
Verification:
- xcodebuild test -skip-testing:ClaveUITests on iPhone 17 / iOS 26.4:
- Pre-baseline (main @ b4c1b72): 240 passed / 0 failed
- Post-deletion (this branch): 232 passed / 0 failed (240 - 8)
- ** TEST SUCCEEDED ** in both runs, identical surviving tests pass
AppState.swift: 2,261 -> 1,934 LOC (-327, -14.5%).
Total diff: -540 / +16 across two files.
Stages 3-4 of the AppState split (ProfileFetcher, NostrConnectCoordinator,
ProxyClient, AccountManager, PendingApprovalCoordinator) remain in
BACKLOG; each gets its own PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three doc/comment blocks referenced the now-deleted Task 8 migration:
- loadCachedProfileImage doc said 'Task 8 will move' the legacy
per-pubkey image filename — but the cachedImageURL computed property
already returns the per-pubkey path. Stale present-tense aspiration
about deleted code.
- deleteAccount step 6 sweep called the legacy global 'profile_image.jpg'
cleanup 'meaningful only for users who never went through Task 8
migration' — reframe as defensive build-31-era orphan sweep since
there's no migration path now.
- An archaeological comment block above renamePetname referenced
'Task 8 will explicitly remove' the legacy cachedProfileKey — also
stale; trimmed to just describe current state.
Comment-only changes; no executable code touched. xcodebuild test
result unchanged (232/232).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@DocNR
DocNR merged commit 7a4641a into mainMay 7, 2026
@DocNR
DocNR deleted the chore/delete-legacy-migrations branch May 7, 2026 20:53
DocNR added a commit that referenced this pull request May 7, 2026
…light (#33)
Internal-only build for on-device verification of the AppState legacy
migration deletion (#32). Bumps CURRENT_PROJECT_VERSION 64 -> 65
across all 4 targets (Clave, ClaveTests, ClaveUITests, ClaveNSE) in
both Debug and Release configs.
Stage 2 verification gates:
- xcodebuild test passes pre/post (240/240 -> 232/232; 8 deleted
tests covered the deleted migration paths) ✓
- Build 65 archives + uploads to TestFlight (this PR enables)
- Multi-account flow exercises live loadState() path on device:
fresh install, addAccount, switchAccount, deleteAccount,
scenePhase .active re-register, nostrconnect pair + sign
Stages 3-4 of the AppState god-object split remain in BACKLOG.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@DocNR