mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-29 07:27:16 +00:00
fix: silent launch DM restore + change-gated last-active persist (#1064)
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) <noreply@anthropic.com>
This commit is contained in:
parent
2ed52ebf4f
commit
0cc4e9821e
@ -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
|
||||
|
||||
@ -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() {
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
|
||||
@ -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()
|
||||
|
||||
@ -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()
|
||||
|
||||
@ -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()
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user