refactor: coordinator boundary review + AppState façade (#50 + #48)

[variablefate]

Summary

• Documents the RoadFlareCore ↔ RidestrSDK coordinator boundary via ADR-0011 (closes Review remaining RoadFlareCore vs RidestrSDK coordinator boundary #50)
• Adds app-facing read APIs and action methods to AppState/RoadFlareCore so SwiftUI views no longer bypass the façade to reach SDK repositories directly (closes Review AppState facade boundary over SDK repositories/services #48)

Scope

#50 — Coordinator boundary review (ADR-0011):

• Reviewed RideCoordinator, ChatCoordinator, AppState, SyncCoordinator, LocationCoordinator
• Decision: all five stay in RoadFlareCore — the SDK already owns the protocol-level types they delegate to
• Filed Consider extracting ChatCoordinator message-list logic into SDK ChatMessageStore #60 as a deferred follow-up: ChatCoordinator message-list deduplication/sorting is a natural SDK extraction if a driver-side app is ever built

#48 — AppState façade:

• Added façade extensions to AppState covering: connectivity (isRelayConnected()), drivers (read + follow/unfollow/note actions), ride history (read + delete + backup), saved locations (read + pin/save/remove/clear), settings (badge counts)
• Refactored 8 views: DriversTab, DriverDetailSheet, AddDriverSheet, RideRequestView, HistoryTab, SettingsTab, SavedLocationsView, ConnectivityIndicator
• Views no longer call repository or service methods directly — all mutations go through AppState façade actions
• import RidestrSDK remains in refactored views because they render SDK value types (FollowedDriver, RideHistoryEntry, CachedDriverLocation, etc.) — eliminating those imports is the job of Review view-layer coupling to SDK models and add app presentation models #49 (PR refactor: app presentation types for view-layer decoupling (#49) #59, presentation types)

Merge order

This PR merges before #59 (presentation types), which rebases onto it.

Test plan

• xcodebuild clean build passes — confirmed on iPhone 17 simulator
• Views compile against new façade APIs without direct repository/service calls
• ADR-0011 is present in decisions/

Closes #50
Closes #48

🤖 Generated with Claude Code

[variablefate]

PROCESS CORRECTION for agent a7cd1ce30141c2ef4 — please acknowledge and adjust your approach:

Your finish line is marking the draft PR ready for review, NOT merging it. Specifically:

• Do NOT merge the PR
• Do NOT delete the branch or worktree
• After your self-review pass, mark the PR ready with: gh pr ready 58
• Leave all cleanup (branch delete, worktree remove) for after the human reviews and merges manually

Everything else in your instructions stands. Continue your work.

[variablefate]

Code review

Found 4 issues (3 fixed, 1 discarded after verification):

---

Surfaced findings and disposition:

Fixed — 9f23c7c: Incomplete façade — rideCoordinator bypasses left in views

Three call sites still reached through appState.rideCoordinator directly, and RideTab still used appState.relayManager directly, contrary to the PR's stated goal. Added three façade methods (publishDriversList(), requestDriverKeyRefresh(driverPubkey:), checkForStaleDriverKeys()) and updated all four call sites.

roadflare-ios/RoadFlare/RoadFlare/Views/Drivers/AddDriverSheet.swift

Lines 303 to 316 in 449c466

	// If still no key, send a stale ack to request one from the driver.
	Task {
	await appState.rideCoordinator?.publishFollowedDriversList()
	await appState.sendFollowNotification(driverPubkey: hexPubkey)
	// If no key locally, try restoring from our Kind 30011 backup on the relay
	if !appState.hasKeyForDriver(pubkey: hexPubkey) {
	await appState.restoreKeyFromBackup(driverPubkey: hexPubkey)
	}
	// If still no key after restore attempt, request from driver
	if !appState.hasKeyForDriver(pubkey: hexPubkey) {
	await appState.rideCoordinator?.requestKeyRefresh(driverPubkey: hexPubkey)
	}
	}

roadflare-ios/RoadFlare/RoadFlare/Views/Drivers/DriversTab.swift

Lines 158 to 161 in 449c466

	private func refreshDrivers() async {
	appState.refreshDriverLocations()
	await appState.rideCoordinator?.checkForStaleKeys()
	}

roadflare-ios/RoadFlare/RoadFlare/Views/Ride/RideTab.swift

Lines 92 to 95 in 449c466

	private func monitorConnection() async {
	while !Task.isCancelled {
	if let rm = appState.relayManager { isOffline = !(await rm.isConnected) }
	try? await Task.sleep(for: .seconds(10))

---

Fixed — 9a8951f: RideRequestView blank screen for zero-driver users

if appState.driversRepository != nil (service-ready check, always true once auth is done) was replaced with if appState.hasFollowedDrivers (empty when no drivers are followed). The new guard left the RideTab showing nothing for a freshly-signed-in user with no drivers. Added a "No Drivers Added" empty state with a CTA to the Drivers tab, matching the existing pattern in DriversTab.

roadflare-ios/RoadFlare/RoadFlare/Views/Ride/RideRequestView.swift

Lines 41 to 45 in 449c466

	ScrollView {
	VStack(spacing: 16) {
	if appState.hasFollowedDrivers {
	if onlineDrivers.isEmpty {
	VStack(spacing: 24) {

---

Fixed — 33c3bde: ADR-0011 Affected Files list incomplete

SavedLocationsView.swift, ConnectivityIndicator.swift, and RideTab.swift were all modified as Phase B view migrations but were absent from the ADR's Affected Files section.

https://github.com/variablefate/roadflare-ios/blob/449c466fdcac179459e29cf4e69f2987099ba2e2/decisions/0011-coordinator-boundary.md#L168-L181

---

Discarded — withAnimation wrapping backupRideHistory() in HistoryTab

Flagged because a prior commit (5e5d133) explicitly separated removeRide (inside withAnimation) from backupRideHistory() (outside). After verification: backupRideHistory() is synchronous at the call site and spawns a detached Task internally, so it escapes the animation transaction regardless. No actual behavior difference; discarded.

---

🤖 Generated with Claude Code

_{If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[variablefate]

Code review — second pass

4 findings this pass; all fixed in bec34a0. 3 others discarded after verification.

---

Fixed — ActiveRideView was the last remaining driversRepository bypass

ActiveRideView was not in the PR diff, so the first pass treated it as pre-existing and left it. It called appState.driversRepository?.cachedDriverName(pubkey:) directly; appState.driverDisplayName(pubkey:) already existed and is the correct façade call.

roadflare-ios/RoadFlare/RoadFlare/Views/Ride/ActiveRideView.swift

Lines 22 to 26 in 33c3bde

	?? appState.settings.roadflarePaymentMethods,
	driverName: coordinator?.session.driverPubkey.flatMap {
	appState.driversRepository?.cachedDriverName(pubkey: $0)
	},
	pickupAddress: coordinator?.pickupLocation?.address,

---

Fixed — appState.settings.allPaymentMethodNames read still bypassed façade in RideRequestView

The Settings façade block added read-only helpers for badge counts and profileName, but omitted allPaymentMethodNames, which RideRequestView uses for the payment display line in the fare section. Added public var allPaymentMethodNames: [String] to the Settings façade block and updated the call site.

roadflare-ios/RoadFlare/RoadFlare/Views/Ride/RideRequestView.swift

Lines 196 to 200 in 33c3bde

	Image(systemName: "creditcard")
	.foregroundColor(Color.rfPrimary)
	Text(appState.settings.allPaymentMethodNames.joined(separator: ", "))
	.font(RFFont.caption(12))
	.foregroundColor(Color.rfOnSurfaceVariant)

---

Fixed — addDriver() publish asymmetry was undocumented

removeDriver() and updateDriverNote() both auto-publish the drivers list internally. addDriver() does not — callers must call publishDriversList() separately. No doc comment warned of this, making the asymmetry a silent trap for any future second call site. Added a note to the doc comment.

roadflare-ios/RoadFlare/RoadFlareCore/ViewModels/AppState.swift

Lines 709 to 718 in 33c3bde

	/// Add a driver and cache their profile and name if available.
	public func addDriver(_ driver: FollowedDriver, profile: UserProfileContent? = nil, name: String? = nil) {
	driversRepository?.addDriver(driver)
	if let profile {
	driversRepository?.cacheDriverProfile(pubkey: driver.pubkey, profile: profile)
	} else if let name, !name.isEmpty {
	driversRepository?.cacheDriverName(pubkey: driver.pubkey, name: name)
	}
	}

---

Fixed — ADR-0011 Affected Files and Settings MARK still stale

ActiveRideView.swift (now fixed) was absent from the ADR Affected Files list. The // MARK: - Façade: Settings (read-only convenience) comment implied settings writes also go through the façade, but SettingsTab still calls appState.settings.setProfileName() directly; the comment now reflects that writes are intentionally deferred.

https://github.com/variablefate/roadflare-ios/blob/33c3bdeb7f562786ee9819f4bd02a7a6b441a993/decisions/0011-coordinator-boundary.md#L178-L184

---

Discarded findings:

• appState.rideCoordinator reference in ActiveRideView/RideRequestView — RideCoordinator is app-layer (RoadFlareCore), not an SDK service; its presence in views is deliberate and deferred to PR refactor: app presentation types for view-layer decoupling (#49) #59.
• public private(set) on driversRepository/relayManager staying readable — narrowing access is an architectural change beyond this PR's scope.
• Task interleave on rapid removeDriver + updateDriverNote — both mutations are synchronous on @MainActor before either Task runs; both publish current repo state at call time; low real-world probability and a pre-existing structural pattern.
• DriverDetailSheet note lost on interactive-dismiss swipe — pre-existing; the note is also irrelevant if the user proceeds to "Remove Driver".

---

What this pass caught that the first missed:

The first pass focused on files in the diff and noted ActiveRideView as pre-existing but left it. This pass treated the façade's completeness as the unit of review rather than the diff boundary, which surfaced both the ActiveRideView bypass and the allPaymentMethodNames gap in a file the PR did touch. The addDriver() asymmetry was also a documentation blind spot the first pass did not flag.

🤖 Generated with Claude Code

_{If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[variablefate]

Code review — third pass

4 findings; 1 is a real user-facing bug the first two passes missed. Fixed in 0b2e519 and 4cdf7b7.

---

Fixed (bug) — Stale-keyed drivers were bookable from the ride-request flow

DriverDetailSheet.canRequestRide, RideRequestView.onlineDrivers, and DriversTab.isOnline all gated on hasKey && online without excluding drivers with a stale encryption key. A driver broadcasting "online" with a stale key would surface as bookable; the rider's encrypted offer would not be decryptable on the driver side and the request would silently fail. The SDK blocks this at the ping path (canPingDriver returns .ineligible when staleKeyPubkeys.contains(driver.pubkey)) but had no equivalent guard for ride offers. All three predicates now also require !appState.isDriverKeyStale(pubkey:).

PR #59's review surfaced the same defect in the new presentation-layer types; the root cause lives here.

roadflare-ios/RoadFlare/RoadFlare/Views/Drivers/DriverDetailSheet.swift

Lines 103 to 107 in bec34a0

	private var canRequestRide: Bool {
	currentDriver.hasKey && currentLocation?.status == "online"
	}

roadflare-ios/RoadFlare/RoadFlare/Views/Ride/RideRequestView.swift

Lines 24 to 28 in bec34a0

	private var onlineDrivers: [FollowedDriver] {
	appState.followedDrivers.filter { driver in
	driver.hasKey && appState.driverLocation(pubkey: driver.pubkey)?.status == "online"
	}
	}

roadflare-ios/RoadFlare/RoadFlare/Views/Drivers/DriversTab.swift

Lines 366 to 368 in bec34a0

	private var isOnline: Bool {
	driver.hasKey && location?.status == "online"
	}

---

Fixed — SettingsTab wasn't using façade properties it added in the same PR

5 reads still went through appState.settings.profileName / appState.settings.roadflarePaymentMethods.count despite appState.profileName and appState.paymentMethodCount having been added in this PR for exactly that. Migrated the 5 read sites.

roadflare-ios/RoadFlare/RoadFlare/Views/Settings/SettingsTab.swift

Lines 45 to 49 in bec34a0

	VStack(alignment: .leading, spacing: 2) {
	Text(appState.settings.profileName.isEmpty ? "Set your name" : appState.settings.profileName)
	.font(RFFont.title(16))
	.foregroundColor(appState.settings.profileName.isEmpty ? Color.rfOnSurfaceVariant : Color.rfOnSurface)
	Text("Edit Profile")

roadflare-ios/RoadFlare/RoadFlare/Views/Settings/SettingsTab.swift

Lines 110 to 112 in bec34a0

	Spacer()
	Text("\(appState.settings.roadflarePaymentMethods.count)")
	.font(RFFont.caption(12))

---

Fixed — Stale comment in DriversTab.pingDriver

Comment at line 131 claimed appState.driversRepository is "accessible without a hop." After the façade migration, no such access occurs in the function — the Task body now uses appState.sendDriverPing / appState.driverDisplayName. Dropped the stale sentence.

roadflare-ios/RoadFlare/RoadFlare/Views/Drivers/DriversTab.swift

Lines 127 to 132 in bec34a0

	private func pingDriver(_ driver: FollowedDriver) {
	// Capture pubkey as a plain String (Sendable) before the task boundary so
	// `driver` (a struct that may not be Sendable) doesn't need to cross the
	// isolation boundary. The view is @MainActor so the Task body inherits that
	// isolation; `appState.driversRepository` is accessible without a hop.
	let driverPubkey = driver.pubkey

---

Fixed — ADR-0011 self-contradictory LOC claim

ADR describes AppState as "~650 LOC" but the file is 870 lines after this PR's façade additions. Replaced the figure with prose that stays accurate as the file evolves.

roadflare-ios/decisions/0011-coordinator-boundary.md

Lines 26 to 28 in bec34a0

	The five coordinators are:
	- `AppState` — owns SDK services, auth lifecycle, orchestration (~650 LOC)
	- `SyncCoordinator` — Nostr sync orchestration, startup resolution, dirty-tracking

---

Discarded after verification:

• Task interleave on removeDriver + updateDriverNote — both mutations are synchronous on @MainActor and run to completion before either Task body begins. By the time the two Tasks execute, they see identical post-mutation repo state. Not a race.
• sendRideOffer / coordinator mutation bypasses — RideCoordinator is app-layer; the ride-flow views hold coordinator handles to compose multi-field state (pickup, destination, fare estimate). Proper fix requires a larger façade surface for the ride flow, deferred to PR refactor: app presentation types for view-layer decoupling (#49) #59.
• keyManager.exportNsec() bypass in backup flow — different service family (not one of the three the ADR scopes).
• Write bypasses in untouched files (SyncScreen, PaymentMethodsScreen, PaymentSetupView) — Settings writes are explicitly deferred per the Settings façade MARK comment.

---

What this pass caught that earlier passes missed:

The stale-key bug is the substantive one. The first two passes audited the façade by walking each view's diff for SDK-type accesses and never examined the views' own business-logic predicates. canRequestRide and onlineDrivers were considered "unchanged logic" and glossed over — but the façade had added the isDriverKeyStale(pubkey:) method and no view was using it in the predicates that gate ride-offer sending. PR #59's review found the same symptom one layer downstream; this pass caught it at the source. The remaining three findings are polish: adopt-your-own-façade and keep docs accurate to the current state.

🤖 Generated with Claude Code

_{If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[variablefate]

Code review — fourth pass (convergence check on e68ec8c)

No new findings worth acting on in this PR. The canRequestRide refactor + tests converges cleanly:

• FollowedDriversRepository.canRequestRide(_:) placement follows CLAUDE.md SDK-vs-app split (protocol-state predicate on the SDK repo, parallel to canPingDriver).
• Lock usage matches the established @unchecked Sendable + NSLock pattern; single atomic lock.withLock into a *Locked helper, symmetric with driverPingPreflightLocked.
• 8 new tests mirror CanPingDriverTests structure (unknown driver, missing key, stale key, offline, on_ride, online-with-key, plus two stale-caller-snapshot cases). Build green, 109 passed / 0 failed.
• The three view call sites (DriverDetailSheet.canRequestRide, RideRequestView.onlineDrivers, DriversTab.isOnline) collapse to the single façade call, killing the three-way drift risk the prior inline fix had.

---

Two forward-looking observations (not flagged for fix in this PR — flagging here for visibility):

1. Send-time preflight gap on publishRideOffer (follow-up issue candidate)

The ping path has UI-side canPingDriver + SDK-side driverPingPreflight + an AppState send-time re-check in sendDriverPing. The ride path now has the UI-side canRequestRide but RiderRideDomainService.publishRideOffer has no analogous preflight — only a state-machine stage check. A stale-key event arriving in the window between button tap and relay publish would still let the offer go out and silently fail on decrypt.

The UI fix closes the user-visible case; this would close the race window. Scope is adjacent to this PR, not in it — suitable as a follow-up issue. Pattern precedent is commit bd2b3f1, which introduced driverPingPreflight for the equivalent ping-side gap.

2. PR #59 presentation-type factories inline the predicate

PR #59's DriverDetailViewState and RideRequestDriverOption factories inline driver.hasKey && !isKeyStale && location?.status == "online" rather than calling the new SDK method. Once #58 merges, there will be two implementations of the same logic, and PR #59's factories lack the "driver exists in repo" guard that CanRequestRideTests.staleCallerSnapshot_hasKeyButRepoMissingKey_returnsFalse pins down. Belongs in PR #59's review — posting here so the author sees it when #58 lands.

---

Branch is ready to merge. This pass found no issues in #58's own scope.

🤖 Generated with Claude Code

_{If this code review was useful, please react with 👍. Otherwise, react with 👎.}