Nearby notes: tap-to-reveal (consent-gate the location subscription) + subscription hygiene (#1422)

* Nearby notes: tap-to-reveal before any relay REQ, plus shared subscription pool

The nearby-notes counter used to open a live building-precision (precision-8)
geohash REQ to the closest geo relays whenever the mesh public timeline was
visible — a passive location side-channel with no opt-in. Now nothing
subscribes until one explicit act reveals the counter for the session: tapping
the new "check for notes left here" line on the empty mesh timeline (static,
no network), opening the notices sheet's geo tab, or a successful /drop. The
app-info setting stays the default-ON kill switch and still gates /drop.

Subscription hygiene alongside:
- LocationNotesManager.deinit now unsubscribes its live REQ (hopping to the
  main actor like the timer teardown) instead of only invalidating timers.
- New refcounted LocationNotesPool dedupes the counter's and the notices
  sheet's identical 9-cell kind-1 REQs into one shared manager per geohash;
  both callers release-and-reacquire instead of retargeting in place, and the
  sheet releases its ref on dismissal.

The reveal affordance is localized across all 29 catalog locales, and new
NearbyNotesCounterTests cover the no-REQ-before-reveal contract, the 9-cell
filter, NIP-40 expiry handling, single unsubscribe on deactivate, and pool
refcounting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Review fixes: permission-gate the hint, exclude building cell from pre-reveal sampling, explicit-act reveal only

Fixes the verified review findings on the tap-to-reveal PR:

- Permission dead-end: the "check for notes left here" hint now renders
  only when location permission is already authorized (it never prompts).
  Previously a location-denied install could tap it, flip the sticky
  revealed flag, and get nothing for the rest of the session because
  retarget() guards on authorization. Predicate lives on
  NearbyNotesCounter.offersRevealHint(permissionState:) and is reactive
  to the published permission state.

- Building-cell sampling: background geohash sampling subscribed
  geo-sample-<gh> for every regional level including the precision-8
  building cell, pre-reveal — contradicting the claim that nothing
  building-precision hits a relay before the explicit act.
  GeoChannelCoordinator now excludes the building level until
  NearbyNotesCounter.revealed (injectable publisher for tests); coarser
  levels keep the nearby-conversation hint and participant counts
  working, and bookmarks stay exempt (bookmarking is explicit).

- Implicit reveal: opening the notices sheet no longer reveals — the
  sheet auto-lands on the geo tab whenever a location channel is
  selected, so browsing a remote geohash and opening notices revealed
  the LOCAL building subscription. reveal() now fires only on the
  person actively picking the geo segment, and only when the sheet has
  a geo scope (the empty-mesh hint tap and /drop stay as before).

- Subscription hygiene: switching the sheet geo → mesh releases the
  pooled notes manager (the REQ was left streaming behind the mesh
  board); switching back re-acquires from the pool. The dismissal
  release stays balanced — liveGeoManager is nil after the tab-switch
  release.

- VoiceOver: the hint button exposes the plain localized action text
  instead of the decorated "* 📍 … *" label.

- Removed LocationNotesManager.setGeohash: zero callers, and calling it
  on a pooled instance would corrupt the pool's keying and refcounts.

New tests: hint permission gate, explicit-geo-tab reveal contract,
building-cell sampling exclusion before/after reveal, pool
release/re-acquire round trip. Full suite (1467) green; iOS simulator
build green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Deflake peer-snapshot binding waits: longTimeout for the multi-hop pipeline

CI failed once on initialization_bindsPeerSnapshotsIntoAllPeers: the
snapshot -> allPeers binding crosses the mock transport's unstructured
Task, UnifiedPeerService.updatePeers, a receive(on: main), and another
Task { @MainActor } in bindPeerService — all contending with every
parallel worker. On the failing runner the whole suite took 10.1s
(usually ~4.4s locally), so the positive 5s defaultTimeout wait lost
the race. That's exactly the case TestConstants.longTimeout documents;
passing runs return as soon as the condition holds and never pay it.

Not caused by the tap-to-reveal changes: nothing on that pipeline was
touched, and 20 full parallel-suite loops each on the branch and on
origin/main reproduce zero failures locally. The two sibling waits on
the same pipeline in this file get the same timeout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

bitchatTests/ChatViewModelTests.swift

@@ -85,9 +85,14 @@ struct ChatViewModelInitializationTests {
             )
         ])
 
