From 8289a6d05e44e8508bdd3bfcc48e14b07bd281c1 Mon Sep 17 00:00:00 2001 From: jack Date: Thu, 11 Jun 2026 14:09:43 +0200 Subject: [PATCH] Republish mirrored conversations on shared-instance status changes When a private message is mirrored into stable-key and ephemeral-peer conversations as one shared BitchatMessage instance, the first conversation's status apply mutated the shared object and the second skipped as already-equal - state stayed correct but the mirrored conversation never republished, so a view observing it rendered stale delivery/read status. The ID-only fan-out now republishes and emits .statusChanged for every skipped conversation whose message holds the applied status; genuinely-rejected distinct copies (downgrades) stay untouched, and duplicate acks still publish nothing. Found by Codex review on #1334. Co-Authored-By: Claude Fable 5 --- bitchat/App/ConversationStore.swift | 38 +++++++++++++++++---- bitchatTests/ConversationStoreTests.swift | 40 +++++++++++++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/bitchat/App/ConversationStore.swift b/bitchat/App/ConversationStore.swift index da7a8dee..73b57b34 100644 --- a/bitchat/App/ConversationStore.swift +++ b/bitchat/App/ConversationStore.swift @@ -135,6 +135,17 @@ final class Conversation: ObservableObject, Identifiable { return true } + /// Republishes a message without changing state. Used for mirrored + /// copies that share a BitchatMessage instance: the first conversation's + /// status apply mutated the shared object, so this conversation's + /// observers still need an @Published emission to re-render. + @discardableResult + fileprivate func republishMessage(withID messageID: String) -> Bool { + guard let index = indexByMessageID[messageID] else { return false } + messages[index] = messages[index] + return true + } + @discardableResult fileprivate func setUnread(_ unread: Bool) -> Bool { guard isUnread != unread else { return false } @@ -345,17 +356,32 @@ final class ConversationStore: ObservableObject { /// downgrade — read beats delivered beats sent). /// /// `BitchatMessage` is a reference type, so mirrored copies sharing one - /// instance are updated by the first apply; subsequent conversations - /// skip as already-equal (state stays correct everywhere, the - /// `.statusChanged` event fires for the conversation that applied). + /// instance are mutated by the first conversation's apply. The skipped + /// conversations still hold the changed message, so they get an explicit + /// republish and `.statusChanged` event - otherwise a view observing the + /// mirrored conversation would render stale status. Distinct copies whose + /// update was genuinely rejected (downgrade) are left untouched, guarded + /// by status equality. @discardableResult func setDeliveryStatus(_ status: DeliveryStatus, forMessageID messageID: String) -> Bool { guard let ids = conversationIDsByMessageID[messageID] else { return false } var applied = false - for id in ids where setDeliveryStatus(status, forMessageID: messageID, in: id) { - applied = true + var skipped: [ConversationID] = [] + for id in ids { + if setDeliveryStatus(status, forMessageID: messageID, in: id) { + applied = true + } else { + skipped.append(id) + } } - return applied + guard applied else { return false } + for id in skipped { + guard let conversation = conversationsByID[id], + conversation.message(withID: messageID)?.deliveryStatus == status, + conversation.republishMessage(withID: messageID) else { continue } + changes.send(.statusChanged(id, messageID: messageID, status)) + } + return true } /// Current delivery status of `messageID` in whichever conversation diff --git a/bitchatTests/ConversationStoreTests.swift b/bitchatTests/ConversationStoreTests.swift index efb03a89..52598db9 100644 --- a/bitchatTests/ConversationStoreTests.swift +++ b/bitchatTests/ConversationStoreTests.swift @@ -714,4 +714,44 @@ struct ConversationStoreTests { } } } + + @Test("mirrored conversations both republish and emit on a shared-instance status change") + @MainActor + func sharedInstanceMirroredCopiesBothRepublishOnStatusChange() { + let store = ConversationStore() + let stable = makeDirectConversationID("stable") + let ephemeral = makeDirectConversationID("ephemeral") + let message = makeMessage(id: "dm-2", timestamp: 1, isPrivate: true, deliveryStatus: .sent) + store.upsertByID(message, in: stable) + store.upsertByID(message, in: ephemeral) + + var cancellables = Set() + var publishedIDs: [ConversationID] = [] + for id in [stable, ephemeral] { + store.conversation(for: id).objectWillChange + .sink { publishedIDs.append(id) } + .store(in: &cancellables) + } + var statusChangedIDs: [ConversationID] = [] + store.changes + .sink { change in + if case .statusChanged(let id, "dm-2", _) = change { statusChangedIDs.append(id) } + } + .store(in: &cancellables) + + let read = DeliveryStatus.read(by: "bob", at: Date()) + #expect(store.setDeliveryStatus(read, forMessageID: "dm-2")) + + // The shared instance is mutated once, but a view observing EITHER + // conversation must re-render, and both emit a change event. + #expect(Set(publishedIDs) == Set([stable, ephemeral])) + #expect(Set(statusChangedIDs) == Set([stable, ephemeral])) + + // A duplicate ack applies nowhere and must publish nothing. + publishedIDs.removeAll() + statusChangedIDs.removeAll() + #expect(!store.setDeliveryStatus(read, forMessageID: "dm-2")) + #expect(publishedIDs.isEmpty) + #expect(statusChangedIDs.isEmpty) + } }