Skip to content

Commit 86e03b9

Browse files
louroulourou
committed
fix(leave): process the self-leave echo on paired sibling installations
Device pairing puts the same inbox on multiple installations, and a leave-request echo names the local inbox as both initiator and removed member on every one of them. The ingest guard skipped the removal whenever the initiator was the local inbox, which is correct on the installation that ran the leave flow (its consent-hide already handled it) but wrong on a paired sibling: the finalization commit reports self-leaves via leftInboxes, which is deliberately not mapped, so the echo is the only removal signal the sibling gets and the conversation stayed visible there until consent sync converged. The sender installation id is not exposed on the SDK's DecodedMessage, so the discriminator is local state: the initiating installation is already consent-denied when the echo lands, while a sibling is still allowed and now processes the removal (marker plus live-hide notification). The decision moved into isRemovedFromConversation, a pure helper with direct test coverage of the sibling, initiator, admin- removal, and unrelated-update cases.
1 parent 0052334 commit 86e03b9

2 files changed

Lines changed: 103 additions & 9 deletions

File tree

‎ConvosCore/Sources/ConvosCore/Storage/Writers/IncomingMessageWriter.swift‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,11 @@ class IncomingMessageWriter: IncomingMessageWriterProtocol, @unchecked Sendable
100100

101101
let messageExistsInDB = try DBMessage.exists(db, key: message.id)
102102
let localInboxId = try DBInbox.currentInboxId(db)
103-
let wasRemovedFromConversation: Bool = {
104-
guard let localInboxId, let update = message.update else { return false }
105-
// A self-leave echo (leave-request the local user sent) names the
106-
// local inbox as both initiator and removed member; it must not
107-
// set the removed-by-someone-else marker -- the leave flow already
108-
// hid the conversation via the consent path.
109-
guard update.initiatedByInboxId != localInboxId else { return false }
110-
return update.removedInboxIds.contains(localInboxId)
111-
}()
103+
let wasRemovedFromConversation = Self.isRemovedFromConversation(
104+
update: message.update,
105+
localInboxId: localInboxId,
106+
conversationConsent: conversation.consent
107+
)
112108

113109
Log.debug("Storing incoming message \(message.id) localId \(message.clientMessageId) echoDateNs=\(message.dateNs)")
114110
if !message.attachmentUrls.isEmpty {
@@ -222,6 +218,31 @@ class IncomingMessageWriter: IncomingMessageWriterProtocol, @unchecked Sendable
222218
)
223219
}
224220

221+
/// True when the update removes the local user from the conversation, so
222+
/// the removed marker and the live-hide notification must fire.
223+
///
224+
/// A self-leave echo (leave-request the local user's inbox sent) names
225+
/// the local inbox as both initiator and removed member. On the
226+
/// installation that initiated the leave, the leave flow already hid the
227+
/// conversation via the consent path, so the echo must not re-mark it.
228+
/// A paired sibling installation of the same inbox receives the identical
229+
/// echo but never ran the local leave flow, and the finalization commit
230+
/// reports self-leaves via `leftInboxes`, which is deliberately not
231+
/// mapped -- the echo is the only removal signal the sibling gets before
232+
/// consent sync converges. The conversation's consent state is the
233+
/// discriminator: the initiator is already denied when the echo lands,
234+
/// while a sibling is still allowed and must process the removal.
235+
static func isRemovedFromConversation(
236+
update: DBMessage.Update?,
237+
localInboxId: String?,
238+
conversationConsent: Consent
239+
) -> Bool {
240+
guard let localInboxId, let update else { return false }
241+
guard update.removedInboxIds.contains(localInboxId) else { return false }
242+
guard update.initiatedByInboxId == localInboxId else { return true }
243+
return conversationConsent != .denied
244+
}
245+
225246
/// Marks the conversation as removed-for-the-local-user. Idempotent and
226247
/// deliberately independent of `messageAlreadyExists`: when the NSE saves
227248
/// the removal `GroupUpdated` message first, the main app re-encounters it

‎ConvosCore/Tests/ConvosCoreTests/ConversationRemovedStateTests.swift‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,79 @@ struct ConversationRemovedStateTests {
9898
.fetchOne(db)
9999
}
100100

101+
// MARK: - isRemovedFromConversation
102+
103+
private static func membershipUpdate(
104+
initiatedBy: String,
105+
removed: [String]
106+
) -> DBMessage.Update {
107+
DBMessage.Update(
108+
initiatedByInboxId: initiatedBy,
109+
addedInboxIds: [],
110+
removedInboxIds: removed,
111+
metadataChanges: [],
112+
expiresAt: nil
113+
)
114+
}
115+
116+
@Test("Admin removal naming the local inbox is processed as a removal")
117+
func testAdminRemovalIsProcessed() throws {
118+
let removed = IncomingMessageWriter.isRemovedFromConversation(
119+
update: Self.membershipUpdate(initiatedBy: Self.otherInboxId, removed: [Self.currentInboxId]),
120+
localInboxId: Self.currentInboxId,
121+
conversationConsent: .allowed
122+
)
123+
#expect(removed)
124+
}
125+
126+
@Test("Self-leave echo on a sibling installation (consent still allowed) is processed as a removal")
127+
func testSelfLeaveEchoOnSiblingInstallationIsProcessed() throws {
128+
// Device pairing puts the same inbox on multiple installations. The
129+
// sibling never ran the local leave flow, and the finalization commit
130+
// is not mapped for self-leaves, so the echo must set the marker or
131+
// the conversation stays visible until consent sync converges.
132+
let removed = IncomingMessageWriter.isRemovedFromConversation(
133+
update: Self.membershipUpdate(initiatedBy: Self.currentInboxId, removed: [Self.currentInboxId]),
134+
localInboxId: Self.currentInboxId,
135+
conversationConsent: .allowed
136+
)
137+
#expect(removed)
138+
}
139+
140+
@Test("Self-leave echo on the initiating installation (consent already denied) stays skipped")
141+
func testSelfLeaveEchoOnInitiatorIsSkipped() throws {
142+
// The leave flow already hid the conversation via the consent path;
143+
// the echo must not re-mark it.
144+
let removed = IncomingMessageWriter.isRemovedFromConversation(
145+
update: Self.membershipUpdate(initiatedBy: Self.currentInboxId, removed: [Self.currentInboxId]),
146+
localInboxId: Self.currentInboxId,
147+
conversationConsent: .denied
148+
)
149+
#expect(!removed)
150+
}
151+
152+
@Test("Updates that do not remove the local inbox never mark a removal")
153+
func testUnrelatedUpdateIsIgnored() throws {
154+
let removed = IncomingMessageWriter.isRemovedFromConversation(
155+
update: Self.membershipUpdate(initiatedBy: Self.otherInboxId, removed: [Self.otherInboxId]),
156+
localInboxId: Self.currentInboxId,
157+
conversationConsent: .allowed
158+
)
159+
#expect(!removed)
160+
let noUpdate = IncomingMessageWriter.isRemovedFromConversation(
161+
update: nil,
162+
localInboxId: Self.currentInboxId,
163+
conversationConsent: .allowed
164+
)
165+
#expect(!noUpdate)
166+
let noInbox = IncomingMessageWriter.isRemovedFromConversation(
167+
update: Self.membershipUpdate(initiatedBy: Self.otherInboxId, removed: [Self.currentInboxId]),
168+
localInboxId: nil,
169+
conversationConsent: .allowed
170+
)
171+
#expect(!noInbox)
172+
}
173+
101174
// MARK: - persistRemovedMarker
102175

103176
@Test("Marks the conversation removed and unpins it")

0 commit comments

Comments
 (0)