From 5a890559c185bcc181489ee8fe87cb9242a9a1b9 Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 5 Jul 2026 17:25:24 -0700 Subject: [PATCH 01/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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)) From ff4ec5dd3fbeb48a26834aa04b0edc2c10f2724d Mon Sep 17 00:00:00 2001 From: ecgang Date: Wed, 26 Aug 2026 09:51:11 -0600 Subject: [PATCH 15/19] Say the mesh channel is public once, not twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1595 landed because two surfaces describing the same security property had drifted apart. This PR was about to recreate that: the new header caption content.header.public_caption ("public · nearby") and main's existing composer placeholder content.input.placeholder.mesh ("message #mesh — public, nearby") say the same thing on the same screen, in 30 locales each, with no mechanism keeping them in step. Keep the caption and drop the clause from the placeholder. The caption is the better carrier of the two: it is visible whenever the public timeline is, while the placeholder disappears the moment someone starts typing — which is exactly when they are about to disclose something. The 30 placeholder translations are cut at the em-dash rather than retranslated, so each locale keeps its own wording for the half it still carries. The caption's comment now says it is the sole carrier, so the next person to add a disclosure string has somewhere to look. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/Localizable.xcstrings | 62 +++++++++++++++++------------------ 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/bitchat/Localizable.xcstrings b/bitchat/Localizable.xcstrings index 5f159f53..3d657e29 100644 --- a/bitchat/Localizable.xcstrings +++ b/bitchat/Localizable.xcstrings @@ -56140,7 +56140,7 @@ } }, "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).", + "comment" : "Trust caption under the public mesh timeline: this channel is public and reaches nearby devices. Sole carrier of that disclosure — the composer placeholder deliberately does not repeat it (#1595).", "extractionState" : "manual", "localizations" : { "ar" : { @@ -57445,181 +57445,181 @@ "ar" : { "stringUnit" : { "state" : "needs_review", - "value" : "رسالة #mesh — عام، قريب" + "value" : "رسالة #mesh" } }, "bn" : { "stringUnit" : { "state" : "needs_review", - "value" : "#mesh-এ বার্তা — পাবলিক, কাছাকাছি" + "value" : "#mesh-এ বার্তা" } }, "de" : { "stringUnit" : { "state" : "needs_review", - "value" : "nachricht an #mesh — öffentlich, in der nähe" + "value" : "nachricht an #mesh" } }, "en" : { "stringUnit" : { "state" : "translated", - "value" : "message #mesh — public, nearby" + "value" : "message #mesh" } }, "es" : { "stringUnit" : { "state" : "translated", - "value" : "mensaje #mesh — público, cerca" + "value" : "mensaje #mesh" } }, "fa" : { "stringUnit" : { "state" : "translated", - "value" : "پیام به #mesh — عمومی، اطراف شما" + "value" : "پیام به #mesh" } }, "fil" : { "stringUnit" : { "state" : "needs_review", - "value" : "mensahe sa #mesh — pampubliko, malapit" + "value" : "mensahe sa #mesh" } }, "fr" : { "stringUnit" : { "state" : "needs_review", - "value" : "message vers #mesh — public, à proximité" + "value" : "message vers #mesh" } }, "he" : { "stringUnit" : { "state" : "needs_review", - "value" : "הודעה ל-#mesh — ציבורי, קרוב" + "value" : "הודעה ל-#mesh" } }, "hi" : { "stringUnit" : { "state" : "needs_review", - "value" : "संदेश #mesh — सार्वजनिक, आसपास" + "value" : "संदेश #mesh" } }, "id" : { "stringUnit" : { "state" : "needs_review", - "value" : "pesan #mesh — publik, terdekat" + "value" : "pesan #mesh" } }, "it" : { "stringUnit" : { "state" : "needs_review", - "value" : "messaggio a #mesh — pubblico, nelle vicinanze" + "value" : "messaggio a #mesh" } }, "ja" : { "stringUnit" : { "state" : "needs_review", - "value" : "#mesh にメッセージ — 公開、近くの人へ" + "value" : "#mesh にメッセージ" } }, "ko" : { "stringUnit" : { "state" : "needs_review", - "value" : "#mesh 에 메시지 — 공개, 근처" + "value" : "#mesh 에 메시지" } }, "ms" : { "stringUnit" : { "state" : "needs_review", - "value" : "pesan #mesh — awam, berdekatan" + "value" : "pesan #mesh" } }, "ne" : { "stringUnit" : { "state" : "needs_review", - "value" : "#mesh मा सन्देश — सार्वजनिक, नजिकको" + "value" : "#mesh मा सन्देश" } }, "nl" : { "stringUnit" : { "state" : "needs_review", - "value" : "bericht naar #mesh — openbaar, in de buurt" + "value" : "bericht naar #mesh" } }, "pl" : { "stringUnit" : { "state" : "needs_review", - "value" : "wiadomość #mesh — publiczna, w pobliżu" + "value" : "wiadomość #mesh" } }, "pt" : { "stringUnit" : { "state" : "needs_review", - "value" : "mensagem para #mesh — público, próximo" + "value" : "mensagem para #mesh" } }, "pt-BR" : { "stringUnit" : { "state" : "needs_review", - "value" : "mensagem para #mesh — público, por perto" + "value" : "mensagem para #mesh" } }, "ru" : { "stringUnit" : { "state" : "needs_review", - "value" : "сообщение в #mesh — публичное, рядом" + "value" : "сообщение в #mesh" } }, "sv" : { "stringUnit" : { "state" : "needs_review", - "value" : "meddelande #mesh — publikt, i närheten" + "value" : "meddelande #mesh" } }, "ta" : { "stringUnit" : { "state" : "needs_review", - "value" : "#mesh க்கு செய்தி — பொது, அருகில்" + "value" : "#mesh க்கு செய்தி" } }, "th" : { "stringUnit" : { "state" : "needs_review", - "value" : "ส่งข้อความถึง #mesh — สาธารณะ, ใกล้เคียง" + "value" : "ส่งข้อความถึง #mesh" } }, "tr" : { "stringUnit" : { "state" : "needs_review", - "value" : "mesaj #mesh — herkese açık, yakında" + "value" : "mesaj #mesh" } }, "uk" : { "stringUnit" : { "state" : "needs_review", - "value" : "повідомлення #mesh — публічне, поблизу" + "value" : "повідомлення #mesh" } }, "ur" : { "stringUnit" : { "state" : "needs_review", - "value" : "پیغام #mesh — عوامی، قریبی" + "value" : "پیغام #mesh" } }, "vi" : { "stringUnit" : { "state" : "needs_review", - "value" : "nhắn #mesh — công khai, gần đây" + "value" : "nhắn #mesh" } }, "zh-Hans" : { "stringUnit" : { "state" : "needs_review", - "value" : "发消息到 #mesh — 公开、附近" + "value" : "发消息到 #mesh" } }, "zh-Hant" : { "stringUnit" : { "state" : "needs_review", - "value" : "發訊息到 #mesh — 公開、附近" + "value" : "發訊息到 #mesh" } } } From be2275ec718a9e3d98d743e895dd6ba96d6f7d7a Mon Sep 17 00:00:00 2001 From: ecgang Date: Wed, 26 Aug 2026 10:02:52 -0600 Subject: [PATCH 16/19] Clear the sidebar latch on a panic wipe too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving showSidebar onto AppChromeModel (so launch restore could raise the conversation list) made it a fourth presentation latch on that model, and panicClearAllData resets the other three. ContentView presents the people sheet whenever showSidebar is set (isPeopleSheetPresented), so a wipe with it up leaves a sheet over the root timeline — which is where both the success message and the failed-wipe banner render. The one signal telling you whether your data is actually gone ends up behind a sheet. The launch path makes this reachable without anyone touching the screen: restore raises the sidebar, then a triple-tap wipes. Caught by a cross-model review pass on the merge, not by me. Test: extends panicWipeDismissesChromePresentation with the latch. Verified red-first against the unfixed model — the run failed at AppArchitectureTests.swift:516 with `showSidebar → true`, exit 65 — then passed after the fix, exit 0 (macOS scheme, 1 test). Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppChromeModel.swift | 5 +++++ bitchatTests/AppArchitectureTests.swift | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/bitchat/App/AppChromeModel.swift b/bitchat/App/AppChromeModel.swift index bee6ad08..bfaa3bb8 100644 --- a/bitchat/App/AppChromeModel.swift +++ b/bitchat/App/AppChromeModel.swift @@ -146,6 +146,11 @@ final class AppChromeModel: ObservableObject { isLocationChannelsSheetPresented = false isNoticesSheetPresented = false showingFingerprintFor = nil + // The people/conversation-list sheet is presented on this latch too + // (`ContentView.isPeopleSheetPresented`), and launch restore can raise + // it without anyone touching the screen — so it hides the outcome + // exactly like the sheets above unless it is cleared here. + showSidebar = false prepareForPanic?() onPanicWipe() diff --git a/bitchatTests/AppArchitectureTests.swift b/bitchatTests/AppArchitectureTests.swift index 747e737b..270ad76d 100644 --- a/bitchatTests/AppArchitectureTests.swift +++ b/bitchatTests/AppArchitectureTests.swift @@ -501,6 +501,11 @@ struct AppArchitectureTests { chromeModel.isLocationChannelsSheetPresented = true chromeModel.presentNotices() chromeModel.showFingerprint(for: PeerID(str: "peer-3")) + // The people/conversation-list sheet counts: ContentView presents it + // whenever `showSidebar || selectedPrivatePeerID != nil`, so a latch + // left set covers the root timeline exactly like the others. Launch + // restore (#1064) can raise it without the person touching anything. + chromeModel.showSidebar = true chromeModel.panicClearAllData() @@ -508,6 +513,7 @@ struct AppArchitectureTests { #expect(!chromeModel.isLocationChannelsSheetPresented) #expect(!chromeModel.isNoticesSheetPresented) #expect(chromeModel.showingFingerprintFor == nil) + #expect(!chromeModel.showSidebar) } @Test("Recent chats list direct conversations with people absent from the rosters") From c1baf8392b3bf17a6816a1cdfa8f27e691b617c3 Mon Sep 17 00:00:00 2001 From: ecgang Date: Wed, 26 Aug 2026 10:07:18 -0600 Subject: [PATCH 17/19] Land on the conversation list, not in the last DM #1064 asked that the app stop dropping people into a plaintext broadcast they never chose. Re-opening the last private chat answered that, but it paid for it with a worse disclosure: launch put a contact's nickname and message history on screen before anyone chose to show them, to whoever is holding an unlocked phone or standing behind it. Nothing gates that today - the app-lock work is #1664. And the restorability rule selected favorites, so the conversations it surfaced were the ones worth most to an observer. Present the conversation list instead. It gets #1064's actual ask, and since #1598 the list carries a recent-chats section deduped by fingerprint, so the last conversation is one tap away rather than lost - which is what made the auto-reopen worth its cost in the first place. A persisted DM and a stale one now take the same branch, so the machinery that told them apart has nothing left to decide: isDirectChatRestorable, shouldPresentConversationList and the suppressSystemMessages plumbing are deleted, and restoreLastActiveConversation drops its resolvability predicate. ChatPeerIdentityCoordinator returns to main's signature exactly. Tests: ConversationStoreLastActiveTests goes 32 -> 7 (610 -> 180 lines); the ~25 that covered the deleted predicate went with it. The survivors pin persistence and the launch decision, and one new test pins the security property directly - a persisted DM presents the list and never the chat, whoever the peer is. Verified by running, not compiling: 7 XCTest + 31 swift-testing across ConversationStoreLastActiveTests, AppArchitectureTests and ChatPeerIdentityCoordinatorContextTests, 0 failures, exit 0 (macOS). Raised by Chessing234 in review. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppRuntime.swift | 218 +------ bitchat/App/ConversationStore.swift | 26 +- .../ChatPeerIdentityCoordinator.swift | 22 +- bitchat/ViewModels/ChatViewModel.swift | 18 +- ...tPeerIdentityCoordinatorContextTests.swift | 42 -- .../ConversationStoreLastActiveTests.swift | 550 ++---------------- 6 files changed, 98 insertions(+), 778 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 8686e530..d21d0507 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -159,217 +159,23 @@ final class AppRuntime: ObservableObject { 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. + /// #1064: decide what launch presents. A persisted public channel defers + /// to the existing mesh / `GeoChannelCoordinator` restore (the sole + /// launch-time writer of `activeChannel`), so there is no race; everything + /// else raises the conversation list. + /// + /// Launch deliberately does not re-open a persisted DM. Opening it would + /// put a contact's name and message history on screen before anyone chose + /// to show them, and nothing gates that today (the app-lock work is #1664). + /// The list is what #1064 actually asked for, and since #1598 it carries a + /// recent-chats section, so the last conversation is one tap away rather + /// than lost. private func restoreLastActiveConversationOnLaunch() { - let presentation = conversations.restoreLastActiveConversation( - isPeerResolvable: { - Self.isDirectChatRestorable( - $0, - favorites: .shared, - hasStoredCryptographicIdentity: { - !chatViewModel.identityManager - .getCryptoIdentitiesByPeerIDPrefix($0.toShort()) - .isEmpty - }, - isPeerBlocked: { chatViewModel.isPeerBlocked($0) } - ) - } - ) - var didOpenDirectChat = false - if case .restoredDirectChat(let peerID) = presentation { - // `startPrivateChat`'s gate (ChatPeerIdentityCoordinator) rejects a - // 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. - // - // 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 - } - // 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) { + if conversations.restoreLastActiveConversation() == .conversationList { appChromeModel.showSidebar = true } } - /// Whether a persisted last-active DM peer is genuinely restorable at - /// 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 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 - /// 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. - /// - /// 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"), 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 - /// 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 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 — 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. - 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 - // 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 - /// 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). - /// - /// 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( - peerID, - isPeerFavorited: { - favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false - }, - hasStoredCryptographicIdentity: hasStoredCryptographicIdentity, - isPeerBlocked: isPeerBlocked - ) - } - - /// 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 - } - } - /// Drops media that has outlived the retention window, then applies the /// explicit protection class to files that older builds wrote without /// one. Expiry runs first so the migration never touches files the diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index d82ae435..b5b33865 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -407,11 +407,14 @@ final class ConversationStore: ObservableObject { /// What launch should present, decided purely from `restoredLastActive`. enum LaunchPresentation: Equatable { - /// First-ever launch, or a persisted DM whose peer no longer resolves. + /// First-ever launch, or a persisted DM. Launch never re-opens the + /// DM itself: landing on a name and message history would make + /// startup a disclosure to anyone holding an unlocked phone, and the + /// old restorability rule selected favorites — the relationships worth + /// most to an observer. The list still gets #1064's ask (don't start + /// typing into a broadcast nobody chose), and #1598's recent-chats + /// section puts the conversation one tap away. 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 @@ -483,18 +486,19 @@ final class ConversationStore: ObservableObject { /// 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 { + /// the location-channel restore. + /// + /// A persisted DM resolves to the conversation list, not to the DM, which + /// makes the peer's addressability irrelevant here — that is why no + /// resolvability predicate is threaded in. A restorable peer and a stale + /// one take the same branch, so there is nothing left for one to decide. + func restoreLastActiveConversation() -> 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) + return .conversationList } } diff --git a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift index 22017ddf..e2d0fae8 100644 --- a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift @@ -319,7 +319,7 @@ final class ChatPeerIdentityCoordinator { } @MainActor - func startPrivateChat(with peerID: PeerID, suppressSystemMessages: Bool = false) { + func startPrivateChat(with peerID: PeerID) { guard peerID != context.myPeerID else { return } // Group chats are virtual conversations: no peer identity, favorites, @@ -334,18 +334,16 @@ final class ChatPeerIdentityCoordinator { let peerNickname = context.peerNickname(for: peerID) ?? "unknown" if context.unifiedIsBlocked(peerID) { - 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 - ) + context.addSystemMessage( + String( + format: String( + localized: "system.chat.blocked", + comment: "System message when starting chat fails because peer is blocked" + ), + locale: .current, + peerNickname ) - } + ) return } diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index a963bfb7..640641a1 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1498,28 +1498,12 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage /// 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. - /// 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. + /// - Note: Switches the UI to private chat mode and loads message history @MainActor func startPrivateChat(with peerID: PeerID) { peerIdentityCoordinator.startPrivateChat(with: peerID) } - /// #1064 launch-restore variant of `startPrivateChat`. When - /// `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 - 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 67c29f72..0bfcd306 100644 --- a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift @@ -303,48 +303,6 @@ 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_oneWayFavoriteOpensChatSilently() async { - // #1064 × #1415: the mutual-favorite gate is gone (store-and-forward - // handles offline non-mutual favorites), so a one-way favorite now - // opens the chat — and under suppression still emits no system message. - 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 == [peerID]) - #expect(context.consolidatedPeers.map(\.peerID) == [peerID]) - } - @Test @MainActor func updatePrivateChatPeerIfNeeded_migratesChatStateByFingerprint() async { let context = MockChatPeerIdentityContext() diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index f7fc07b3..b67bc992 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -4,9 +4,14 @@ // // 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. +// what to present — a persisted DM or a first-ever launch presents the +// conversation list, and a public channel defers to the existing +// GeoChannelCoordinator restore. +// +// Launch never re-opens the DM itself. The tests below pin that as a +// property, not an implementation detail: it is what keeps startup from +// disclosing a contact's name and message history to whoever is holding +// the phone. // // This is free and unencumbered software released into the public domain. // For more information, see @@ -25,42 +30,50 @@ final class ConversationStoreLastActiveTests: XCTestCase { func test_persistsLastActiveOnEverySwitch() { let storage = makeStorage() - // Open a DM: the next launch should restore it. + // Open a DM: the next launch presents the list. let session1 = ConversationStore(storage: storage) session1.setSelectedPrivatePeer(peerID) XCTAssertEqual( - ConversationStore(storage: storage).restoreLastActiveConversation(isPeerResolvable: { _ in true }), - .restoredDirectChat(peerID) + ConversationStore(storage: storage).restoreLastActiveConversation(), + .conversationList ) // 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. + // so the next launch now defers to the channel restore. session1.setSelectedPrivatePeer(nil) XCTAssertEqual( - ConversationStore(storage: storage).restoreLastActiveConversation(isPeerResolvable: { _ in true }), + ConversationStore(storage: storage).restoreLastActiveConversation(), .deferToChannelRestore ) } - func test_restoresValidDirectChat() { + func test_persistedDirectChatPresentsTheListAndNeverTheChat() { + // The security property this design turns on: whatever was persisted, + // a DM never re-opens itself at launch. There is deliberately no + // "restorable peer" branch to take — a favorite, a merely-known peer + // and a stale one all land on the list — so no addressability check + // can put a name and a history on screen before anyone asked. let storage = makeStorage() ConversationStore(storage: storage).setSelectedPrivatePeer(peerID) - let restored = ConversationStore(storage: storage) - .restoreLastActiveConversation(isPeerResolvable: { $0.isValid }) - - XCTAssertEqual(restored, .restoredDirectChat(peerID)) + XCTAssertEqual( + ConversationStore(storage: storage).restoreLastActiveConversation(), + .conversationList + ) } - func test_staleDirectChatFallsBackToConversationList() { + func test_launchDefersToChannelRestore_forAPersistedPublicChannel() { + // A public-channel restore is owned by GeoChannelCoordinator; the + // launch decision must not present the list on top of it. let storage = makeStorage() - ConversationStore(storage: storage).setSelectedPrivatePeer(peerID) + let session = ConversationStore(storage: storage) + session.setSelectedPrivatePeer(peerID) + session.setSelectedPrivatePeer(nil) - // The persisted peer no longer resolves at launch. - let restored = ConversationStore(storage: storage) - .restoreLastActiveConversation(isPeerResolvable: { _ in false }) - - XCTAssertEqual(restored, .conversationList) + XCTAssertEqual( + ConversationStore(storage: storage).restoreLastActiveConversation(), + .deferToChannelRestore + ) } func test_failedDirectRestoreDoesNotEraseDirectRecord() { @@ -71,19 +84,19 @@ final class ConversationStoreLastActiveTests: XCTestCase { 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`. + // selected, and GeoChannelCoordinator re-asserts the SAME (default + // mesh) active channel. Because the channel does not actually change, + // the persist is guarded out — so the `.direct` record on disk must + // survive rather than be overwritten with `.mesh`. The distinction + // still matters with the DM no longer re-opening: `.direct` presents + // the list, `.mesh` would defer to the channel restore instead. let launch = ConversationStore(storage: storage) launch.setActiveChannel(launch.activeChannel) - // Session 3: the DM record is intact and still restores. + // Session 3: the DM record is intact, so launch still presents the list. XCTAssertEqual( - ConversationStore(storage: storage) - .restoreLastActiveConversation(isPeerResolvable: { _ in true }), - .restoredDirectChat(peerID) + ConversationStore(storage: storage).restoreLastActiveConversation(), + .conversationList ) } @@ -92,13 +105,13 @@ final class ConversationStoreLastActiveTests: XCTestCase { // wiped DM/channel cannot be restored on the next launch. let storage = makeStorage() - // A DM is persisted and would otherwise restore. + // A channel record is persisted and would otherwise defer. let session = ConversationStore(storage: storage) session.setSelectedPrivatePeer(peerID) + session.setSelectedPrivatePeer(nil) XCTAssertEqual( - ConversationStore(storage: storage) - .restoreLastActiveConversation(isPeerResolvable: { _ in true }), - .restoredDirectChat(peerID) + ConversationStore(storage: storage).restoreLastActiveConversation(), + .deferToChannelRestore ) // The panic path erases the pointer through the store's own storage. @@ -106,8 +119,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { // The next launch has nothing to restore and falls back to the list. XCTAssertEqual( - ConversationStore(storage: storage) - .restoreLastActiveConversation(isPeerResolvable: { _ in true }), + ConversationStore(storage: storage).restoreLastActiveConversation(), .conversationList ) } @@ -116,28 +128,20 @@ final class ConversationStoreLastActiveTests: XCTestCase { // #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: + // 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: // 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`. + // Without the fix those resets re-persist a `.mesh` record, and the next + // launch would `.deferToChannelRestore` instead of `.conversationList`. + // That is what makes this assertion load-bearing rather than trivially + // true: `.conversationList` is also the no-record answer, so the test + // distinguishes "pointer absent" from "pointer rewritten as .mesh". 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() @@ -148,8 +152,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { // 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 }), + ConversationStore(storage: storage).restoreLastActiveConversation(), .conversationList ) } @@ -157,442 +160,9 @@ final class ConversationStoreLastActiveTests: XCTestCase { 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 }, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - } - - 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 }, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - } - - 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 }, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - } - - 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 }, - 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 - // the retained outbox. Restoring it is not a phantom. - XCTAssertTrue( - AppRuntime.isDirectChatRestorable( - peerID, - isPeerFavorited: { _ 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 }, - 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 }, - 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 }, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - } - - 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_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)) - 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. - XCTAssertFalse( - AppRuntime.isDirectChatRestorable( - peerID, - isPeerFavorited: { _ in true }, - hasStoredCryptographicIdentity: { _ in false }, - 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) - XCTAssertFalse( - AppRuntime.isDirectChatRestorable( - geoDMPeer, - isPeerFavorited: { _ in false }, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ 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 }, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - } - ) - - XCTAssertEqual(restored, .conversationList) - } - - // MARK: - Production wiring (real FavoritesPersistenceService) - - 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. 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, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - } - - 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") - - let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) - XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) - XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) - XCTAssertTrue( - AppRuntime.isDirectChatRestorable( - fullHexPeer, - favorites: favorites, - hasStoredCryptographicIdentity: { _ in false }, - 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, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in true } - ) - ) - } - - 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, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - } - - 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. 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. - // - // 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") - 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) - XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) - XCTAssertFalse( - AppRuntime.isDirectChatRestorable( - fullHexPeer, - favorites: favorites, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) + XCTAssertEqual( + ConversationStore(storage: storage).restoreLastActiveConversation(), + .conversationList ) } From 2f3524fa8a3ce2a05ae12de1ee29393a6a5ca6c0 Mon Sep 17 00:00:00 2001 From: ecgang Date: Wed, 26 Aug 2026 10:15:16 -0600 Subject: [PATCH 18/19] Make the launch and panic-wipe tests able to fail Three tests here could pass while the behaviour was wrong. A review pass found all three; the mutation runs below are what settled them. The launch decision had no test at all. Deleting shouldPresentConversationList along with the restorability predicate took the seam with it, leaving `presentation == .conversationList -> showSidebar = true` executed only in production. That one comparison decides whether a public timeline or a conversation list ends up on screen. It comes back as a pure one-case function with both branches asserted. The panic-wipe test asserted only what launch presents, and an earlier version of its comment claimed that distinguished "pointer absent" from "pointer rewritten as .mesh". It does distinguish that one. It does not distinguish a surviving .direct record, which also presents as .conversationList - so a regression where finishPanicWipe stopped deleting the key, while suppression kept working, would leave the peer id on disk and the test would stay green. For a panic wipe that is the whole feature. It now reads the stored key directly. Proved rather than assumed: with the deletion removed from finishPanicWipe, exactly one of nine tests fails, on the new XCTAssertNil (ConversationStoreLastActiveTests.swift:162, "45 bytes"). The presentation assertion passes against that mutant, which is the point - it could not have caught it. The chrome panic test asserted one arm of isPeopleSheetPresented while its own comment named both. It now selects a private peer first and asserts the selection is cleared too, so it covers the cross-model invariant it claimed rather than the half this PR happens to own. 9 XCTest + 31 swift-testing, 0 failures, exit 0 (macOS). Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/AppRuntime.swift | 18 +++++++++- bitchatTests/AppArchitectureTests.swift | 7 ++++ .../ConversationStoreLastActiveTests.swift | 34 ++++++++++++++++--- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index d21d0507..15b8e88a 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -171,11 +171,27 @@ final class AppRuntime: ObservableObject { /// recent-chats section, so the last conversation is one tap away rather /// than lost. private func restoreLastActiveConversationOnLaunch() { - if conversations.restoreLastActiveConversation() == .conversationList { + if Self.shouldPresentConversationList(for: conversations.restoreLastActiveConversation()) { appChromeModel.showSidebar = true } } + /// The launch decision itself, split out as a pure function so the mapping + /// is testable without standing up an `AppRuntime`. It is one comparison, + /// but it is the comparison that decides whether a public timeline or a + /// list ends up on screen, and inverting it silently is exactly the kind of + /// mistake the store-level tests cannot see. + static func shouldPresentConversationList( + for presentation: ConversationStore.LaunchPresentation + ) -> Bool { + switch presentation { + case .conversationList: + return true + case .deferToChannelRestore: + return false + } + } + /// Drops media that has outlived the retention window, then applies the /// explicit protection class to files that older builds wrote without /// one. Expiry runs first so the migration never touches files the diff --git a/bitchatTests/AppArchitectureTests.swift b/bitchatTests/AppArchitectureTests.swift index 270ad76d..ca4fd621 100644 --- a/bitchatTests/AppArchitectureTests.swift +++ b/bitchatTests/AppArchitectureTests.swift @@ -506,6 +506,12 @@ struct AppArchitectureTests { // left set covers the root timeline exactly like the others. Launch // restore (#1064) can raise it without the person touching anything. chromeModel.showSidebar = true + // The OTHER arm of `isPeopleSheetPresented`. Asserting only the latch + // would leave the test green while a selected DM kept the sheet over + // the wipe result — the selection is cleared by pre-existing code in + // ChatViewModel.panicClearAllData, so this pins a cross-model + // invariant that neither half owns alone. + viewModel.startPrivateChat(with: PeerID(str: "1122334455667788")) chromeModel.panicClearAllData() @@ -514,6 +520,7 @@ struct AppArchitectureTests { #expect(!chromeModel.isNoticesSheetPresented) #expect(chromeModel.showingFingerprintFor == nil) #expect(!chromeModel.showSidebar) + #expect(viewModel.selectedPrivateChatPeer == nil) } @Test("Recent chats list direct conversations with people absent from the rosters") diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index b67bc992..d04d6908 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -135,9 +135,15 @@ final class ConversationStoreLastActiveTests: XCTestCase { // beginPanicWipe → setSelectedPrivatePeer(nil) → setActiveChannel(.mesh) → finishPanicWipe // Without the fix those resets re-persist a `.mesh` record, and the next // launch would `.deferToChannelRestore` instead of `.conversationList`. - // That is what makes this assertion load-bearing rather than trivially - // true: `.conversationList` is also the no-record answer, so the test - // distinguishes "pointer absent" from "pointer rewritten as .mesh". + // + // The presentation assertion alone is NOT enough, and the earlier + // version of this comment overclaimed. `.conversationList` is the + // answer for an absent record AND for a surviving `.direct` one, so a + // regression where `finishPanicWipe` stopped removing the key while + // suppression still worked would leave the peer id on disk and this + // test would stay green. For a panic wipe that is the whole point of + // the feature, so assert the stored key is gone, not just that launch + // looks right. let storage = makeStorage() let session = ConversationStore(storage: storage) @@ -149,14 +155,32 @@ final class ConversationStoreLastActiveTests: XCTestCase { 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`. + // The pointer is truly absent, not rewritten as `.mesh` and not left + // behind as `.direct`. Read the key directly: this is the only + // assertion that separates "erased" from "still on disk", since both + // present as `.conversationList` at launch. + XCTAssertNil(storage.data(forKey: "conversation.lastActive")) + + // And launch presents the list rather than deferring to a resurrected + // `.mesh` record — the suppression half of the same fix. XCTAssertEqual( ConversationStore(storage: storage).restoreLastActiveConversation(), .conversationList ) } + // MARK: - Launch decision + + func test_launchPresentsTheListForAConversationListPresentation() { + XCTAssertTrue(AppRuntime.shouldPresentConversationList(for: .conversationList)) + } + + func test_launchLeavesAPublicChannelRestoreAlone() { + // Inverting this is how a geohash restore would end up buried under the + // people sheet, or a DM launch would land on the public timeline. + XCTAssertFalse(AppRuntime.shouldPresentConversationList(for: .deferToChannelRestore)) + } + func test_firstLaunchPresentsConversationList() { let storage = makeStorage() From 3fc9f4db82b20ca067bffa613070776fbe997145 Mon Sep 17 00:00:00 2001 From: ecgang Date: Wed, 26 Aug 2026 10:35:16 -0600 Subject: [PATCH 19/19] Ask the store whether the pointer is gone, not a string literal The panic-wipe assertion added in the previous commit read storage.data(forKey: "conversation.lastActive") by literal, because lastActiveKey is private. Rename that key and production moves to the new name while the test keeps checking the old one - which nothing writes - so XCTAssertNil passes for the wrong reason. That is the same "green while the behaviour is wrong" defect the previous commit exists to close, reintroduced through the coupling. clearPersistedLastActive's own comment says the key stays private on purpose, so relax nothing: add _test_hasPersistedLastActive, which reads through the same private constant. A rename now moves the test with production instead of stranding it. Matches the _test_ accessor idiom already used across BLEService. Re-proved after the change, since the assertion mechanism changed and the earlier proof was against the literal: with the deletion removed from finishPanicWipe, exactly one of nine tests fails, on the new XCTAssertFalse (ConversationStoreLastActiveTests.swift:164). Restored, 9 XCTest + 31 swift-testing, 0 failures, exit 0. Co-Authored-By: Claude Opus 5 (1M context) --- bitchat/App/ConversationStore.swift | 12 ++++++++++++ bitchatTests/ConversationStoreLastActiveTests.swift | 10 ++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index b5b33865..89c803ea 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -472,6 +472,18 @@ final class ConversationStore: ObservableObject { storage.removeObject(forKey: lastActiveKey) } + /// Whether a last-active pointer is on disk right now. Test-only, and it + /// exists because the panic wipe's guarantee is that the pointer is *gone* + /// — which launch cannot report: an absent record and a surviving + /// `.direct` one both present as `.conversationList`, so a test reading + /// only the presentation passes while a peer id survives an emergency + /// clear. Goes through the same private `lastActiveKey` production uses, + /// so renaming the key moves the test with it instead of leaving it + /// checking a string nothing writes. + var _test_hasPersistedLastActive: Bool { + storage.data(forKey: lastActiveKey) != nil + } + /// 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 } diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index d04d6908..d9041ce3 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -156,10 +156,12 @@ final class ConversationStoreLastActiveTests: XCTestCase { session.finishPanicWipe() // removes the pointer once // The pointer is truly absent, not rewritten as `.mesh` and not left - // behind as `.direct`. Read the key directly: this is the only - // assertion that separates "erased" from "still on disk", since both - // present as `.conversationList` at launch. - XCTAssertNil(storage.data(forKey: "conversation.lastActive")) + // behind as `.direct`. This is the only assertion that separates + // "erased" from "still on disk", since both present as + // `.conversationList` at launch. Asked through the store rather than + // by key literal: a literal here would go quietly vacuous the day the + // key is renamed, checking a string nothing writes. + XCTAssertFalse(ConversationStore(storage: storage)._test_hasPersistedLastActive) // And launch presents the list rather than deferring to a resurrected // `.mesh` record — the suppression half of the same fix.