refactor(drivestr): extract AvailabilityCoordinator, AcceptanceCoordinator, RoadflareDriverCoordinator

[variablefate]

Closes #66.

Summary

Decomposes DriverViewModel into three shared domain coordinators extracted to :common/coordinator/. The coordinators handle availability broadcasts, offer acceptance with wallet-pubkey handshake and payment-path derivation, and RoadFlare driver state sync. Consolidates the driver-app-local PaymentStatus enum with the shared production values in common/payment/PaymentModels.kt.

Coordinators

• AvailabilityCoordinator (:common/coordinator/) — Kind 30173 periodic broadcast loop, NIP-09 batch deletion on go-offline/ride-accept, time + distance throttle guards, and one-shot publish with optional event-ID tracking (track: Boolean).
• AcceptanceCoordinator (:common/coordinator/) — direct + broadcast offer acceptance. Kind 3174 publish, wallet pubkey / mint URL snapshot, PaymentPath derivation, and broadcast first-acceptance-wins CAS gate. Returns the sealed AcceptBroadcastOutcome (Success / DuplicateBlocked / PublishFailed) so callers can distinguish a silent dedup from a real publish failure.
• RoadflareDriverCoordinator (:common/coordinator/) — union-merge Kind 30012 state sync, mergeFollowerLists / mergeMutedLists, RoadflareLocationBroadcaster lifecycle, and final OFFLINE Kind 30014 publish.

