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 ) }