From 0cc4e9821e19d7f17a9e075d0680de707521201e Mon Sep 17 00:00:00 2001 From: ecgang Date: Mon, 6 Jul 2026 10:29:54 -0700 Subject: [PATCH] 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()