+        // The snapshot → allPeers binding hops the transport's unstructured
+        // Task, UnifiedPeerService, a receive(on: main), and another Task —
+        // all contending with every parallel worker, so a loaded CI runner
+        // can exceed defaultTimeout (observed: one 5s miss on a run where
+        // the whole suite took 10s instead of the usual ~4s).
         let updated = await TestHelpers.waitUntil({
             viewModel.allPeers.contains { $0.peerID == peerID && $0.nickname == "Alice" }
-        }, timeout: TestConstants.defaultTimeout)
+        }, timeout: TestConstants.longTimeout)
 
         #expect(updated)
     }
@@ -119,9 +124,10 @@ struct ChatViewModelIdentityTests {
             )
         ])
 
+        // Same multi-hop snapshot pipeline as above: longTimeout for load.
         let oldPeerBound = await TestHelpers.waitUntil({
             viewModel.connectedPeers.contains(oldPeerID)
-        }, timeout: TestConstants.defaultTimeout)
+        }, timeout: TestConstants.longTimeout)
         #expect(oldPeerBound)
 
         let existingMessage = BitchatMessage(
@@ -155,7 +161,7 @@ struct ChatViewModelIdentityTests {
 
         let newPeerBound = await TestHelpers.waitUntil({
             viewModel.connectedPeers.contains(newPeerID) && !viewModel.connectedPeers.contains(oldPeerID)
-        }, timeout: TestConstants.defaultTimeout)
+        }, timeout: TestConstants.longTimeout)
         #expect(newPeerBound)
 
         viewModel.updatePrivateChatPeerIfNeeded()

bitchatTests/GeoChannelCoordinatorContextTests.swift

@@ -14,6 +14,7 @@
 //
 
 import Testing
+import Combine
 import Foundation
 import CoreLocation
 import Tor
@@ -168,6 +169,44 @@ struct GeoChannelCoordinatorContextTests {
         #expect(await waitUntil { context.endSamplingCount > endCountBeforeRefresh })
     }
 
