From 5a890559c185bcc181489ee8fe87cb9242a9a1b9 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 5 Jul 2026 17:25:24 -0700 Subject: [PATCH 01/14] feat: restore last-active conversation on launch; label mesh as public MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bitchat always opened into the public mesh timeline regardless of where the user last was. Geohash channels are already restored at launch (LocationStateManager + GeoChannelCoordinator), but first-ever launches and DM-last sessions still landed in mesh, and nothing signalled that mesh is public and unencrypted. - Persist the last-active conversation in ConversationStore under a new `conversation.lastActive` key (mirrors the `locationChannel.selected` idiom), written on every switch in the single-writer selection paths. - At launch, restore a last-used DM via the normal private-chat path, fall back to the conversation list on a first-ever launch or a stale peer, and defer public-channel restore to the existing GeoChannelCoordinator, so no second launch-time writer of activeChannel is introduced. - Add a light "public · unencrypted" label to the mesh header. - Cover persistence and the launch decision with unit tests. activeChannel keeps its non-optional ChannelID type; no coordinator or send-path signatures change. --- bitchat/App/AppChromeModel.swift | 4 + bitchat/App/AppRuntime.swift | 81 +++++++ bitchat/App/ConversationStore.swift | 84 ++++++++ bitchat/Views/ContentHeaderView.swift | 9 + bitchat/Views/ContentView.swift | 3 +- .../ConversationStoreLastActiveTests.swift | 197 ++++++++++++++++++ 6 files changed, 377 insertions(+), 1 deletion(-) create mode 100644 bitchatTests/ConversationStoreLastActiveTests.swift diff --git a/bitchat/App/AppChromeModel.swift b/bitchat/App/AppChromeModel.swift index 3f9cacd7..ea4e86df 100644 --- a/bitchat/App/AppChromeModel.swift +++ b/bitchat/App/AppChromeModel.swift @@ -14,6 +14,10 @@ final class AppChromeModel: ObservableObject { @Published var bluetoothAlertMessage = "" @Published var bluetoothState: CBManagerState = .unknown @Published var showScreenshotPrivacyWarning = false + /// #1064: set once at launch when the last-active conversation resolves to + /// "present the conversation list" (first-ever launch or a stale DM peer). + /// `ContentView` folds this into the people-sheet presentation binding. + @Published var presentsConversationListOnLaunch = false private let chatViewModel: ChatViewModel private var cancellables = Set() diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index a706cfac..e37eb8c1 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -122,11 +122,92 @@ final class AppRuntime: ObservableObject { NetworkActivationService.shared.start() GeohashPresenceService.shared.start() checkForSharedContent() + restoreLastActiveConversationOnLaunch() record(.launched) record(.startupCompleted) } + /// #1064: restore the last-active conversation at launch. A persisted DM + /// re-opens via the normal private-chat path (which never writes + /// `activeChannel`); a first-ever launch or a stale DM peer presents the + /// conversation list; a public channel defers to the existing mesh / + /// `GeoChannelCoordinator` restore (the sole launch-time writer of + /// `activeChannel`), so there is no race. + private func restoreLastActiveConversationOnLaunch() { + let presentation = conversations.restoreLastActiveConversation( + isPeerResolvable: { Self.isDirectChatRestorable($0, favorites: .shared) } + ) + var didOpenDirectChat = false + if case .restoredDirectChat(let peerID) = presentation { + // `startPrivateChat` silently no-ops when the peer is now blocked + // or fails the mutual-favorite-and-connected gate + // (ChatPeerIdentityCoordinator.startPrivateChat), leaving no chat + // open — `selectedPrivateChatPeer` is only set on the success path. + chatViewModel.startPrivateChat(with: peerID) + didOpenDirectChat = chatViewModel.selectedPrivateChatPeer == peerID + } + // Fall back to the conversation list rather than silently landing on + // the public mesh timeline when a restore target existed but could not + // be opened. + if Self.shouldPresentConversationList(for: presentation, didOpenDirectChat: didOpenDirectChat) { + appChromeModel.presentsConversationListOnLaunch = true + } + } + + /// Whether a persisted last-active DM peer is genuinely restorable at + /// launch — validated against *durable* conversation state, never live + /// presence (mesh discovery is async, so no peer is connected yet). A + /// syntactically valid `PeerID` is NOT sufficient: an unknown peer would + /// otherwise fall straight through `startPrivateChat` into an empty phantom + /// DM. Restorable iff the peerID is a geohash/Nostr DM (its stable Nostr + /// identity is embedded in the id itself) or the peer is a persisted + /// favorite (keychain-backed, stored by stable Noise public key across + /// launches). Presence-independent and pure, so it is unit-testable. + static func isDirectChatRestorable( + _ peerID: PeerID, + isPeerFavorited: (PeerID) -> Bool + ) -> Bool { + if peerID.isGeoDM { return true } + return isPeerFavorited(peerID) + } + + /// Production wiring of `isDirectChatRestorable`, extracted so the real + /// favorites lookup (not just a stub predicate) is unit-testable via an + /// injected in-memory-keychain-backed `FavoritesPersistenceService`. + /// `migrateSelectedConversationIfNeeded` can persist the last-active peer in + /// full 64-hex Noise-key form, but the favorites store is keyed by the + /// short, Noise-key-derived id — so normalize with `toShort()` (a no-op on + /// an already-short id) before the lookup, or favorited DMs silently fail to + /// restore. + static func isDirectChatRestorable( + _ peerID: PeerID, + favorites: FavoritesPersistenceService + ) -> Bool { + isDirectChatRestorable(peerID, isPeerFavorited: { + favorites.getFavoriteStatus(forPeerID: $0.toShort()) != nil + }) + } + + /// Pure launch-effect decision, extracted so the fallback is unit-testable + /// without constructing `AppRuntime`: present the conversation list on a + /// first-ever launch, or when a persisted DM could not actually be opened + /// (blocked / stale / gated peer). A public-channel restore is left to + /// `GeoChannelCoordinator`. + static func shouldPresentConversationList( + for presentation: ConversationStore.LaunchPresentation, + didOpenDirectChat: Bool + ) -> Bool { + switch presentation { + case .conversationList: + return true + case .restoredDirectChat: + return !didOpenDirectChat + case .deferToChannelRestore: + return false + } + } + func handleOpenURL(_ url: URL) { record(.openedURL(url.absoluteString)) diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index 88d72085..f1bcc872 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -339,6 +339,88 @@ final class ConversationStore: ObservableObject { let changes = PassthroughSubject() + // MARK: Last-active persistence (#1064) + + /// Persisted last-active conversation, so launch can restore where the + /// user left off instead of always dropping into the public mesh + /// timeline. Mirrors `LocationStateManager`'s `locationChannel.selected` + /// idiom: injected `UserDefaults`, JSON-encoded value. Location channels + /// are deliberately NOT restored here — `GeoChannelCoordinator` already + /// owns launch-time channel restore; persisting a `.location` marker only + /// lets us tell "was in a channel" apart from a first-ever launch. + private let storage: UserDefaults + private let lastActiveKey = "conversation.lastActive" + + /// Snapshot of the persisted value read once at init, before any launch + /// writer (e.g. `GeoChannelCoordinator` re-applying the location channel) + /// can overwrite the key. Restore decisions read this, never the disk. + private let restoredLastActive: LastActiveRecord? + + /// Codable form of the last-active conversation. `peerID` is a peer's + /// `PeerID.id` string (`PeerID` is not itself `Codable`). + private struct LastActiveRecord: Codable, Equatable { + enum Kind: String, Codable { case mesh, location, direct } + let kind: Kind + let peerID: String? + } + + /// What launch should present, decided purely from `restoredLastActive`. + enum LaunchPresentation: Equatable { + /// First-ever launch, or a persisted DM whose peer no longer resolves. + case conversationList + /// A valid persisted DM — the caller re-opens it via the normal + /// private-chat path (this axis never writes `activeChannel`). + case restoredDirectChat(PeerID) + /// Last-active was a public channel — defer to the existing mesh / + /// `GeoChannelCoordinator` restore; do nothing here. + case deferToChannelRestore + } + + init(storage: UserDefaults = .standard) { + self.storage = storage + if let data = storage.data(forKey: lastActiveKey), + let record = try? JSONDecoder().decode(LastActiveRecord.self, from: data) { + self.restoredLastActive = record + } else { + self.restoredLastActive = nil + } + } + + /// Persists the current foreground conversation. Called from the two + /// single-writer selection paths on every switch; an open DM wins over + /// the active channel. + private func persistLastActive() { + let record: LastActiveRecord + if let peerID = selectedPrivatePeerID { + record = LastActiveRecord(kind: .direct, peerID: peerID.id) + } else if activeChannel.isLocation { + record = LastActiveRecord(kind: .location, peerID: nil) + } else { + record = LastActiveRecord(kind: .mesh, peerID: nil) + } + if let data = try? JSONEncoder().encode(record) { + storage.set(data, forKey: lastActiveKey) + } + } + + /// Decides what to present at launch from the value persisted last + /// session. Pure aside from reading the init snapshot: performs no + /// selection mutation and never writes `activeChannel`, so it cannot race + /// the location-channel restore. `isPeerResolvable` lets the caller reject + /// a stale/unaddressable DM peer, falling back to the conversation list. + func restoreLastActiveConversation(isPeerResolvable: (PeerID) -> Bool) -> LaunchPresentation { + guard let record = restoredLastActive else { return .conversationList } + switch record.kind { + case .mesh, .location: + return .deferToChannelRestore + case .direct: + guard let raw = record.peerID, !raw.isEmpty else { return .conversationList } + let peerID = PeerID(str: raw) + guard isPeerResolvable(peerID) else { return .conversationList } + return .restoredDirectChat(peerID) + } + } + // MARK: Intent API /// Returns the conversation for `id`, creating it (with the cap policy @@ -488,6 +570,7 @@ final class ConversationStore: ObservableObject { activeChannel = channelID } refreshDerivedSelection() + persistLastActive() } /// Opens a private chat (`nil` closes it, returning the selection to the @@ -497,6 +580,7 @@ final class ConversationStore: ObservableObject { selectedPrivatePeerID = peerID } refreshDerivedSelection() + persistLastActive() } private func refreshDerivedSelection() { diff --git a/bitchat/Views/ContentHeaderView.swift b/bitchat/Views/ContentHeaderView.swift index eb188ed7..fe027b67 100644 --- a/bitchat/Views/ContentHeaderView.swift +++ b/bitchat/Views/ContentHeaderView.swift @@ -186,6 +186,15 @@ struct ContentHeaderView: View { } .buttonStyle(.plain) + if case .mesh = locationChannelsModel.selectedChannel { + Text(verbatim: "public · unencrypted") + .bitchatFont(size: 10) + .foregroundColor(palette.secondary) + .lineLimit(1) + .fixedSize(horizontal: true, vertical: false) + .accessibilityLabel(Text(verbatim: "public, unencrypted")) + } + Button(action: { withAnimation(.easeInOut(duration: TransportConfig.uiAnimationMediumSeconds)) { showSidebar.toggle() diff --git a/bitchat/Views/ContentView.swift b/bitchat/Views/ContentView.swift index 86bb31ad..49d4bae8 100644 --- a/bitchat/Views/ContentView.swift +++ b/bitchat/Views/ContentView.swift @@ -102,10 +102,11 @@ struct ContentView: View { } .sheet( isPresented: Binding( - get: { showSidebar || selectedPrivatePeerID != nil }, + get: { showSidebar || selectedPrivatePeerID != nil || appChromeModel.presentsConversationListOnLaunch }, set: { isPresented in if !isPresented { showSidebar = false + appChromeModel.presentsConversationListOnLaunch = false privateConversationModel.endConversation() } } diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift new file mode 100644 index 00000000..292947fd --- /dev/null +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -0,0 +1,197 @@ +// +// ConversationStoreLastActiveTests.swift +// bitchatTests +// +// Tests for #1064 last-active persistence: ConversationStore records the +// foreground conversation on every switch and, at the next launch, decides +// what to present — a valid DM restores, a stale DM or a first-ever launch +// falls back to the conversation list, and a public channel defers to the +// existing GeoChannelCoordinator restore. +// +// This is free and unencumbered software released into the public domain. +// For more information, see +// + +import BitFoundation +import Foundation +import XCTest +@testable import bitchat + +@MainActor +final class ConversationStoreLastActiveTests: XCTestCase { + /// A structurally valid short (16-hex) peer id. + private let peerID = PeerID(str: "0123456789abcdef") + + func test_persistsLastActiveOnEverySwitch() { + let storage = makeStorage() + + // Open a DM: the next launch should restore it. + let session1 = ConversationStore(storage: storage) + session1.setSelectedPrivatePeer(peerID) + XCTAssertEqual( + ConversationStore(storage: storage).restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .restoredDirectChat(peerID) + ) + + // Close the DM back to the mesh channel: the write must happen again, + // so the next launch now defers to the channel restore, not the DM. + session1.setSelectedPrivatePeer(nil) + XCTAssertEqual( + ConversationStore(storage: storage).restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .deferToChannelRestore + ) + } + + func test_restoresValidDirectChat() { + let storage = makeStorage() + ConversationStore(storage: storage).setSelectedPrivatePeer(peerID) + + let restored = ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { $0.isValid }) + + XCTAssertEqual(restored, .restoredDirectChat(peerID)) + } + + func test_staleDirectChatFallsBackToConversationList() { + let storage = makeStorage() + ConversationStore(storage: storage).setSelectedPrivatePeer(peerID) + + // The persisted peer no longer resolves at launch. + let restored = ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in false }) + + XCTAssertEqual(restored, .conversationList) + } + + func test_firstLaunchPresentsConversationList() { + let storage = makeStorage() + + let restored = ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in true }) + + XCTAssertEqual(restored, .conversationList) + } + + // MARK: - Launch effect (silent-mesh fallback) + + func test_launchPresentsList_whenRestoredDirectChatDidNotOpen() { + // A persisted DM whose peer is now blocked/stale/gated: startPrivateChat + // no-ops, so no chat opens — must fall back to the conversation list, + // never silently land on the public mesh timeline. + XCTAssertTrue( + AppRuntime.shouldPresentConversationList( + for: .restoredDirectChat(peerID), + didOpenDirectChat: false + ) + ) + } + + func test_launchDoesNotPresentList_whenRestoredDirectChatOpened() { + XCTAssertFalse( + AppRuntime.shouldPresentConversationList( + for: .restoredDirectChat(peerID), + didOpenDirectChat: true + ) + ) + } + + func test_launchDefersToChannelRestore_withoutPresentingList() { + // A public-channel restore is owned by GeoChannelCoordinator; the + // launch decision must not present the list on top of it. + XCTAssertFalse( + AppRuntime.shouldPresentConversationList( + for: .deferToChannelRestore, + didOpenDirectChat: false + ) + ) + } + + // MARK: - Restorability predicate (durable state, not syntax) + + func test_unknownButSyntacticallyValidPeerIsNotRestorable() { + // Regression guard: a well-formed 16-hex peer id we have NO durable + // relationship with must NOT be treated as restorable. Otherwise it + // falls straight through startPrivateChat into an empty phantom DM. + // (The syntax-only `{ $0.isValid }` resolver missed exactly this.) + XCTAssertTrue(peerID.isValid) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable(peerID, isPeerFavorited: { _ in false }) + ) + } + + func test_favoritePeerIsRestorable() { + // A persisted favorite is stored by stable Noise public key and survives + // restart — the one durable, presence-independent mesh relationship. + XCTAssertTrue( + AppRuntime.isDirectChatRestorable(peerID, isPeerFavorited: { _ in true }) + ) + } + + func test_geoDMPeerIsRestorable_withoutFavorite() { + // Geohash/Nostr DM ids embed a stable Nostr identity in the id itself, + // so they are restorable even though they are not favorites. + let geoDMPeer = PeerID(str: "nostr_0123456789abcdef") + XCTAssertTrue(geoDMPeer.isGeoDM) + XCTAssertTrue( + AppRuntime.isDirectChatRestorable(geoDMPeer, isPeerFavorited: { _ in false }) + ) + } + + func test_restoreWithProductionShapedResolver_unknownPeerYieldsConversationList() { + // End-to-end through ConversationStore using the PRODUCTION predicate + // shape (not a bare `{ _ in false }`): a persisted DM whose peer is + // unknown/unfavorited must present the conversation list, never restore + // a phantom DM. This is the test that would have caught the hole. + let storage = makeStorage() + ConversationStore(storage: storage).setSelectedPrivatePeer(peerID) + + let restored = ConversationStore(storage: storage).restoreLastActiveConversation( + isPeerResolvable: { + AppRuntime.isDirectChatRestorable($0, isPeerFavorited: { _ in false }) + } + ) + + XCTAssertEqual(restored, .conversationList) + } + + // MARK: - Production wiring (real FavoritesPersistenceService) + + func test_production_fullHexFavoritePeerIsRestorable() { + // migrateSelectedConversationIfNeeded persists the peer in FULL 64-hex + // Noise-key form; the favorites store is keyed by the short derived id. + // The production resolver must normalize (`toShort()`) so a favorited DM + // still restores. Regression for fix-round-3 finding 1. + let favorites = FavoritesPersistenceService(keychain: MockKeychain()) + let noiseKey = Data((0..<32).map(UInt8.init)) + favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") + + let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) + XCTAssertFalse(fullHexPeer.isShort) // 64-hex, not the short form + XCTAssertTrue( + AppRuntime.isDirectChatRestorable(fullHexPeer, favorites: favorites) + ) + } + + func test_production_unknownShortPeerIsNotRestorable() { + // A syntactically valid short peer with no favorite relationship must + // NOT restore (would otherwise open an empty phantom DM). Pins the real + // wiring so it cannot drift back to a syntax-only check. + let favorites = FavoritesPersistenceService(keychain: MockKeychain()) + XCTAssertTrue(peerID.isValid) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable(peerID, favorites: favorites) + ) + } + + // MARK: - Helpers + + private func makeStorage() -> UserDefaults { + let suiteName = "ConversationStoreLastActiveTests-\(UUID().uuidString)" + let storage = UserDefaults(suiteName: suiteName)! + storage.removePersistentDomain(forName: suiteName) + addTeardownBlock { + storage.removePersistentDomain(forName: suiteName) + } + return storage + } +} From c01be3b35995b414bc15ec84966047cb95a209e3 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 5 Jul 2026 20:51:23 -0700 Subject: [PATCH 02/14] fix: gate in-sheet actions on launch presentation; require isFavorite for DM restore Addresses two Codex P2 review comments on #1364: - ContentSheetViews fingerprint drill-down and image-picker gates missed the launch-presentation flag, so on the launch-fallback path (only presentsConversationListOnLaunch true) tapping a peer fingerprint was a silent no-op. Add || appChromeModel.presentsConversationListOnLaunch to both in-sheet gate conditions to match the ContentView call site. - isDirectChatRestorable treated any non-nil favorite record as restorable, but removeFavorite retains a record (isFavorite: false, theyFavoritedUs: true) when the peer still favorites us, so a DM to an unfavorited peer reopened on restart. Key the resolver on isFavorite, not record existence. Adds a regression test for the unfavorited-but-still-favorited-by-them peer. --- bitchat/App/AppRuntime.swift | 2 +- bitchat/Views/ContentSheetViews.swift | 4 ++-- .../ConversationStoreLastActiveTests.swift | 21 +++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index e37eb8c1..98379cc6 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -185,7 +185,7 @@ final class AppRuntime: ObservableObject { favorites: FavoritesPersistenceService ) -> Bool { isDirectChatRestorable(peerID, isPeerFavorited: { - favorites.getFavoriteStatus(forPeerID: $0.toShort()) != nil + favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false }) } diff --git a/bitchat/Views/ContentSheetViews.swift b/bitchat/Views/ContentSheetViews.swift index dd72d8b6..ac0c5b61 100644 --- a/bitchat/Views/ContentSheetViews.swift +++ b/bitchat/Views/ContentSheetViews.swift @@ -85,7 +85,7 @@ struct ContentPeopleSheetView: View { } } .navigationDestination(isPresented: Binding( - get: { appChromeModel.showingFingerprintFor != nil && (showSidebar || privateConversationModel.selectedPeerID != nil) }, + get: { appChromeModel.showingFingerprintFor != nil && (showSidebar || privateConversationModel.selectedPeerID != nil || appChromeModel.presentsConversationListOnLaunch) }, set: { isPresented in if !isPresented { appChromeModel.clearFingerprint() @@ -105,7 +105,7 @@ struct ContentPeopleSheetView: View { #endif #if os(iOS) .fullScreenCover(isPresented: Binding( - get: { showImagePicker && (showSidebar || privateConversationModel.selectedPeerID != nil) }, + get: { showImagePicker && (showSidebar || privateConversationModel.selectedPeerID != nil || appChromeModel.presentsConversationListOnLaunch) }, set: { newValue in if !newValue { showImagePicker = false diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index 292947fd..dbd3ebaa 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -183,6 +183,27 @@ final class ConversationStoreLastActiveTests: XCTestCase { ) } + func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() { + // removeFavorite RETAINS a record (isFavorite: false, theyFavoritedUs: + // true) when the peer still favorites us. The resolver must key on + // isFavorite, not mere record existence — otherwise a DM to a peer we + // deliberately unfavorited reopens on restart, contradicting the + // "is a persisted favorite" contract. Regression for Codex P2 review. + let favorites = FavoritesPersistenceService(keychain: MockKeychain()) + let noiseKey = Data((0..<32).map(UInt8.init)) + favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") + favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true) + favorites.removeFavorite(peerNoisePublicKey: noiseKey) + + let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) + // The record survives (they still favorite us) but isFavorite is false. + XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())) + XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable(fullHexPeer, favorites: favorites) + ) + } + // MARK: - Helpers private func makeStorage() -> UserDefaults { From 7db98c4d400d4c3ff43e362ac1692b44125e42f4 Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:18:38 -0700 Subject: [PATCH 03/14] test: isolate ConversationStore default storage from .standard The ~48 no-arg `ConversationStore()` call sites in the test target run in the real app host and wrote `conversation.lastActive` into `.standard`, polluting the developer's actual app state and letting back-to-back local runs see each other's persisted selection. Point the init default at a new `defaultStorage` static that returns `.standard` in production but a wiped ephemeral `UserDefaults(suiteName:)` under test, mirroring the existing `ChatViewModel.defaultReadReceiptsDefaults` idiom. Production behavior is unchanged (still `.standard`); no call sites change. Co-Authored-By: Claude Opus 4.8 (1M context) --- bitchat/App/ConversationStore.swift | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index f1bcc872..94e563fd 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -376,7 +376,21 @@ final class ConversationStore: ObservableObject { case deferToChannelRestore } - init(storage: UserDefaults = .standard) { + /// Default persistence store for the last-active conversation. Production + /// uses `.standard`. Under test, a dedicated scratch suite is used instead + /// — wiped at first use per process — so the ~48 no-arg `ConversationStore()` + /// tests never pollute the real app's `.standard` `conversation.lastActive` + /// and back-to-back local runs never see each other's persisted selection. + /// Mirrors `ChatViewModel.defaultReadReceiptsDefaults`. + static let defaultStorage: UserDefaults = { + guard TestEnvironment.isRunningTests else { return .standard } + let suiteName = "chat.bitchat.tests.conversationStore" + guard let scratch = UserDefaults(suiteName: suiteName) else { return .standard } + scratch.removePersistentDomain(forName: suiteName) + return scratch + }() + + init(storage: UserDefaults = ConversationStore.defaultStorage) { self.storage = storage if let data = storage.data(forKey: lastActiveKey), let record = try? JSONDecoder().decode(LastActiveRecord.self, from: data) { From 2ed52ebf4f96ebb93aec79892c92387e88932c06 Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:24:14 -0700 Subject: [PATCH 04/14] fix: require mutual-favorite + unblocked for launch DM restore (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops the `isGeoDM` short-circuit in `isDirectChatRestorable`: a restored `nostr_` geohash-DM id cannot rebuild its full Nostr key at launch (that map is repopulated only from inbound ephemeral events), so it opened an unsendable phantom DM. The predicate now mirrors the open-path gate `ChatPeerIdentityCoordinator.startPrivateChat` — restorable iff the peer is a MUTUAL favorite (we favorite them AND they favorite us) and NOT blocked. The gate's `isConnected` relaxation term is always false at launch, so it drops out of the launch-effective predicate. Threads durable `theyFavoritedUs`/`isPeerBlocked` lookups into the pure predicate the same way `isPeerFavorited` was injected; the production overload wires them via `FavoritesPersistenceService` and the gate's `unifiedIsBlocked` source. Also corrects the doc comment that falsely claimed geohash/Nostr DMs carry a self-resolving identity. Updates the predicate tests: geoDM-without-favorite is now NOT restorable, the "favorite" cases require mutuality, and adds one-way-favorite / blocked-mutual negative cases at both the pure and production-wired layers. Co-Authored-By: Claude Opus 4.8 (1M context) --- bitchat/App/AppRuntime.swift | 64 +++++--- .../ConversationStoreLastActiveTests.swift | 141 +++++++++++++++--- 2 files changed, 167 insertions(+), 38 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 98379cc6..ec60a099 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -136,7 +136,13 @@ final class AppRuntime: ObservableObject { /// `activeChannel`), so there is no race. private func restoreLastActiveConversationOnLaunch() { let presentation = conversations.restoreLastActiveConversation( - isPeerResolvable: { Self.isDirectChatRestorable($0, favorites: .shared) } + isPeerResolvable: { + Self.isDirectChatRestorable( + $0, + favorites: .shared, + isPeerBlocked: { chatViewModel.isPeerBlocked($0) } + ) + } ) var didOpenDirectChat = false if case .restoredDirectChat(let peerID) = presentation { @@ -156,37 +162,55 @@ final class AppRuntime: ObservableObject { } /// Whether a persisted last-active DM peer is genuinely restorable at - /// launch — validated against *durable* conversation state, never live + /// launch — validated against *durable* relationship state, never live /// presence (mesh discovery is async, so no peer is connected yet). A /// syntactically valid `PeerID` is NOT sufficient: an unknown peer would /// otherwise fall straight through `startPrivateChat` into an empty phantom - /// DM. Restorable iff the peerID is a geohash/Nostr DM (its stable Nostr - /// identity is embedded in the id itself) or the peer is a persisted - /// favorite (keychain-backed, stored by stable Noise public key across - /// launches). Presence-independent and pure, so it is unit-testable. + /// DM. Mirrors the open-path gate + /// (`ChatPeerIdentityCoordinator.startPrivateChat`): restorable iff the peer + /// is a MUTUAL favorite (we favorite them AND they favorite us) and NOT + /// blocked. The gate's third relaxation term, `isConnected`, is always false + /// at launch, so it drops out of the launch-effective predicate. A geohash/ + /// Nostr DM is *not* special-cased: its full Nostr key is rebuilt only from + /// inbound ephemeral events, so at launch a restored `nostr_` id cannot + /// resolve and would open an unsendable phantom unless it is also a mutual + /// favorite. Favorites are keychain-backed and keyed by stable Noise public + /// key, so this is presence-independent and pure, hence unit-testable. static func isDirectChatRestorable( _ peerID: PeerID, - isPeerFavorited: (PeerID) -> Bool + isPeerFavorited: (PeerID) -> Bool, + theyFavoritedUs: (PeerID) -> Bool, + isPeerBlocked: (PeerID) -> Bool ) -> Bool { - if peerID.isGeoDM { return true } - return isPeerFavorited(peerID) + guard !isPeerBlocked(peerID) else { return false } + return isPeerFavorited(peerID) && theyFavoritedUs(peerID) } /// Production wiring of `isDirectChatRestorable`, extracted so the real - /// favorites lookup (not just a stub predicate) is unit-testable via an - /// injected in-memory-keychain-backed `FavoritesPersistenceService`. - /// `migrateSelectedConversationIfNeeded` can persist the last-active peer in - /// full 64-hex Noise-key form, but the favorites store is keyed by the - /// short, Noise-key-derived id — so normalize with `toShort()` (a no-op on - /// an already-short id) before the lookup, or favorited DMs silently fail to - /// restore. + /// favorites/block lookups (not just stub predicates) are unit-testable via + /// an injected in-memory-keychain-backed `FavoritesPersistenceService` and a + /// block closure. `migrateSelectedConversationIfNeeded` can persist the + /// last-active peer in full 64-hex Noise-key form, but the favorites store is + /// keyed by the short, Noise-key-derived id — so normalize with `toShort()` + /// (a no-op on an already-short id) before the lookup, or favorited DMs + /// silently fail to restore. The block lookup mirrors the open-path gate's + /// `unifiedIsBlocked` (fingerprint-resolved, so it works for offline + /// favorites). static func isDirectChatRestorable( _ peerID: PeerID, - favorites: FavoritesPersistenceService + favorites: FavoritesPersistenceService, + isPeerBlocked: (PeerID) -> Bool ) -> Bool { - isDirectChatRestorable(peerID, isPeerFavorited: { - favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false - }) + isDirectChatRestorable( + peerID, + isPeerFavorited: { + favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false + }, + theyFavoritedUs: { + favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false + }, + isPeerBlocked: isPeerBlocked + ) } /// Pure launch-effect decision, extracted so the fallback is unit-testable diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index dbd3ebaa..3c1edae6 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -115,25 +115,72 @@ final class ConversationStoreLastActiveTests: XCTestCase { // (The syntax-only `{ $0.isValid }` resolver missed exactly this.) XCTAssertTrue(peerID.isValid) XCTAssertFalse( - AppRuntime.isDirectChatRestorable(peerID, isPeerFavorited: { _ in false }) + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + isPeerBlocked: { _ in false } + ) ) } - func test_favoritePeerIsRestorable() { - // A persisted favorite is stored by stable Noise public key and survives - // restart — the one durable, presence-independent mesh relationship. + func test_mutualFavoritePeerIsRestorable() { + // A persisted MUTUAL favorite is stored by stable Noise public key and + // survives restart — the one durable, presence-independent mesh + // relationship. Mirrors the open-path gate, which only lets an offline + // favorite through when the favorite is mutual. XCTAssertTrue( - AppRuntime.isDirectChatRestorable(peerID, isPeerFavorited: { _ in true }) + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in true }, + theyFavoritedUs: { _ in true }, + isPeerBlocked: { _ in false } + ) ) } - func test_geoDMPeerIsRestorable_withoutFavorite() { - // Geohash/Nostr DM ids embed a stable Nostr identity in the id itself, - // so they are restorable even though they are not favorites. + func test_oneWayFavoritePeerIsNotRestorable() { + // We favorite them but they do NOT favorite us: the open-path gate + // rejects this at launch (isConnected is false), so auto-restoring it + // would inject a "requires favorite" system message into the public + // timeline. The predicate must refuse it up front. + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in true }, + theyFavoritedUs: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_blockedMutualFavoriteIsNotRestorable() { + // A blocked peer is never restorable, even if the favorite is mutual — + // mirrors the gate's first (block) reject. + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in true }, + theyFavoritedUs: { _ in true }, + isPeerBlocked: { _ in true } + ) + ) + } + + func test_geoDMPeerWithoutMutualFavoriteIsNotRestorable() { + // #1064 phantom-DM fix: a geohash/Nostr DM id is NO LONGER special-cased + // as restorable. Its full Nostr key is rebuilt only from inbound + // ephemeral events, so at launch a restored `nostr_` id cannot resolve + // and would open an unsendable phantom. Only a mutual favorite restores. let geoDMPeer = PeerID(str: "nostr_0123456789abcdef") XCTAssertTrue(geoDMPeer.isGeoDM) - XCTAssertTrue( - AppRuntime.isDirectChatRestorable(geoDMPeer, isPeerFavorited: { _ in false }) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + geoDMPeer, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + isPeerBlocked: { _ in false } + ) ) } @@ -147,7 +194,12 @@ final class ConversationStoreLastActiveTests: XCTestCase { let restored = ConversationStore(storage: storage).restoreLastActiveConversation( isPeerResolvable: { - AppRuntime.isDirectChatRestorable($0, isPeerFavorited: { _ in false }) + AppRuntime.isDirectChatRestorable( + $0, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + isPeerBlocked: { _ in false } + ) } ) @@ -156,19 +208,64 @@ final class ConversationStoreLastActiveTests: XCTestCase { // MARK: - Production wiring (real FavoritesPersistenceService) - func test_production_fullHexFavoritePeerIsRestorable() { + func test_production_fullHexMutualFavoritePeerIsRestorable() { // migrateSelectedConversationIfNeeded persists the peer in FULL 64-hex // Noise-key form; the favorites store is keyed by the short derived id. // The production resolver must normalize (`toShort()`) so a favorited DM - // still restores. Regression for fix-round-3 finding 1. + // still restores. Regression for fix-round-3 finding 1. The favorite must + // be MUTUAL to mirror the open-path gate. + let favorites = FavoritesPersistenceService(keychain: MockKeychain()) + let noiseKey = Data((0..<32).map(UInt8.init)) + favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") + favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true) + + let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) + XCTAssertFalse(fullHexPeer.isShort) // 64-hex, not the short form + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + fullHexPeer, + favorites: favorites, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_production_oneWayFavoriteIsNotRestorable() { + // We favorite them but they never favorited us: not mutual, so the + // open-path gate would reject it at launch. The production resolver must + // refuse it rather than auto-open a gated DM. let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) - XCTAssertFalse(fullHexPeer.isShort) // 64-hex, not the short form - XCTAssertTrue( - AppRuntime.isDirectChatRestorable(fullHexPeer, favorites: favorites) + XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) + XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + fullHexPeer, + favorites: favorites, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_production_blockedMutualFavoriteIsNotRestorable() { + // A mutual favorite we have since blocked must NOT restore — mirrors the + // gate's block reject. Block state is injected (it lives in the identity + // manager, not the favorites store). + let favorites = FavoritesPersistenceService(keychain: MockKeychain()) + let noiseKey = Data((0..<32).map(UInt8.init)) + favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") + favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true) + + let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + fullHexPeer, + favorites: favorites, + isPeerBlocked: { _ in true } + ) ) } @@ -179,7 +276,11 @@ final class ConversationStoreLastActiveTests: XCTestCase { let favorites = FavoritesPersistenceService(keychain: MockKeychain()) XCTAssertTrue(peerID.isValid) XCTAssertFalse( - AppRuntime.isDirectChatRestorable(peerID, favorites: favorites) + AppRuntime.isDirectChatRestorable( + peerID, + favorites: favorites, + isPeerBlocked: { _ in false } + ) ) } @@ -200,7 +301,11 @@ final class ConversationStoreLastActiveTests: XCTestCase { XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) XCTAssertFalse( - AppRuntime.isDirectChatRestorable(fullHexPeer, favorites: favorites) + AppRuntime.isDirectChatRestorable( + fullHexPeer, + favorites: favorites, + isPeerBlocked: { _ in false } + ) ) } From 0cc4e9821e19d7f17a9e075d0680de707521201e Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:29:54 -0700 Subject: [PATCH 05/14] fix: silent launch DM restore + change-gated last-active persist (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled fixes on the launch restore/persist path. Ask 3b — suppress launch system messages: the restore comment claimed `startPrivateChat` "silently no-ops" on a gate reject, but the gate emits a `blocked` / `requires_favorite` system message into the CURRENT (public mesh) timeline before returning. Adds an opt-in `suppressSystemMessages` param threaded from the launch restore call through `ChatViewModel` into `ChatPeerIdentityCoordinator.startPrivateChat`; when set, the two gate rejects no-op silently (the chat still does not open, so the caller falls back to the list). Kept as a distinct non-defaulted `ChatViewModel` overload so the zero-argument witness still satisfies the CommandProcessor / ChatNostrCoordinator context protocols. This is the belt-and-suspenders second line behind the predicate fix — a future gate/predicate divergence can no longer leak to the public timeline. Ask 2 — change-gated persist: `setActiveChannel` / `setSelectedPrivatePeer` called `persistLastActive()` unconditionally, outside the equality guard. A redundant channel re-apply at launch (GeoChannelCoordinator re-asserting the default mesh channel) therefore overwrote a persisted `.direct` record after a failed DM restore. Moves the persist inside the actual-change branch in both writers. Tests: a suppressed blocked / one-way-favorite restore emits ZERO system messages and opens no chat; a redundant channel re-apply does not erase the persisted `.direct` record. Co-Authored-By: Claude Opus 4.8 (1M context) --- bitchat/App/AppRuntime.swift | 14 ++++--- bitchat/App/ConversationStore.swift | 10 ++++- .../ChatPeerIdentityCoordinator.swift | 42 ++++++++++--------- bitchat/ViewModels/ChatViewModel.swift | 17 +++++++- ...tPeerIdentityCoordinatorContextTests.swift | 42 +++++++++++++++++++ .../ConversationStoreLastActiveTests.swift | 24 +++++++++++ 6 files changed, 122 insertions(+), 27 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index ec60a099..846b6451 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -146,11 +146,15 @@ final class AppRuntime: ObservableObject { ) var didOpenDirectChat = false if case .restoredDirectChat(let peerID) = presentation { - // `startPrivateChat` silently no-ops when the peer is now blocked - // or fails the mutual-favorite-and-connected gate - // (ChatPeerIdentityCoordinator.startPrivateChat), leaving no chat - // open — `selectedPrivateChatPeer` is only set on the success path. - chatViewModel.startPrivateChat(with: peerID) + // `startPrivateChat`'s gate (ChatPeerIdentityCoordinator) rejects a + // now-blocked or non-mutual-favorite peer by emitting a system + // message and returning WITHOUT opening the chat. At launch that + // message would land in the current (public mesh) timeline, so pass + // `suppressSystemMessages: true` — the reject stays silent and we + // detect it via `selectedPrivateChatPeer`, which is only set on the + // success path. `isDirectChatRestorable` already screens for the + // same conditions; this is the belt-and-suspenders second line. + chatViewModel.startPrivateChat(with: peerID, suppressSystemMessages: true) didOpenDirectChat = chatViewModel.selectedPrivateChatPeer == peerID } // Fall back to the conversation list rather than silently landing on diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index 94e563fd..4a201c90 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -582,9 +582,13 @@ final class ConversationStore: ObservableObject { func setActiveChannel(_ channelID: ChannelID) { if activeChannel != channelID { activeChannel = channelID + // Persist only on an ACTUAL change. A redundant re-apply of the + // current channel (e.g. `GeoChannelCoordinator` re-asserting the + // default mesh channel at launch) must not overwrite a persisted + // `.direct` record when a DM restore has just failed (#1064). + persistLastActive() } refreshDerivedSelection() - persistLastActive() } /// Opens a private chat (`nil` closes it, returning the selection to the @@ -592,9 +596,11 @@ final class ConversationStore: ObservableObject { func setSelectedPrivatePeer(_ peerID: PeerID?) { if selectedPrivatePeerID != peerID { selectedPrivatePeerID = peerID + // Persist only on an actual selection change; the people-sheet + // dismissal re-invokes this with the same (nil) value. + persistLastActive() } refreshDerivedSelection() - persistLastActive() } private func refreshDerivedSelection() { diff --git a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift index 46e0787a..e97f2214 100644 --- a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift @@ -320,37 +320,41 @@ final class ChatPeerIdentityCoordinator { } @MainActor - func startPrivateChat(with peerID: PeerID) { + func startPrivateChat(with peerID: PeerID, suppressSystemMessages: Bool = false) { guard peerID != context.myPeerID else { return } let peerNickname = context.peerNickname(for: peerID) ?? "unknown" if context.unifiedIsBlocked(peerID) { - context.addSystemMessage( - String( - format: String( - localized: "system.chat.blocked", - comment: "System message when starting chat fails because peer is blocked" - ), - locale: .current, - peerNickname + if !suppressSystemMessages { + context.addSystemMessage( + String( + format: String( + localized: "system.chat.blocked", + comment: "System message when starting chat fails because peer is blocked" + ), + locale: .current, + peerNickname + ) ) - ) + } return } if let peer = context.unifiedPeer(for: peerID), peer.isFavorite && !peer.theyFavoritedUs && !peer.isConnected { - context.addSystemMessage( - String( - format: String( - localized: "system.chat.requires_favorite", - comment: "System message when mutual favorite requirement blocks chat" - ), - locale: .current, - peerNickname + if !suppressSystemMessages { + context.addSystemMessage( + String( + format: String( + localized: "system.chat.requires_favorite", + comment: "System message when mutual favorite requirement blocks chat" + ), + locale: .current, + peerNickname + ) ) - ) + } return } diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 803fec7f..49f38560 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1091,12 +1091,27 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele /// Initiates a private chat session with a peer. /// - Parameter peerID: The peer's ID to start chatting with - /// - Note: Switches the UI to private chat mode and loads message history + /// - Note: Switches the UI to private chat mode and loads message history. + /// This zero-extra-argument signature is the witness for the + /// `CommandProcessor` / `ChatNostrCoordinator` context protocols, so it is + /// kept intact; the launch-restore variant is a separate overload below. @MainActor func startPrivateChat(with peerID: PeerID) { peerIdentityCoordinator.startPrivateChat(with: peerID) } + /// #1064 launch-restore variant of `startPrivateChat`. When + /// `suppressSystemMessages` is `true`, a gate rejection (blocked / + /// non-mutual favorite) no-ops silently instead of emitting a system message + /// into the current (public mesh) timeline, so a rejected DM restore falls + /// back to the conversation list cleanly. Kept as a distinct non-defaulted + /// overload — a defaulted extra parameter would not satisfy the context + /// protocols above and a default would make the plain call ambiguous. + @MainActor + func startPrivateChat(with peerID: PeerID, suppressSystemMessages: Bool) { + peerIdentityCoordinator.startPrivateChat(with: peerID, suppressSystemMessages: suppressSystemMessages) + } + @MainActor func endPrivateChat() { peerIdentityCoordinator.endPrivateChat() diff --git a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift index b756999b..17061343 100644 --- a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift @@ -304,6 +304,48 @@ struct ChatPeerIdentityCoordinatorContextTests { #expect(context.markedReadPeers.isEmpty) } + @Test @MainActor + func startPrivateChat_suppressed_blockedPeerEmitsNoSystemMessage() async { + // #1064: at launch a now-blocked (e.g. favorited-then-blocked) peer must + // reject SILENTLY — no "blocked" line leaked into the public timeline — + // while still not opening the chat, so the caller falls back to the list. + let context = MockChatPeerIdentityContext() + let coordinator = ChatPeerIdentityCoordinator(context: context) + let peerID = PeerID(str: "1122334455667788") + context.blockedPeers = [peerID] + + coordinator.startPrivateChat(with: peerID, suppressSystemMessages: true) + + #expect(context.systemMessages.isEmpty) + #expect(context.begunChatSessions.isEmpty) + #expect(context.consolidatedPeers.isEmpty) + #expect(context.markedReadPeers.isEmpty) + } + + @Test @MainActor + func startPrivateChat_suppressed_oneWayFavoriteEmitsNoSystemMessage() async { + // #1064: a one-way favorite (we favorite them, they don't favorite us, + // not connected) trips the mutual-favorite gate. Under suppression the + // reject must emit no "requires favorite" system message and open no chat. + let context = MockChatPeerIdentityContext() + let coordinator = ChatPeerIdentityCoordinator(context: context) + let peerID = PeerID(str: "1122334455667788") + let noiseKey = Data((0..<32).map(UInt8.init)) + var peer = BitchatPeer(peerID: peerID, noisePublicKey: noiseKey, nickname: "alice") + peer.favoriteStatus = makeFavoriteRelationship( + noiseKey: noiseKey, + isFavorite: true, + theyFavoritedUs: false + ) + context.peersByID[peerID] = peer + + coordinator.startPrivateChat(with: peerID, suppressSystemMessages: true) + + #expect(context.systemMessages.isEmpty) + #expect(context.begunChatSessions.isEmpty) + #expect(context.consolidatedPeers.isEmpty) + } + @Test @MainActor func updatePrivateChatPeerIfNeeded_migratesChatStateByFingerprint() async { let context = MockChatPeerIdentityContext() diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index 3c1edae6..cd57869b 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -63,6 +63,30 @@ final class ConversationStoreLastActiveTests: XCTestCase { XCTAssertEqual(restored, .conversationList) } + func test_failedDirectRestoreDoesNotEraseDirectRecord() { + // #1064: a redundant channel re-apply must not clobber a persisted DM. + let storage = makeStorage() + + // Session 1: the user is in a DM — persists a `.direct` record. + ConversationStore(storage: storage).setSelectedPrivatePeer(peerID) + + // Session 2 (launch): a fresh store starts with no private peer + // selected. The DM restore has just failed, and GeoChannelCoordinator + // re-asserts the SAME (default mesh) active channel. Because the channel + // does not actually change, the persist is now guarded out — so the + // `.direct` record on disk must survive rather than be overwritten with + // `.mesh`. + let launch = ConversationStore(storage: storage) + launch.setActiveChannel(launch.activeChannel) + + // Session 3: the DM record is intact and still restores. + XCTAssertEqual( + ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .restoredDirectChat(peerID) + ) + } + func test_firstLaunchPresentsConversationList() { let storage = makeStorage() From e3acdfb599a2a3ac1a4d7824b1a70af54a825580 Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:35:31 -0700 Subject: [PATCH 06/14] fix: single showSidebar latch for launch list, drop one-shot flag (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The launch conversation-list signal was a separate `AppChromeModel` one-shot, `presentsConversationListOnLaunch`, OR'd only into the people-sheet presentation binding. The competing root sheets/covers — the fingerprint sheet and the image pickers — gate on `!showSidebar && selectedPrivatePeerID == nil` and never subtracted that launch term, so when the launch flag raised the people sheet while `showSidebar` was still false, both the people sheet and (if `showingFingerprintFor != nil`) the fingerprint sheet evaluated their `isPresented` binding to true at once — a SwiftUI single-sheet conflict. Rather than thread the launch term through every competing binding, consume the launch signal once through the existing `showSidebar` latch. `showSidebar` moves from a `ContentView` `@State` local onto `AppChromeModel` (`ContentView` already observes it as an `@EnvironmentObject`, and `$appChromeModel.showSidebar` yields the same `Binding` the child views were already handed), so non-view launch code in `AppRuntime` can raise it directly. Every competing sheet already keys off `showSidebar`, so with the launch signal folded into that one latch there is no separate flag to reconcile and the collision cannot occur. `presentsConversationListOnLaunch` and its every reference are deleted. Behavior-preserving: `showSidebar` had a single owning `ContentView` instance, so promoting it to the shared chrome model does not change its lifetime or semantics. Co-Authored-By: Claude Opus 4.8 (1M context) --- bitchat/App/AppChromeModel.swift | 13 +++++++++---- bitchat/App/AppRuntime.swift | 2 +- bitchat/Views/ContentSheetViews.swift | 4 ++-- bitchat/Views/ContentView.swift | 24 ++++++++++++------------ 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/bitchat/App/AppChromeModel.swift b/bitchat/App/AppChromeModel.swift index ea4e86df..76611564 100644 --- a/bitchat/App/AppChromeModel.swift +++ b/bitchat/App/AppChromeModel.swift @@ -14,10 +14,15 @@ final class AppChromeModel: ObservableObject { @Published var bluetoothAlertMessage = "" @Published var bluetoothState: CBManagerState = .unknown @Published var showScreenshotPrivacyWarning = false - /// #1064: set once at launch when the last-active conversation resolves to - /// "present the conversation list" (first-ever launch or a stale DM peer). - /// `ContentView` folds this into the people-sheet presentation binding. - @Published var presentsConversationListOnLaunch = false + /// Latch for the people / conversation-list sheet. Owned here (rather than as + /// `ContentView` local `@State`) so non-view launch code can raise it: on launch + /// `AppRuntime` sets this to `true` when the last-active conversation resolves to + /// "present the conversation list" (first-ever launch or a stale/unrestorable DM + /// peer). `ContentView` binds the people sheet directly to this, and every + /// competing sheet/cover already gates on `!showSidebar`, so a single latch keeps + /// the launch presentation from colliding with the fingerprint / image-picker + /// sheets (#1064). + @Published var showSidebar = false private let chatViewModel: ChatViewModel private var cancellables = Set() diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 846b6451..8c1f9ef2 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -161,7 +161,7 @@ final class AppRuntime: ObservableObject { // the public mesh timeline when a restore target existed but could not // be opened. if Self.shouldPresentConversationList(for: presentation, didOpenDirectChat: didOpenDirectChat) { - appChromeModel.presentsConversationListOnLaunch = true + appChromeModel.showSidebar = true } } diff --git a/bitchat/Views/ContentSheetViews.swift b/bitchat/Views/ContentSheetViews.swift index ac0c5b61..dd72d8b6 100644 --- a/bitchat/Views/ContentSheetViews.swift +++ b/bitchat/Views/ContentSheetViews.swift @@ -85,7 +85,7 @@ struct ContentPeopleSheetView: View { } } .navigationDestination(isPresented: Binding( - get: { appChromeModel.showingFingerprintFor != nil && (showSidebar || privateConversationModel.selectedPeerID != nil || appChromeModel.presentsConversationListOnLaunch) }, + get: { appChromeModel.showingFingerprintFor != nil && (showSidebar || privateConversationModel.selectedPeerID != nil) }, set: { isPresented in if !isPresented { appChromeModel.clearFingerprint() @@ -105,7 +105,7 @@ struct ContentPeopleSheetView: View { #endif #if os(iOS) .fullScreenCover(isPresented: Binding( - get: { showImagePicker && (showSidebar || privateConversationModel.selectedPeerID != nil || appChromeModel.presentsConversationListOnLaunch) }, + get: { showImagePicker && (showSidebar || privateConversationModel.selectedPeerID != nil) }, set: { newValue in if !newValue { showImagePicker = false diff --git a/bitchat/Views/ContentView.swift b/bitchat/Views/ContentView.swift index 49d4bae8..6d3c16dd 100644 --- a/bitchat/Views/ContentView.swift +++ b/bitchat/Views/ContentView.swift @@ -41,7 +41,8 @@ struct ContentView: View { @FocusState private var isTextFieldFocused: Bool @Environment(\.colorScheme) var colorScheme @Environment(\.appTheme) private var appTheme - @State private var showSidebar = false + // `showSidebar` (the people/conversation-list sheet latch) lives on + // `AppChromeModel` so non-view launch code can raise it; see that property. @State private var selectedMessageSender: String? @State private var selectedMessageSenderID: PeerID? @FocusState private var isNicknameFieldFocused: Bool @@ -97,16 +98,15 @@ struct ContentView: View { #endif .onChange(of: selectedPrivatePeerID) { newValue in if newValue != nil { - showSidebar = true + appChromeModel.showSidebar = true } } .sheet( isPresented: Binding( - get: { showSidebar || selectedPrivatePeerID != nil || appChromeModel.presentsConversationListOnLaunch }, + get: { appChromeModel.showSidebar || selectedPrivatePeerID != nil }, set: { isPresented in if !isPresented { - showSidebar = false - appChromeModel.presentsConversationListOnLaunch = false + appChromeModel.showSidebar = false privateConversationModel.endConversation() } } @@ -114,7 +114,7 @@ struct ContentView: View { ) { #if os(iOS) ContentPeopleSheetView( - showSidebar: $showSidebar, + showSidebar: $appChromeModel.showSidebar, messageText: $messageText, selectedMessageSender: $selectedMessageSender, selectedMessageSenderID: $selectedMessageSenderID, @@ -132,7 +132,7 @@ struct ContentView: View { ) #else ContentPeopleSheetView( - showSidebar: $showSidebar, + showSidebar: $appChromeModel.showSidebar, messageText: $messageText, selectedMessageSender: $selectedMessageSender, selectedMessageSenderID: $selectedMessageSenderID, @@ -153,7 +153,7 @@ struct ContentView: View { AppInfoView() } .sheet(isPresented: Binding( - get: { appChromeModel.showingFingerprintFor != nil && !showSidebar && selectedPrivatePeerID == nil }, + get: { appChromeModel.showingFingerprintFor != nil && !appChromeModel.showSidebar && selectedPrivatePeerID == nil }, set: { _ in appChromeModel.clearFingerprint() } )) { if let peerID = appChromeModel.showingFingerprintFor { @@ -163,7 +163,7 @@ struct ContentView: View { } #if os(iOS) .fullScreenCover(isPresented: Binding( - get: { showImagePicker && !showSidebar && selectedPrivatePeerID == nil }, + get: { showImagePicker && !appChromeModel.showSidebar && selectedPrivatePeerID == nil }, set: { newValue in if !newValue { showImagePicker = false @@ -179,7 +179,7 @@ struct ContentView: View { #endif #if os(macOS) .sheet(isPresented: Binding( - get: { showMacImagePicker && !showSidebar && selectedPrivatePeerID == nil }, + get: { showMacImagePicker && !appChromeModel.showSidebar && selectedPrivatePeerID == nil }, set: { newValue in if !newValue { showMacImagePicker = false @@ -268,7 +268,7 @@ struct ContentView: View { private var headerView: some View { ContentHeaderView( - showSidebar: $showSidebar, + showSidebar: $appChromeModel.showSidebar, showVerifySheet: $showVerifySheet, showLocationNotes: $showLocationNotes, notesGeohash: $notesGeohash, @@ -289,7 +289,7 @@ struct ContentView: View { imagePreviewURL: $imagePreviewURL, windowCountPublic: $windowCountPublic, windowCountPrivate: $windowCountPrivate, - showSidebar: $showSidebar, + showSidebar: $appChromeModel.showSidebar, isTextFieldFocused: $isTextFieldFocused ) } From 87f1561f342a1f33e5eb4e8f3035522ce3f3d6e3 Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:41:04 -0700 Subject: [PATCH 07/14] fix: move public trust caption to a localized caption band (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ask 6 — SE-width header overflow: the mesh trust caption lived as a `.fixedSize(horizontal: true)` `Text` inside the header's trailing `.layoutPriority(3)` (non-compressible) cluster, so on a narrow (SE-width) header it forced the row to overflow. Relocated it out of the header into a persistent full-width caption band under the public timeline — the parity twin of the DM sheet's `privacyCaption` band (#1366): same structure (full width, `.themedSurface()`, centered, combined a11y), rendered above the composer in both the glass and matrix layouts, mesh-only (geohash/location channels carry no such caption). It is muted (`palette.secondary`), not orange, because orange is the DM privacy signal and this surface is deliberately public. `ContentView` gains `locationChannelsModel` (already injected on its environment in `BitchatApp`) to gate the mesh condition. Ask 7 — untranslatable hardcoded vocabulary: the caption and its accessibility label were `Text(verbatim:)` literals ("public · unencrypted" / "public, unencrypted") that bypassed `Localizable.xcstrings`. Replaced with two new localized keys — `content.header.public_caption` ("public · nearby", visible, `·` separator matching the DM caption band's "private · end-to-end encrypted") and `content.header.public_caption.a11y` ("public, nearby", VoiceOver, comma form). Both reuse the established public-surface vocabulary from #1357's `content.input.placeholder.mesh` ("… public, nearby") rather than coining a new "unencrypted" string. Only the `en` source value is seeded; other locales are left for translators, matching the existing DM-caption keys. NOT locally compiled (Swift/iOS; CI runs Build iOS + Swift Tests). Visual placement/spacing of the new band is unverified — needs an SE-width eyeball. Co-Authored-By: Claude Opus 4.8 (1M context) --- bitchat/Localizable.xcstrings | 24 ++++++++++++++++++++++++ bitchat/Views/ContentHeaderView.swift | 9 --------- bitchat/Views/ContentView.swift | 26 +++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/bitchat/Localizable.xcstrings b/bitchat/Localizable.xcstrings index 7301e613..eb7cbd5b 100644 --- a/bitchat/Localizable.xcstrings +++ b/bitchat/Localizable.xcstrings @@ -18051,6 +18051,30 @@ } } }, + "content.header.public_caption" : { + "comment" : "Trust caption under the public mesh timeline: this channel is public and reaches nearby devices. Reuses the composer placeholder vocabulary (content.input.placeholder.mesh).", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "public · nearby" + } + } + } + }, + "content.header.public_caption.a11y" : { + "comment" : "Accessibility label for the public mesh trust caption; comma form of content.header.public_caption for VoiceOver.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "public, nearby" + } + } + } + }, "content.help.verification" : { "extractionState" : "manual", "localizations" : { diff --git a/bitchat/Views/ContentHeaderView.swift b/bitchat/Views/ContentHeaderView.swift index fe027b67..eb188ed7 100644 --- a/bitchat/Views/ContentHeaderView.swift +++ b/bitchat/Views/ContentHeaderView.swift @@ -186,15 +186,6 @@ struct ContentHeaderView: View { } .buttonStyle(.plain) - if case .mesh = locationChannelsModel.selectedChannel { - Text(verbatim: "public · unencrypted") - .bitchatFont(size: 10) - .foregroundColor(palette.secondary) - .lineLimit(1) - .fixedSize(horizontal: true, vertical: false) - .accessibilityLabel(Text(verbatim: "public, unencrypted")) - } - Button(action: { withAnimation(.easeInOut(duration: TransportConfig.uiAnimationMediumSeconds)) { showSidebar.toggle() diff --git a/bitchat/Views/ContentView.swift b/bitchat/Views/ContentView.swift index 6d3c16dd..503a2bf7 100644 --- a/bitchat/Views/ContentView.swift +++ b/bitchat/Views/ContentView.swift @@ -35,6 +35,7 @@ struct ContentView: View { @EnvironmentObject private var privateConversationModel: PrivateConversationModel @EnvironmentObject private var verificationModel: VerificationModel @EnvironmentObject private var conversationUIModel: ConversationUIModel + @EnvironmentObject private var locationChannelsModel: LocationChannelsModel @StateObject private var voiceRecordingVM = VoiceRecordingViewModel() @State private var messageText = "" @@ -239,7 +240,10 @@ struct ContentView: View { } .safeAreaInset(edge: .bottom, spacing: 0) { if selectedPrivatePeerID == nil { - composerView + VStack(spacing: 0) { + meshPrivacyCaption + composerView + } } } } else { @@ -260,12 +264,32 @@ struct ContentView: View { Divider() if selectedPrivatePeerID == nil { + meshPrivacyCaption composerView } } } } + /// Persistent trust caption under the PUBLIC mesh timeline — the parity + /// twin of the DM sheet's `privacyCaption` (#1366). Moved here out of the + /// header's non-compressible trailing cluster, where its `.fixedSize` text + /// overflowed narrow (SE-width) headers. Mesh-only: geohash/location + /// channels carry no such caption. Muted rather than orange — orange is the + /// DM privacy signal; this surface is deliberately public. + @ViewBuilder + private var meshPrivacyCaption: some View { + if case .mesh = locationChannelsModel.selectedChannel { + Text("content.header.public_caption") + .bitchatFont(size: 11, weight: .medium) + .foregroundColor(palette.secondary) + .frame(maxWidth: .infinity) + .padding(.vertical, 4) + .themedSurface() + .accessibilityLabel(Text("content.header.public_caption.a11y")) + } + } + private var headerView: some View { ContentHeaderView( showSidebar: $appChromeModel.showSidebar, From 8428dfed666df2868038146b9a25f86872a689c7 Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:42:40 -0700 Subject: [PATCH 08/14] fix: panic wipe erases persisted last-active conversation (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `panicClearAllData` wiped messages, keychain, identity defaults and location state, but never the new `conversation.lastActive` pointer, so after an emergency wipe the next launch could still restore the just-wiped DM or channel — a privacy-load-bearing gap for an activist-safety wipe. Adds `ConversationStore.clearPersistedLastActive()`, which removes the key through the store's own injected `storage` (never by reaching into `.standard`, and keeping `lastActiveKey` private), and calls it from the panic path right after `conversations.clearAll()`. Test: after a persisted restorable DM, a `clearPersistedLastActive()` makes the next launch fall back to the conversation list. NOT locally compiled (Swift/iOS; CI runs Build iOS + Swift Tests). Co-Authored-By: Claude Opus 4.8 (1M context) --- bitchat/App/ConversationStore.swift | 8 ++++++ bitchat/ViewModels/ChatViewModel.swift | 3 +++ .../ConversationStoreLastActiveTests.swift | 25 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index 4a201c90..df467075 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -417,6 +417,14 @@ final class ConversationStore: ObservableObject { } } + /// Erases the persisted last-active conversation. Called from the panic + /// wipe so a restored DM/channel pointer cannot survive an emergency clear. + /// The store owns its injected `storage`, so the key is removed through it + /// (never by reaching into `.standard`), and `lastActiveKey` stays private. + func clearPersistedLastActive() { + storage.removeObject(forKey: lastActiveKey) + } + /// Decides what to present at launch from the value persisted last /// session. Pure aside from reading the init snapshot: performs no /// selection mutation and never writes `activeChannel`, so it cannot race diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 49f38560..15088037 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1177,6 +1177,9 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele // single-writer ConversationStore; the derived `messages` view and // the legacy mirror empty with it) conversations.clearAll() + // Also erase the persisted last-active pointer (#1064) so a wiped + // DM/channel cannot be restored on the next launch. + conversations.clearPersistedLastActive() pendingGeohashSystemMessages.removeAll() // Delete all keychain data (including Noise and Nostr keys) diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index cd57869b..faf5adc4 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -87,6 +87,31 @@ final class ConversationStoreLastActiveTests: XCTestCase { ) } + func test_clearPersistedLastActiveErasesRestoreRecord() { + // #1064 panic wipe: clearing the persisted last-active pointer means a + // wiped DM/channel cannot be restored on the next launch. + let storage = makeStorage() + + // A DM is persisted and would otherwise restore. + let session = ConversationStore(storage: storage) + session.setSelectedPrivatePeer(peerID) + XCTAssertEqual( + ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .restoredDirectChat(peerID) + ) + + // The panic path erases the pointer through the store's own storage. + session.clearPersistedLastActive() + + // The next launch has nothing to restore and falls back to the list. + XCTAssertEqual( + ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .conversationList + ) + } + func test_firstLaunchPresentsConversationList() { let storage = makeStorage() From 325a65ce6f50c0ab8738c4fc33660743798948ad Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 11:16:06 -0700 Subject: [PATCH 09/14] fix: suppress last-active persistence during panic wipe (#1064) panicClearAllData() cleared conversation.lastActive, but the trailing selectedPrivateChatPeer=nil / activeChannel=.mesh resets route through the change-gated setters and re-persist a .mesh record via persistLastActive() (the sole write site), so the removal was a no-op and the pointer survived. Wrap the wipe in a beginPanicWipe()/finishPanicWipe() suppression window so the resets can't re-write the pointer; remove the key once at the end. The pointer now ends absent -> next launch hits the first-launch/list fallback. Adds test_panicWipeOrderLeavesNoRestoreRecord replaying the full panic order. --- bitchat/App/ConversationStore.swift | 19 +++++++++ bitchat/ViewModels/ChatViewModel.swift | 15 +++++-- .../ConversationStoreLastActiveTests.swift | 42 +++++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index df467075..8c669bcc 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -351,6 +351,11 @@ final class ConversationStore: ObservableObject { private let storage: UserDefaults private let lastActiveKey = "conversation.lastActive" + /// True for the duration of a panic wipe. Suppresses `persistLastActive()` + /// so the selection/channel resets the wipe performs cannot re-write the + /// pointer we are about to remove. + private var isPanicWiping = false + /// Snapshot of the persisted value read once at init, before any launch /// writer (e.g. `GeoChannelCoordinator` re-applying the location channel) /// can overwrite the key. Restore decisions read this, never the disk. @@ -404,6 +409,9 @@ final class ConversationStore: ObservableObject { /// single-writer selection paths on every switch; an open DM wins over /// the active channel. private func persistLastActive() { + // Panic wipe suppresses last-active persistence so the selection/channel + // resets that follow it can't re-write the pointer being cleared. + guard !isPanicWiping else { return } let record: LastActiveRecord if let peerID = selectedPrivatePeerID { record = LastActiveRecord(kind: .direct, peerID: peerID.id) @@ -425,6 +433,17 @@ final class ConversationStore: ObservableObject { storage.removeObject(forKey: lastActiveKey) } + /// Begins a panic wipe: suppress last-active persistence so the selection/channel + /// resets that follow cannot re-write the pointer we're about to remove. + func beginPanicWipe() { isPanicWiping = true } + + /// Finishes a panic wipe: remove the persisted pointer and re-enable persistence. + /// MUST be called AFTER all selection/channel state has been reset. + func finishPanicWipe() { + clearPersistedLastActive() + isPanicWiping = false + } + /// Decides what to present at launch from the value persisted last /// session. Pure aside from reading the init snapshot: performs no /// selection mutation and never writes `activeChannel`, so it cannot race diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 15088037..541509b2 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1177,9 +1177,11 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele // single-writer ConversationStore; the derived `messages` view and // the legacy mirror empty with it) conversations.clearAll() - // Also erase the persisted last-active pointer (#1064) so a wiped - // DM/channel cannot be restored on the next launch. - conversations.clearPersistedLastActive() + // Begin suppressing last-active persistence (#1064). The selection and + // channel resets below route through the store's setters, which would + // otherwise re-persist a `.mesh` pointer; the wipe is finished (and the + // pointer removed once) at the very end of this method. + conversations.beginPanicWipe() pendingGeohashSystemMessages.removeAll() // Delete all keychain data (including Noise and Nostr keys) @@ -1307,6 +1309,13 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele #endif } + // Finish the panic wipe (#1064): must run AFTER `selectedPrivateChatPeer + // = nil` / `activeChannel = .mesh` above so no setter re-persists the + // pointer. Removes the last-active key once and re-enables persistence, + // leaving the key ABSENT so next launch hits the conversation-list + // first-launch fallback. + conversations.finishPanicWipe() + // Force immediate UI update for panic mode // UI updates immediately - no flushing needed diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index faf5adc4..9f4523bd 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -112,6 +112,48 @@ final class ConversationStoreLastActiveTests: XCTestCase { ) } + func test_panicWipeOrderLeavesNoRestoreRecord() { + // #1064 panic wipe, FULL ORDER: `clearPersistedLastActive()` alone is + // insufficient because `panicClearAllData()` also runs + // `selectedPrivateChatPeer = nil` and `activeChannel = .mesh` AFTER it — + // both route through the store's setters, whose guarded `persistLastActive()` + // re-writes a `.mesh` record on a real state change, resurrecting the + // pointer. The begin/finish suppression window fixes that. This test + // mirrors panicClearAllData's exact order against a single store: + // beginPanicWipe → setSelectedPrivatePeer(nil) → setActiveChannel(.mesh) → finishPanicWipe + // Without the fix (i.e. without begin/finish), those selection/channel + // resets re-persist a `.mesh` record — here `setSelectedPrivatePeer(nil)` + // is the real state change that writes it (the store's default channel + // is already `.mesh`, so `setActiveChannel(.mesh)` is the second trigger + // whenever panic runs from a non-mesh channel). Either way the pointer + // survives as `.mesh` and the next launch would `.deferToChannelRestore` + // instead of `.conversationList`. + let storage = makeStorage() + + // The user is in a DM: a `.direct` record is persisted and would restore. + let session = ConversationStore(storage: storage) + session.setSelectedPrivatePeer(peerID) + XCTAssertEqual( + ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .restoredDirectChat(peerID) + ) + + // Reproduce panicClearAllData's order on the SAME store. + session.beginPanicWipe() + session.setSelectedPrivatePeer(nil) // re-persist trigger #1 (suppressed) + session.setActiveChannel(.mesh) // re-persist trigger #2 (suppressed) + session.finishPanicWipe() // removes the pointer once + + // A fresh store on the SAME storage finds NO record and falls back to + // the conversation list — the pointer is truly absent, not `.mesh`. + XCTAssertEqual( + ConversationStore(storage: storage) + .restoreLastActiveConversation(isPeerResolvable: { _ in true }), + .conversationList + ) + } + func test_firstLaunchPresentsConversationList() { let storage = makeStorage() From 432475ee5dadc63f966a251877558ff0e7c05535 Mon Sep 17 00:00:00 2001 From: ecgang Date: Wed, 15 Jul 2026 10:04:22 -0700 Subject: [PATCH 10/14] Localize the public-caption keys for all 28 supported locales MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main's localization coverage gate (#1391) now requires every catalog key to cover every locale. Derive the caption and its VoiceOver comma form per locale from content.input.placeholder.mesh's 'public, nearby' suffix — the vocabulary the key's comment already says it reuses. Co-Authored-By: Claude Fable 5 --- bitchat/Localizable.xcstrings | 336 ++++++++++++++++++++++++++++++++++ 1 file changed, 336 insertions(+) diff --git a/bitchat/Localizable.xcstrings b/bitchat/Localizable.xcstrings index 19270cd3..12803d6a 100644 --- a/bitchat/Localizable.xcstrings +++ b/bitchat/Localizable.xcstrings @@ -33758,11 +33758,179 @@ "comment" : "Trust caption under the public mesh timeline: this channel is public and reaches nearby devices. Reuses the composer placeholder vocabulary (content.input.placeholder.mesh).", "extractionState" : "manual", "localizations" : { + "ar" : { + "stringUnit" : { + "state" : "translated", + "value" : "عام · قريب" + } + }, + "bn" : { + "stringUnit" : { + "state" : "translated", + "value" : "পাবলিক · কাছাকাছি" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "öffentlich · in der nähe" + } + }, "en" : { "stringUnit" : { "state" : "translated", "value" : "public · nearby" } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "público · cerca" + } + }, + "fil" : { + "stringUnit" : { + "state" : "translated", + "value" : "pampubliko · malapit" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "public · à proximité" + } + }, + "he" : { + "stringUnit" : { + "state" : "translated", + "value" : "ציבורי · קרוב" + } + }, + "hi" : { + "stringUnit" : { + "state" : "translated", + "value" : "सार्वजनिक · आसपास" + } + }, + "id" : { + "stringUnit" : { + "state" : "translated", + "value" : "publik · terdekat" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "pubblico · nelle vicinanze" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "公開 · 近くの人へ" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "공개 · 근처" + } + }, + "ms" : { + "stringUnit" : { + "state" : "translated", + "value" : "awam · berdekatan" + } + }, + "ne" : { + "stringUnit" : { + "state" : "translated", + "value" : "सार्वजनिक · नजिकको" + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "openbaar · in de buurt" + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "publiczna · w pobliżu" + } + }, + "pt" : { + "stringUnit" : { + "state" : "translated", + "value" : "público · próximo" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "público · por perto" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "публичное · рядом" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "publikt · i närheten" + } + }, + "ta" : { + "stringUnit" : { + "state" : "translated", + "value" : "பொது · அருகில்" + } + }, + "th" : { + "stringUnit" : { + "state" : "translated", + "value" : "สาธารณะ · ใกล้เคียง" + } + }, + "tr" : { + "stringUnit" : { + "state" : "translated", + "value" : "herkese açık · yakında" + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "публічне · поблизу" + } + }, + "ur" : { + "stringUnit" : { + "state" : "translated", + "value" : "عوامی · قریبی" + } + }, + "vi" : { + "stringUnit" : { + "state" : "translated", + "value" : "công khai · gần đây" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "公开 · 附近" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "公開 · 附近" + } } } }, @@ -33770,11 +33938,179 @@ "comment" : "Accessibility label for the public mesh trust caption; comma form of content.header.public_caption for VoiceOver.", "extractionState" : "manual", "localizations" : { + "ar" : { + "stringUnit" : { + "state" : "translated", + "value" : "عام، قريب" + } + }, + "bn" : { + "stringUnit" : { + "state" : "translated", + "value" : "পাবলিক, কাছাকাছি" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "öffentlich, in der nähe" + } + }, "en" : { "stringUnit" : { "state" : "translated", "value" : "public, nearby" } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "público, cerca" + } + }, + "fil" : { + "stringUnit" : { + "state" : "translated", + "value" : "pampubliko, malapit" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "public, à proximité" + } + }, + "he" : { + "stringUnit" : { + "state" : "translated", + "value" : "ציבורי, קרוב" + } + }, + "hi" : { + "stringUnit" : { + "state" : "translated", + "value" : "सार्वजनिक, आसपास" + } + }, + "id" : { + "stringUnit" : { + "state" : "translated", + "value" : "publik, terdekat" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "pubblico, nelle vicinanze" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "公開、近くの人へ" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "공개, 근처" + } + }, + "ms" : { + "stringUnit" : { + "state" : "translated", + "value" : "awam, berdekatan" + } + }, + "ne" : { + "stringUnit" : { + "state" : "translated", + "value" : "सार्वजनिक, नजिकको" + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "openbaar, in de buurt" + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "publiczna, w pobliżu" + } + }, + "pt" : { + "stringUnit" : { + "state" : "translated", + "value" : "público, próximo" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "público, por perto" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "публичное, рядом" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "publikt, i närheten" + } + }, + "ta" : { + "stringUnit" : { + "state" : "translated", + "value" : "பொது, அருகில்" + } + }, + "th" : { + "stringUnit" : { + "state" : "translated", + "value" : "สาธารณะ, ใกล้เคียง" + } + }, + "tr" : { + "stringUnit" : { + "state" : "translated", + "value" : "herkese açık, yakında" + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "публічне, поблизу" + } + }, + "ur" : { + "stringUnit" : { + "state" : "translated", + "value" : "عوامی، قریبی" + } + }, + "vi" : { + "stringUnit" : { + "state" : "translated", + "value" : "công khai, gần đây" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "公开、附近" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "公開、附近" + } } } }, From baacc6a85b6aab1a9abfdaefed619d25c8cfb830 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 26 Jul 2026 12:14:32 -0700 Subject: [PATCH 11/14] Relax launch restore to match chat entry after #1415 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1415 removed the mutual-favorite gate from startPrivateChat — store-and-forward needs only the recipient's noise key, so "the router decides what delivery looks like, not chat entry". That left this branch's launch predicate stricter than chat entry, and its doc comment claiming to mirror a gate that no longer exists. A one-way-favorite DM that the outbox or a courier would deliver happily failed to restore and dropped the user on the conversation list. Restorable is now: not blocked, AND either side has favorited the other, OR we hold a stored cryptographic identity for them. Blocked stays a veto rather than becoming one term among several, and geohash DMs are refused the identity term outright. That refusal is written down rather than relied on: a `nostr_` id does satisfy `isShort`, since the prefix is not part of the length check, and today the lookup misses only because the id's prefix is re-attached and no hex fingerprint starts with it. Luck, not a rule — and the cost of it changing is a phantom geoDM that opens with no error at all, because startPrivateChat skips the handshake for them. The outbox and live Noise session state are deliberately not consulted. Both read empty at launch whatever the truth — the outbox defers loading until protected data is available, session state is in-memory — so a predicate built on them would refuse to restore the very conversations it is meant to admit. The identity lookup is injected through the existing SecureIdentityStateManagerProtocol rather than reached for as a singleton, so these tests stub it the way they already stub favorites instead of sharing process-wide state with the rest of the suite. Three doc comments cited the removed gate and are rewritten, including the one claiming startPrivateChat is a second line of defence here. It is not any more: post-#1415 it screens only self, group and blocked, so this predicate stands alone and is written to hold on its own. Tests invert rather than disappear — they are the record of what changed. Mutation-verified: dropping the geoDM guard fails only the geoDM case, dropping the identity term fails only the established-peer case, and demoting the block veto fails four. One behaviour change deserves a second opinion, called out on the PR: a peer we unfavorited who still favorites us now restores, reversing a guard added for an earlier review. Its premise was the "is a persisted favorite" contract, which #1415 dissolved. Also adds Persian for the two caption keys, which main's fa localization landed after this branch wrote them. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppRuntime.swift | 83 ++++++++-- bitchat/Localizable.xcstrings | 12 ++ bitchat/ViewModels/ChatViewModel.swift | 9 +- .../ConversationStoreLastActiveTests.swift | 152 ++++++++++++++++-- 4 files changed, 219 insertions(+), 37 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 715a7703..eda16d4b 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -171,6 +171,11 @@ final class AppRuntime: ObservableObject { Self.isDirectChatRestorable( $0, favorites: .shared, + hasStoredCryptographicIdentity: { + !chatViewModel.identityManager + .getCryptoIdentitiesByPeerIDPrefix($0.toShort()) + .isEmpty + }, isPeerBlocked: { chatViewModel.isPeerBlocked($0) } ) } @@ -178,13 +183,18 @@ final class AppRuntime: ObservableObject { var didOpenDirectChat = false if case .restoredDirectChat(let peerID) = presentation { // `startPrivateChat`'s gate (ChatPeerIdentityCoordinator) rejects a - // now-blocked or non-mutual-favorite peer by emitting a system - // message and returning WITHOUT opening the chat. At launch that - // message would land in the current (public mesh) timeline, so pass + // now-blocked peer by emitting a system message and returning + // WITHOUT opening the chat. At launch that message would land in + // the current (public mesh) timeline, so pass // `suppressSystemMessages: true` — the reject stays silent and we // detect it via `selectedPrivateChatPeer`, which is only set on the - // success path. `isDirectChatRestorable` already screens for the - // same conditions; this is the belt-and-suspenders second line. + // success path. + // + // Not a second line of defence any more. Post-#1415 that gate + // screens only self, group and blocked, so it catches nothing + // `isDirectChatRestorable` has not already caught. It is kept for + // the narrow race where the peer is blocked between the predicate + // and this call, and for the silent-failure detection above. chatViewModel.startPrivateChat(with: peerID, suppressSystemMessages: true) didOpenDirectChat = chatViewModel.selectedPrivateChatPeer == peerID } @@ -201,24 +211,54 @@ final class AppRuntime: ObservableObject { /// presence (mesh discovery is async, so no peer is connected yet). A /// syntactically valid `PeerID` is NOT sufficient: an unknown peer would /// otherwise fall straight through `startPrivateChat` into an empty phantom - /// DM. Mirrors the open-path gate - /// (`ChatPeerIdentityCoordinator.startPrivateChat`): restorable iff the peer - /// is a MUTUAL favorite (we favorite them AND they favorite us) and NOT - /// blocked. The gate's third relaxation term, `isConnected`, is always false - /// at launch, so it drops out of the launch-effective predicate. A geohash/ - /// Nostr DM is *not* special-cased: its full Nostr key is rebuilt only from - /// inbound ephemeral events, so at launch a restored `nostr_` id cannot - /// resolve and would open an unsendable phantom unless it is also a mutual - /// favorite. Favorites are keychain-backed and keyed by stable Noise public - /// key, so this is presence-independent and pure, hence unit-testable. + /// DM. + /// + /// Restorable iff the peer is NOT blocked and we hold durable evidence the + /// conversation is real: either side has favorited the other, or we have a + /// stored cryptographic identity for them. + /// + /// This tracks `ChatPeerIdentityCoordinator.startPrivateChat`, which #1415 + /// relaxed — it no longer requires a mutual favorite, on the grounds that + /// store-and-forward (couriers, bridge drops, retained outbox) needs only + /// the recipient's noise key, so "the router decides what delivery looks + /// like, not chat entry". Keeping the old mutual-favorite rule here would + /// have made launch-restore stricter than chat entry: a one-way-favorite or + /// merely-known peer whose DM is perfectly sendable would fail to restore + /// and drop the user on the conversation list instead. + /// + /// It cannot simply defer to that gate, though. Post-#1415 the open path + /// screens only self, group and blocked, so this predicate is the sole + /// defence against restoring into a phantom DM, and it has to hold the line + /// on its own. + /// + /// The terms are chosen for durability at launch. Favorites are + /// keychain-backed; stored cryptographic identities are on disk. The outbox + /// and live Noise session state are deliberately NOT consulted — the outbox + /// defers loading until protected data is available and session state is + /// in-memory, so both read empty at launch regardless of the truth. + /// + /// Geohash/Nostr DMs stay excluded: a `nostr_` id's full Nostr key is + /// rebuilt only from inbound ephemeral events, so at launch it cannot + /// resolve, and `startPrivateChat` skips the handshake for geoDMs — a + /// phantom would open with no error at all. Their one durable anchor is a + /// favorite record, so for them the favorite terms decide and the identity + /// term is refused outright. static func isDirectChatRestorable( _ peerID: PeerID, isPeerFavorited: (PeerID) -> Bool, theyFavoritedUs: (PeerID) -> Bool, + hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { + // Blocked stays a veto, never one term among several. guard !isPeerBlocked(peerID) else { return false } - return isPeerFavorited(peerID) && theyFavoritedUs(peerID) + if isPeerFavorited(peerID) || theyFavoritedUs(peerID) { return true } + // Explicit rather than incidental: a `nostr_` id does satisfy + // `isShort` (the prefix is not part of the length check), and today it + // misses only because the identity lookup re-attaches that prefix and + // no hex fingerprint starts with it. That is luck, not a rule. + guard !peerID.isGeoDM, !peerID.isGeoChat else { return false } + return hasStoredCryptographicIdentity(peerID) } /// Production wiring of `isDirectChatRestorable`, extracted so the real @@ -231,9 +271,19 @@ final class AppRuntime: ObservableObject { /// silently fail to restore. The block lookup mirrors the open-path gate's /// `unifiedIsBlocked` (fingerprint-resolved, so it works for offline /// favorites). + /// + /// The identity lookup is passed in rather than reached for: it lives on + /// the injected `SecureIdentityStateManagerProtocol`, so tests stub it the + /// same way they stub the favorites service instead of sharing + /// process-wide state. It needs the same `toShort()` normalization — + /// `getCryptoIdentitiesByPeerIDPrefix` guards on `isShort` and returns an + /// empty result otherwise, so a 64-hex id would read as "no identity" + /// rather than as an error. Synchronous and disk-backed, so it is safe on + /// the launch path. static func isDirectChatRestorable( _ peerID: PeerID, favorites: FavoritesPersistenceService, + hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { isDirectChatRestorable( @@ -244,6 +294,7 @@ final class AppRuntime: ObservableObject { theyFavoritedUs: { favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false }, + hasStoredCryptographicIdentity: hasStoredCryptographicIdentity, isPeerBlocked: isPeerBlocked ) } diff --git a/bitchat/Localizable.xcstrings b/bitchat/Localizable.xcstrings index f7141e82..2d2759a3 100644 --- a/bitchat/Localizable.xcstrings +++ b/bitchat/Localizable.xcstrings @@ -37946,6 +37946,12 @@ "value" : "público · cerca" } }, + "fa" : { + "stringUnit" : { + "state" : "translated", + "value" : "عمومی · اطراف شما" + } + }, "fil" : { "stringUnit" : { "state" : "translated", @@ -38126,6 +38132,12 @@ "value" : "público, cerca" } }, + "fa" : { + "stringUnit" : { + "state" : "translated", + "value" : "عمومی، اطراف شما" + } + }, "fil" : { "stringUnit" : { "state" : "translated", diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 9ab6b66b..2c89dd22 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1502,10 +1502,11 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage } /// #1064 launch-restore variant of `startPrivateChat`. When - /// `suppressSystemMessages` is `true`, a gate rejection (blocked / - /// non-mutual favorite) no-ops silently instead of emitting a system message - /// into the current (public mesh) timeline, so a rejected DM restore falls - /// back to the conversation list cleanly. Kept as a distinct non-defaulted + /// `suppressSystemMessages` is `true`, a gate rejection no-ops silently + /// instead of emitting a system message into the current (public mesh) + /// timeline, so a rejected DM restore falls back to the conversation list + /// cleanly. Since #1415 removed the mutual-favorite gate that is exactly + /// one message — the blocked one. Kept as a distinct non-defaulted /// overload — a defaulted extra parameter would not satisfy the context /// protocols above and a default would make the plain call ambiguous. @MainActor diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index 9f4523bd..ab27e6bd 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -210,6 +210,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { peerID, isPeerFavorited: { _ in false }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -225,21 +226,104 @@ final class ConversationStoreLastActiveTests: XCTestCase { peerID, isPeerFavorited: { _ in true }, theyFavoritedUs: { _ in true }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_oneWayFavoritePeerIsNotRestorable() { - // We favorite them but they do NOT favorite us: the open-path gate - // rejects this at launch (isConnected is false), so auto-restoring it - // would inject a "requires favorite" system message into the public - // timeline. The predicate must refuse it up front. - XCTAssertFalse( + func test_oneWayFavoritePeerIsRestorable() { + // INVERTED by #1415, deliberately kept as the record of that change. + // This used to be false: the open path required a mutual favorite, so + // restoring a one-way favorite would have injected a "requires + // favorite" system message into the public timeline. #1415 removed that + // gate — store-and-forward needs only the recipient's noise key — so a + // one-way favorite is now perfectly sendable, and refusing to restore + // it would make launch stricter than chat entry. + XCTAssertTrue( AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_peerWhoFavoritedUsIsRestorable() { + // The other direction of the same relaxation. + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in true }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_establishedNonFavoritePeerIsRestorable() { + // No favorite either way, but we hold a stored cryptographic identity — + // durable, on disk, and enough for the router to deliver via courier or + // the retained outbox. Restoring it is not a phantom. + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_blockedEstablishedPeerIsNotRestorable() { + // Blocked is a veto, not one term among several: a stored identity must + // not buy its way past the block the way it passes the favorite terms. + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in true } + ) + ) + } + + func test_geoDMWithStoredIdentityIsNotRestorable() { + // The identity term must NOT extend to geohash DMs. A `nostr_` id's + // full Nostr key is rebuilt only from inbound ephemeral events, and + // startPrivateChat skips the handshake for geoDMs, so a phantom would + // open with no error at all. Today the lookup would miss anyway (the + // id's prefix is re-attached and no hex fingerprint starts with it), + // which is luck — this pins the refusal so a change to that lookup + // cannot quietly turn geoDM phantoms back on. + let geoDMPeer = PeerID(str: "nostr_0123456789abcdef") + XCTAssertTrue(geoDMPeer.isGeoDM) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + geoDMPeer, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_geoDMWithFavoriteIsRestorable() { + // A favorite record IS a geoDM's durable anchor (it carries + // peerNostrPublicKey), so the favorite terms still admit one. + let geoDMPeer = PeerID(str: "nostr_0123456789abcdef") + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + geoDMPeer, + isPeerFavorited: { _ in true }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -253,6 +337,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { peerID, isPeerFavorited: { _ in true }, theyFavoritedUs: { _ in true }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in true } ) ) @@ -270,6 +355,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { geoDMPeer, isPeerFavorited: { _ in false }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -289,6 +375,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { $0, isPeerFavorited: { _ in false }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) } @@ -316,15 +403,17 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_production_oneWayFavoriteIsNotRestorable() { - // We favorite them but they never favorited us: not mutual, so the - // open-path gate would reject it at launch. The production resolver must - // refuse it rather than auto-open a gated DM. + func test_production_oneWayFavoriteIsRestorable() { + // INVERTED by #1415, through the real favorites wiring. This used to + // refuse a non-mutual favorite because the open-path gate would have + // rejected it at launch; that gate is gone, so refusing here would make + // launch stricter than chat entry for a DM the router can deliver. let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") @@ -332,10 +421,11 @@ final class ConversationStoreLastActiveTests: XCTestCase { let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) - XCTAssertFalse( + XCTAssertTrue( AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -355,6 +445,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in true } ) ) @@ -370,17 +461,31 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() { + func test_production_unfavoritedPeerWhoStillFavoritesUsIsRestorable() { + // INVERTED by #1415, and the inversion worth arguing about. + // // removeFavorite RETAINS a record (isFavorite: false, theyFavoritedUs: - // true) when the peer still favorites us. The resolver must key on - // isFavorite, not mere record existence — otherwise a DM to a peer we - // deliberately unfavorited reopens on restart, contradicting the - // "is a persisted favorite" contract. Regression for Codex P2 review. + // true) when the peer still favorites us. This previously refused to + // restore, so that a DM to a peer we deliberately unfavorited would not + // reopen on restart — the "is a persisted favorite" contract, added for + // an earlier Codex review. + // + // #1415 dissolved that contract: chat entry no longer consults + // favorites at all, so this conversation is openable by hand and + // sendable through the router. Refusing to restore it would single out + // launch for a stricter rule than every other way in. + // + // Note the protection is largely moot in practice regardless: any peer + // we have actually corresponded with has a stored cryptographic + // identity, which admits them through the identity term whatever the + // favorite record says. Unfavoriting is not blocking — blocking still + // refuses, and that is the control for "do not reopen this". let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") @@ -391,11 +496,24 @@ final class ConversationStoreLastActiveTests: XCTestCase { // The record survives (they still favorite us) but isFavorite is false. XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) + XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + fullHexPeer, + favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + + // Blocking is the control that still refuses, with everything else + // about this peer unchanged. XCTAssertFalse( AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, - isPeerBlocked: { _ in false } + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in true } ) ) } From a3ee2fbb46dd5da9b66cb00ded197409c51bdd28 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 26 Jul 2026 12:20:47 -0700 Subject: [PATCH 12/14] Drop theyFavoritedUs as a restore term, and screen geohash ids first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both cross-model reviewers landed on the same two defects in the previous commit, and they agreed with the doubt I had flagged for the PR. `theyFavoritedUs` was admitting the exact phantom this predicate exists to refuse. A peer who favorited us, whom we never favorited and hold no stored identity for, has no durable local evidence we can address them at all — their opinion of us is not a key. It is gone as a term, so restoring now requires local evidence: our own favorite, or a stored cryptographic identity. That also settles the behaviour change I was going to ask Jack to arbitrate. The earlier review's guard — a peer we deliberately unfavorited must not reopen on restart — SURVIVES intact rather than being reversed, because it never depended on #1415's premise. Its test goes back to its original name and expectation. With a stored identity such a peer does restore, and that is the intended line: the evidence is then our own capability, not the other side's opinion. Unfavoriting is not blocking; blocking still refuses. The geohash screen also ran too late. It sat after the favorite term, so a favorited geoChat id — a public channel identifier, not a direct chat at all — was reported restorable as a DM. Both the geoChat rejection and the geoDM restriction now run before any other term, so nothing can match past them. Mutation-verified, each guard by the test that fails without it: moving the geoChat screen back after the favorite term fails the geoChat case, re-adding an unconditional term fails five phantom-guard tests, and letting geoDMs reach the identity term fails the geoDM case. Two reviewer points I did not act on. Codex reads the identity lookup's `hasPrefix` as a loose prefix match that a collision could satisfy; it is exact by construction, since a short peer ID *is* the first 16 hex of the fingerprint, and every peer-ID lookup in the app shares that property. Codex also proposed making a retained `isFavorite == false` an explicit veto; that would refuse to restore a conversation the user was in when they quit, and the last-active pointer is the more recent signal of intent. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppRuntime.swift | 49 +++++++----- .../ConversationStoreLastActiveTests.swift | 77 ++++++++----------- 2 files changed, 62 insertions(+), 64 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index eda16d4b..77b7fc78 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -213,9 +213,16 @@ final class AppRuntime: ObservableObject { /// otherwise fall straight through `startPrivateChat` into an empty phantom /// DM. /// - /// Restorable iff the peer is NOT blocked and we hold durable evidence the - /// conversation is real: either side has favorited the other, or we have a - /// stored cryptographic identity for them. + /// Restorable iff the peer is NOT blocked and we hold durable evidence + /// *locally* that the conversation is addressable: we favorited them, or we + /// have a stored cryptographic identity for them. + /// + /// `theyFavoritedUs` is deliberately NOT a term. It is remote state, and on + /// its own it proves nothing about our ability to address the peer — a peer + /// who favorited us, whom we never favorited and hold no identity for, is + /// exactly the unaddressable phantom this predicate exists to refuse. It + /// would also override a deliberate local unfavorite on the strength of the + /// other side's opinion. /// /// This tracks `ChatPeerIdentityCoordinator.startPrivateChat`, which #1415 /// relaxed — it no longer requires a mutual favorite, on the grounds that @@ -237,28 +244,31 @@ final class AppRuntime: ObservableObject { /// defers loading until protected data is available and session state is /// in-memory, so both read empty at launch regardless of the truth. /// - /// Geohash/Nostr DMs stay excluded: a `nostr_` id's full Nostr key is - /// rebuilt only from inbound ephemeral events, so at launch it cannot - /// resolve, and `startPrivateChat` skips the handshake for geoDMs — a - /// phantom would open with no error at all. Their one durable anchor is a - /// favorite record, so for them the favorite terms decide and the identity - /// term is refused outright. + /// Geohash/Nostr ids are screened first, before any other term, so nothing + /// can bypass the check by matching earlier. A geoChat id is not a direct + /// chat at all. A geoDM's full Nostr key is rebuilt only from inbound + /// ephemeral events, so at launch it cannot resolve, and `startPrivateChat` + /// skips the handshake for geoDMs — a phantom would open with no error at + /// all. Its one durable anchor is OUR OWN favorite record, which carries + /// `peerNostrPublicKey`, so a geoDM restores on that and nothing else. static func isDirectChatRestorable( _ peerID: PeerID, isPeerFavorited: (PeerID) -> Bool, - theyFavoritedUs: (PeerID) -> Bool, hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { - // Blocked stays a veto, never one term among several. + // Blocked is a veto, never one term among several. guard !isPeerBlocked(peerID) else { return false } - if isPeerFavorited(peerID) || theyFavoritedUs(peerID) { return true } - // Explicit rather than incidental: a `nostr_` id does satisfy - // `isShort` (the prefix is not part of the length check), and today it - // misses only because the identity lookup re-attaches that prefix and - // no hex fingerprint starts with it. That is luck, not a rule. - guard !peerID.isGeoDM, !peerID.isGeoChat else { return false } - return hasStoredCryptographicIdentity(peerID) + // A geohash channel is not a direct chat. + guard !peerID.isGeoChat else { return false } + // Screened before the identity term rather than after, so no earlier + // match can skip it. The identity lookup does not reject these on its + // own merits either: a `nostr_` id satisfies `isShort` (the prefix is + // not part of the length check), and today it misses only because the + // lookup re-attaches that prefix and no hex fingerprint starts with it. + // Luck, not a rule. + if peerID.isGeoDM { return isPeerFavorited(peerID) } + return isPeerFavorited(peerID) || hasStoredCryptographicIdentity(peerID) } /// Production wiring of `isDirectChatRestorable`, extracted so the real @@ -291,9 +301,6 @@ final class AppRuntime: ObservableObject { isPeerFavorited: { favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false }, - theyFavoritedUs: { - favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false - }, hasStoredCryptographicIdentity: hasStoredCryptographicIdentity, isPeerBlocked: isPeerBlocked ) diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index ab27e6bd..4c4df1ef 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -209,7 +209,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -225,7 +224,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in true }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -244,26 +242,42 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_peerWhoFavoritedUsIsRestorable() { - // The other direction of the same relaxation. - XCTAssertTrue( + func test_peerWhoOnlyFavoritedUsIsNotRestorable() { + // Remote state alone is not evidence we can address them. They favorited + // us, we never favorited them, and we hold no stored identity — the + // unaddressable phantom this predicate exists to refuse. Their opinion + // of us says nothing about whether we have a key for them. + XCTAssertFalse( AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in true }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } + func test_geoChatIdIsNeverRestorableAsADirectChat() { + // A geohash channel id is not a direct chat at all, and the screen runs + // before the favorite term so nothing can match past it. + let geoChatID = PeerID(str: "nostr:01234567") + XCTAssertTrue(geoChatID.isGeoChat) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + geoChatID, + isPeerFavorited: { _ in true }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in false } + ) + ) + } + func test_establishedNonFavoritePeerIsRestorable() { // No favorite either way, but we hold a stored cryptographic identity — // durable, on disk, and enough for the router to deliver via courier or @@ -272,7 +286,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in true }, isPeerBlocked: { _ in false } ) @@ -286,7 +299,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in true }, isPeerBlocked: { _ in true } ) @@ -307,7 +319,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( geoDMPeer, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in true }, isPeerBlocked: { _ in false } ) @@ -322,7 +333,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( geoDMPeer, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -336,7 +346,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in true }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in true } ) @@ -354,7 +363,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( geoDMPeer, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -374,7 +382,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( $0, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -467,25 +474,20 @@ final class ConversationStoreLastActiveTests: XCTestCase { ) } - func test_production_unfavoritedPeerWhoStillFavoritesUsIsRestorable() { - // INVERTED by #1415, and the inversion worth arguing about. - // + func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() { // removeFavorite RETAINS a record (isFavorite: false, theyFavoritedUs: - // true) when the peer still favorites us. This previously refused to - // restore, so that a DM to a peer we deliberately unfavorited would not - // reopen on restart — the "is a persisted favorite" contract, added for - // an earlier Codex review. + // true) when the peer still favorites us. The resolver must key on + // isFavorite, not mere record existence — otherwise a DM to a peer we + // deliberately unfavorited reopens on restart. Regression for an + // earlier Codex review, and it SURVIVES the #1415 relaxation: the + // relaxed predicate admits our own favorite or a stored identity, and + // `theyFavoritedUs` is deliberately not a term, so the unfavorite still + // stands on its own. // - // #1415 dissolved that contract: chat entry no longer consults - // favorites at all, so this conversation is openable by hand and - // sendable through the router. Refusing to restore it would single out - // launch for a stricter rule than every other way in. - // - // Note the protection is largely moot in practice regardless: any peer - // we have actually corresponded with has a stored cryptographic - // identity, which admits them through the identity term whatever the - // favorite record says. Unfavoriting is not blocking — blocking still - // refuses, and that is the control for "do not reopen this". + // With a stored identity this peer would restore — see + // test_establishedNonFavoritePeerIsRestorable. That is the intended + // line: the durable evidence is then local capability rather than the + // other side's opinion of us. let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") @@ -497,23 +499,12 @@ final class ConversationStoreLastActiveTests: XCTestCase { XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) - XCTAssertTrue( - AppRuntime.isDirectChatRestorable( - fullHexPeer, - favorites: favorites, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - - // Blocking is the control that still refuses, with everything else - // about this peer unchanged. XCTAssertFalse( AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in true } + isPeerBlocked: { _ in false } ) ) } From 53b74bd1e91c0480fd24e3ee96e45bf812fdecce Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 26 Jul 2026 13:30:15 -0700 Subject: [PATCH 13/14] Let private groups restore, and stop claiming geoDM restore works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps a review found in the launch-restore predicate. A private group could never restore. A group id is `group_` plus 32 hex, so both peer lookups guard on `isShort` and return empty without ever consulting the group — leaving `isDirectChatRestorable` to refuse every group, silently, every time. That is backwards: `startPrivateChat` gates group re-entry on nothing at all, because a group is local state rather than a claim about reaching a peer, so there is no phantom to guard against. Groups are now admitted on their own branch, ahead of the peer terms and behind the block veto. One test pins the admission and a second pins that neither peer term is consulted, so a group cannot be let in by an unrelated term happening to match. The geoDM reasoning was also describing behaviour the code does not have. It claimed a geoDM restores on its own favorite record; in fact `FavoritesPersistenceService` is keyed by Noise public key alone, and `getFavoriteStatus(forPeerID:)` matches by rebuilding `PeerID(publicKey:)`, which carries no prefix and so can never equal a `nostr_`-prefixed id. A geoDM therefore never restores today. The branch is the right shape for the day a Nostr-keyed lookup exists; until then it resolves to false. Documented rather than fixed, because adding that lookup means new favorites plumbing and this change does not touch that service. Mutation-proven: removing the group branch fails both group tests and nothing else, while the blocked-group test keeps passing on its own veto. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppRuntime.swift | 40 ++++++++++++--- .../ConversationStoreLastActiveTests.swift | 49 +++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 77b7fc78..9b550842 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -244,21 +244,45 @@ final class AppRuntime: ObservableObject { /// defers loading until protected data is available and session state is /// in-memory, so both read empty at launch regardless of the truth. /// - /// Geohash/Nostr ids are screened first, before any other term, so nothing - /// can bypass the check by matching earlier. A geoChat id is not a direct - /// chat at all. A geoDM's full Nostr key is rebuilt only from inbound - /// ephemeral events, so at launch it cannot resolve, and `startPrivateChat` - /// skips the handshake for geoDMs — a phantom would open with no error at - /// all. Its one durable anchor is OUR OWN favorite record, which carries - /// `peerNostrPublicKey`, so a geoDM restores on that and nothing else. + /// A private group is a virtual conversation, so none of the peer terms + /// apply to it and none of them would pass: a group id is `group_` plus 32 + /// hex, and both lookups guard on `isShort` (a 16-hex bare), so they return + /// empty for every group without ever consulting the group. Left to the + /// peer terms a group would therefore never restore — silently, every + /// time. `startPrivateChat` gates group re-entry on nothing at all ("no + /// peer identity, favorites, handshake … just select the chat"), and there + /// is no phantom to guard against: the conversation is local state, not a + /// claim about reachability. So groups are admitted outright. + /// + /// Geohash/Nostr ids are screened before the peer terms, so nothing can + /// bypass the check by matching earlier. A geoChat id is not a direct chat + /// at all. A geoDM's full Nostr key is rebuilt only from inbound ephemeral + /// events, so at launch it cannot resolve, and `startPrivateChat` skips the + /// handshake for geoDMs — a phantom would open with no error at all. Its + /// only conceivable durable anchor is our own favorite record. + /// + /// In practice that anchor does not exist yet, so **a geoDM never restores + /// today**. `FavoritesPersistenceService` is keyed by Noise public key + /// alone, and `getFavoriteStatus(forPeerID:)` matches by rebuilding + /// `PeerID(publicKey:)` — which carries no prefix — so it can never equal a + /// `nostr_`-prefixed id no matter what is favorited. The geoDM branch below + /// is the correct shape for the day a Nostr-keyed lookup exists; until + /// then it resolves to `false` and the fallback to the conversation list is + /// the whole behaviour. Documented rather than fixed here: adding that + /// lookup means new favorites plumbing, which is not this change. static func isDirectChatRestorable( _ peerID: PeerID, isPeerFavorited: (PeerID) -> Bool, hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { - // Blocked is a veto, never one term among several. + // Blocked is a veto, never one term among several. Kept ahead of the + // group branch so the veto is unconditional; a group id is never + // blocked in practice, so the order costs nothing. guard !isPeerBlocked(peerID) else { return false } + // A private group is local state, not a claim about reaching a peer. + // The peer terms below cannot represent it and would refuse it. + if peerID.isGroup { return true } // A geohash channel is not a direct chat. guard !peerID.isGeoChat else { return false } // Screened before the identity term rather than after, so no earlier diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index 4c4df1ef..60464b29 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -339,6 +339,55 @@ final class ConversationStoreLastActiveTests: XCTestCase { ) } + func test_privateGroupIsRestorable() { + // A group id is `group_` plus 32 hex, so both peer lookups guard on + // `isShort` and return empty for it. Left to those terms a group would + // never restore, silently, every time — even though `startPrivateChat` + // gates group re-entry on nothing at all. + let group = PeerID(str: "group_" + String(repeating: "ab", count: 16)) + XCTAssertTrue(group.isGroup, "test fixture is not a group id") + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + group, + isPeerFavorited: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_privateGroupNeverConsultsThePeerTerms() { + // Admitting groups by accident — because some peer term happened to + // match — would be a different bug wearing the same green check. The + // group must be admitted on its own branch, before either lookup runs. + let group = PeerID(str: "group_" + String(repeating: "cd", count: 16)) + var favoriteLookups = 0 + var identityLookups = 0 + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + group, + isPeerFavorited: { _ in favoriteLookups += 1; return false }, + hasStoredCryptographicIdentity: { _ in identityLookups += 1; return false }, + isPeerBlocked: { _ in false } + ) + ) + XCTAssertEqual(favoriteLookups, 0, "group restore consulted the favorites term") + XCTAssertEqual(identityLookups, 0, "group restore consulted the identity term") + } + + func test_blockedGroupIsNotRestorable() { + // Block stays an unconditional veto, ahead of the group branch. + let group = PeerID(str: "group_" + String(repeating: "ef", count: 16)) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + group, + isPeerFavorited: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in true } + ) + ) + } + func test_blockedMutualFavoriteIsNotRestorable() { // A blocked peer is never restorable, even if the favorite is mutual — // mirrors the gate's first (block) reject. From 5a81fcf3dd9102186f81c153027d8ec59a637ab0 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 26 Jul 2026 13:34:52 -0700 Subject: [PATCH 14/14] Bound the group claim to what the code actually proves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cross-model pass caught the new doc comment claiming more than the code does. `isGroup` tests the `group_` prefix only — `PeerID(str:)` assigns a prefix by `hasPrefix` and never validates the bare — so the branch admits any persisted id claiming to be a group, not a group proven to exist. The comment now says so, along with why that is acceptable here: the value is written by our own selection path into local state, never parsed from the network, and a corrupted one restores an empty group rather than the phantom DM this predicate exists to prevent. Two other overstatements struck. "A group id is never blocked in practice" was unsupported and load-bearing for nothing. And the geoDM branch resolves to false for the *production* closure only — the injected seam will return true for a stub that accepts a geoDM, so a passing test there is not evidence that geoDM restore works. Said plainly, since the whole point of the previous commit was to stop the comment claiming behaviour the code lacks. Two tests: id classes are mutually exclusive, so the branch ordering is not load-bearing the day that stops being true; and the malformed-group case is pinned as deliberate, so if the id ever becomes untrusted this test is what has to change. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppRuntime.swift | 41 ++++++++++++++----- .../ConversationStoreLastActiveTests.swift | 38 +++++++++++++++++ 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 9b550842..0823647e 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -250,9 +250,29 @@ final class AppRuntime: ObservableObject { /// empty for every group without ever consulting the group. Left to the /// peer terms a group would therefore never restore — silently, every /// time. `startPrivateChat` gates group re-entry on nothing at all ("no - /// peer identity, favorites, handshake … just select the chat"), and there - /// is no phantom to guard against: the conversation is local state, not a - /// claim about reachability. So groups are admitted outright. + /// peer identity, favorites, handshake … just select the chat"), so + /// admitting groups here restores what re-entry would have opened. (Not + /// quite an identity: the block veto above has no counterpart in + /// `startPrivateChat`'s group branch, so restore is the narrower of the + /// two. Moot today — `isBlocked` needs a resolved fingerprint and a group + /// id never has one — but stated rather than leaned on.) + /// + /// Note the bound on that admission: `isGroup` tests the `group_` prefix + /// only — `PeerID(str:)` assigns a prefix by `hasPrefix` and never + /// validates the bare — so this branch admits any persisted id *claiming* + /// to be a group, not a group proven to exist. + /// + /// That is acceptable, but for a narrower reason than "the id is trusted". + /// A group id can certainly originate remotely: an invite carries one, and + /// `GroupProtocol` derives the conversation id from it. Every such id is + /// built by `PeerID(groupID:)`, which hex-encodes the raw bytes, so a + /// group learned from the network still has a structurally valid bare. A + /// malformed `group_` id therefore implies corrupted local persistence + /// rather than hostile input. And the failure it produces is an empty + /// group, not a DM compose box aimed at an unreachable peer — which is the + /// specific hazard this predicate exists to prevent. Admitting only groups + /// that still exist locally would need a membership lookup this predicate + /// does not take. /// /// Geohash/Nostr ids are screened before the peer terms, so nothing can /// bypass the check by matching earlier. A geoChat id is not a direct chat @@ -266,19 +286,20 @@ final class AppRuntime: ObservableObject { /// alone, and `getFavoriteStatus(forPeerID:)` matches by rebuilding /// `PeerID(publicKey:)` — which carries no prefix — so it can never equal a /// `nostr_`-prefixed id no matter what is favorited. The geoDM branch below - /// is the correct shape for the day a Nostr-keyed lookup exists; until - /// then it resolves to `false` and the fallback to the conversation list is - /// the whole behaviour. Documented rather than fixed here: adding that - /// lookup means new favorites plumbing, which is not this change. + /// is kept because it is the right shape once a Nostr-keyed lookup exists, + /// but read it precisely: it resolves to `false` for the *production* + /// closure only. The injected seam will happily return `true` for a stub + /// that accepts a geoDM, so a passing test here is not evidence that geoDM + /// restore works. Documented rather than fixed: wiring the real lookup + /// means new favorites plumbing, which is not this change. static func isDirectChatRestorable( _ peerID: PeerID, isPeerFavorited: (PeerID) -> Bool, hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { - // Blocked is a veto, never one term among several. Kept ahead of the - // group branch so the veto is unconditional; a group id is never - // blocked in practice, so the order costs nothing. + // Blocked is a veto, never one term among several — including over the + // group branch below, so no id class can sidestep it. guard !isPeerBlocked(peerID) else { return false } // A private group is local state, not a claim about reaching a peer. // The peer terms below cannot represent it and would refuse it. diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index 60464b29..f7fc07b3 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -375,6 +375,44 @@ final class ConversationStoreLastActiveTests: XCTestCase { XCTAssertEqual(identityLookups, 0, "group restore consulted the identity term") } + func test_idClassesAreMutuallyExclusive() { + // The group branch sits between the geoChat guard and the geoDM one, + // so its placement would be load-bearing if an id could belong to two + // classes at once. `PeerID` assigns exactly one prefix, so it cannot — + // pin that, because the day it stops being true the ordering silently + // decides which rule wins. + let group = PeerID(str: "group_" + String(repeating: "ab", count: 16)) + let geoDM = PeerID(str: "nostr_0123456789abcdef") + let geoChat = PeerID(str: "nostr:someGeohashChannel") + + XCTAssertTrue(group.isGroup) + XCTAssertFalse(group.isGeoDM) + XCTAssertFalse(group.isGeoChat) + + XCTAssertFalse(geoDM.isGroup) + XCTAssertFalse(geoChat.isGroup) + } + + func test_malformedGroupIDIsStillAdmitted_documentingTheBound() { + // `isGroup` tests the prefix only — `PeerID(str:)` never validates the + // bare — so this branch admits any persisted id claiming to be a + // group. That is deliberate and bounded: the value is written by our + // own selection path into local state, never parsed from the network, + // and the failure it can produce is an empty group rather than the + // phantom DM this predicate exists to prevent. Pinned so that if the + // id ever becomes untrusted, this test is the thing that has to change. + let malformed = PeerID(str: "group_not-hex") + XCTAssertTrue(malformed.isGroup) + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + malformed, + isPeerFavorited: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + func test_blockedGroupIsNotRestorable() { // Block stays an unconditional veto, ahead of the group branch. let group = PeerID(str: "group_" + String(repeating: "ef", count: 16))