Migrate UIKit navigation to SwiftUI NavigationStack

.claude/agents/change-reviewer.md

@@ -14,9 +14,10 @@ Review all recent code changes thoroughly and provide a structured, actionable a
 
 ## Project Context
 
-- **Architecture**: MVVM-C with Combine (main branch), NavigationStack + Combine (navigation-stack branch), AsyncSequence + @Observable (observation branch)
+- **Branch**: feature/navigation-stack — Pure SwiftUI, NavigationPath, single AppCoordinator (ObservableObject), Combine
 - **Packages**: `FunCore` → `FunModel` → `FunViewModel` / `FunServices` → `FunUI` → `FunCoordinator`
 - **Dependency direction**: Never import upward. ViewModel must NOT import UI or Coordinator.
+- **UIKit**: Zero UIKit in this branch — flag any `import UIKit` as a critical issue
 - **DI**: ServiceLocator with `@Service` property wrapper, session-scoped (LoginSession / AuthenticatedSession)
 - **Testing**: Swift Testing framework, mocks in FunModelTestSupport
 - **Lint**: SwiftLint with custom rules (no_print, weak_coordinator_in_viewmodel, no_direct_userdefaults)
@@ -33,26 +34,23 @@ Review all recent code changes thoroughly and provide a structured, actionable a
 - **Similar patterns elsewhere**: Search the codebase for code following the same pattern. If the same improvement applies elsewhere, flag each location.
 - **Consistency**: Do changes follow existing patterns?
 - **No orphaned references**: Stale imports, unused variables, dead code paths?
-- **Edge cases**: Boundary conditions, nil/optional handling, error paths?
 
 ### Step 3: Architecture Check
 - Package dependency direction respected?
-- No `import UIKit` in ViewModel or Model
+- No `import UIKit` — pure SwiftUI branch
 - No coordinator references in ViewModels (except weak closures)
 - No `print()` — use LoggerService
 - No `UserDefaults.standard` outside Services
-- Navigation logic only in Coordinators
+- Navigation logic only in Coordinators (AppCoordinator)
+- NavigationPath mutations only in coordinator, not in Views
 - Protocols in Core (reusable) or Model (domain), never in Services/ViewModel/UI/Coordinator
-- Detect which branch you're on and enforce the right reactive pattern:
-  - `main`: Combine + UIKit coordinators
-  - `feature/navigation-stack`: Combine + NavigationPath + ObservableObject
-  - `feature/observation`: AsyncSequence + StreamBroadcaster + @Observable, zero Combine
+- Reactive pattern: Combine (`@Published`, `@StateObject`, `@ObservedObject`, `.sink`)
 
 ### Step 4: Correctness Check
 - **Logic errors**: Algorithms, conditions, control flow
 - **Type safety**: Force unwraps, force casts, unsafe assumptions
-- **Concurrency**: `@MainActor` isolation, `Sendable` conformance, thread safety (Swift 6 strict)
-- **Memory management**: `[weak self]` in closures, no retain cycles. `self?.` preferred over `guard let self` for async ViewModel work.
+- **Concurrency**: `@MainActor` isolation, `Sendable` conformance, Swift 6 strict
+- **Memory management**: `[weak self]` and `[weak coordinator]` in closures
 - **API contracts**: Public interfaces used correctly
 
 ### Step 5: Quality Check
@@ -91,15 +89,7 @@ Ship it | Minor fixes needed | Needs significant work
 1. **Be calibrated**: This is a demo/portfolio app. Don't demand enterprise patterns.
 2. **Be specific**: Reference exact files and lines. No vague feedback.
 3. **Be actionable**: Every finding must include a concrete recommendation.
-4. **Don't over-engineer**: If the codebase uses a pattern (e.g., `fatalError` for service resolution), don't flag it.
+4. **Don't over-engineer**: If the codebase uses a pattern, don't flag it.
 5. **Focus on the diff**: Review what changed, not pre-existing code.
 6. **Verify before flagging**: Read actual code before claiming something is missing.
 7. **Count honestly**: Fewer than 3 issues? That's fine. Don't inflate.
-
-## Self-Verification
-
-Before delivering your review:
-- Re-read each finding: "Is this actually a problem, or am I being overly cautious?"
-- "Did I miss any changed files?"
-- "Are my recommendations correct and compatible with the codebase?"
-- "Would I stand behind each finding in a review discussion?"

