[PM-26606] Fix most strict warnings in AuthenticatorBridgeKit

[KatherineInCode]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-26606
https://bitwarden.atlassian.net/browse/PM-26770
https://bitwarden.atlassian.net/browse/PM-26754

📔 Objective

This turns on strict concurrency warnings in AuthenticatorBridgeKit and AuthenticatorBridgeKitTests, and then fixes most of the warnings, partly through @preconcurrency and partially through using a DispatchQueue to enforce thread safety.

There is one warning remaining in AuthenticatorBridgeKitTests, relating to TestData; this will be covered in PM-27143.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[github-actions]

Checkmarx One – Scan Summary & Details – 8b43968e-87a7-4eb1-83ad-240f293db51e

Great job! No new security vulnerabilities introduced in this pull request

[codecov]

Codecov Report

❌ Patch coverage is 8.33333% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.97%. Comparing base (8f4b365) to head (4a5d4cc).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...icatorBridgeKit/AuthenticatorBridgeDataStore.swift	8.33%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2054      +/-   ##
==========================================
- Coverage   85.31%   82.97%   -2.35%     
==========================================
  Files        1691     1988     +297     
  Lines      144372   161216   +16844     
==========================================
+ Hits       123166   133761   +10595     
- Misses      21206    27455    +6249     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fedemkr]

🤔 I was wondering whether we should move this to be an actor and then create persistentContainer.newBackgroundContext per function instead of reusing the same background context all the time which would fix these warnings as well.
Although not sure if that could cause any race condition on non-atomic usage of it by any of the functions.
Perhaps it's just better this way for now not to change a lot of stuff at once.

[KatherineInCode]

My understanding is that generally with CoreData you only want the one NSManagedObjectContext per NSPersistentContainer, which in this case is specific to the BWA-PM shared CoreData.

Another option was moving the initialization of the context into the initializer, but I assumed there was a good reason we made it lazy to begin with, probably along the performance line.

Moving it to an actor might not be a bad idea, though 🤔 Because looking at it again, I don't know that it makes sense for it to be nonisolated. The other option would be to make it run on the main actor, since that's the default that Swift is moving towards.

I can poke at it a bit.

[KatherineInCode]

I asked Claude for the implications of migrating the class to be an actor, and it generated:

Core Data Specific Concerns

The biggest concern would be how actor isolation interacts with Core Data's queue-based concurrency model. Core Data contexts have their own internal queues, and actor isolation might conflict with this, potentially leading to:

• Deadlocks if Core Data operations try to synchronously access actor-isolated properties
• Performance issues from double-queuing (actor queue + Core Data queue)

Recommendation

For this specific use case with Core Data, I'd recommend keeping it as a class because:

1. The current solution works well and maintains API compatibility
2. Core Data's queue-based concurrency model is already well-established
3. The performance overhead of making all access async might not be worth it
4. The manual synchronization is well-contained and documented

The actor model shines more for business logic objects rather than data store wrappers around frameworks like Core Data that have their own concurrency models.

[KatherineInCode]

Asking Claude more questions, its generation very much wants this class to be nonisolated, actually. And doing a bit of digging, I think that makes sense, especially as NSManagedObjectContext is itself marked nonisolated. Strict concurrency seems to interact weirdly with Core Data 🫠

[fedemkr]

Alright we can keep the class for now and then check whether we find a more up-to-date solution for this that is not a workaround to properly use it. I wouldn't use @MainActor as for long-running operations it will affect UI performance.
I'll research more about this during the week.

[KatherineInCode]

After some back-and-forth with Claude and with digging around in Apple forums and such, I think the best option is for the AuthenticatorBridgeDataStore to be nonisolated, so I've marked it explicitly such, to avoid issues if/when we switch the default isolation to MainActor in line with current Swift defaults.