From 132f56d91b93de31afff8fa971062d4f70dc62c0 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 15:50:04 -0700 Subject: [PATCH 1/8] feat(roadflare): silent mute filter for Kind 3173 + 3189 (#82 Scope A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the receive-side filter promised by issue #80's lightweight mute. Pre-fix, muting a rider only suppressed key delivery — they could still spam the driver with offers and pings that landed in the inbox / pinged the driver awake. - New `DriverRoadflareRepository.isAnyMuted(pubkey)` helper unifying the heavyweight (`MutedRider` / "Remove") and lightweight (`RoadflareFollower.mutedAt` / "Mute") checks. Single source so the four receive-side handlers (Kind 3173 / 3187 / 3188 / 3189) can't drift. - `DriverViewModel.processIncomingOffer` (Kind 3173): drop muted offers before the dedup / staleness checks. Twitter-style silent mute — the rider sees no response, can fall back to another app. - `RoadflareListenerService.processPingEvent` (Kind 3189): the existing `getMutedPubkeys()` heavyweight-only check is upgraded to `isAnyMuted`, so lightweight-muted riders also can't wake the driver via ping. Kind 3187 (`handleFollowNotification`, PR #79) and Kind 3188 (ack handler, PR #81) already check both paths — no change needed there. Closes part of #82 (Scope A). Scope B (stale-key offer flow) follows. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/ridestr/common/data/DriverRoadflareRepository.kt | 9 +++++++++ .../com/drivestr/app/service/RoadflareListenerService.kt | 8 ++++++-- .../java/com/drivestr/app/viewmodels/DriverViewModel.kt | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt index 7293e27..e111199 100644 --- a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt @@ -440,6 +440,15 @@ class DriverRoadflareRepository(context: Context) { return _state.value?.followers?.any { it.pubkey == pubkey && it.mutedAt != null } ?: false } + /** + * True if the rider is muted via either path (issue #82). Single helper so receive-side + * filters across the codebase use the same check. Callers: + * - `DriverViewModel.processIncomingOffer` — Kind 3173 ride offers (silent-drop muted) + * - `RoadflareListenerService` Kind 3189 handler — driver pings (silent-drop muted) + * - `MainActivity` Kind 3187 follow + Kind 3188 ack handlers — already used for both paths + */ + fun isAnyMuted(pubkey: String): Boolean = isMuted(pubkey) || isFollowerMuted(pubkey) + // In-memory flag (resets across process restarts) tracking whether the lightweight-mute // reconciliation against Kind 30177 has run at least once this session. // diff --git a/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt b/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt index 3d4081f..ffae4eb 100644 --- a/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt +++ b/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt @@ -264,8 +264,12 @@ class RoadflareListenerService : Service() { // requiring a service restart (unlike the Kind 3173 snapshot approach). // O(n) in muted-count per event; acceptable up to ~200 muted pubkeys. // Revisit with a cached snapshot if drivers report >1000 followers or high muted counts. - val mutedPubkeys = driverRoadflareRepo?.getMutedPubkeys() ?: emptySet() - if (event.pubKey in mutedPubkeys) { + // + // Issue #82: now checks BOTH mute paths (heavyweight MutedRider + lightweight + // RoadflareFollower.mutedAt) via the unified `isAnyMuted` helper. Pre-#82 only + // the heavyweight list was checked, so a lightweight-muted rider could still wake + // the driver up with a Kind 3189 ping. + if (driverRoadflareRepo?.isAnyMuted(event.pubKey) == true) { Log.d(TAG, "Discarding authenticated ping from muted rider ${event.pubKey.take(8)}") return } diff --git a/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt b/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt index 7976cac..f15d6dd 100644 --- a/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt +++ b/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt @@ -3085,6 +3085,15 @@ class DriverViewModel @Inject constructor( val offerType = if (offer.isRoadflare) "RoadFlare" else "Direct" Log.d(TAG, "Received $offerType offer from ${offer.riderPubKey.take(8)}...") + // Issue #82 Scope A — silent mute filter. Drop offers from heavyweight-muted + // ("Removed") AND lightweight-muted riders. The rider sees no response (matches + // standard "soft block" UX from Twitter/Instagram) and can fall back to another + // app — explicit product decision: muted means muted, downgraded UX is acceptable. + if (driverRoadflareRepository.isAnyMuted(offer.riderPubKey)) { + Log.d(TAG, "Filtering offer from muted rider ${offer.riderPubKey.take(8)}...") + return + } + // Filter out offers we've already accepted (prevents duplicates after ride completion) if (offer.eventId in acceptedOfferEventIds) { Log.d(TAG, "Ignoring already-accepted offer: ${offer.eventId.take(8)}") From 98ef6ed4ccb86193fc6a9cadd78c1ed0d555e8c8 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 15:53:09 -0700 Subject: [PATCH 2/8] test(roadflare): pin isAnyMuted helper contract (#82 Scope A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single regression test covering the four cases: - unmuted rider → false - lightweight-only muted → true - heavyweight-only muted → true - both paths muted → true - unknown pubkey → false Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/roadflare/RoadflareMuteTest.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt b/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt index 9a7c94e..d36fa32 100644 --- a/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt +++ b/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt @@ -275,6 +275,32 @@ class RoadflareMuteTest { assertEquals(setOf("rider-A"), needing) } + @Test + fun `isAnyMuted returns true for either heavyweight or lightweight mute`() { + // Issue #82 helper — single check used by all four receive-side handlers + // (Kind 3173 / 3187 / 3188 / 3189). Must catch both mute paths. + seedFollower("rider-A", mutedAt = null) // unmuted baseline + seedFollower("rider-B", mutedAt = 100L) // lightweight only + seedFollower("rider-C", mutedAt = null) // will become heavyweight + seedFollower("rider-D", mutedAt = 100L) // both paths + + val current = repository.state.value!! + repository.restoreFromBackup( + current.copy( + muted = listOf( + MutedRider(pubkey = "rider-C", mutedAt = 50L), + MutedRider(pubkey = "rider-D", mutedAt = 50L) + ) + ) + ) + + assertFalse("unmuted rider must NOT be flagged", repository.isAnyMuted("rider-A")) + assertTrue("lightweight-muted rider must be flagged", repository.isAnyMuted("rider-B")) + assertTrue("heavyweight-muted rider must be flagged", repository.isAnyMuted("rider-C")) + assertTrue("rider muted via both paths must be flagged", repository.isAnyMuted("rider-D")) + assertFalse("unknown pubkey must NOT be flagged", repository.isAnyMuted("ghost-rider")) + } + @Test fun `lightweight mute is independent of heavyweight MutedRider`() { seedFollower("rider-A", mutedAt = 1L) From 015af4c8cfa6403b5486b3719dd75f7f7e6b3cd1 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 16:03:23 -0700 Subject: [PATCH 3/8] feat(roadflare): track driver presence from public status tag (#82 Scope B foundation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #82's stale-key offer flow needs the rider to know a driver is available even when their stored RoadFlare key can't decrypt the location. The protocol already exposes the necessary info: \`RoadflareLocationEvent.create()\` emits \`status\` and \`key_version\` as **public tags** alongside the encrypted lat/lon content. No schema change needed. This commit adds the data layer + coordinator wiring so the rider's repository captures the public presence regardless of decryption outcome. - New \`CachedDriverPresence(status, timestamp, keyVersion)\` data class on \`FollowedDriversRepository\` plus \`driverPresence\` StateFlow and \`updateDriverPresence\` / \`removeDriverPresence\` / \`clearDriverPresence\` helpers. Out-of-order guard built in. - \`RoadflareDriverPresenceCoordinator.resubscribe\`: - Subscribes to ALL followed drivers, not just those with a current key. - Always extracts the public \`status\` and \`key_version\` tags (via the existing \`getStatus(event)\` / \`getKeyVersion(event)\` helpers) and updates \`driverPresence\` first. - Best-effort decryption follows; if it succeeds, also updates the existing \`driverLocations\`. If it fails (no key, stale key, etc.) the rider still sees the driver as available via \`driverPresence\`. - Out-of-order guard moved up so it applies to presence-only events too. - \`stop()\` deliberately doesn't clear repository state — left for the repo's own clear lifecycle (e.g., logout). UI consumers (RoadflareTab / RiderModeScreen / DriverQuoteCoordinator) will be updated in follow-up commits to read both signals and treat presence-only as \"available, fall back to rider-route fare on offer-send.\" Refs #82 Scope B, #83. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/data/FollowedDriversRepository.kt | 48 +++++++++++ .../RoadflareDriverPresenceCoordinator.kt | 82 ++++++++++++------- 2 files changed, 102 insertions(+), 28 deletions(-) diff --git a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt index d040e4c..0a24d15 100644 --- a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt @@ -25,6 +25,27 @@ data class CachedDriverLocation( val keyVersion: Int = 0 ) +/** + * Cached driver presence — read from the **public** `status` tag on Kind 30014 events + * (no decryption required). Issue #82: drives the rider's "this driver is available + * even though I can't see their location" UX so a stale or missing RoadFlare key never + * blocks ride-on-demand. + * + * Distinct from [CachedDriverLocation] which carries the decrypted lat/lon/timestamp + * from the event content. Presence is a strict subset that any follower can read + * regardless of key state — the protocol exposes `status` publicly via the event tags + * (see `RoadflareLocationEvent.create()` line 86 + `getStatus()` helper line 163). + * + * @param status `"online"` / `"on_ride"` / `"offline"` from the public tag + * @param timestamp `event.createdAt` of the latest 30014 event for this driver + * @param keyVersion Driver's current RoadFlare key version, also from a public tag + */ +data class CachedDriverPresence( + val status: String, + val timestamp: Long, + val keyVersion: Int = 0 +) + /** * Repository for managing rider's followed drivers list for RoadFlare. * Stores drivers and their RoadFlare decryption keys in SharedPreferences. @@ -76,6 +97,33 @@ class FollowedDriversRepository(context: Context) { _driverLocations.value = emptyMap() } + /** + * Cached driver presence — populated from the public `status` tag on Kind 30014 + * regardless of whether the encrypted content can be decrypted. Issue #82. + */ + private val _driverPresence = MutableStateFlow>(emptyMap()) + val driverPresence: StateFlow> = _driverPresence.asStateFlow() + + /** + * Update a driver's cached presence from a Kind 30014 event's PUBLIC tags. + * Skips the update if an existing entry has a newer timestamp (out-of-order delivery). + */ + fun updateDriverPresence(pubkey: String, status: String, timestamp: Long, keyVersion: Int = 0) { + val existing = _driverPresence.value[pubkey] + if (existing != null && existing.timestamp >= timestamp) return + _driverPresence.value = _driverPresence.value + (pubkey to CachedDriverPresence(status, timestamp, keyVersion)) + } + + /** Remove a driver's cached presence. */ + fun removeDriverPresence(pubkey: String) { + _driverPresence.value = _driverPresence.value - pubkey + } + + /** Clear all cached driver presence. */ + fun clearDriverPresence() { + _driverPresence.value = emptyMap() + } + /** * Load cached driver names from SharedPreferences. */ diff --git a/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt b/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt index d98a197..adeecb3 100644 --- a/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt +++ b/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt @@ -48,22 +48,53 @@ class RoadflareDriverPresenceCoordinator( locationSubId?.let { nostrService.closeRoadflareSubscription(it) } locationSubId = null - val withKeys = drivers.filter { it.roadflareKey != null } - if (withKeys.isEmpty()) { - Log.d(TAG, "No drivers with keys to subscribe to (total: ${drivers.size})") + // Issue #82: subscribe to ALL followed drivers, not just those with a current key. + // The PUBLIC `status` tag on Kind 30014 lets us track availability for stale-key / + // missing-key drivers without ever decrypting the encrypted lat/lon content. The + // decryption attempt below is best-effort — if it fails we still surface the driver + // as available via `updateDriverPresence`, so the rider can fall back to the + // rider-route fare and request the ride anyway. + if (drivers.isEmpty()) { + Log.d(TAG, "No followed drivers to subscribe to") return } - val driverPubkeys = withKeys.map { it.pubkey } - Log.d(TAG, "Subscribing to ${driverPubkeys.size} driver locations") + val driverPubkeys = drivers.map { it.pubkey } + val withKeysCount = drivers.count { it.roadflareKey != null } + Log.d(TAG, "Subscribing to ${driverPubkeys.size} driver locations ($withKeysCount with keys)") locationSubId = nostrService.subscribeToRoadflareLocations(driverPubkeys) { event, relayUrl -> val driverPubKey = event.pubKey - val driver = withKeys.find { it.pubkey == driverPubKey } - val roadflareKey = driver?.roadflareKey + val eventCreatedAt = event.createdAt + val isExpired = RoadflareLocationEvent.isExpired(event) + val lastSeen = lastLocationCreatedAt[driverPubKey] ?: 0L + val isOutOfOrder = eventCreatedAt < lastSeen + if (isExpired || isOutOfOrder) { + Log.d(TAG, "Rejected stale/out-of-order 30014 from ${driverPubKey.take(8)}: expired=$isExpired, outOfOrder=$isOutOfOrder") + return@subscribeToRoadflareLocations + } + + // Mark this event as the latest seen for the driver, regardless of decryption + // outcome — the out-of-order guard at the top now applies to presence-only + // events too, so a stale Kind 30014 can't overwrite a fresher presence update. + lastLocationCreatedAt[driverPubKey] = eventCreatedAt + + // Always update presence from the PUBLIC tags — works regardless of key state. + val publicStatus = RoadflareLocationEvent.getStatus(event) + val publicKeyVersion = RoadflareLocationEvent.getKeyVersion(event) + followedDriversRepository.updateDriverPresence( + pubkey = driverPubKey, + status = publicStatus, + timestamp = eventCreatedAt, + keyVersion = publicKeyVersion + ) + + // Best-effort location decryption — only succeeds if we have the current key. + val driver = drivers.find { it.pubkey == driverPubKey } + val roadflareKey = driver?.roadflareKey if (roadflareKey == null) { - Log.w(TAG, "No RoadFlare key for driver ${driverPubKey.take(8)}") + Log.d(TAG, "No RoadFlare key for ${driverPubKey.take(8)} — presence-only update") return@subscribeToRoadflareLocations } @@ -74,27 +105,19 @@ class RoadflareDriverPresenceCoordinator( ) if (locationData != null) { - val eventCreatedAt = event.createdAt - - val isExpired = RoadflareLocationEvent.isExpired(event) - val lastSeen = lastLocationCreatedAt[driverPubKey] ?: 0L - val isOutOfOrder = eventCreatedAt < lastSeen - - if (!isExpired && !isOutOfOrder) { - lastLocationCreatedAt[driverPubKey] = eventCreatedAt - followedDriversRepository.updateDriverLocation( - pubkey = driverPubKey, - lat = locationData.location.lat, - lon = locationData.location.lon, - status = locationData.tagStatus, - timestamp = eventCreatedAt, - keyVersion = locationData.keyVersion - ) - } else { - Log.d(TAG, "Rejected stale/out-of-order 30014 from ${driverPubKey.take(8)}: expired=$isExpired, outOfOrder=$isOutOfOrder") - } + followedDriversRepository.updateDriverLocation( + pubkey = driverPubKey, + lat = locationData.location.lat, + lon = locationData.location.lon, + status = locationData.tagStatus, + timestamp = eventCreatedAt, + keyVersion = locationData.keyVersion + ) } else { - Log.w(TAG, "Failed to decrypt location from ${driverPubKey.take(8)}") + // Decryption failed despite having a key — likely stale (key was rotated). + // Presence already updated above; rider will see the driver as available + // and fall through to the rider-route fare path on offer-send. + Log.w(TAG, "Failed to decrypt location from ${driverPubKey.take(8)} (presence-only, likely stale key)") } } } @@ -121,6 +144,9 @@ class RoadflareDriverPresenceCoordinator( locationSubId?.let { nostrService.closeRoadflareSubscription(it) } locationSubId = null lastLocationCreatedAt.clear() + // Don't clear presence/locations from the repository here — other ViewModel + // observers may briefly read them across configuration changes. The repository + // owns its own clear lifecycle (e.g., on logout). } companion object { From 4068fe9b64bc4ec07203dc6a6bb2bbf8887c3312 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 16:10:59 -0700 Subject: [PATCH 4/8] feat(rider-app): consume presence channel + stale-signal piggyback (#82 Scope B, #83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the presence channel into the rider's request flow so stale-key drivers are no longer treated as offline. Closes the user-visible portion of #82 Scope B for rider-app + drivestr (the ridestr platform). - \`RiderModeScreen.kt\` driver-list pipeline now reads \`followedDriversRepository.driverPresence\` alongside \`driverLocations\`. A driver is "online" if EITHER the decrypted location says so OR the public \`status\` tag does (presence-only path). Per-driver fare quote still requires a known location; without one, the offer falls back to the rider-route fare automatically via the existing \`state.fareEstimate\` path. - \`isBroadcastEligible\` deliberately tightens to \`hasKey && locationOnline\` so the broadcast count matches what \`sendRoadflareToAll\` will actually attempt (its geographic too-far cap requires real locations). - \`isDirectSelectable\` stays loose — single-tap to a stale-key driver works via the rider-route fare fallback. This is the primary UX win. - \`RiderViewModel.sendRoadflareOffer\` and \`sendRoadflareOfferWithAlternatePayment\` now fire a Kind 3188 \`status="stale"\` alongside any offer sent without a known location. The driver receives offer + key-refresh request together; by the time they accept, the rider has the new key and can decrypt in-ride Kind 30014 updates. Fire-and-forget with explicit \`CancellationException\` rethrow; \`RoadflareTab.checkStaleKeys\` remains the periodic backstop. - \`FollowedDriversRepositoryPresenceTest\` (8 cases): pins out-of-order rejection, equal-timestamp dedup, the independence of \`driverPresence\` and \`driverLocations\` flows, and the basic CRUD path. Closes the rider-app + drivestr scope of #82 + #83. roadflare-rider's \`DriverQuoteCoordinator\` and related screens will get the same treatment in a separate PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../FollowedDriversRepositoryPresenceTest.kt | 109 ++++++++++++++++++ .../rider/ui/screens/RiderModeScreen.kt | 30 ++++- .../rider/viewmodels/RiderViewModel.kt | 46 ++++++++ 3 files changed, 182 insertions(+), 3 deletions(-) create mode 100644 common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt diff --git a/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt new file mode 100644 index 0000000..74fb07d --- /dev/null +++ b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt @@ -0,0 +1,109 @@ +package com.ridestr.common.data + +import androidx.test.core.app.ApplicationProvider +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Tests for issue #82's lightweight presence channel — the public-tag-only path on + * Kind 30014 that lets the rider know a driver is available even when their stored + * RoadFlare key can't decrypt the encrypted lat/lon content. + * + * Robolectric provides Android Context for SharedPreferences. Presence itself is + * in-memory only (matches `CachedDriverLocation`'s ephemeral semantic) but the repo + * needs Context to construct. + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [28]) +class FollowedDriversRepositoryPresenceTest { + + private lateinit var repo: FollowedDriversRepository + + @Before + fun setUp() { + repo = FollowedDriversRepository(ApplicationProvider.getApplicationContext()) + } + + @Test + fun `driverPresence starts empty`() { + assertTrue(repo.driverPresence.value.isEmpty()) + } + + @Test + fun `updateDriverPresence stores entry keyed by pubkey`() { + repo.updateDriverPresence(pubkey = "rider-A", status = "online", timestamp = 1_000L, keyVersion = 5) + + val entry = repo.driverPresence.value["rider-A"] + assertEquals("online", entry?.status) + assertEquals(1_000L, entry?.timestamp) + assertEquals(5, entry?.keyVersion) + } + + @Test + fun `updateDriverPresence overwrites with newer timestamp`() { + repo.updateDriverPresence("rider-A", "online", 1_000L, 5) + repo.updateDriverPresence("rider-A", "on_ride", 2_000L, 6) + + val entry = repo.driverPresence.value["rider-A"] + assertEquals("on_ride", entry?.status) + assertEquals(2_000L, entry?.timestamp) + assertEquals(6, entry?.keyVersion) + } + + @Test + fun `updateDriverPresence skips out-of-order updates`() { + // Issue #82: protocol guarantee: Kind 30014 events have monotonic createdAt per driver + // when delivered in order, but relay reordering can deliver an older event after a newer + // one. The repository must reject the older one to keep presence consistent. + repo.updateDriverPresence("rider-A", "on_ride", 2_000L, 6) + repo.updateDriverPresence("rider-A", "online", 1_500L, 5) // older — must be skipped + + val entry = repo.driverPresence.value["rider-A"] + assertEquals("on_ride", entry?.status) + assertEquals(2_000L, entry?.timestamp) + assertEquals(6, entry?.keyVersion) + } + + @Test + fun `updateDriverPresence skips equal-timestamp duplicate`() { + // Same event arriving from two relays must not overwrite the first. + repo.updateDriverPresence("rider-A", "online", 2_000L, 5) + repo.updateDriverPresence("rider-A", "on_ride", 2_000L, 5) + + // The first wins because the second is not strictly newer. + assertEquals("online", repo.driverPresence.value["rider-A"]?.status) + } + + @Test + fun `removeDriverPresence drops entry`() { + repo.updateDriverPresence("rider-A", "online", 1_000L, 5) + repo.removeDriverPresence("rider-A") + assertNull(repo.driverPresence.value["rider-A"]) + } + + @Test + fun `clearDriverPresence wipes all entries`() { + repo.updateDriverPresence("rider-A", "online", 1_000L) + repo.updateDriverPresence("rider-B", "on_ride", 1_500L) + repo.clearDriverPresence() + assertTrue(repo.driverPresence.value.isEmpty()) + } + + @Test + fun `driverPresence and driverLocations are independent state flows`() { + repo.updateDriverPresence("rider-A", "online", 1_000L, 5) + repo.updateDriverLocation("rider-B", lat = 36.0, lon = -115.0, status = "online", timestamp = 1_000L) + + assertTrue("rider-A appears in presence", repo.driverPresence.value.containsKey("rider-A")) + assertFalse("rider-A NOT in locations", repo.driverLocations.value.containsKey("rider-A")) + assertTrue("rider-B appears in locations", repo.driverLocations.value.containsKey("rider-B")) + assertFalse("rider-B NOT in presence (writes are independent)", repo.driverPresence.value.containsKey("rider-B")) + } +} diff --git a/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt b/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt index 73efb9b..a0c8e38 100644 --- a/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt +++ b/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt @@ -2939,6 +2939,10 @@ private fun RoadflareDriverSelectionSheet( // Use repository cache for names and locations (survives sheet close/reopen) val cachedLocations by followedDriversRepository.driverLocations.collectAsState() + // Issue #82: presence channel — drives availability for drivers whose Kind 30014 + // location can't be decrypted (stale key, missing key). Read from the public + // `status` tag, no decryption required. + val cachedPresence by followedDriversRepository.driverPresence.collectAsState() val driverNames by followedDriversRepository.driverNames.collectAsState() // Location subscriptions and profile fetching are now managed by @@ -3047,14 +3051,27 @@ private fun RoadflareDriverSelectionSheet( val driverEntries = drivers.map { driver -> val cachedState = cachedLocations[driver.pubkey] + val cachedPres = cachedPresence[driver.pubkey] val driverLocation = cachedState?.let { Location(lat = it.lat, lon = it.lon) } val driverName = driverNames[driver.pubkey] ?: driver.pubkey.take(8) + "..." val hasKey = driver.roadflareKey != null - val isOnline = cachedState != null && + // Issue #82: a driver is "online" if EITHER the decrypted location says so + // (cachedState path — drivers with a fresh key) OR the public `status` tag + // on Kind 30014 says so (cachedPres path — drivers with stale or missing + // key). The presence-only path drives the stale-key offer flow: rider can + // still request the ride even though they can't see the driver's lat/lon. + val locationOnline = cachedState != null && cachedState.status == com.ridestr.common.nostr.events.RoadflareLocationEvent.Status.ONLINE && (nowSec - cachedState.timestamp) < staleThresholdSec - + val presenceOnline = cachedPres != null && + cachedPres.status == com.ridestr.common.nostr.events.RoadflareLocationEvent.Status.ONLINE && + (nowSec - cachedPres.timestamp) < staleThresholdSec + val isOnline = locationOnline || presenceOnline + + // Per-driver fare quote needs the actual location. When location is + // unavailable (stale key) we omit the quote — the offer-send path falls + // back to the rider-route fare via `state.fareEstimate` automatically. val quote = if (driverLocation != null && pickupLocation != null) { RoadflareFarePolicy.quoteDriver( pickupLat = pickupLocation.lat, @@ -3097,7 +3114,14 @@ private fun RoadflareDriverSelectionSheet( pickupMiles = quote?.pickupMiles, fareState = quote?.fareState ?: FareState.ESTIMATED, isTooFar = isTooFar, - isBroadcastEligible = hasKey && isOnline && !isTooFar, + // Broadcast requires a known location for the geographic too-far cap + // — `sendRoadflareToAll` filters out null-location drivers anyway. + // Use the strict `locationOnline` here so the count + button match + // what the batch will actually attempt. + isBroadcastEligible = hasKey && locationOnline && !isTooFar, + // Direct selection works on the looser `isOnline` (location OR + // presence). Sending an offer with `driverLocation = null` falls + // back to the rider-route fare via `state.fareEstimate`. isDirectSelectable = hasKey && !isSending ) ) diff --git a/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt b/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt index 461d787..496dde8 100644 --- a/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt +++ b/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt @@ -1826,6 +1826,16 @@ class RiderViewModel @Inject constructor( val eventId = sendOfferToNostr(params, pickupRoute) if (eventId != null) { Log.d(TAG, "Sent RoadFlare offer to ${driverPubKey.take(16)}: $eventId") + // Issue #82: stale-key piggyback. If we just sent an offer to a driver + // we can't see the location of (no key OR stale key), fire a Kind 3188 + // status="stale" alongside so the driver re-delivers the current key in + // the same beat. By the time they accept, we have the new key and can + // decrypt the in-ride Kind 30014 updates. Fire-and-forget — failure + // doesn't affect the offer, and `RoadflareTab.checkStaleKeys` covers the + // periodic backfill. + if (driverLocation == null) { + requestKeyRefreshAlongsideOffer(driverPubKey) + } setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) applyOfferSuccessState(params, eventId) } else { @@ -1834,6 +1844,38 @@ class RiderViewModel @Inject constructor( } } + /** + * Fire a Kind 3188 with `status="stale"` to a driver alongside the just-sent ride + * offer. Issue #82: the driver receives both events together; the existing Kind 3188 + * ack handler re-delivers the current Kind 3186 key share in response. Best-effort — + * any failure is logged and otherwise ignored. + */ + private suspend fun requestKeyRefreshAlongsideOffer(driverPubKey: String) { + try { + val storedKey = followedDriversRepository.drivers.value + .find { it.pubkey == driverPubKey } + ?.roadflareKey + val storedKeyUpdatedAt = storedKey?.keyUpdatedAt ?: 0L + val storedVersion = storedKey?.version ?: 0 + + val ackEventId = nostrService.publishRoadflareKeyAck( + driverPubKey = driverPubKey, + keyVersion = storedVersion, + keyUpdatedAt = storedKeyUpdatedAt, + status = "stale" + ) + if (ackEventId != null) { + Log.d(TAG, "Stale-signal piggyback: requested key refresh from ${driverPubKey.take(8)}... (offer sent without location)") + } else { + Log.w(TAG, "Stale-signal piggyback failed for ${driverPubKey.take(8)}... (offer still went through)") + } + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: Exception) { + Log.w(TAG, "Stale-signal piggyback threw for ${driverPubKey.take(8)}...", e) + } + } + /** * Send a RoadFlare offer with an alternate (non-bitcoin) payment method. * Called when rider chooses "Continue with Alternate Payment" from insufficient funds dialog. @@ -1892,6 +1934,10 @@ class RiderViewModel @Inject constructor( val eventId = sendOfferToNostr(params, pickupRoute) if (eventId != null) { Log.d(TAG, "Sent RoadFlare offer with $paymentMethod to ${driverPubKey.take(16)}: $eventId") + // Issue #82: stale-key piggyback. See sendRoadflareOffer() for the rationale. + if (driverLocation == null) { + requestKeyRefreshAlongsideOffer(driverPubKey) + } setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) applyOfferSuccessState(params, eventId) } else { From 653edef198522ccba5bbb03e39cf38c96323df4b Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 17:33:52 -0700 Subject: [PATCH 5/8] =?UTF-8?q?fix(roadflare):=20code-review=20pass=201=20?= =?UTF-8?q?=E2=80=94=20close=20mute=20gaps=20+=20ordering=20race=20(#82)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the code-review pass on PR #85, all addressed: 1. **Broadcast offer mute gap.** `DriverViewModel.subscribeToBroadcastRequests` callback had no mute check, so a muted rider's broadcast Kind 3173 still landed in the inbox. The PR's claim that "muted rider's Kind 3173 offers don't appear in offer inbox" was only true for the direct/RoadFlare path. Added the same `driverRoadflareRepository.isAnyMuted(...)` guard. 2. **`RoadflareListenerService.subscribeToRoadflareRequests` not migrated.** Two compounding issues in this callback: (a) it captured `getMutedPubkeys()` at subscribe-open time so mute changes after subscribe were invisible until service restart, AND (b) the heavyweight-only set meant lightweight-muted (`RoadflareFollower.mutedAt`) riders bypassed it entirely. Migrated to a live per-event `isAnyMuted` call covering both paths. 3. **`FollowedDriversRepository.clearAll()` omitted `_driverPresence`.** The logout reset path cleared `_driverLocations` but the new presence channel was left intact. After logout + login as a different user in the same process, the previous user's presence map survived until the coordinator's next resubscribe. Added the clear, updated the KDoc. 4. **`requestKeyRefreshAlongsideOffer` ordering race.** The piggyback fired BEFORE `setupOfferSubscriptions`. The piggyback issues a Nostr publish round-trip; a fast-responding driver's Kind 3174 acceptance could arrive on the relay before the acceptance subscription was armed and be silently missed. Reordered: subscription first, piggyback second, in both `sendRoadflareOffer` and `sendRoadflareOfferWithAlternatePayment`. `:common:assembleDebug` + `:rider-app:assembleDebug` + `:drivestr:assembleDebug` clean, all unit test suites green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/data/FollowedDriversRepository.kt | 6 ++++- .../app/service/RoadflareListenerService.kt | 12 ++++++---- .../app/viewmodels/DriverViewModel.kt | 8 +++++++ .../rider/viewmodels/RiderViewModel.kt | 23 +++++++++++-------- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt index 0a24d15..408afd8 100644 --- a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt @@ -316,7 +316,7 @@ class FollowedDriversRepository(context: Context) { } /** - * Clear all followed drivers and cached names/locations (for logout). + * Clear all followed drivers and cached names/locations/presence (for logout). */ fun clearAll() { prefs.edit() @@ -326,6 +326,10 @@ class FollowedDriversRepository(context: Context) { _drivers.value = emptyList() _driverNames.value = emptyMap() _driverLocations.value = emptyMap() + // Issue #82: presence has the same in-memory-only semantic as locations and + // belongs in the logout reset path so the next user's session doesn't inherit + // stale presence data from the previous identity. + _driverPresence.value = emptyMap() } companion object { diff --git a/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt b/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt index ffae4eb..8d55b52 100644 --- a/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt +++ b/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt @@ -185,9 +185,6 @@ class RoadflareListenerService : Service() { } private fun subscribeToRoadflareRequests(driverPubKey: String) { - // Get muted pubkeys to filter - val mutedPubkeys = driverRoadflareRepo?.getMutedPubkeys()?.toSet() ?: emptySet() - subscriptionId = nostrService?.relayManager?.subscribe( kinds = listOf(RideshareEventKinds.RIDE_OFFER), tags = mapOf( @@ -198,8 +195,13 @@ class RoadflareListenerService : Service() { // Skip if already seen (atomic check-and-add) if (!seenRequests.add(event.id)) return@subscribe - // Skip if from muted rider - if (event.pubKey in mutedPubkeys) { + // Issue #82: live mute check via the unified `isAnyMuted` helper. + // Pre-fix this captured a `getMutedPubkeys()` snapshot at subscribe-open + // time (heavyweight only) — which had two compounding gaps: mutes applied + // after subscribe were invisible until service restart, AND lightweight-muted + // riders (`RoadflareFollower.mutedAt`) bypassed the check entirely. The + // live `isAnyMuted` call covers both paths and refreshes per event. + if (driverRoadflareRepo?.isAnyMuted(event.pubKey) == true) { Log.d(TAG, "Ignoring RoadFlare from muted rider ${event.pubKey.take(8)}") return@subscribe } diff --git a/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt b/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt index f15d6dd..4a1c34d 100644 --- a/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt +++ b/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt @@ -3425,6 +3425,14 @@ class DriverViewModel @Inject constructor( Log.d(TAG, "Received broadcast ride request from ${request.riderPubKey.take(8)}, fare=${request.fareEstimate}") + // Issue #82 Scope A — silent mute filter for the broadcast Kind 3173 path. + // Mirrors the check in `processIncomingOffer` for the direct/RoadFlare path. + // Without this, a muted rider's broadcast request still lands in the inbox. + if (driverRoadflareRepository.isAnyMuted(request.riderPubKey)) { + Log.d(TAG, "Filtering broadcast request from muted rider ${request.riderPubKey.take(8)}...") + return@subscribeToBroadcastRideRequests + } + // Filter out requests we've already accepted if (request.eventId in acceptedOfferEventIds) { Log.d(TAG, "Ignoring already-accepted request: ${request.eventId.take(8)}") diff --git a/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt b/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt index 496dde8..5d98301 100644 --- a/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt +++ b/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt @@ -1826,18 +1826,20 @@ class RiderViewModel @Inject constructor( val eventId = sendOfferToNostr(params, pickupRoute) if (eventId != null) { Log.d(TAG, "Sent RoadFlare offer to ${driverPubKey.take(16)}: $eventId") - // Issue #82: stale-key piggyback. If we just sent an offer to a driver - // we can't see the location of (no key OR stale key), fire a Kind 3188 + // Acceptance subscription MUST be armed before any further round-trip + // so a fast-responding driver's Kind 3174 isn't missed. + setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) + applyOfferSuccessState(params, eventId) + // Issue #82: stale-key piggyback. If we sent an offer to a driver we + // can't see the location of (no key OR stale key), fire a Kind 3188 // status="stale" alongside so the driver re-delivers the current key in // the same beat. By the time they accept, we have the new key and can - // decrypt the in-ride Kind 30014 updates. Fire-and-forget — failure - // doesn't affect the offer, and `RoadflareTab.checkStaleKeys` covers the - // periodic backfill. + // decrypt the in-ride Kind 30014 updates. Fire-and-forget — runs AFTER + // the acceptance subscription is live so its publish round-trip can't + // race the driver's response. if (driverLocation == null) { requestKeyRefreshAlongsideOffer(driverPubKey) } - setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) - applyOfferSuccessState(params, eventId) } else { applyOfferFailureState("Failed to send RoadFlare offer") } @@ -1934,12 +1936,13 @@ class RiderViewModel @Inject constructor( val eventId = sendOfferToNostr(params, pickupRoute) if (eventId != null) { Log.d(TAG, "Sent RoadFlare offer with $paymentMethod to ${driverPubKey.take(16)}: $eventId") - // Issue #82: stale-key piggyback. See sendRoadflareOffer() for the rationale. + // Acceptance subscription armed BEFORE the piggyback round-trip — see + // sendRoadflareOffer() for the ordering rationale. + setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) + applyOfferSuccessState(params, eventId) if (driverLocation == null) { requestKeyRefreshAlongsideOffer(driverPubKey) } - setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) - applyOfferSuccessState(params, eventId) } else { applyOfferFailureState("Failed to send RoadFlare offer") } From e8def8f75cd38449e78fe491aa129ee38156b483 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 17:44:29 -0700 Subject: [PATCH 6/8] docs(roadflare): correct isAnyMuted KDoc caller list (#82) Pass 2 review noted the KDoc said Kind 3187 / 3188 handlers "use" `isAnyMuted` when they actually inline the equivalent `isMuted` + `isFollowerMuted` calls separately. Fixed the caller list to enumerate the actual call sites (`processIncomingOffer`, `subscribeToBroadcastRequests`, `subscribeToRoadflareRequests`, `processPingEvent`) and explain that the Kind 3187 / 3188 handlers implement the equivalent semantic inline. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/data/DriverRoadflareRepository.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt index e111199..00d5f7f 100644 --- a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt @@ -443,9 +443,17 @@ class DriverRoadflareRepository(context: Context) { /** * True if the rider is muted via either path (issue #82). Single helper so receive-side * filters across the codebase use the same check. Callers: - * - `DriverViewModel.processIncomingOffer` — Kind 3173 ride offers (silent-drop muted) - * - `RoadflareListenerService` Kind 3189 handler — driver pings (silent-drop muted) - * - `MainActivity` Kind 3187 follow + Kind 3188 ack handlers — already used for both paths + * - `DriverViewModel.processIncomingOffer` — Kind 3173 direct/RoadFlare ride offers + * - `DriverViewModel.subscribeToBroadcastRequests` — Kind 3173 broadcast offers + * - `RoadflareListenerService.subscribeToRoadflareRequests` — Kind 3173 RoadFlare offers + * reaching the foreground service + * - `RoadflareListenerService.processPingEvent` — Kind 3189 driver pings + * + * Kind 3187 (`RoadflareKeyManager.handleFollowNotification`) and Kind 3188 + * (`MainActivity` ack handler) implement the equivalent two-path check inline via + * [isMuted] + [isFollowerMuted] separately, predating this helper. Functionally + * equivalent — kept inline because each call site has additional surrounding logic + * that wraps the mute decision. */ fun isAnyMuted(pubkey: String): Boolean = isMuted(pubkey) || isFollowerMuted(pubkey) From 11ac75cb847f5612b437c1655c0a5582f5ccbc49 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 18:20:50 -0700 Subject: [PATCH 7/8] refactor(roadflare): consolidate Kind 3188 ack mute check on isAnyMuted (#82) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass 2 review noted the `MainActivity` Kind 3188 ack handler was the only remaining receive-side callback that inlined the dual `getMutedPubkeys() + follower?.mutedAt` check rather than calling the unified `isAnyMuted` helper introduced earlier in this PR. Functionally equivalent, but having the helper exist while one call site doesn't use it is exactly the entropy that makes "add a new mute path" mistakes likely (see PR #79 / #81 / #82 review history where this pattern repeated). - Collapse the `isMuted` + `isLightMuted` locals into a single `driverRoadflareRepo.isAnyMuted(ackData.riderPubKey)` call. The locals were only used in the combined `!isMuted && !isLightMuted` clause; no separate logging or other use was lost. - Update `isAnyMuted` KDoc caller list to reflect Kind 3188 is now a direct caller, and explain why Kind 3187's `handleFollowNotification` deliberately keeps the inline check (the two outcomes map to distinct return values `AlreadyMuted` vs `AlreadyLightMuted`, which `isAnyMuted` would erase). - New regression test `clearAll wipes the presence channel along with locations` in `FollowedDriversRepositoryPresenceTest` — pins the pass-1 fix that added `_driverPresence` to `clearAll()` so a future refactor adding another in-memory state can't silently regress the logout cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/data/DriverRoadflareRepository.kt | 12 ++++++------ .../FollowedDriversRepositoryPresenceTest.kt | 16 ++++++++++++++++ .../main/java/com/drivestr/app/MainActivity.kt | 14 ++++++++------ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt index 00d5f7f..9a17f79 100644 --- a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt @@ -442,18 +442,18 @@ class DriverRoadflareRepository(context: Context) { /** * True if the rider is muted via either path (issue #82). Single helper so receive-side - * filters across the codebase use the same check. Callers: + * filters across the codebase use the same check and can't drift. Callers: * - `DriverViewModel.processIncomingOffer` — Kind 3173 direct/RoadFlare ride offers * - `DriverViewModel.subscribeToBroadcastRequests` — Kind 3173 broadcast offers * - `RoadflareListenerService.subscribeToRoadflareRequests` — Kind 3173 RoadFlare offers * reaching the foreground service * - `RoadflareListenerService.processPingEvent` — Kind 3189 driver pings + * - `MainActivity` Kind 3188 ack handler — refuses key re-delivery to muted riders * - * Kind 3187 (`RoadflareKeyManager.handleFollowNotification`) and Kind 3188 - * (`MainActivity` ack handler) implement the equivalent two-path check inline via - * [isMuted] + [isFollowerMuted] separately, predating this helper. Functionally - * equivalent — kept inline because each call site has additional surrounding logic - * that wraps the mute decision. + * Kind 3187 (`RoadflareKeyManager.handleFollowNotification`) deliberately calls + * [isMuted] and [isFollowerMuted] separately because the two outcomes map to + * distinct return values (`AlreadyMuted` vs `AlreadyLightMuted`) — collapsing + * to `isAnyMuted` there would lose the semantic distinction the caller depends on. */ fun isAnyMuted(pubkey: String): Boolean = isMuted(pubkey) || isFollowerMuted(pubkey) diff --git a/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt index 74fb07d..6c4bc54 100644 --- a/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt +++ b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt @@ -106,4 +106,20 @@ class FollowedDriversRepositoryPresenceTest { assertTrue("rider-B appears in locations", repo.driverLocations.value.containsKey("rider-B")) assertFalse("rider-B NOT in presence (writes are independent)", repo.driverPresence.value.containsKey("rider-B")) } + + @Test + fun `clearAll wipes the presence channel along with locations`() { + // Issue #82 pass-1 fix: presence has the same in-memory-only semantic as locations + // and MUST be cleared on logout so the next user's session doesn't inherit stale + // data from the previous identity. The test pins this so a future refactor that + // adds another in-memory state can't silently regress the cleanup. + repo.updateDriverPresence("driver-A", "online", 1_000L, 5) + repo.updateDriverPresence("driver-B", "on_ride", 1_500L, 6) + repo.updateDriverLocation("driver-C", lat = 36.0, lon = -115.0, status = "online", timestamp = 1_000L) + + repo.clearAll() + + assertTrue("presence map must be empty after clearAll", repo.driverPresence.value.isEmpty()) + assertTrue("locations map must be empty after clearAll", repo.driverLocations.value.isEmpty()) + } } diff --git a/drivestr/src/main/java/com/drivestr/app/MainActivity.kt b/drivestr/src/main/java/com/drivestr/app/MainActivity.kt index 46c690b..6e5ea96 100644 --- a/drivestr/src/main/java/com/drivestr/app/MainActivity.kt +++ b/drivestr/src/main/java/com/drivestr/app/MainActivity.kt @@ -478,13 +478,15 @@ fun DrivestrApp(settingsRepository: SettingsRepository) { android.util.Log.w("MainActivity", "Key ack pubkey mismatch - ignoring (claimed=${ackData.riderPubKey.take(8)}, signer=${event.pubKey.take(8)})") } else { // Verify authorized follower (approved + not muted via either path). - // - Heavyweight mute (`MutedRider` / "Remove" UX) excludes the rider entirely. - // - Lightweight mute (issue #80, `RoadflareFollower.mutedAt`) suppresses - // key delivery — re-sends in response to acks would defeat the mute. + // `isAnyMuted` covers BOTH heavyweight (`MutedRider` / "Remove" UX, + // which excludes the rider entirely) AND lightweight (issue #80, + // `RoadflareFollower.mutedAt`, which suppresses key delivery — re-sends + // in response to acks would defeat the mute). Single helper so the + // four receive-side handlers (Kind 3173 broadcast/direct, 3188, 3189) + // can't drift on which paths they cover. val follower = driverRoadflareRepo.getFollowers().find { it.pubkey == ackData.riderPubKey } - val isMuted = driverRoadflareRepo.getMutedPubkeys().contains(ackData.riderPubKey) - val isLightMuted = follower?.mutedAt != null - val isAuthorized = follower != null && follower.approved && !isMuted && !isLightMuted + val isAuthorized = follower != null && follower.approved && + !driverRoadflareRepo.isAnyMuted(ackData.riderPubKey) if (!isAuthorized) { android.util.Log.w("MainActivity", "Key ack from unauthorized follower - ignoring") From c016c93c20a1782bf30f96b912f64981d429a7a3 Mon Sep 17 00:00:00 2001 From: variablefate Date: Tue, 5 May 2026 18:56:54 -0700 Subject: [PATCH 8/8] =?UTF-8?q?fix(roadflare):=20code-review=20pass=203=20?= =?UTF-8?q?=E2=80=94=20close=20presence-channel=20cleanup=20gaps=20+=20ato?= =?UTF-8?q?mic=20guard=20(#82)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from pass-3 review on PR #85: 1. **`removeDriver` leaked the presence entry.** It was symmetrically clearing names + locations but missed `_driverPresence`. Concrete failure: removing then re-adding the same driver in one session left `_driverPresence[pubkey]` intact. The same Kind 30014 event (still within its 5-min relay TTL) would then hit the `existing.timestamp >= timestamp` guard in `updateDriverPresence` — driver shows offline until next broadcast tick, defeating the stale-key UX in a real scenario. Added the symmetric clear. 2. **`updateDriverPresence` had non-atomic check-then-act.** Direct `_driverPresence.value =` assignment with a separate read for the guard means two relay threads (`Dispatchers.IO`) calling for the same pubkey can both pass the guard before either writes — and the lower-timestamp write could win, defeating the out-of-order protection the PR's KDoc explicitly advertises. Migrated to `StateFlow.update {}` for CAS-retried atomic read-modify-write. The other in-memory caches (`_driverLocations`) use the simpler unprotected pattern, but those don't claim a guard contract — the presence channel does, so it's held to a stricter standard. 3. **`lastLocationCreatedAt` not pruned when drivers list shrinks.** If a driver is removed and re-added in one session, the prior session's `lastLocationCreatedAt[pubkey]` remained — and gated the same Kind 30014 event at the `eventCreatedAt < lastSeen` boundary. Defense-in-depth fix alongside #1: `resubscribe` now intersects the in-memory map keys with the current drivers list. Without this, even if a future change re-introduced the presence leak, the coordinator-level guard would still mask the failure. Tests: - `removeDriver clears the presence entry for that pubkey` — pins the cleanup. - `updateDriverPresence converges to highest timestamp under concurrent updates` — sanity check launching N JVM threads with shuffled-order timestamps and verifying the highest wins. Smoke test rather than rigorous concurrency proof, but surfaces obvious regressions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/data/FollowedDriversRepository.kt | 29 ++++++++-- .../RoadflareDriverPresenceCoordinator.kt | 9 ++++ .../FollowedDriversRepositoryPresenceTest.kt | 53 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt index 408afd8..27d1cc0 100644 --- a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt @@ -7,6 +7,7 @@ import com.ridestr.common.nostr.events.RoadflareKey import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import org.json.JSONArray import org.json.JSONObject @@ -106,12 +107,27 @@ class FollowedDriversRepository(context: Context) { /** * Update a driver's cached presence from a Kind 30014 event's PUBLIC tags. - * Skips the update if an existing entry has a newer timestamp (out-of-order delivery). + * Skips the update if an existing entry has a newer-or-equal timestamp (out-of-order + * delivery + same-event multi-relay dedup). + * + * Issue #82: uses [kotlinx.coroutines.flow.MutableStateFlow.update] so the read / + * timestamp-compare / write executes atomically under CAS. Direct + * `_driverPresence.value =` would race when multiple relay callbacks (`Dispatchers.IO`) + * update the same pubkey concurrently — both reading the same `existing` before either + * writes — and the lower-timestamp write could win, defeating the guard. The other + * in-memory caches in this file use the simpler unprotected pattern; this one is held + * to a stricter standard because the PR's KDoc explicitly advertises the out-of-order + * guard as a correctness property. */ fun updateDriverPresence(pubkey: String, status: String, timestamp: Long, keyVersion: Int = 0) { - val existing = _driverPresence.value[pubkey] - if (existing != null && existing.timestamp >= timestamp) return - _driverPresence.value = _driverPresence.value + (pubkey to CachedDriverPresence(status, timestamp, keyVersion)) + _driverPresence.update { current -> + val existing = current[pubkey] + if (existing != null && existing.timestamp >= timestamp) { + current + } else { + current + (pubkey to CachedDriverPresence(status, timestamp, keyVersion)) + } + } } /** Remove a driver's cached presence. */ @@ -239,6 +255,11 @@ class FollowedDriversRepository(context: Context) { } // Remove from location cache (in-memory only, no persist needed) _driverLocations.value = _driverLocations.value - pubkey + // Issue #82: also remove the presence entry so a re-add of the same driver + // in the same session doesn't see a stale `(timestamp >= timestamp)` guard + // hit and silently render the re-added driver as offline until their next + // broadcast tick. + _driverPresence.value = _driverPresence.value - pubkey } /** diff --git a/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt b/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt index adeecb3..559b0d3 100644 --- a/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt +++ b/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt @@ -48,6 +48,15 @@ class RoadflareDriverPresenceCoordinator( locationSubId?.let { nostrService.closeRoadflareSubscription(it) } locationSubId = null + // Issue #82: prune lastLocationCreatedAt for drivers no longer in the list. Without + // this, removing + re-adding the same driver in a single session leaves the prior + // session's `lastLocationCreatedAt[pubkey]` intact, which then gates the same Kind + // 30014 event (createdAt unchanged within the 5-min TTL window) at the + // `eventCreatedAt < lastSeen` boundary — driver shows as offline until the next + // broadcast tick, defeating the stale-key UX in a real scenario. + val currentPubkeys = drivers.map { it.pubkey }.toSet() + lastLocationCreatedAt.keys.retainAll(currentPubkeys) + // Issue #82: subscribe to ALL followed drivers, not just those with a current key. // The PUBLIC `status` tag on Kind 30014 lets us track availability for stale-key / // missing-key drivers without ever decrypting the encrypted lat/lon content. The diff --git a/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt index 6c4bc54..0746dc0 100644 --- a/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt +++ b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt @@ -3,6 +3,7 @@ package com.ridestr.common.data import androidx.test.core.app.ApplicationProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before @@ -122,4 +123,56 @@ class FollowedDriversRepositoryPresenceTest { assertTrue("presence map must be empty after clearAll", repo.driverPresence.value.isEmpty()) assertTrue("locations map must be empty after clearAll", repo.driverLocations.value.isEmpty()) } + + @Test + fun `removeDriver clears the presence entry for that pubkey`() { + // Issue #82 pass-3 fix: removeDriver was clearing names + locations but not presence. + // The leak meant a re-add of the same driver in a single session could see the + // `(timestamp >= timestamp)` guard hit on the same Kind 30014 event (still within + // its 5-min relay TTL), rendering the re-added driver as offline until the next + // broadcast tick. Pin the cleanup symmetric with locations. + // (Note: this repo doesn't expose addDriver as a unit-test seam — `_drivers` is only + // mutated via the wider Kind 30011 sync path. The presence clear is unconditional on + // pubkey match, so test it directly without going through the drivers list.) + repo.updateDriverPresence("driver-A", "online", 1_000L, 5) + repo.updateDriverPresence("driver-B", "online", 1_000L, 5) + repo.updateDriverLocation("driver-A", lat = 36.0, lon = -115.0, status = "online", timestamp = 1_000L) + + repo.removeDriver("driver-A") + + assertNull("driver-A presence must be cleared", repo.driverPresence.value["driver-A"]) + assertNull("driver-A location must be cleared", repo.driverLocations.value["driver-A"]) + assertNotNull("driver-B presence must NOT be touched", repo.driverPresence.value["driver-B"]) + } + + @Test + fun `updateDriverPresence converges to highest timestamp under concurrent updates`() { + // Issue #82 pass-3 fix: updateDriverPresence used direct `_driverPresence.value =` + // assignment with a non-atomic read-then-check-then-write guard. Two relay threads + // calling updateDriverPresence for the same pubkey with different timestamps could + // both pass the `existing.timestamp >= timestamp` guard (both reading the same + // `existing` before either writes), and the lower-timestamp write could win the + // StateFlow assignment. The fix uses StateFlow.update {} for CAS-retried + // read-modify-write. + // + // Sanity check: launch many concurrent updates with strictly-ascending timestamps + // (in shuffled order to avoid happens-before ordering by accident) and confirm the + // highest timestamp wins. Uses plain JVM threads to avoid coroutine-scope plumbing + // in a Robolectric test. + val concurrency = 64 + val timestamps = (1..concurrency).toList().shuffled() + val threads = timestamps.map { ts -> + Thread { repo.updateDriverPresence("driver-X", "online", ts.toLong(), 0) } + } + threads.forEach { it.start() } + threads.forEach { it.join() } + + val finalEntry = repo.driverPresence.value["driver-X"] + assertNotNull("driver-X presence must be set", finalEntry) + assertEquals( + "highest timestamp must win after concurrent updates", + concurrency.toLong(), + finalEntry?.timestamp + ) + } }