Refactor ServiceLocator to session-scoped ownership by g-enius · Pull Request #7 · g-enius/Fun-iOS

g-enius · 2026-03-20T11:45:05Z

Summary

Each Session now creates and owns its own ServiceLocator instead of receiving a shared one from SceneDelegate
On session transition (login <-> main), the old ServiceLocator is released with the session — no stale services lingering in a shared registry
Session protocol extends ServiceLocatorProvider; LoginSession/AuthenticatedSession create their ServiceLocator in-line
SessionFactory.makeSession no longer takes a serviceLocator parameter
BaseCoordinator.serviceLocator changed from let to internal(set) var so AppCoordinator can update it on session transition
ServiceLocatorProvider protocol made @MainActor (all conformers already were)
SceneDelegate derives its serviceLocator from AppCoordinator instead of creating one

Test plan

All 83 unit tests pass
Build and run on simulator — verify login -> main -> logout flow works
Verify dark mode toggle still applies after session transitions
Confirm no retain cycles via Instruments (Leaks)

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

BaseCoordinator is now pure navigation. Each coordinator that creates ViewModels stores its own serviceLocator reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add SessionProvider protocol: auto-provides serviceLocator from session - ViewModels conform to SessionProvider, store session instead of serviceLocator - Coordinators pass session to VMs, no force unwraps - SceneDelegate resolves services directly, no ServiceLocatorProvider conformance - BaseCoordinator is pure navigation, no DI concerns - Tests use MockSession wrapper Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Session is created in init, activated in start(). No separate serviceLocator storage — SessionProvider extension provides it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

g-enius · 2026-03-21T02:33:32Z