The ViewModel retains all UI-state composition, subscription management (via SubscriptionManager), and ride-session lifecycle. Coordinators are unit-testable without Android context — constructor-injected via provider lambdas until Hilt migration (#52) lands.

Side cleanups

• stageBeforeRide moved from a loose ViewModel var into DriverRideSession so it auto-resets by construction via resetRideUiState(), matching CLAUDE.md's "Consolidated State Resets" pattern.
• Removed redundant always-true/always-false null checks flagged by the compiler in handlePreimageShare.

Review passes

Five rounds of review landed 15 real fixes:

• Pass 1 (Sonnet): BuildConfig.DEBUG guards, double walletServiceProvider() calls, conditional publishedEventIds.clear() regression, orphaned ROADFLARE_ONLY presence event, stale KDoc, throttle log detail.
• Pass 2 (Sonnet): CancellationException gate reset in acceptBroadcastRequest, resetBroadcastGate() unconditional in handleConfirmationTimeout, deleteAllAvailabilityEvents revert to unconditional clear, redundant stopBroadcasting(), misplaced TODO(#52) on enum.
• Pass 3 (Sonnet): Convergence check — no new issues (investigated 6 candidates, all cleared).
• Pass 4 (Opus): AcceptBroadcastOutcome sealed-type split to distinguish CAS-dedup from publish failure; removed dead scope: CoroutineScope param on RoadflareDriverCoordinator.
• Pre-finalization cleanup: trackPublishedEvent → track: Boolean param on publishAvailability; stageBeforeRide consolidated into DriverRideSession; dead null checks removed.

Tests

33 new unit tests in common/src/test/java/com/ridestr/common/coordinator/:

• AcceptanceCoordinatorTest — CAS gate Success/DuplicateBlocked/PublishFailed outcomes, CancellationException gate reset + rethrow, PaymentPath derivation (SAME_MINT / CROSS_MINT / FIAT_CASH), AcceptanceResult shape for broadcast acceptance.
• AvailabilityCoordinatorTest — throttle guards, publishAvailability track=true/false append semantics, deleteAllAvailabilityEvents no-op when empty + clear on success and failure, clearBroadcastState resetting all three fields.
• RoadflareDriverCoordinatorMergeTest — mergeFollowerLists union, approved logical-OR, keyVersionSent max-with-clamp, addedAt min, remote-newer pruning, local-newer retention; mergeMutedLists union + never auto-unmute.

All tests pass: ./gradlew :common:testDebugUnitTest :drivestr:testDebugUnitTest → BUILD SUCCESSFUL.

Test plan

• :common:testDebugUnitTest passes (33 new coordinator tests + existing)
• :drivestr:testDebugUnitTest passes
• :common:compileDebugKotlin and :drivestr:compileDebugKotlin clean
• Manual smoke: accept a broadcast offer and verify Kind 3174 publishes (AcceptanceCoordinator)
• Manual smoke: ROADFLARE_ONLY mode toggle → go offline → verify presence event is NIP-09 deleted (track = true path)
• Manual smoke: handleConfirmationTimeout with currentLocation == null — verify resetBroadcastGate() fires so the next broadcast offer can be accepted

🤖 Generated with Claude Code

[variablefate]

Code review

Found 1 issue:

1. RoadflareDriverCoordinator drops BuildConfig.DEBUG guards that existed in the original DriverViewModel code, causing two verbose log lines to run unconditionally in release builds. The common/ module has its own BuildConfig (used in RoadflareLocationBroadcaster, WalletService, etc.), so this is fixable. The two affected lines:
   • Line 135: Log.w(TAG, "Kind 30011 query timed out …") — was if (BuildConfig.DEBUG) gated
   • Line 343: Log.d(TAG, "Remote newer; pruning … stale local-only followers") — was if (BuildConfig.DEBUG) gated

ridestr/common/src/main/java/com/ridestr/common/coordinator/RoadflareDriverCoordinator.kt

Lines 133 to 137 in baa8343

	val queryResult = nostrService.queryCurrentFollowerPubkeys(driverPubKey)
	if (!queryResult.success) {
	Log.w(TAG, "Kind 30011 query timed out — using merged followers as fallback")
	mergedFollowers
	} else {

ridestr/common/src/main/java/com/ridestr/common/coordinator/RoadflareDriverCoordinator.kt

Lines 341 to 345 in baa8343

	val localOnlyPubkeys = local.map { it.pubkey }.filter { it !in remotePubkeys }
	if (localOnlyPubkeys.isNotEmpty()) {
	Log.d(TAG, "Remote newer; pruning ${localOnlyPubkeys.size} local-only stale followers")
	localOnlyPubkeys.forEach { byPubkey.remove(it) }
	}

🤖 Generated with Claude Code

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

[variablefate]

Code review — Second pass

Found 5 issues. First pass already fixed 6; these are new findings from the improved state.

Advanced (verified real, fixed in commit 5be8340):

1. acceptBroadcastRequest CAS gate not reset on coroutine cancellation — if the coroutine is cancelled (e.g. during logout) while suspended inside acceptBroadcastRide, CancellationException propagates without resetting hasAcceptedBroadcast, leaving it permanently true until the next resumeOfferSubscriptions(). Added catch (CancellationException) reset + rethrow.

ridestr/common/src/main/java/com/ridestr/common/coordinator/AcceptanceCoordinator.kt

Lines 128 to 141 in 5be8340

	val eventId = try {
	nostrService.acceptBroadcastRide(
	request = request,
	walletPubKey = walletPubKey,
	mintUrl = driverMintUrl,
	paymentMethod = request.paymentMethod
	)
	} catch (e: kotlinx.coroutines.CancellationException) {
	hasAcceptedBroadcast.set(false)
	throw e
	} ?: run {
	Log.e(TAG, "acceptBroadcastRide returned null — Nostr publish failed")
	hasAcceptedBroadcast.set(false) // allow retry

2. handleConfirmationTimeout() skips resetBroadcastGate() when currentLocation is null — the gate was only reset inside if (location != null) which calls resumeOfferSubscriptions. A timeout with no location left hasAcceptedBroadcast = true, silently blocking all future broadcast acceptances until the next proceedGoOnline(). Fixed by calling acceptanceCoordinator.resetBroadcastGate() unconditionally before the location check.

ridestr/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt

Lines 2584 to 2596 in 5be8340

	clearSavedRideState()
	// Always reset broadcast gate so the next broadcast offer can be accepted,
	// even if location is null and we skip resumeOfferSubscriptions() below.
	acceptanceCoordinator.resetBroadcastGate()
	// Resume broadcasting
	val location = _uiState.value.currentLocation
	if (location != null) {
	availabilityCoordinator.clearBroadcastState()
	resumeOfferSubscriptions(location)
	Log.d(TAG, "Resumed broadcasting after confirmation timeout")
	}

3. deleteAllAvailabilityEvents() "retain for retry" was immediately defeated — prior fix commit (06d4951) made it retain IDs on failure, but clearBroadcastState() (called on the very next line in every call site) always cleared them anyway. No retry mechanism exists. Reverted to unconditional clear with an honest KDoc.

ridestr/common/src/main/java/com/ridestr/common/coordinator/AvailabilityCoordinator.kt

Lines 192 to 215 in 5be8340

	* Suspends until the deletion event is confirmed sent (or fails). Event IDs are
	* always cleared after this call — stale events expire naturally if the relay
	* request fails, so no retry is warranted.
	*/
	suspend fun deleteAllAvailabilityEvents() {
	if (publishedEventIds.isEmpty()) {
	Log.d(TAG, "deleteAllAvailabilityEvents: nothing to delete")
	return
	}
	Log.d(TAG, "Deleting ${publishedEventIds.size} availability events")
	val deletionId = nostrService.deleteEvents(
	publishedEventIds.toList(),
	"driver went offline",
	listOf(RideshareEventKinds.DRIVER_AVAILABILITY)
	)
	if (deletionId != null) {
	Log.d(TAG, "Deletion request sent: $deletionId")
	} else {
	Log.w(TAG, "Deletion request failed — events will expire naturally")
	}
	publishedEventIds.clear()
	}
	// -------------------------------------------------------------------------

4. Redundant stopBroadcasting() before startBroadcasting() in handleLocationUpdate() — startBroadcasting() KDoc says it cancels the running job internally; the explicit stop set broadcastJob = null first, making the internal cancel a no-op and contradicting the documented interface.

ridestr/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt

Lines 1148 to 1152 in 5be8340

	availabilityCoordinator.updateThrottle(newLocation)
	// Restart broadcasting with the new location to trigger immediate update.
	// startBroadcasting() cancels the running job internally (see its KDoc).
	startBroadcasting(newLocation)

5. Misplaced TODO(#52) on PaymentStatus enum — enums are never Hilt-injected; the comment had no valid target and was copied from the coordinator classes in error.

ridestr/common/src/main/java/com/ridestr/common/payment/PaymentModels.kt

Lines 39 to 44 in 5be8340

	* This is not a lifecycle enum — it reflects the driver's *readiness to claim*, not the
	* overall payment lifecycle (see [EscrowType] / [EscrowDetails] for lifecycle state).
	*/
	enum class PaymentStatus {
	/** Non-HTLC payment path (fiat cash, cross-mint bridge already complete, or no payment). */
	NO_PAYMENT_EXPECTED,

Verified false positive / pre-existing (not actioned):

• confirmationInFlight AtomicBoolean cited as missing — driver-side does not use this; it is RiderViewModel-only. hasAcceptedBroadcast in AcceptanceCoordinator is the correct driver-side equivalent.
• publishedEventIds thread safety — all access routes through viewModelScope (Dispatchers.Main), single-threaded; no actual data race.
• _uiState.value = vs .update {} — pre-existing throughout ViewModel, not introduced by this PR.
• BuildConfig.DEBUG module mismatch — com.ridestr.common.BuildConfig.DEBUG and drivestr.BuildConfig.DEBUG are equivalent in practice; same build variant sets both.
• onCleared() missing stopBroadcasting() — performLogoutCleanup() already calls it at line 3821.

🤖 Generated with Claude Code

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

[variablefate]

Code review — Third pass

No new findings worth acting on. The extraction is clean.

Checked at HEAD 5be83405f10a88ac23e6251ac206da2093c43c44:

Investigated and cleared:

• ensureStateSynced() returning verifiedFollowerPubkeys = null when stateChanged == false — correct behaviour: when merged state is identical to local state the follower list is already verified, no refresh signal is needed.
• cancelCurrentRide deferring resetRideUiState/clearSavedRideState inside a coroutine — intentional: cancellation event must be sent before the UI resets. Synchronous block still covers closeAllRideSubscriptionsAndJobs, clearDriverStateHistory, stageBeforeRide = null, and service stop. Pre-existing, not introduced by this PR.
• cancelCurrentRide missing resetBroadcastGate() — cancelCurrentRide goes to DriverStage.OFFLINE; gate is reset via resumeOfferSubscriptions() on the next goOnline(). No window where a stuck gate can block offers.
• trackPublishedEvent() + broadcast loop single-delete pattern — Kind 30173 is a parameterised replaceable event; relay-side replacement handles any duplicates. All tracked IDs are batch-deleted by deleteAllAvailabilityEvents() on go-offline.
• publishedEventIds thread safety — all access routes through viewModelScope (Dispatchers.Main), single-threaded dispatcher, no actual data race.
• stageBeforeRide and completeRideInternal patterns — pre-existing, not introduced by the coordinator extraction.
• RoadflareLocationBroadcaster internal IO scope on destroy() — onCleared() calls performLogoutCleanup() which calls roadflareCoordinator.destroy() which calls broadcaster?.destroy() which cancels the internal scope. Path is complete.

Summary across all three passes: 11 real issues caught and fixed (6 in pass 1, 5 in pass 2). Third pass finds the codebase at steady state for this change.

🤖 Generated with Claude Code

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

[variablefate]

Code review — Fourth pass (Opus)

Passes 1-3 used Sonnet agents; this pass re-ran with Opus. Found 2 new real issues.

Advanced (verified real, fixed in commit eedd4b4):

1. CAS-gate-blocked broadcast acceptance surfaced as a user-visible error. AcceptanceCoordinator.acceptBroadcastRequest returned AcceptanceResult?, with null meaning either "CAS gate blocked a duplicate (silent — another caller is handling it)" or "Nostr publish failed (surface to user)". The caller treated both the same and showed "Failed to accept ride request" on multi-relay duplicate deliveries or rapid taps. Split into AcceptBroadcastOutcome sealed type (Success / DuplicateBlocked / PublishFailed); caller handles each appropriately.

ridestr/common/src/main/java/com/ridestr/common/coordinator/AcceptanceCoordinator.kt

Lines 30 to 42 in eedd4b4

	/**
	* Outcome of [AcceptanceCoordinator.acceptBroadcastRequest].
	*
	* Distinguishes CAS-gate deduplication (silent — another caller is handling it) from
	* a real Nostr publish failure (surface to user). Without this split, a rapid duplicate
	* invocation would look identical to a failure at the call site.
	*/
	sealed class AcceptBroadcastOutcome {
	data class Success(val result: AcceptanceResult) : AcceptBroadcastOutcome()
	object DuplicateBlocked : AcceptBroadcastOutcome()
	object PublishFailed : AcceptBroadcastOutcome()
	}

ridestr/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt

Lines 3664 to 3691 in eedd4b4

	is AcceptBroadcastOutcome.Success -> {
	val result = outcome.result
	setupAcceptedRide(
	acceptanceEventId = result.acceptanceEventId,
	offer = result.offer,
	broadcastRequest = result.broadcastRequest,
	walletPubKey = result.walletPubKey,
	driverMintUrl = result.driverMintUrl,
	paymentPath = result.paymentPath,
	cleanupTag = "BROADCAST_ACCEPTANCE"
	)
	}
	AcceptBroadcastOutcome.DuplicateBlocked -> {
	// Another invocation is handling this broadcast; stay silent.
	Log.d(TAG, "Broadcast acceptance deduplicated by CAS gate")
	}
	AcceptBroadcastOutcome.PublishFailed -> {
	_uiState.update { current ->
	current.copy(
	error = "Failed to accept ride request",
	rideSession = current.rideSession.copy(isProcessingOffer = false)
	)
	}
	}
	}
	}
	}

2. Dead scope: CoroutineScope constructor parameter in RoadflareDriverCoordinator. Field was stored but never referenced anywhere in the class — the coordinator's work is done in suspend functions and the wrapped RoadflareLocationBroadcaster owns its own internal scope. Removed parameter and unused imports; updated ViewModel instantiation.

ridestr/common/src/main/java/com/ridestr/common/coordinator/RoadflareDriverCoordinator.kt

Lines 48 to 52 in eedd4b4

	private val driverRoadflareRepository: DriverRoadflareRepository
	) {
	companion object {
	private const val TAG = "RoadflareDriverCoord"

Verified false positive / pre-existing (not actioned):

• deleteAllAvailabilityEvents unconditional clear — pass 2 intentionally reverted conditional-clear after confirming no retry mechanism exists; not a regression to re-revert.
• In-loop deleteEvent missing NIP-09 k-tag — verified pre-existing in DriverViewModel before the refactor; batch deletion at go-offline self-heals.
• CancellationException coverage in acceptBroadcastRequest — verified that all code after the publish suspend is synchronous, so cancellation cannot land between publish success and return.
• Dropped log verbosity in broadcast loop and throttle — cosmetic observability change, not a correctness issue.
• ensureStateSynced retry during delay(1_000) cancel — state unchanged before delay; clean cancellation.

Summary across all four passes: 13 real issues caught and fixed (6 + 5 + 0 + 2). PR converged.

🤖 Generated with Claude Code

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