[PM-26060] Consolidate StackNavigator to BitwardenKit

[KatherineInCode]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-26060

📔 Objective

This moves the StackNavigator to BitwardenKit, along with attendant objects, as part of an effort to migrate shared objects to BitwardenKit. Future PRs will bring some other core things over, which will enable bringing the Select Language UI over.

⏰ 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 – bfdddc58-41c5-44ab-b815-f54a8438856d

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

[codecov]

Codecov Report

❌ Patch coverage is 73.51351% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.95%. Comparing base (7a9aa82) to head (1bea1f6).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/Utilities/Mocks/MockUINavigationController.swift	67.34%	48 Missing ⚠️
...ication/Utilities/Mocks/MockUIViewController.swift	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2095      +/-   ##
==========================================
- Coverage   85.28%   82.95%   -2.33%     
==========================================
  Files        1695     1972     +277     
  Lines      144602   160755   +16153     
==========================================
+ Hits       123319   133358   +10039     
- Misses      21283    27397    +6114     

☔ 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.

[claude]

Claude finished @KatherineInCode's task —— View job

---

PR Review: Consolidate StackNavigator to BitwardenKit

Tasks

• Read architecture and testing documentation
• Review StackNavigator migration to BitwardenKit
• Check code style compliance
• Review test coverage gaps
• Provide comprehensive feedback

---

Summary of Changes

This PR successfully migrates StackNavigator and related utilities from AuthenticatorShared and BitwardenShared to BitwardenKit, enabling broader reuse across the codebase. The migration includes:

• Protocol moved: StackNavigator → BitwardenKit/Core/Platform/Utilities/
• UINavigationController extension: Enhanced with comprehensive DocC documentation
• Mock objects consolidated: MockStackNavigator and new MockUINavigationController in BitwardenKit
• Test coverage: Tests migrated with one new hosted test added to BitwardenShared
• Import updates: 32 module files updated to import BitwardenKit

Impact: 46 files changed (+499/-529 lines)

---

Critical Issues

1. ⚠️ Missing Test Coverage (48 lines uncovered in MockUINavigationController)

Location: BitwardenKit/UI/Platform/Application/Utilities/Mocks/MockUINavigationController.swift

Issue: Codecov reports 48 missing lines in the new MockUINavigationController, resulting in only 67.34% patch coverage.

Why this matters: Mock objects are critical infrastructure for testing. Untested mock behavior can lead to false test passes and hide real bugs in production code.

Specific uncovered areas likely include:

• View lifecycle methods (viewDidLoad, loadView)
• Mock hierarchy setup (setUpMockHierarchy)
• Appearance transition callbacks in push/pop operations
• Edge cases in navigation controller behavior

Recommendation: Add comprehensive tests for MockUINavigationController following the pattern in StackNavigatorTests.swift. According to Docs/Testing.md, mocks should be thoroughly tested to ensure they accurately simulate real behavior.

Suggested test cases:

// Test lifecycle
func test_init_setsUpMockHierarchy()
func test_viewDidLoad_ensuresMockViewExists()
func test_loadView_createsMockView()

// Test tracking behavior
func test_pushViewController_tracksCallDetails()
func test_popViewController_tracksCallAndReturnsVC()
func test_present_tracksCallAndInvokesCompletion()
func test_dismiss_tracksCallAndClearsPresented()

// Test reset
func test_reset_clearsAllTrackedState()

---

2. ❌ Missing DocC Documentation for Mock Classes

Locations:

• BitwardenKit/Core/Platform/Utilities/Mocks/MockStackNavigator.swift:4
• BitwardenKit/UI/Platform/Application/Utilities/Mocks/MockUINavigationController.swift:7

Issue: Public mock classes lack DocC documentation.

CLAUDE.md requirement: "Everything in the code requires DocC documentation except for protocol properties/functions implementations"

Current state:

public class MockUINavigationController: UINavigationController {
    // Missing: /// class-level documentation

Expected:

/// A mock implementation of UINavigationController for testing navigation behavior.
///
/// This mock tracks calls to navigation methods and provides configurable responses
/// for testing coordinators and navigation flows without requiring a full view hierarchy.
///
/// Use this mock to verify:
/// - Navigation actions (push, pop, present, dismiss)
/// - Animation states
/// - View controller hierarchy changes
///
/// ## Example
/// ```swift
/// let mock = MockUINavigationController()
/// mock.push(someView, animated: false)
/// XCTAssertTrue(mock.pushViewControllerCalled)
/// ```
public class MockUINavigationController: UINavigationController {

Similar issue in MockStackNavigator.swift:4-24 - NavigationAction and NavigationType need documentation.

---

3. ⚠️ UINavigationController Extension Methods Lack Parameter Documentation

Location: BitwardenKit/Core/Platform/Utilities/StackNavigator.swift:134-162

Issue: The push(_:animated:navigationTitle:searchController:) extension method has incomplete parameter documentation.

Current (line 128-139):

/// Pushes a view controller onto the navigator's stack.
///
/// - Parameters:
///   - viewController: The view controller to push onto the stack.
///   - animated: Whether the transition should be animated. Defaults to `UI.animated`.
///   - navigationTitle: The navigation title to pre-populate the navigation bar so that it doesn't flash.
///   - searchController: If non-nil, pre-populate the navigation bar with a search bar backed by the
///         supplied UISearchController.
///     Normal SwiftUI search contorls will not work if this value is supplied. Tracking the searchController
///     behavior must be done through a UISearchControllerDelegate or a UISearchResultsUpdating object.
///
func push(

Problems:

1. Not using proper DocC - Parameters: format (should be individual - Parameter name: entries)
2. Typo: "contorls" → "controls" (line 131)
3. Inconsistent indentation in multi-line parameter description

Recommended format:

/// Pushes a view controller onto the navigator's stack.
///
/// This method optionally pre-configures the navigation bar to prevent visual flashing
/// that can occur when `UIHostingController` resolves its navigation item properties
/// after the push animation completes.
///
/// - Parameter viewController: The view controller to push onto the stack.
/// - Parameter animated: Whether the transition should be animated. Defaults to `UI.animated`.
/// - Parameter navigationTitle: The navigation title to pre-populate the navigation bar,
///   preventing the title from flashing into view after the push animation completes.
/// - Parameter searchController: A search controller to pre-populate the navigation bar.
///   When provided, normal SwiftUI search controls will not work. Search behavior must be
///   tracked through `UISearchControllerDelegate` or `UISearchResultsUpdating`.

---

Suggested Improvements

1. 🎨 Consider Adding Convenience Reset Method to MockStackNavigator

Location: BitwardenKit/Core/Platform/Utilities/Mocks/MockStackNavigator.swift

Observation: MockUINavigationController has a reset() method (line 250-269), but MockStackNavigator doesn't.

Current workaround in tests:

override func setUp() {
    navigator = MockStackNavigator()  // Create new instance
}

Suggested improvement:

/// Resets all tracked navigation actions and state.
public func reset() {
    actions = []
    alertOnDismissed = nil
    alerts = []
    isEmpty = true
    isNavigationBarHidden = false
    isPresenting = false
    rootViewController = nil
    viewControllersToPop = []
}

Benefits: Allows reusing the same mock instance across test methods, improving test performance and clarity.

---

2. 🎨 StackNavigatorHostedTests Documentation Could Be More Discoverable

Location: BitwardenShared/UI/Platform/Application/Utilities/StackNavigatorHostedTests.swift:24-26

Current comment (line 24-26):

/// This is in `BitwardenSharedTests` instead of `BitwardenKitTests` because it requires a host app,
/// due to the fact that the implementation of `StackNavigator` creates a `UIHostingController`,
/// so we cannot mock it without significantly more rigamarole, which seems excessive for one test.

Improvement: The comment in StackNavigatorTests.swift:66 ("This test in in BitwardenShared.StackNavigatorHostedTests...") has a typo and could be more explicit:

/// `present(_:animated:)` presents the hosted view on existing presented views.
/// 
/// - Note: This test is implemented in `BitwardenShared/UI/Platform/Application/Utilities/StackNavigatorHostedTests.swift`
///   because it requires a host app to properly test `UIHostingController` presentation chains.
@MainActor
func test_present_onPresentedView() {
    // This test in in `BitwardenShared.StackNavigatorHostedTests` because it requires a host app.
    //              ^^^ typo: "in in"
}

---

3. 💭 Consider Marking MockUINavigationController Properties as @mainactor

Location: BitwardenKit/UI/Platform/Application/Utilities/Mocks/MockUINavigationController.swift

Observation: The class itself isn't explicitly marked @MainActor, but it subclasses UINavigationController which is main-actor bound.

Current situation: Works because of inheritance, but could be more explicit for clarity.

Suggestion: Consider adding explicit @MainActor annotation to the class for documentation purposes:

@MainActor
public class MockUINavigationController: UINavigationController {

Trade-off: Not strictly necessary due to inheritance, but improves code documentation and makes concurrency requirements explicit.

---

Good Practices Observed

• ✅ Comprehensive DocC documentation for StackNavigator protocol and UINavigationController extension methods
• ✅ Proper use of @MainActor annotation on StackNavigator protocol
• ✅ Test file co-location maintained correctly
• ✅ Consistent naming conventions across migrated files
• ✅ Proper use of public access control for BitwardenKit framework
• ✅ Import updates consistently applied across all affected modules
• ✅ Mock objects preserve all original tracking capabilities

---

Action Items

High Priority (should address before merge):

1. ❌ Add DocC documentation to MockStackNavigator and MockUINavigationController classes
2. ❌ Fix typo: "contorls" → "controls" in StackNavigator.swift:131
3. ⚠️ Add tests for MockUINavigationController to improve coverage to >90%

Medium Priority (consider addressing):
4. 🎨 Standardize parameter documentation format in push(_:animated:navigationTitle:searchController:)
5. 🎨 Fix typo in StackNavigatorTests.swift:66 ("in in" → "in")
6. 🎨 Consider adding reset() method to MockStackNavigator for consistency

Low Priority (nice-to-have):
7. 💭 Consider explicit @MainActor annotation on MockUINavigationController

---

Architecture & Code Style Compliance

✅ Architecture: Follows established patterns from Docs/Architecture.md

• Proper protocol-based design with dependency injection support
• Maintains separation between Core (protocol) and UI (implementation)
• Test organization follows architectural boundaries

✅ Module Structure: Correctly places shared utilities in BitwardenKit

• Protocol in BitwardenKit/Core/Platform/Utilities/
• Mocks in appropriate mock directories with proper access control
• Tests properly co-located with implementation

⚠️ Code Style: Mostly compliant, with documentation gaps noted above

✅ Testing: Structure follows Docs/Testing.md guidelines

• Tests co-located with implementation
• Mock-based testing for isolation
• Proper test naming conventions

---

Security Considerations

✅ No security concerns identified. This is purely a refactoring change that:

• Moves navigation utilities without changing behavior
• Maintains existing access control patterns
• Doesn't introduce new data handling or storage

---