Coordinator/Sources/Coordinator/AppCoordinator.swift

    override public func start() {
-        activateSession(for: currentFlow)
+        session.activate()
+        onSessionActivated?()


Why it’s not activateSession?

g-enius

@claude fix those

g-enius · 2026-03-21T02:36:52Z

Core/Sources/Core/ServiceLocator.swift

 // MARK: - ServiceLocatorProvider

 /// Any type that holds a ServiceLocator instance for instance-based DI resolution.
+@MainActor


Why need main actor now but not before?

Claude finished @g-enius's task in 1m 39s —— View job

Why @MainActor was added to ServiceLocatorProvider

Before this PR (main branch): ServiceLocatorProvider had no @MainActor. All concrete conformers (ViewModels) happened to be @MainActor classes, but the protocol requirement itself was nonisolated — Swift accepted this because the concrete types' isolation satisfied the call sites.

What changed: This PR introduced SessionProvider, a @MainActor protocol that inherits from ServiceLocatorProvider and provides a default implementation:

@MainActor public protocol SessionProvider: ServiceLocatorProvider { var session: Session { get } } extension SessionProvider { public var serviceLocator: ServiceLocator { session.serviceLocator } }

The Swift 6 strict concurrency rule: A @MainActor-isolated implementation cannot satisfy a nonisolated protocol requirement. Since SessionProvider is @MainActor, its serviceLocator implementation is main-actor-isolated. If ServiceLocatorProvider.serviceLocator remained nonisolated, Swift 6 would error — the protocol witness table requires isolation to match.

Adding @MainActor to ServiceLocatorProvider aligns the requirement's isolation with the implementation's isolation, resolving the violation cleanly.

In short: It wasn't needed before because no @MainActor protocol inherited from it. Once SessionProvider: ServiceLocatorProvider was introduced, Swift 6 required the requirement and implementation to agree on actor isolation.

· Branch

g-enius · 2026-03-21T02:40:31Z

Core/Sources/Core/ServiceLocator.swift

+    var session: Session { get }
+}
+
+extension SessionProvider {


Should write protocol conformance here

g-enius · 2026-03-21T02:43:06Z

FunApp/FunApp/SceneDelegate.swift


    private func subscribeToDarkMode() {
+        guard let session = appCoordinator?.session else { return }
+        let toggles: FeatureToggleServiceProtocol = session.serviceLocator.resolve(for: .featureToggles)


It’s impossible to use @service here?

g-enius · 2026-03-21T02:45:46Z

Services/Tests/ServicesTests/SessionTests.swift

-            // until next AuthenticatedSession.activate() overwrites it
            main.teardown()
-            let login = LoginSession(serviceLocator: locator)
+            let login = LoginSession()


Better named loginSession

claude · 2026-03-22T10:57:55Z

Claude finished @g-enius's task in 3m 27s —— View job

Review Comments Fixed

All 5 review comments addressed in commit a8bd4ed:

AppCoordinator.swift:59 — Extracted activateCurrentSession() helper so both start() and activateSession(for:) share the same activate-and-notify path, eliminating duplication.
ServiceLocator.swift:65 (@MainActor question) — Added a doc comment to ServiceLocatorProvider explaining: @MainActor is required because ServiceLocator itself is @MainActor, so any property returning one must also be main-actor isolated.
ServiceLocator.swift:79 (protocol conformance) — Added // MARK: - ServiceLocatorProvider above the extension SessionProvider block to clearly identify it as the ServiceLocatorProvider conformance implementation.

Note: Swift doesn't allow extension SomeProtocol: AnotherProtocol when the conformance is already declared via protocol inheritance — the MARK is the idiomatic way to label it.
SceneDelegate.swift:68 (@Service question) — Added a comment explaining why @Service can't be used: SceneDelegate is not a ServiceLocatorProvider, and even if it were, the locator changes on every session transition so re-resolving on each activation is intentional.
SessionTests.swift:88 — Renamed login → loginSession.
· Branch

@mainactor

- Extract activateCurrentSession() helper so start() and activateSession(for:) share one path - Add @mainactor doc comment to ServiceLocatorProvider explaining the isolation requirement - Add MARK comment to SessionProvider extension clarifying ServiceLocatorProvider conformance - Add comment in SceneDelegate explaining why @service can't be used for the dark mode subscription - Rename test variable login → loginSession for clarity Co-authored-by: Charles Wang <g-enius@users.noreply.github.com>

g-enius

LGTM. Sync the other 2 PRs now @claude

claude · 2026-03-22T12:37:53Z

Claude finished @g-enius's task in 29m 5s —— View job

Sync complete

Both feature branches have been rebased onto the new main (which includes this PR's session-scoped ServiceLocator changes).

Gather context and understand request
Check sync script / explore branch structure
Rebase feature/navigation-stack onto main — resolved conflicts in UIKit→SwiftUI migration + session DI
Force-push feature/navigation-stack
Rebase feature/observation onto rebased feature/navigation-stack — resolved conflicts in @observable + AsyncStream + session DI
Force-push feature/observation

What was resolved

feature/navigation-stack (32 commits rebased, 4 conflict rounds):

Deleted UIKit files that navigation-stack removes: BaseCoordinator, LoginCoordinator, SceneDelegate, HomeTabBarController, HomeTabBarViewModel, HomeTabBarViewModelTests
Merged SwiftUI AppCoordinator with SessionProvider conformance (session: Session)
Updated ViewModels and tests to use session: + makeSession() pattern throughout
Merged branch-specific docs (CLAUDE.md, ai-rules/) — session-scoped DI language + NavigationStack architecture

feature/observation (55 commits rebased):

Kept @Observable + @ObservationIgnored patterns from observation branch
Kept session: Session + @Service DI from the navigation-stack base
Updated tests to use makeSession() + MockSession(serviceLocator:) pattern
Merged doc sections for session-scoped + observation-specific content
· Branch

g-enius and others added 8 commits March 20, 2026 22:44

Refactor ServiceLocator to session-scoped ownership

79d9fb4

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update docs for session-scoped ServiceLocator

ce51c79

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove throwaway ServiceLocator from AppCoordinator init

2e12f33

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove serviceLocator from BaseCoordinator

a70476a

BaseCoordinator is now pure navigation. Each coordinator that creates ViewModels stores its own serviceLocator reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fix stale docs and SwiftLint warning from review

15eea16

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

AppCoordinator conforms to SessionProvider, no throwaway ServiceLocator

1afaeb4

Session is created in init, activated in start(). No separate serviceLocator storage — SessionProvider extension provides it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update in-app ServiceLocator description to session-scoped pattern

1c40140

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

g-enius commented Mar 21, 2026

View reviewed changes

g-enius commented Mar 22, 2026

View reviewed changes

g-enius merged commit fbc016b into main Mar 22, 2026
7 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ServiceLocator to session-scoped ownership#7

Refactor ServiceLocator to session-scoped ownership#7
g-enius merged 9 commits intomainfrom
refactor/session-scoped-service-locator

g-enius commented Mar 20, 2026 •

edited

Loading

Uh oh!

g-enius Mar 21, 2026

Uh oh!

g-enius left a comment

Uh oh!

g-enius Mar 21, 2026

Uh oh!

g-enius Mar 22, 2026

Uh oh!

claude bot Mar 22, 2026 •

edited

Loading

Uh oh!

g-enius Mar 21, 2026

Uh oh!

g-enius Mar 21, 2026

Uh oh!

g-enius Mar 21, 2026

Uh oh!

claude bot commented Mar 22, 2026 •

edited

Loading

Uh oh!

g-enius left a comment

Uh oh!

claude bot commented Mar 22, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

g-enius commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

g-enius Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

g-enius left a comment

Choose a reason for hiding this comment

Uh oh!

g-enius Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

g-enius Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Why @MainActor was added to ServiceLocatorProvider

Uh oh!

g-enius Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

g-enius Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

g-enius Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Comments Fixed

Uh oh!

g-enius left a comment

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sync complete

What was resolved

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

g-enius commented Mar 20, 2026 •

edited

Loading

claude bot Mar 22, 2026 •

edited

Loading

Why `@MainActor` was added to `ServiceLocatorProvider`

claude bot commented Mar 22, 2026 •

edited

Loading

claude bot commented Mar 22, 2026 •

edited

Loading