.claude/skills/cross-platform/SKILL.md

@@ -10,7 +10,7 @@ args: "<feature-name>"
 Compare the implementation of a feature across Fun-iOS and Fun-Android to find unintentional divergences.
 
 ## Project Paths
-- **iOS**: `~/Documents/Source/Fun-iOS/`
+- **iOS**: `~/Documents/Source/Fun-iOS-NavigationStack/`
 - **Android**: `~/Documents/Source/Fun-Android/`
 
 ## Steps

.claude/skills/pull-request/SKILL.md

@@ -19,6 +19,7 @@ Create a draft PR following the team's quality standards.
    - `git diff main...HEAD` to review all changes
    - Verify package dependency direction isn't violated
    - Check for any `print()`, `UserDefaults.standard`, or other anti-patterns
+   - Verify zero UIKit imports (this branch is pure SwiftUI)
 
 3. **Accessibility checklist** (for UI changes)
    - Dynamic Type: Do text elements scale with user font size preference?

.claude/skills/review/SKILL.md

@@ -20,20 +20,20 @@ Review all recent code changes for completeness, correctness, and consistency wi
 
 3. **Architecture check**
    - Verify package dependency direction: `Coordinator → UI → ViewModel → Model → Core`, `Services → Model → Core`
-   - No `import UIKit` in ViewModel or Model
+   - No `import UIKit` anywhere — this branch is pure SwiftUI
    - No coordinator references in ViewModels (except weak closures)
    - No `print()` — use LoggerService
    - No `UserDefaults.standard` outside Services
    - Navigation logic only in Coordinators
    - Protocols in Core (reusable) or Model (domain), never in Services/ViewModel/UI/Coordinator
-   - Branch-specific: Combine patterns (this branch uses Combine + UIKit coordinators)
+   - Branch-specific: Combine + NavigationPath + single AppCoordinator (ObservableObject)
 
 4. **Similar pattern search**
    - Search the codebase for code that follows the same pattern as what changed
    - If the same improvement should be applied elsewhere, flag each location
 
 5. **Correctness check**
-   - Logic errors, type safety, concurrency (Swift 6 strict), memory management (`[weak self]`)
+   - Logic errors, type safety, concurrency (Swift 6 strict), memory management (`[weak self]`, `[weak coordinator]`)
    - Verify `@MainActor` isolation, `Sendable` conformance where needed
 
 6. **Cross-platform parity**

CLAUDE.md

@@ -45,21 +45,27 @@ Coordinator → UI → ViewModel → Model → Core
 Never import upward. ViewModel must NOT import UI or Coordinator. Model must NOT import Services.
 
 ## Anti-Patterns (Red Flags)
-- `import UIKit` in ViewModel or Model packages — UIKit belongs in UI and Coordinator only
+- `import UIKit` anywhere — this branch is pure SwiftUI, zero UIKit
 - Coordinator references in ViewModels (except weak optional closures) — retain cycle risk
 - `print()` anywhere — use LoggerService
 - `UserDefaults.standard` outside Services — use FeatureToggleService
 - Adding `fatalError()` for missing services — ServiceLocator.resolve() already crashes with `fatalError` if a service isn't registered; don't add redundant guards
-- Navigation logic in Views — navigation decisions belong in Coordinators only
+- Navigation logic in Views — all navigation (push, pop, tab switch, modal present/dismiss) must go through named AppCoordinator methods (`showDetail`, `selectTab`, `showProfile`, etc.), never inline property manipulation like `coordinator.homePath.append(item)` or `coordinator.isProfilePresented = true`
 - Protocol definitions in Services — domain protocols go in Model, reusable abstractions in Core
+- Wrong ownership annotations — tab content wrappers must use `@StateObject` to own ViewModels (not `@ObservedObject`). `@ObservedObject` on a ViewModel means it gets recreated on every re-render. Conversely, the coordinator must be `let` or `@ObservedObject` (not `@StateObject`) since the wrapper doesn't own it.
 