+    @Test @MainActor
+    func buildingCellJoinsSamplingOnlyAfterNotesReveal() async {
+        TorManager.shared.setAppForeground(true)
+        let locationManager = makeLocationManager()
+        let context = MockGeoChannelContext()
+        let revealed = CurrentValueSubject<Bool, Never>(false)
+        let coordinator = GeoChannelCoordinator(
+            locationManager: locationManager,
+            bookmarksStore: locationManager,
+            torManager: TorManager.shared,
+            notesRevealed: revealed.eraseToAnyPublisher(),
+            context: context
+        )
+        defer { withExtendedLifetime(coordinator) {} }
+
+        // A location fix yields all six channel levels…
+        locationManager.locationManager(
+            CLLocationManager(),
+            didUpdateLocations: [CLLocation(latitude: 21.2850, longitude: -157.8357)]
+        )
+        #expect(await waitUntil {
+            locationManager.availableChannels.contains { $0.level == .building }
+        })
+        let building = locationManager.availableChannels.first { $0.level == .building }!.geohash
+
+        // …but pre-reveal sampling must exclude the building-precision cell:
+        // a passive precision-8 REQ identifies a single address.
+        #expect(await waitUntil {
+            (context.beginSamplingCalls.last?.count ?? 0) == GeohashChannelLevel.allCases.count - 1
+        })
+        #expect(context.beginSamplingCalls.allSatisfy { !$0.contains(building) })
+
+        // The explicit notes act widens sampling to include it.
+        revealed.send(true)
+        #expect(await waitUntil { context.beginSamplingCalls.last?.contains(building) == true })
+        #expect(context.beginSamplingCalls.last?.count == GeohashChannelLevel.allCases.count)
+    }
+
     @Test @MainActor
     func releasedContext_isHeldWeaklyAndSafelyIgnored() async {
         let locationManager = makeLocationManager()

bitchatTests/LocationNotesManagerTests.swift

@@ -122,27 +122,6 @@ struct LocationNotesManagerTests {
         #expect(manager.notes.first?.content == "hi")
     }
 
-    @Test
-    func setGeohash_invalidValueIsIgnored() {
-        var subscribeCount = 0
-        let deps = LocationNotesDependencies(
-            relayLookup: { _, _ in ["wss://relay.one"] },
-            subscribe: { _, _, _, _, _ in
-                subscribeCount += 1
-            },
-            unsubscribe: { _ in },
-            sendEvent: { _, _ in },
-            deriveIdentity: { _ in try NostrIdentity.generate() },
-            now: { Date() }
-        )
-
-        let manager = LocationNotesManager(geohash: "u4pruydq", dependencies: deps)
-        manager.setGeohash("not-valid")
-
-        #expect(manager.geohash == "u4pruydq")
-        #expect(subscribeCount == 1)
-    }
-
     @Test
     func refreshAndCancel_manageSubscriptions() {
         var subscribeIDs: [String] = []

bitchatTests/NearbyNotesCounterTests.swift

@@ -0,0 +1,309 @@
+import Combine
+import CoreLocation
+import XCTest
+@testable import bitchat
+
+/// Tap-to-reveal privacy contract: the nearby-notes counter must not open a
+/// building-precision relay REQ until one explicit act calls `reveal()`, and
+/// the pooled subscription must come up exactly once and go down exactly once.
+@MainActor
+final class NearbyNotesCounterTests: XCTestCase {
+    private var previousNotesEnabled: Any?
+
+    override func setUp() {
+        super.setUp()
+        previousNotesEnabled = UserDefaults.standard.object(forKey: "locationNotes.enabled")
+        UserDefaults.standard.set(true, forKey: "locationNotes.enabled")
+    }
+
+    override func tearDown() {
+        if let previous = previousNotesEnabled as? Bool {
+            UserDefaults.standard.set(previous, forKey: "locationNotes.enabled")
+        } else {
+            UserDefaults.standard.removeObject(forKey: "locationNotes.enabled")
+        }
+        super.tearDown()
+    }
+
+    func test_counterOnlySubscribesAfterReveal_countsUnexpiredNotes_andUnsubscribesOnDeactivate() async throws {
+        let relays = SubscriptionRecorder()
+        let locationManager = try await makeAuthorizedLocationManager()
+        let buildingGeohash = try XCTUnwrap(
+            locationManager.availableChannels.first(where: { $0.level == .building })?.geohash
+        )
+
+        let counter = NearbyNotesCounter(
+            locationManager: locationManager,
+            managerFactory: { LocationNotesManager(geohash: $0, dependencies: relays.dependencies) },
+            releaseManager: { $0?.cancel() }
+        )
+
+        counter.activate()
+        // Let the availableChannels replay and any queued retargets land:
+        // being active is not consent, so still no REQ.
+        try await Task.sleep(nanoseconds: 50_000_000)
+        XCTAssertEqual(relays.subscribeCount, 0)
+        XCTAssertFalse(counter.revealed)
+
+        counter.reveal()
+
+        XCTAssertTrue(counter.revealed)
+        XCTAssertEqual(relays.subscribeCount, 1)
+        let gTags = try XCTUnwrap(geohashTagFilter(of: XCTUnwrap(relays.lastFilter)))
+        XCTAssertEqual(gTags.count, 9)
+        XCTAssertEqual(
+            Set(gTags),
+            Set([buildingGeohash] + Geohash.neighbors(of: buildingGeohash)),
+            "REQ must cover the building geohash plus its 8 neighbors"
+        )
+
+        // An expired NIP-40 note must never count.
+        let identity = try NostrIdentity.generate()
+        let now = Date()
+        let expired = NostrEvent(
+            pubkey: identity.publicKeyHex,
+            createdAt: now.addingTimeInterval(-3600),
+            kind: .textNote,
+            tags: [["g", buildingGeohash], ["expiration", String(Int(now.timeIntervalSince1970) - 60)]],
+            content: "gone"
+        )
+        relays.lastHandler?(try expired.sign(with: identity.schnorrSigningKey()))
+        try await Task.sleep(nanoseconds: 50_000_000)
+        XCTAssertEqual(counter.noteCount, 0)
+
+        // A live matching kind-1 note drives the count to 1.
+        let live = NostrEvent(
+            pubkey: identity.publicKeyHex,
+            createdAt: now,
+            kind: .textNote,
+            tags: [["g", buildingGeohash]],
+            content: "still here"
+        )
+        relays.lastHandler?(try live.sign(with: identity.schnorrSigningKey()))
+        let counted = await waitUntil { counter.noteCount == 1 }
+        XCTAssertTrue(counted)
+        // Still exactly one REQ after all the async retarget re-entries.
+        XCTAssertEqual(relays.subscribeCount, 1)
+
+        counter.deactivate()
+
+        XCTAssertEqual(relays.unsubscribeCount, 1)
+        XCTAssertEqual(counter.noteCount, 0)
+    }
+
+    func test_checkNotesHint_requiresAuthorizedLocationPermission() {
+        let relays = SubscriptionRecorder()
+        let counter = NearbyNotesCounter(
+            locationManager: makeBareLocationManager(),
+            managerFactory: { LocationNotesManager(geohash: $0, dependencies: relays.dependencies) },
+            releaseManager: { $0?.cancel() }
+        )
+
+        // An unauthorized install must never see the hint: the tap can't
+        // subscribe (retarget requires authorization) and must not prompt,
+        // so offering it would be a silent dead-end for the session.
+        XCTAssertFalse(counter.offersRevealHint(permissionState: .notDetermined))
+        XCTAssertFalse(counter.offersRevealHint(permissionState: .denied))
+        XCTAssertFalse(counter.offersRevealHint(permissionState: .restricted))
+        XCTAssertTrue(counter.offersRevealHint(permissionState: .authorized))
+
+        // The app-info kill switch hides it too.
+        LocationNotesSettings.enabled = false
+        XCTAssertFalse(counter.offersRevealHint(permissionState: .authorized))
+        LocationNotesSettings.enabled = true
+
+        // Once revealed, the hint yields to the live strip and count.
+        counter.reveal()
+        XCTAssertFalse(counter.offersRevealHint(permissionState: .authorized))
+        XCTAssertEqual(relays.subscribeCount, 0, "reveal without authorization must not open a REQ")
+    }
+
+    func test_noticesSheet_revealsOnlyOnExplicitGeoTabSelectionWithScope() {
+        // The sheet reveals the local counter only when the person actively
+        // picks the geo tab and the sheet actually has a geo scope —
+        // auto-landing on geo (initial tab) never calls this path at all.
+        XCTAssertTrue(NoticesView.revealsNearbyNotes(onSwitchingTo: .geo, geoGeohash: "u4pruydq"))
+        XCTAssertFalse(NoticesView.revealsNearbyNotes(onSwitchingTo: .geo, geoGeohash: nil))
+        XCTAssertFalse(NoticesView.revealsNearbyNotes(onSwitchingTo: .mesh, geoGeohash: "u4pruydq"))
+    }
+
+    func test_pool_sharesOneManagerPerGeohash_andCancelsOnLastRelease() {
+        let relays = SubscriptionRecorder()
+        let pool = LocationNotesPool(
+            makeManager: { LocationNotesManager(geohash: $0, dependencies: relays.dependencies) }
+        )
+
+        let first = pool.acquire("u4pruydq")
+        let second = pool.acquire("U4PRUYDQ")
+
+        XCTAssertTrue(first === second, "same geohash (case-insensitive) must share one manager")
+        XCTAssertEqual(relays.subscribeCount, 1)
+
+        pool.release(first)
+
+        XCTAssertEqual(relays.unsubscribeCount, 0, "first release keeps the shared REQ live")
+        XCTAssertNotEqual(first.state, .idle)
+
+        pool.release(second)
+
+        XCTAssertEqual(relays.unsubscribeCount, 1)
+        XCTAssertEqual(first.state, .idle)
+
+        // An instance the pool never owned (test-injected) degrades to cancel.
+        let stray = LocationNotesManager(geohash: "u4pruydp", dependencies: relays.dependencies)
+        pool.release(stray)
+        XCTAssertEqual(relays.unsubscribeCount, 2)
+        XCTAssertEqual(stray.state, .idle)
+    }
+
+    func test_pool_reacquireAfterFullRelease_bringsSubscriptionBackUp() {
+        let relays = SubscriptionRecorder()
+        let pool = LocationNotesPool(
+            makeManager: { LocationNotesManager(geohash: $0, dependencies: relays.dependencies) }
+        )
+
+        // The notices sheet's geo → mesh → geo cycle: switching to mesh
+        // releases (REQ goes down), switching back re-acquires (fresh REQ),
+        // with exactly one live REQ at any point.
+        let first = pool.acquire("u4pruydq")
+        XCTAssertEqual(relays.subscribeCount, 1)
+
+        pool.release(first)
+        XCTAssertEqual(relays.unsubscribeCount, 1)
+        XCTAssertEqual(first.state, .idle)
+
+        let second = pool.acquire("u4pruydq")
+        XCTAssertEqual(relays.subscribeCount, 2)
+        XCTAssertNotEqual(second.state, .idle)
+
+        // Dismissal after the round trip releases once more — no double
+        // unsubscribe from the earlier tab-switch release.
+        pool.release(second)
+        XCTAssertEqual(relays.unsubscribeCount, 2)
+    }
+
+    // MARK: - Helpers
+
+    /// A LocationStateManager that never touches CoreLocation; for tests
+    /// that don't need channels or authorization.
+    private func makeBareLocationManager() -> LocationStateManager {
+        let suiteName = "NearbyNotesCounterTests-\(UUID().uuidString)"
+        let storage = UserDefaults(suiteName: suiteName)!
+        storage.removePersistentDomain(forName: suiteName)
+        addTeardownBlock {
+            storage.removePersistentDomain(forName: suiteName)
+        }
+        return LocationStateManager(
+            storage: storage,
+            locationManager: MockLocationManager(authorizationStatus: .denied),
+            geocoder: MockLocationGeocoder(),
+            shouldInitializeCoreLocation: false
+        )
+    }
+
+    /// An authorized LocationStateManager whose availableChannels carry a
+    /// real building-level geohash (Honolulu), built over CoreLocation mocks.
+    private func makeAuthorizedLocationManager() async throws -> LocationStateManager {
+        let suiteName = "NearbyNotesCounterTests-\(UUID().uuidString)"
+        let storage = UserDefaults(suiteName: suiteName)!
+        storage.removePersistentDomain(forName: suiteName)
+        addTeardownBlock {
+            storage.removePersistentDomain(forName: suiteName)
+        }
+
+        let manager = LocationStateManager(
+            storage: storage,
+            locationManager: MockLocationManager(authorizationStatus: .authorizedAlways),
+            geocoder: MockLocationGeocoder(),
+            shouldInitializeCoreLocation: true
+        )
+        let authorized = await waitUntil { manager.permissionState == .authorized }
+        XCTAssertTrue(authorized)
+
+        manager.locationManager(
+            CLLocationManager(),
+            didUpdateLocations: [CLLocation(latitude: 21.2850, longitude: -157.8357)]
+        )
+        let channelsLoaded = await waitUntil {
+            manager.availableChannels.contains { $0.level == .building }
+        }
+        XCTAssertTrue(channelsLoaded)
+        return manager
+    }
+
+    /// The filter's `g` tag values, read through its NIP-01 wire encoding
+    /// (the stored tag filters aren't visible outside the Nostr layer).
+    private func geohashTagFilter(of filter: NostrFilter) throws -> [String]? {
+        let data = try JSONEncoder().encode(filter)
+        let json = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any])
+        return json["#g"] as? [String]
+    }
+
+    private func waitUntil(
+        timeout: TimeInterval = 1.0,
+        condition: @escaping @MainActor () -> Bool
+    ) async -> Bool {
+        let deadline = Date().addingTimeInterval(timeout)
+        while Date() < deadline {
+            if condition() {
+                return true
+            }
+            try? await Task.sleep(nanoseconds: 10_000_000)
+        }
+        return condition()
+    }
+}
+
+/// Stub relay layer: counts REQs, captures the last filter/handler, and never
+/// touches the network.
+@MainActor
+private final class SubscriptionRecorder {
+    private(set) var subscribeCount = 0
+    private(set) var unsubscribeCount = 0
+    private(set) var lastFilter: NostrFilter?
+    private(set) var lastHandler: ((NostrEvent) -> Void)?
+
+    var dependencies: LocationNotesDependencies {
+        LocationNotesDependencies(
+            relayLookup: { _, _ in ["wss://relay.one"] },
+            subscribe: { [weak self] filter, _, _, handler, _ in
+                self?.subscribeCount += 1
+                self?.lastFilter = filter
+                self?.lastHandler = handler
+            },
+            unsubscribe: { [weak self] _ in
+                self?.unsubscribeCount += 1
+            },
+            sendEvent: { _, _ in },
+            deriveIdentity: { _ in try NostrIdentity.generate() },
+            now: { Date() }
+        )
+    }
+}
+
+private final class MockLocationManager: LocationStateManaging {
+    weak var delegate: CLLocationManagerDelegate?
+    var desiredAccuracy: CLLocationAccuracy = 0
+    var distanceFilter: CLLocationDistance = 0
+    var authorizationStatus: CLAuthorizationStatus
+
+    init(authorizationStatus: CLAuthorizationStatus) {
+        self.authorizationStatus = authorizationStatus
+    }
+
+    func requestWhenInUseAuthorization() {}
+    func requestLocation() {}
+    func startUpdatingLocation() {}
+    func stopUpdatingLocation() {}
+}
+
+private final class MockLocationGeocoder: LocationStateGeocoding {
+    func cancelGeocode() {}
+
+    func reverseGeocodeLocation(
+        _ location: CLLocation,
+        completionHandler: @escaping ([CLPlacemark]?, Error?) -> Void
+    ) {
+        completionHandler(nil, nil)
+    }
+}