Skip to content

feat(roadflare): silent mute + decoupled stale-key offer flow (ridestr side)#85

Open
variablefate wants to merge 8 commits intomainfrom
feat/mute-stale-key-flow-82
Open

feat(roadflare): silent mute + decoupled stale-key offer flow (ridestr side)#85
variablefate wants to merge 8 commits intomainfrom
feat/mute-stale-key-flow-82

Conversation

@variablefate
Copy link
Copy Markdown
Owner

Closes the ridestr-side scope of #82 + #83. roadflare-rider's DriverQuoteCoordinator and related screens will follow in a separate PR (matches the user-stated "one big PR per app" plan).

What this changes

Scope A — Silent mute (real)

The lightweight per-follower mute (#80) only suppressed key delivery; muted riders could still spam the driver with offers and pings. This PR adds the receive-side filter:

Twitter-style soft block: muted rider sees no response and falls back to another app (acceptable downgraded UX per product decision documented on #82).

Scope B — Decoupled stale-key offer flow

The RoadFlare key handshake conflated who can decrypt my live location (privacy gate) with who can effectively send me a ride offer (functional gate). Per the analysis on #82, those are independent — and since fare calc never strictly needs driver location (iOS proved this via ADR-0008, and Android already has the state.fareEstimate rider-route fallback at RiderViewModel.kt:1761), stale-key drivers should remain requestable.

The protocol already supports this: RoadflareLocationEvent.create() emits status and key_version as public tags alongside the encrypted lat/lon content. No schema change needed.

  • New CachedDriverPresence(status, timestamp, keyVersion) channel on FollowedDriversRepository. Independent of CachedDriverLocation. 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 + key_version tags via RoadflareLocationEvent.getStatus(event) / getKeyVersion(event) and updates driverPresence first.
    • Best-effort decryption afterwards; success → also updates driverLocations.
    • Out-of-order guard moved up so it applies to presence-only events too.
  • RiderModeScreen.kt driver-list pipeline reads both signals. A driver is isOnline if EITHER decrypted location OR public presence says so. Per-driver fare quote still requires a known location; without one, the offer falls back to the rider-route fare automatically.
  • isBroadcastEligible deliberately tightens to hasKey && locationOnline so the broadcast count matches what sendRoadflareToAll will actually attempt (geographic too-far cap requires real locations).
  • isDirectSelectable stays loose — single-tap to a stale-key driver works.

Stale-signal piggyback (inline key recovery)

When the rider sends an offer to a driver they can't see the location of, fire a Kind 3188 status="stale" alongside. Driver receives both events together; the existing ack handler (#81) re-delivers the current Kind 3186 in response. By the time the driver accepts, the rider has the new key and can decrypt in-ride Kind 30014 updates.

  • New private RiderViewModel.requestKeyRefreshAlongsideOffer(driverPubKey). Wired into sendRoadflareOffer and sendRoadflareOfferWithAlternatePayment. Fire-and-forget with explicit CancellationException rethrow; RoadflareTab.checkStaleKeys remains the periodic backstop for users not in the request flow.

Out of scope

  • roadflare-rider (Android RoadFlare-only rider app): same Scope B treatment for DriverQuoteCoordinator, DriverNetworkTab, DriverSelectionScreen, RideTab. Will land in a follow-up PR per the "one big PR per app" plan.
  • No UI hint for fallback fare — explicit product decision per fix(rider-app): wire per-driver-fare fallback to also cover stale/missing key case #83. Driver shows as available; rider sees the rider-route fare without qualifier; UX matches iOS.
  • Heavyweight/lightweight mute API rename — naming is confusing but the rename is its own concern; tracked separately.

Acceptance criteria from #82 / #83

  • Driver-side: muted rider's Kind 3173 offers don't appear in offer inbox.
  • Driver-side: muted rider's Kind 3189 driver pings drop silently.
  • Single isAnyMuted(pubkey) helper used by all four handlers (3173, 3187, 3188, 3189).
  • Rider-side: stale-key driver appears as available in the driver list (no special UI state, matches iOS).
  • requestRide() succeeds from stale-key state; sends Kind 3188 stale-signal alongside the offer.
  • Driver-side: receiving an offer + stale-signal triggers Kind 3186 re-delivery via the existing feat(drivestr): per-follower lightweight mute (#80) #81 ack handler.
  • :common:assembleDebug + :rider-app:assembleDebug + :drivestr:assembleDebug + :roadflare-rider:assembleDebug clean.
  • :common:testDebugUnitTest + :rider-app:testDebugUnitTest + :drivestr:testDebugUnitTest green.
  • Manual: drivestr + 2 rider clients. Mute rider A. Have A send a request → A sees no response, B unaffected.
  • Manual: drivestr offline >12h, rider with stale key sends a request → driver receives offer + Kind 3188 stale-signal when next online; key re-delivers; ride proceeds.

🤖 Generated with Claude Code

variablefate and others added 4 commits May 5, 2026 15:50
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…ope B foundation)

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) <noreply@anthropic.com>
… Scope B, #83)

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) <noreply@anthropic.com>
@variablefate
Copy link
Copy Markdown
Owner Author

Code review

Found 4 issues:

  1. Broadcast offer mute gap. subscribeToBroadcastRequests callback has no mute check, so a muted rider's broadcast Kind 3173 still lands in the driver's offer inbox. The PR's stated acceptance criterion ("muted rider's Kind 3173 offers don't appear in offer inbox") is only true for the direct/RoadFlare callback. Same driverRoadflareRepository.isAnyMuted(...) one-liner needed at the top of this callback.