-## Architecture (this branch: main)
-- **Entry point**: UIKit `AppDelegate` + `SceneDelegate` (scene-based lifecycle)
-- **Navigation**: 6 UIKit coordinators — `AppCoordinator`, `BaseCoordinator`, `LoginCoordinator`, `HomeCoordinator`, `ItemsCoordinator`, `SettingsCoordinator`
-- **Views**: SwiftUI views embedded in UIHostingController via UIViewControllers
-- **Reactive**: Combine (`@Published`, `CurrentValueSubject`, `.sink`)
-- **ViewModel → Coordinator**: Optional closures (`onShowDetail`, `onShowProfile`, etc.)
-- **DI**: Session-scoped ServiceLocator — no `.shared` singleton. Each `Session` creates and owns its own `ServiceLocator`. On session transition, the old ServiceLocator is released with the session (no stale services). `@Service` property wrapper resolves via `static subscript(_enclosingInstance:)` from the enclosing type's `serviceLocator` (requires `ServiceLocatorProvider` conformance). Coordinators and ViewModels receive the current session's ServiceLocator via constructor injection.
+## Architecture (this branch: feature/navigation-stack)
+- **Entry point**: SwiftUI `@main App` struct (`FunApp.swift`) — no AppDelegate or SceneDelegate
+- **Navigation**: Single `AppCoordinator: ObservableObject, SessionProvider` with per-tab `NavigationPath`
+- **Views**: Pure SwiftUI views, no UIHostingController or UIViewControllers
+- **Reactive**: Combine (`@Published`, `@StateObject`, `@ObservedObject`, `.sink`)
+- **ViewModel → Coordinator**: Optional closures wired in tab content wrappers via `.task { viewModel.onShowDetail = { ... } }`
+- **Tab bar**: SwiftUI `TabView(selection: $coordinator.selectedTab)`
+- **Push nav**: `coordinator.showDetail(item, in: .home)` — named methods on AppCoordinator
+- **Modals**: `.sheet(isPresented: $coordinator.isProfilePresented)`
+- **DI**: Session-scoped ServiceLocator — no `.shared` singleton. Each `Session` creates and owns its own `ServiceLocator`. On session transition, the old ServiceLocator is released with the session (no stale services). `@Service` property wrapper resolves via `static subscript(_enclosingInstance:)` from the enclosing type's `serviceLocator` (requires `ServiceLocatorProvider` conformance). Coordinators and ViewModels receive the current session via constructor injection.
+- **Coordinator-owned views**: `AppRootView`, `MainTabView`, and tab content wrappers live in `Coordinator` (not `FunUI`) because they depend on `AppCoordinator`. Moving them to `FunUI` would create a circular dependency (`Coordinator → UI → Coordinator`). Pure reusable views (`HomeView`, `DetailView`, etc.) stay in `FunUI`.
+- **Ownership wrappers**: Tab content wrappers (`HomeTabContent`, `ItemsTabContent`, etc.) use `@StateObject` to **own** their ViewModel and `@ObservedObject` (or `let`) for the coordinator passed from the parent. `@StateObject` ensures the ViewModel survives re-renders; `@ObservedObject` means the wrapper doesn't own the coordinator. Pure views in `FunUI` take `@ObservedObject var viewModel` since the wrapper owns it.
 
 ## Rule Index
 Consult these files for detailed guidance (not auto-loaded — read on demand):
@@ -69,8 +75,9 @@ Consult these files for detailed guidance (not auto-loaded — read on demand):
 
 ## Code Style
 - Swift 6 strict concurrency, iOS 17+
-- SwiftUI + UIKit hybrid, MVVM-C with Combine
-- ViewModels use closures for navigation (no coordinator protocols)
+- Pure SwiftUI (NavigationStack), MVVM-C with Combine
+- Single AppCoordinator: ObservableObject with @Published NavigationPath per tab
+- ViewModels use closures for navigation, wired in tab content wrappers
 - Navigation logic ONLY in Coordinators, never in Views
 - Protocol placement: Core = reusable abstractions, Model = domain-specific
 - Session-scoped ServiceLocator with `@Service` property wrapper — ViewModels conform to `SessionProvider`, store `let session: Session`

Coordinator/Package.swift

@@ -4,8 +4,8 @@ import PackageDescription
 let package = Package(
     name: "Coordinator",
     platforms: [
-        .iOS(.v15),
-        .macCatalyst(.v15),
+        .iOS(.v16),
+        .macCatalyst(.v16),
     ],
     products: [
         .library(name: "FunCoordinator", targets: ["FunCoordinator"]),