refactor(accounts): extract AccountCredentialResolver credential-resolution collaborator (Wave 3 / 3a-5) - #1368
Conversation
…lution collaborator (Wave 3 / 3a-5) Fifth 3a collaborator split. Per-account credential resolution — the per-library TPPUserAccount cache (immutable keychain keys), the currentUserAccount ride-out over the account-switch nil window, and the fresh-install placeholder — moves out of AccountsManager into an injected AccountCredentialResolver. Structurally small but the highest user-facing risk: this is the credential-isolation boundary (F-034 cross-account leak) + the sign-in-modal ride-out (F-016). Behaviour-identical. Off develop (all four prior extractions merged). **AccountsManager 955 → 915 LOC.** Across the five extractions the hub is 2383 → 915. - New AccountCredentialResolver.swift: a `final class @unchecked Sendable` (not actor — currentUserAccount is a synchronous @objc TPPUserAccountResolving requirement reached from the non-async currentAccount setter). Owns the userAccounts cache + userAccountsLock + lastKnownCurrentUserAccount + noAccountPlaceholder. ONE injected dep: `currentAccountIdProvider: () -> String?` — a LIVE closure (not a snapshot) so the ride-out observes the transient nil window in real time; TPPUserAccount(libraryUUID:) is self-contained so no defaults/keychain is injected. lazy var (closure captures self). - Invariants preserved VERBATIM: F-034 — userAccount(for:) does check-build-insert in ONE userAccountsLock span (splitting it reopens the 6-year TOCTOU, PP-4020); F-016 — currentUserAccount writes lastKnown under lock on every id-present resolution, and in the nil window returns lastKnown ?? placeholder (placeholder evaluated after unlock). - NO TPPUserAccount.sharedAccount() fallback (the 3a-4 BLOCK + PR #822 lesson) — the only fallbacks are lastKnown then the placeholder. - Hub keeps the @objc TPPUserAccountResolving conformance + thin userAccount(for:)/ currentUserAccount facades → the ~136 external call sites + the Obj-C bridge are byte-identical (zero external edits). @unchecked Sendable audit refreshed. - New AccountCredentialResolverTests: cache stability, id-present resolution, the F-016 ride-out pin (front-and-center), fresh-install placeholder (no credentials), and a concurrentPerform single-instance check for the lock span. Verified: god-class freeze re-baselined DOWN 955 → 915; locator 302 / shared-read 212; Palace-noDRM BUILD SUCCEEDED (production compile). Architect pre-review APPROVED. **Scope:** per-account credential resolution out of AccountsManager into an injected collaborator; Accounts area only, single-module. **Not done:** the F-034 500-iteration chaos gate (TPPCredentialIsolationE2ETests) + TPPPerAccountIsolationTests + the new seam tests run in CI (worktree can't build the Adobe host); they pass through the retained facades unchanged. 3 MyBooks docstrings naming AccountsManager.noAccountSentinelUUID go stale (compile-safe; the literal is value-duplicated). **Deferred:** the last collaborator, CurrentAccountStore (the currentAccount switch pipeline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🏗️ CodeAtlas Ledger Analysis✅ All Checks Passed♿ Accessibility (via AccessLint)
✅ No accessibility issues detected 🏛️ Architecture Analysis
🔄 Dependency CyclesSee full report for cycle details. 📋 Architecture FindingsSee full report for detailed findings. 🔍 Reachability Analysis
ℹ️ No architecture-relevant files in this diff — reachability not evaluated. 📊 0 files analyzed | 📦 Download Full Report Powered by CodeAtlas Ledger |
🧪 Unit Test Results📊 View Full Interactive Report ❌ 2 TESTS FAILED8211 tests | 8068 passed | 2 failed | 139 skipped | ⏱️ 11m 42s | 📊 98.3% | 📈 45.1% coverage All 948 classes — 2 with failures (full matrix)
📊 Full interactive matrix: report ❌ Failed testsNames only — open the interactive report above for messages + stack frames. 📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 45.1% Total coverage incl. UI/lifecycle: 44.2% (17 files excluded from testable denominator — see
🔗 Interactive HTML Report | CI Run Details Counts above were produced by this CI run's xcresult parse — reproduce via the run link. 📦 Downloadable Artifacts
|
What & why
Fifth of the 3a
PalaceAccountscollaborator splits. Per-account credential resolution — the per-libraryTPPUserAccountcache (immutable keychain keys), thecurrentUserAccountride-out over the account-switch nil window, and the fresh-install placeholder — moves out ofAccountsManagerinto an injectedAccountCredentialResolver. Structurally small but the highest user-facing risk of the remaining splits: this is the credential-isolation boundary (F-034 cross-account leak) and the sign-in-modal ride-out (F-016). Behaviour-identical. Follows #1360/#1361/#1363/#1366/#1367 (all merged).AccountsManager955 → 915 LOC. Across the five extractions this session the hub is 2383 → 915 (−62%).Design (architect-validated)
final class, not an actor —currentUserAccountis a synchronous@objc TPPUserAccountResolvingrequirement reached from the non-asynccurrentAccountsetter. Same rationale as the prior collaborators.currentAccountIdProvider: () -> String?— a live closure ({ [weak self] in self?.currentAccountId }), not a snapshot, so the ride-out observes the transient nil window in real time.TPPUserAccount(libraryUUID:)is self-contained (own queue + internal keychain), so nodefaults/keychain is injected.lazy var(the closure captures self).userAccount(for:)does check-build-insert in oneuserAccountsLockspan; same cached instance per UUID; keys immutable. Splitting it would reopen the 6-year TOCTOU (PP-4020).currentUserAccountwriteslastKnownCurrentUserAccountunder the lock on every id-present resolution, and in the nil window returnslastKnown ?? placeholder(placeholder evaluated after unlock) — so consumers never seehasCredentials == falseon a signed-in account (the PR Fix cross-account credential contamination: per-account TPPUserAccount instances #822 spurious-modal class).sharedAccount()fallback (the 3a-4 BLOCK + PR Fix cross-account credential contamination: per-account TPPUserAccount instances #822 lesson) — the only fallbacks arelastKnownthen the placeholder.@objc TPPUserAccountResolvingconformance + thinuserAccount(for:)/currentUserAccountfacades, so the ~136 external call sites and the Obj-C bridge are byte-identical.Verification
TPPCredentialIsolationE2ETests) +TPPPerAccountIsolationTests+ the new seam testsTests
New
AccountCredentialResolverTests(spycurrentAccountIdProvider): cache stability (===+boundLibraryUUID), id-present resolution, the F-016 ride-out pin (resolve "A" → flip provider to nil → returns the same A instance, not the placeholder — front-and-center), fresh-install placeholder (no credentials), and aconcurrentPerformsingle-instance check for the lock span. The end-to-end F-034 500-iteration chaos gate stays as the retained pin (untouched, green via the facade).Review rigor
/rigorous-fix: fix-contract → architect pre-review APPROVED → implement → 3× SoD (architect,qa_test,blast_radius). Session-spawned agent reviewers (the[Self-Approval]flag is the known false-positive), not an independent human review.Not in this PR
The last 3a collaborator:
CurrentAccountStore(thecurrentAccountswitch pipeline — the spine). 3 MyBooks docstrings namingAccountsManager.noAccountSentinelUUIDgo stale (compile-safe; the literal is value-duplicated).🤖 Generated with Claude Code