subs.set(SubKeys.BROADCAST_REQUESTS, nostrService.subscribeToBroadcastRideRequests(
location = location,
expandSearch = expandSearch
) { request ->
// Observability: should never fire after accept-time subscription closure
if (BuildConfig.DEBUG) {
val stage = _uiState.value.stage
if (stage != DriverStage.AVAILABLE && stage != DriverStage.ROADFLARE_ONLY) {
Log.w(TAG, "subscribeToBroadcastRequests callback during $stage — sub should be closed")
}
}
Log.d(TAG, "Received broadcast ride request from ${request.riderPubKey.take(8)}, fare=${request.fareEstimate}")
// Filter out requests we've already accepted
if (request.eventId in acceptedOfferEventIds) {
Log.d(TAG, "Ignoring already-accepted request: ${request.eventId.take(8)}")
return@subscribeToBroadcastRideRequests
}

  1. RoadflareListenerService.subscribeToRoadflareRequests was not migrated to isAnyMuted. Two compounding gaps in the same callback: (a) it captures getMutedPubkeys() at subscribe-open time so mute changes after subscribe are invisible until service restart; (b) the heavyweight-only mutedPubkeys set means lightweight-muted (RoadflareFollower.mutedAt) riders still trigger the foreground-service notification path. Should use the live isAnyMuted(event.pubKey) check the PR introduces.

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(
"p" to listOf(driverPubKey),
"t" to listOf(RideOfferEvent.ROADFLARE_TAG)
)
) { event, relayUrl ->
// 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) {
Log.d(TAG, "Ignoring RoadFlare from muted rider ${event.pubKey.take(8)}")
return@subscribe
}

  1. FollowedDriversRepository.clearAll() omits the new _driverPresence map. The KDoc on clearAll() says "for logout" and explicitly clears _driverLocations (which has the same in-memory-only semantic), but the new _driverPresence is left intact. After logout + login as a different user in the same process, the previous user's presence data survives until the coordinator's next resubscribe overwrites it.

* Clear all followed drivers and cached names/locations (for logout).
*/
fun clearAll() {
prefs.edit()
.remove(KEY_DRIVERS)
.remove(KEY_DRIVER_NAMES)
.apply()
_drivers.value = emptyList()
_driverNames.value = emptyMap()
_driverLocations.value = emptyMap()
}

  1. requestKeyRefreshAlongsideOffer fires BEFORE setupOfferSubscriptions in both sendRoadflareOffer and sendRoadflareOfferWithAlternatePayment. 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 is armed, and be silently missed. The piggyback should fire AFTER setupOfferSubscriptions, or the two should be launched concurrently.

val pickupRoute = calculatePickupRoute(driverLocation, pickup)
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 {
applyOfferFailureState("Failed to send RoadFlare offer")
}
}

🤖 Generated with Claude Code

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

variablefate and others added 2 commits May 5, 2026 17:33
…82)

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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@variablefate
Copy link
Copy Markdown
Owner Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Pass 2 verified the four pass-1 fixes (broadcast mute guard, RoadflareListenerService live mute check, clearAll presence, piggyback ordering) introduce no regressions. Re-audited the full PR for missed receivers, presence-channel observability, batch send + null-location, and out-of-order edge cases — all clean. One minor KDoc inaccuracy on isAnyMuted caller list addressed in e8def8f. PR ready for review.

🤖 Generated with Claude Code

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

@variablefate variablefate marked this pull request as ready for review May 6, 2026 00:44
variablefate and others added 2 commits May 5, 2026 18:20
…ed (#82)

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) <noreply@anthropic.com>
…aps + atomic guard (#82)

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) <noreply@anthropic.com>
@variablefate
Copy link
Copy Markdown
Owner Author

Code review

Found 3 issues (all addressed in c016c93):

  1. removeDriver leaked the presence entry — symmetric clear was missed alongside locations + names. Concrete failure: remove + re-add same driver in one session, the same Kind 30014 (still within 5-min TTL) hits the existing.timestamp >= timestamp guard and the driver shows offline until next broadcast tick. Defeats the stale-key UX.

https://github.com/variablefate/ridestr/blob/11ac75cb04ea25a3ce3b566f06bd1e6c8a4e63c5/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt#L232-L242

  1. updateDriverPresence had non-atomic check-then-act. Direct _driverPresence.value = assignment with a separate guard read means two relay threads (Dispatchers.IO) for the same pubkey can both pass the guard before either writes, and the lower-timestamp write can win — defeating the out-of-order protection the KDoc advertises. Migrated to StateFlow.update { } for CAS-retried atomic read-modify-write.

https://github.com/variablefate/ridestr/blob/11ac75cb04ea25a3ce3b566f06bd1e6c8a4e63c5/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt#L107-L114

  1. lastLocationCreatedAt not pruned when drivers list shrinks. Same root cause as Driver app needs wallet setup screen in onboarding flow #1 — re-adding a removed driver in the same session leaves the prior session's lastLocationCreatedAt[pubkey] intact, gating the event at eventCreatedAt < lastSeen. Defense-in-depth fix alongside Driver app needs wallet setup screen in onboarding flow #1.

https://github.com/variablefate/ridestr/blob/11ac75cb04ea25a3ce3b566f06bd1e6c8a4e63c5/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt#L47-L60

🤖 Generated with Claude Code

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant