From 8a000a18920d11c7065b49e51bb9ad33c4fc50f3 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 1 Sep 2026 15:49:10 -0400 Subject: [PATCH 1/3] feat(chat): overlay optimistic edits and deletes in the conversation store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MutationEntry` is the mirror of `PendingEntry`: it describes a mutation that has been issued but not confirmed, and `displayedMessages` applies it over the stored row so the transcript reflects an edit or delete immediately. The overlay is bounded by `expectedSequence` — the `eventSequence` the message carried when the mutation was issued, and the same value sent as `expected_event_sequence`. Once the stored row moves past it, the server's answer has landed and the overlay stops applying, so a refused mutation reverts without the caller having to notice. --- .../Conversation/ConversationStore.swift | 41 ++++++ .../Models/Conversation/MutationEntry.swift | 32 +++++ .../ConversationStoreMutationTests.swift | 122 ++++++++++++++++++ 3 files changed, 195 insertions(+) create mode 100644 FlipcashCore/Sources/FlipcashCore/Models/Conversation/MutationEntry.swift create mode 100644 FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreMutationTests.swift diff --git a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift index c072b4ab3..ac276feb2 100644 --- a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift +++ b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationStore.swift @@ -19,6 +19,8 @@ public struct ConversationStore: Sendable { /// after (its `anchor`) and a monotonic send `sequence`, so the transcript orders it relative to /// confirmed rows without comparing client and server wall-clock times. private var pendingByConversation: [ConversationID: [PendingEntry]] = [:] + /// Optimistic edits and deletes, keyed by message so reissuing one replaces it. + private var mutationsByConversation: [ConversationID: [MessageID: MutationEntry]] = [:] /// Monotonic counter stamped on each optimistic send, breaking ties among rows that share an anchor /// and keeping send order stable across out-of-order reconciles. private var pendingSequence: UInt64 = 0 @@ -68,6 +70,7 @@ public struct ConversationStore: Sendable { /// to clock skew), a failed send keeps its place as newer messages arrive, and out-of-order /// reconciles never reshuffle. public func displayedMessages(for conversationID: ConversationID, over confirmed: [ConversationMessage]) -> [ConversationMessage] { + let confirmed = overlaid(confirmed, in: conversationID) guard let pending = pendingByConversation[conversationID], !pending.isEmpty else { return confirmed } // An anchor of 0 means the send predates any loaded history — it is the newest thing the user @@ -109,6 +112,44 @@ public struct ConversationStore: Sendable { !(pendingByConversation[conversationID]?.isEmpty ?? true) } + /// Records an optimistic edit or delete. One per message — reissuing replaces the previous entry. + public mutating func applyMutation(_ entry: MutationEntry, in conversationID: ConversationID) { + mutationsByConversation[conversationID, default: [:]][entry.messageID] = entry + } + + /// Removes the overlay, whether because the server confirmed it, overrode it, or refused it. + public mutating func dropMutation(for messageID: MessageID, in conversationID: ConversationID) { + mutationsByConversation[conversationID]?.removeValue(forKey: messageID) + if mutationsByConversation[conversationID]?.isEmpty == true { + mutationsByConversation.removeValue(forKey: conversationID) + } + } + + /// Replaces each stored message that has a live mutation with its optimistic form. A mutation + /// whose message has already advanced past `expectedSequence` no longer applies: the server's + /// answer has landed, and whatever it says wins. + private func overlaid(_ confirmed: [ConversationMessage], in conversationID: ConversationID) -> [ConversationMessage] { + guard let mutations = mutationsByConversation[conversationID], !mutations.isEmpty else { return confirmed } + + return confirmed.map { message in + guard let entry = mutations[message.id], message.eventSequence <= entry.expectedSequence else { + return message + } + switch entry.kind { + case .edited(let text): + // The stamp only drives the "Edited" marker, so the message's own date stands in for + // the server's — using a live clock here would make the store's output time-dependent. + return message.replacingContent(.text(text), lastEditedTs: message.lastEditedTs ?? message.date) + case .deleted: + // Delete is only ever offered on your own message, so its sender is its deleter. + return message.replacingContent( + .deleted(.init(deletedBy: message.senderID, deletedAt: message.date)), + lastEditedTs: message.lastEditedTs + ) + } + } + } + /// Add an optimistic message the server hasn't confirmed yet, anchored to the caller-supplied /// newest confirmed id at send time so the transcript can position it without a wall-clock compare. public mutating func insertPending(_ message: ConversationMessage, anchoredTo anchor: UInt64, into conversationID: ConversationID) { diff --git a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/MutationEntry.swift b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/MutationEntry.swift new file mode 100644 index 000000000..dea8eca95 --- /dev/null +++ b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/MutationEntry.swift @@ -0,0 +1,32 @@ +// +// MutationEntry.swift +// FlipcashCore +// +// Copyright © 2026 Code Inc. All rights reserved. +// + +import Foundation + +/// An edit or delete that has been issued but not yet confirmed, overlaid on the stored message so +/// the transcript reflects it immediately. The mirror image of `PendingEntry`, which does the same +/// job for a message that has not been sent yet. +public struct MutationEntry: Hashable, Sendable { + + public enum Kind: Hashable, Sendable { + case edited(String) + case deleted + } + + public let messageID: MessageID + public let kind: Kind + /// The `eventSequence` the message carried when the mutation was issued — the same value sent as + /// `expected_event_sequence`. A stored row that exceeds it is the server's newer answer, and the + /// overlay stops applying. + public let expectedSequence: UInt64 + + public init(messageID: MessageID, kind: Kind, expectedSequence: UInt64) { + self.messageID = messageID + self.kind = kind + self.expectedSequence = expectedSequence + } +} diff --git a/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreMutationTests.swift b/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreMutationTests.swift new file mode 100644 index 000000000..484cbac5b --- /dev/null +++ b/FlipcashCore/Tests/FlipcashCoreTests/ConversationStoreMutationTests.swift @@ -0,0 +1,122 @@ +// +// ConversationStoreMutationTests.swift +// FlipcashCoreTests +// +// Copyright © 2026 Code Inc. All rights reserved. +// + +import Testing +import Foundation +@testable import FlipcashCore + +@Suite("ConversationStore mutation overlay") +struct ConversationStoreMutationTests { + + private let sender = UUID() + + private func conversationID(_ byte: UInt8) -> ConversationID { + ConversationID(data: Data(repeating: byte, count: 32)) + } + + private func message(_ id: UInt64, _ text: String, eventSequence: UInt64) -> ConversationMessage { + ConversationMessage( + id: MessageID(value: id), senderID: sender, content: .text(text), + date: Date(timeIntervalSince1970: TimeInterval(id)), unreadSeq: id, eventSequence: eventSequence + ) + } + + private func texts(_ messages: [ConversationMessage]) -> [String] { + messages.map { + switch $0.content { + case .text(let value): value + case .deleted: "" + case .cash: "" + } + } + } + + @Test("An edit overlay replaces the stored text") + func editOverlayReplacesText() { + var store = ConversationStore() + let id = conversationID(1) + store.applyMutation( + MutationEntry(messageID: MessageID(value: 2), kind: .edited("after"), expectedSequence: 5), + in: id + ) + + let displayed = store.displayedMessages( + for: id, + over: [message(1, "one", eventSequence: 4), message(2, "before", eventSequence: 5)] + ) + #expect(texts(displayed) == ["one", "after"]) + } + + @Test("An edit overlay marks the message as edited so the marker shows immediately") + func editOverlayMarksEdited() { + var store = ConversationStore() + let id = conversationID(1) + store.applyMutation( + MutationEntry(messageID: MessageID(value: 2), kind: .edited("after"), expectedSequence: 5), + in: id + ) + + let displayed = store.displayedMessages(for: id, over: [message(2, "before", eventSequence: 5)]) + #expect(displayed.first?.lastEditedTs != nil) + } + + @Test("A delete overlay turns the message into a tombstone attributed to its sender") + func deleteOverlayTombstones() { + var store = ConversationStore() + let id = conversationID(1) + store.applyMutation( + MutationEntry(messageID: MessageID(value: 2), kind: .deleted, expectedSequence: 5), + in: id + ) + + let displayed = store.displayedMessages(for: id, over: [message(2, "before", eventSequence: 5)]) + guard case .deleted(let deletion) = displayed.first?.content else { + Issue.record("expected a tombstone") + return + } + #expect(deletion.deletedBy == sender) + } + + @Test("A stored row that out-versions the overlay wins — the server's answer landed") + func newerStoredRowBeatsTheOverlay() { + var store = ConversationStore() + let id = conversationID(1) + store.applyMutation( + MutationEntry(messageID: MessageID(value: 2), kind: .edited("mine"), expectedSequence: 5), + in: id + ) + + let displayed = store.displayedMessages(for: id, over: [message(2, "theirs", eventSequence: 6)]) + #expect(texts(displayed) == ["theirs"]) + } + + @Test("Dropping the overlay restores the stored text") + func droppingOverlayRestoresStoredText() { + var store = ConversationStore() + let id = conversationID(1) + store.applyMutation( + MutationEntry(messageID: MessageID(value: 2), kind: .edited("after"), expectedSequence: 5), + in: id + ) + store.dropMutation(for: MessageID(value: 2), in: id) + + let displayed = store.displayedMessages(for: id, over: [message(2, "before", eventSequence: 5)]) + #expect(texts(displayed) == ["before"]) + } + + @Test("An overlay in one conversation does not leak into another") + func overlayIsScopedToItsConversation() { + var store = ConversationStore() + store.applyMutation( + MutationEntry(messageID: MessageID(value: 2), kind: .edited("after"), expectedSequence: 5), + in: conversationID(1) + ) + + let displayed = store.displayedMessages(for: conversationID(2), over: [message(2, "before", eventSequence: 5)]) + #expect(texts(displayed) == ["before"]) + } +} From 60c43cb0a61ac1edd0183b1d1b03987d930ca0f5 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 1 Sep 2026 15:51:16 -0400 Subject: [PATCH 2/3] feat(chat): add edit and delete message RPCs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both requests carry `expected_event_sequence`, the server's optimistic-concurrency guard, and both responses return a message on `CONFLICT` as well as on `OK` — the state that won. `MessageMutation` keeps that message alongside the conflict flag, so a caller reconciling a lost race has the winning copy rather than having to wait for the next delta. `conflict` reports at `.info`: it is the guard working, not a client defect. --- .../Controllers/FlipClient+Protocols.swift | 6 + .../Clients/Flip API/FlipClient+Chat.swift | 34 +++++ .../Services/ChatMessagingService.swift | 142 ++++++++++++++++++ .../TestSupport/MockConversations.swift | 59 ++++++++ 4 files changed, 241 insertions(+) diff --git a/Flipcash/Core/Controllers/FlipClient+Protocols.swift b/Flipcash/Core/Controllers/FlipClient+Protocols.swift index deb70c743..ac38f90a3 100644 --- a/Flipcash/Core/Controllers/FlipClient+Protocols.swift +++ b/Flipcash/Core/Controllers/FlipClient+Protocols.swift @@ -82,6 +82,12 @@ protocol ConversationMessaging: AnyObject, Sendable { onBatch: @MainActor @Sendable @escaping (_ messages: [ConversationMessage], _ checkpoint: UInt64?) -> Void ) async throws -> UInt64 func sendMessage(owner: KeyPair, conversationID: ConversationID, text: String, clientMessageID: UUID) async throws -> ConversationMessage + /// Replaces a message's text. `expectedEventSequence` is the optimistic-concurrency guard: the + /// server applies the edit only if the message still carries that sequence, and reports a + /// conflict with the winning state otherwise. + func editMessage(owner: KeyPair, conversationID: ConversationID, messageID: MessageID, text: String, expectedEventSequence: UInt64) async throws -> MessageMutation + /// Tombstones a message, guarded by `expectedEventSequence` the same way as `editMessage`. + func deleteMessage(owner: KeyPair, conversationID: ConversationID, messageID: MessageID, expectedEventSequence: UInt64) async throws -> MessageMutation func markRead(owner: KeyPair, conversationID: ConversationID, messageID: MessageID) async throws func notifyIsTyping(owner: KeyPair, conversationID: ConversationID, state: TypingState) async throws } diff --git a/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/FlipClient+Chat.swift b/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/FlipClient+Chat.swift index 688616a63..a380d7ba2 100644 --- a/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/FlipClient+Chat.swift +++ b/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/FlipClient+Chat.swift @@ -61,6 +61,40 @@ extension FlipClient { } } + public func editMessage( + owner: KeyPair, + conversationID: ConversationID, + messageID: MessageID, + text: String, + expectedEventSequence: UInt64 + ) async throws -> MessageMutation { + try await withCheckedThrowingContinuation { c in + chatMessagingService.editMessage( + owner: owner, + conversationID: conversationID, + messageID: messageID, + text: text, + expectedEventSequence: expectedEventSequence + ) { c.resume(with: $0) } + } + } + + public func deleteMessage( + owner: KeyPair, + conversationID: ConversationID, + messageID: MessageID, + expectedEventSequence: UInt64 + ) async throws -> MessageMutation { + try await withCheckedThrowingContinuation { c in + chatMessagingService.deleteMessage( + owner: owner, + conversationID: conversationID, + messageID: messageID, + expectedEventSequence: expectedEventSequence + ) { c.resume(with: $0) } + } + } + public func markRead(owner: KeyPair, conversationID: ConversationID, messageID: MessageID) async throws { try await withCheckedThrowingContinuation { c in chatMessagingService.advancePointer(owner: owner, conversationID: conversationID, messageID: messageID) { c.resume(with: $0) } diff --git a/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/Services/ChatMessagingService.swift b/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/Services/ChatMessagingService.swift index 808ed6e9e..0942073d0 100644 --- a/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/Services/ChatMessagingService.swift +++ b/FlipcashCore/Sources/FlipcashCore/Clients/Flip API/Services/ChatMessagingService.swift @@ -11,6 +11,18 @@ import GRPCCore private let logger = Logger(label: "flipcash.chat-messaging-service") +/// The outcome of an edit or delete: the message state the server now holds, and whether that state +/// came back as a conflict — meaning another client's change won and this one did not apply. +public struct MessageMutation: Sendable, Equatable { + public let message: ConversationMessage + public let isConflict: Bool + + public init(message: ConversationMessage, isConflict: Bool) { + self.message = message + self.isConflict = isConflict + } +} + /// Wraps the Core `messaging.v1` service (chat messages). Named distinctly from /// the Payments-domain `MessagingService`, which handles bill rendezvous. final class ChatMessagingService: Sendable { @@ -138,6 +150,88 @@ final class ChatMessagingService: Sendable { } } + func editMessage( + owner: KeyPair, + conversationID: ConversationID, + messageID: MessageID, + text: String, + expectedEventSequence: UInt64, + completion: @Sendable @escaping (Result) -> Void + ) { + let request = Flipcash_Messaging_V1_EditMessageRequest.with { + $0.chatID = conversationID.proto + $0.messageID = messageID.proto + $0.content = [.with { $0.text = .with { $0.text = text } }] + $0.expectedEventSequence = expectedEventSequence + $0.auth = owner.authFor(message: $0) + } + + Task { + do { + let response = try await service.editMessage(request, options: .unaryDefault) + let error = ErrorEditMessage(rawValue: response.result.rawValue) ?? .unknown + switch error { + case .ok, .conflict: + guard response.hasMessage, let message = ConversationMessage(response.message) else { + logger.error("Edit message response carried no message") + await MainActor.run { completion(.failure(error == .ok ? .unknown : error)) } + return + } + await MainActor.run { + completion(.success(MessageMutation(message: message, isConflict: error == .conflict))) + } + case .denied, .messageNotFound, .cannotEdit, .unknown, .transportFailure, .cancelled, .rejected: + logger.error("Failed to edit message") + await MainActor.run { completion(.failure(error)) } + } + } catch let error as RPCError { + await MainActor.run { completion(.failure(.from(transportError: error))) } + } catch { + await MainActor.run { completion(.failure(.unknown)) } + } + } + } + + func deleteMessage( + owner: KeyPair, + conversationID: ConversationID, + messageID: MessageID, + expectedEventSequence: UInt64, + completion: @Sendable @escaping (Result) -> Void + ) { + let request = Flipcash_Messaging_V1_DeleteMessageRequest.with { + $0.chatID = conversationID.proto + $0.messageID = messageID.proto + $0.expectedEventSequence = expectedEventSequence + $0.auth = owner.authFor(message: $0) + } + + Task { + do { + let response = try await service.deleteMessage(request, options: .unaryDefault) + let error = ErrorDeleteMessage(rawValue: response.result.rawValue) ?? .unknown + switch error { + case .ok, .conflict: + guard response.hasMessage, let message = ConversationMessage(response.message) else { + logger.error("Delete message response carried no message") + await MainActor.run { completion(.failure(error == .ok ? .unknown : error)) } + return + } + await MainActor.run { + completion(.success(MessageMutation(message: message, isConflict: error == .conflict))) + } + case .denied, .messageNotFound, .cannotDelete, .unknown, .transportFailure, .cancelled, .rejected: + logger.error("Failed to delete message") + await MainActor.run { completion(.failure(error)) } + } + } catch let error as RPCError { + await MainActor.run { completion(.failure(.from(transportError: error))) } + } catch { + await MainActor.run { completion(.failure(.unknown)) } + } + } + } + func advancePointer(owner: KeyPair, conversationID: ConversationID, messageID: MessageID, completion: @Sendable @escaping (Result) -> Void) { let request = Flipcash_Messaging_V1_AdvancePointerRequest.with { $0.chatID = conversationID.proto @@ -211,6 +305,30 @@ public enum ErrorSendMessage: Int, Error { case rejected = -4 } +public enum ErrorEditMessage: Int, Error { + case ok + case denied + case messageNotFound + case cannotEdit + case conflict + case unknown = -1 + case transportFailure = -2 + case cancelled = -3 + case rejected = -4 +} + +public enum ErrorDeleteMessage: Int, Error { + case ok + case denied + case messageNotFound + case cannotDelete + case conflict + case unknown = -1 + case transportFailure = -2 + case cancelled = -3 + case rejected = -4 +} + public enum ErrorAdvancePointer: Int, Error { case ok case denied @@ -264,6 +382,30 @@ extension ErrorSendMessage: ServerError, TransportClassifiableError { } } +extension ErrorEditMessage: ServerError, TransportClassifiableError { + public var reportingLevel: ErrorReportingLevel { + switch self { + case .ok, .transportFailure: .suppressed + case .cancelled: .info + // `conflict` is the concurrency guard doing its job, and the rest are expected + // membership/business outcomes — none is a client defect. + case .denied, .messageNotFound, .cannotEdit, .conflict: .info + case .unknown, .rejected: .error + } + } +} + +extension ErrorDeleteMessage: ServerError, TransportClassifiableError { + public var reportingLevel: ErrorReportingLevel { + switch self { + case .ok, .transportFailure: .suppressed + case .cancelled: .info + case .denied, .messageNotFound, .cannotDelete, .conflict: .info + case .unknown, .rejected: .error + } + } +} + extension ErrorAdvancePointer: ServerError, TransportClassifiableError { public var reportingLevel: ErrorReportingLevel { switch self { diff --git a/FlipcashTests/TestSupport/MockConversations.swift b/FlipcashTests/TestSupport/MockConversations.swift index 5d0ea7717..ebd9a6009 100644 --- a/FlipcashTests/TestSupport/MockConversations.swift +++ b/FlipcashTests/TestSupport/MockConversations.swift @@ -16,6 +16,19 @@ final class MockConversations: ConversationFetching, ConversationMessaging, Conv /// A scripted `GetDelta` batch: one `onBatch` call with these messages + checkpoint. struct DeltaBatch: Sendable { let messages: [ConversationMessage]; let checkpoint: UInt64? } + struct Edited: Sendable, Equatable { + let conversationID: ConversationID + let messageID: MessageID + let text: String + let expectedEventSequence: UInt64 + } + + struct Deleted: Sendable, Equatable { + let conversationID: ConversationID + let messageID: MessageID + let expectedEventSequence: UInt64 + } + private let lock = NSLock() private var _feed: [Conversation] = [] @@ -27,6 +40,12 @@ final class MockConversations: ConversationFetching, ConversationMessaging, Conv private var _sendError: Error? private var _sentClientIDs: [UUID] = [] private var _sent: [Sent] = [] + private var _edited: [Edited] = [] + private var _deleted: [Deleted] = [] + private var _editResult: MessageMutation? + private var _editError: (any Error)? + private var _deleteResult: MessageMutation? + private var _deleteError: (any Error)? private var _markedRead: [MessageID] = [] private var _typingCalls: [TypingCall] = [] private var _typingCallsBegun = 0 @@ -72,6 +91,28 @@ final class MockConversations: ConversationFetching, ConversationMessaging, Conv /// The client message ids `sendMessage` was called with, in order. var sentClientIDs: [UUID] { lock.withLock { _sentClientIDs } } var sent: [Sent] { lock.withLock { _sent } } + /// The edits `editMessage` was called with, in order. + var edited: [Edited] { lock.withLock { _edited } } + /// The deletes `deleteMessage` was called with, in order. + var deleted: [Deleted] { lock.withLock { _deleted } } + var editResult: MessageMutation? { + get { lock.withLock { _editResult } } + set { lock.withLock { _editResult = newValue } } + } + /// When set, `editMessage` throws this instead of returning a mutation. + var editError: (any Error)? { + get { lock.withLock { _editError } } + set { lock.withLock { _editError = newValue } } + } + var deleteResult: MessageMutation? { + get { lock.withLock { _deleteResult } } + set { lock.withLock { _deleteResult = newValue } } + } + /// When set, `deleteMessage` throws this instead of returning a mutation. + var deleteError: (any Error)? { + get { lock.withLock { _deleteError } } + set { lock.withLock { _deleteError = newValue } } + } var markedRead: [MessageID] { lock.withLock { _markedRead } } var typingCalls: [TypingCall] { lock.withLock { _typingCalls } } /// Number of `notifyIsTyping` calls entered, counted before any artificial delay — @@ -154,6 +195,24 @@ final class MockConversations: ConversationFetching, ConversationMessaging, Conv ) } + func editMessage(owner: KeyPair, conversationID: ConversationID, messageID: MessageID, text: String, expectedEventSequence: UInt64) async throws -> MessageMutation { + lock.withLock { + _edited.append(Edited(conversationID: conversationID, messageID: messageID, text: text, expectedEventSequence: expectedEventSequence)) + } + if let editError { throw editError } + guard let editResult else { throw ErrorEditMessage.unknown } + return editResult + } + + func deleteMessage(owner: KeyPair, conversationID: ConversationID, messageID: MessageID, expectedEventSequence: UInt64) async throws -> MessageMutation { + lock.withLock { + _deleted.append(Deleted(conversationID: conversationID, messageID: messageID, expectedEventSequence: expectedEventSequence)) + } + if let deleteError { throw deleteError } + guard let deleteResult else { throw ErrorDeleteMessage.unknown } + return deleteResult + } + func getDelta( owner: KeyPair, conversationID: ConversationID, From 36eb1e5c48e6bdbc30d42775883fef0436f7e7e9 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 1 Sep 2026 15:54:06 -0400 Subject: [PATCH 3/3] feat(chat): reconcile optimistic message edits and deletes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `edit` and `delete` overlay the change, issue the RPC, then persist whatever the server returns. Accepted and conflicted responses take the same path: a conflict's payload is the state that won, which is exactly what has to land locally. There is no retry — reissuing would clobber the change that beat this one. `expected_event_sequence` is read from the database, never from the overlay. A second edit that read its own optimistic value would send a sequence the server never assigned and conflict every time. A message with sequence 0 is unconfirmed and cannot produce a valid request, so both methods refuse it before the call. The failure path drops the overlay and reports through `mutationAlert`; the transcript reverts on its own once the overlay is gone. --- ...versationController+MessageMutations.swift | 147 +++++++++++++++ .../Controllers/ConversationController.swift | 41 ++++- FlipcashTests/ConversationMutationTests.swift | 167 ++++++++++++++++++ 3 files changed, 347 insertions(+), 8 deletions(-) create mode 100644 Flipcash/Core/Controllers/ConversationController+MessageMutations.swift create mode 100644 FlipcashTests/ConversationMutationTests.swift diff --git a/Flipcash/Core/Controllers/ConversationController+MessageMutations.swift b/Flipcash/Core/Controllers/ConversationController+MessageMutations.swift new file mode 100644 index 000000000..480453477 --- /dev/null +++ b/Flipcash/Core/Controllers/ConversationController+MessageMutations.swift @@ -0,0 +1,147 @@ +// +// ConversationController+MessageMutations.swift +// Flipcash +// +// Copyright © 2026 Code Inc. All rights reserved. +// + +import Foundation +import FlipcashCore + +nonisolated private let logger = Logger(label: "flipcash.conversation-controller") + +/// What a mutation attempt did. +enum MutationOutcome: Equatable { + /// The server accepted it; the transcript shows the result. + case applied + /// Another client's change won; the transcript shows that change instead. + case conflicted + /// Nothing applied; the transcript has reverted to what it showed before. + case failed +} + +@MainActor +extension ConversationController { + + /// Replaces a message's text. The transcript updates immediately from an overlay; the server's + /// answer then replaces it, whether that answer is the edit or somebody else's. + @discardableResult + func edit(messageID: MessageID, in conversationID: ConversationID, to text: String) async -> MutationOutcome { + guard let current = confirmedMessage(messageID, in: conversationID), current.eventSequence > 0 else { + logger.error("Refusing to edit a message with no confirmed sequence", metadata: [ + "conversationID": "\(conversationID)", + "messageID": "\(messageID)", + ]) + return .failed + } + + store.applyMutation( + MutationEntry(messageID: messageID, kind: .edited(text), expectedSequence: current.eventSequence), + in: conversationID + ) + bumpMessageRevision() + + do { + let outcome = try await messaging.editMessage( + owner: owner, + conversationID: conversationID, + messageID: messageID, + text: text, + expectedEventSequence: current.eventSequence + ) + settle(outcome, messageID: messageID, in: conversationID, operation: "edit-message") + if outcome.isConflict { + mutationAlert = MutationAlert(action: .edit, kind: .conflict) + return .conflicted + } + return .applied + } catch { + store.dropMutation(for: messageID, in: conversationID) + bumpMessageRevision() + logger.error("Failed to edit conversation message", metadata: [ + "conversationID": "\(conversationID)", + "error": "\(error)", + ]) + ErrorReporting.captureError(error, reason: "Failed to edit conversation message") + mutationAlert = MutationAlert(action: .edit, kind: .failure) + return .failed + } + } + + /// Deletes a message for everyone in the conversation. The row is not removed — it becomes a + /// tombstone, so message ordering stays gapless and a reply quoting it still has a target. + @discardableResult + func delete(messageID: MessageID, in conversationID: ConversationID) async -> MutationOutcome { + guard let current = confirmedMessage(messageID, in: conversationID), current.eventSequence > 0 else { + logger.error("Refusing to delete a message with no confirmed sequence", metadata: [ + "conversationID": "\(conversationID)", + "messageID": "\(messageID)", + ]) + return .failed + } + + store.applyMutation( + MutationEntry(messageID: messageID, kind: .deleted, expectedSequence: current.eventSequence), + in: conversationID + ) + bumpMessageRevision() + + do { + let outcome = try await messaging.deleteMessage( + owner: owner, + conversationID: conversationID, + messageID: messageID, + expectedEventSequence: current.eventSequence + ) + settle(outcome, messageID: messageID, in: conversationID, operation: "delete-message") + if outcome.isConflict { + mutationAlert = MutationAlert(action: .delete, kind: .conflict) + return .conflicted + } + return .applied + } catch { + store.dropMutation(for: messageID, in: conversationID) + bumpMessageRevision() + logger.error("Failed to delete conversation message", metadata: [ + "conversationID": "\(conversationID)", + "error": "\(error)", + ]) + ErrorReporting.captureError(error, reason: "Failed to delete conversation message") + mutationAlert = MutationAlert(action: .delete, kind: .failure) + return .failed + } + } + + /// The stored copy. The overlay is deliberately not consulted: `expected_event_sequence` has to + /// come from server truth, or a second edit would send the sequence the first one optimistically + /// assumed and conflict against the server every time. + private func confirmedMessage(_ messageID: MessageID, in conversationID: ConversationID) -> ConversationMessage? { + do { + return try database.message(id: messageID, conversationID: conversationID) + } catch { + logger.error("Failed to read message for mutation", metadata: [ + "conversationID": "\(conversationID)", + "error": "\(error)", + ]) + return nil + } + } + + /// Persists whatever the server says the message now is and drops the overlay. Identical for an + /// accepted mutation and a conflict — a conflict's payload is the state that won, which is + /// exactly what has to land locally. There is no retry: reissuing would clobber the change that + /// beat this one. + private func settle( + _ outcome: MessageMutation, + messageID: MessageID, + in conversationID: ConversationID, + operation: String + ) { + _ = persist(operation: operation) { + try database.upsertConversationMessages([outcome.message], conversationID: conversationID) + } + store.dropMutation(for: messageID, in: conversationID) + refreshFeedPreview(for: conversationID) + persistConversation(conversationID) + } +} diff --git a/Flipcash/Core/Controllers/ConversationController.swift b/Flipcash/Core/Controllers/ConversationController.swift index d35ddd5bf..a0824e4c7 100644 --- a/Flipcash/Core/Controllers/ConversationController.swift +++ b/Flipcash/Core/Controllers/ConversationController.swift @@ -100,18 +100,18 @@ final class ConversationController { /// it's excluded from observation. @ObservationIgnored var visibleConversationID: ConversationID? - private var store = ConversationStore() + var store = ConversationStore() /// The current blocklist (wired to `BlocklistController`), used to reconcile /// which conversations are hidden from the feed. @ObservationIgnored var blockedUserIDs: () -> Set = { [] } @ObservationIgnored private let fetching: any ConversationFetching - @ObservationIgnored private let messaging: any ConversationMessaging + @ObservationIgnored let messaging: any ConversationMessaging @ObservationIgnored private let streaming: any ConversationEventStreaming @ObservationIgnored private let contactNaming: any DMContactNaming - @ObservationIgnored private let database: Database - @ObservationIgnored private let owner: KeyPair + @ObservationIgnored let database: Database + @ObservationIgnored let owner: KeyPair @ObservationIgnored private var startTask: Task? @ObservationIgnored private var streamTask: Task? @ObservationIgnored private var connectionStateTask: Task? @@ -695,7 +695,7 @@ final class ConversationController { /// Recompute the feed row's preview from the newest persisted *visible* message (the store no longer /// holds the confirmed transcript to derive it from). - private func refreshFeedPreview(for conversationID: ConversationID) { + func refreshFeedPreview(for conversationID: ConversationID) { let visible = (try? database.latestMessage(conversationID: conversationID)) ?? nil // A newest row that is itself invisible (a tombstone) is the one case the preview must regress — // the never-regress guard would otherwise keep showing the deleted content. @@ -709,7 +709,7 @@ final class ConversationController { /// Persists the store's current version of a conversation. No-ops for /// conversations the store doesn't know yet. - private func persistConversation(_ conversationID: ConversationID) { + func persistConversation(_ conversationID: ConversationID) { guard let conversation = store.conversations.first(where: { $0.id == conversationID }) else { return } persistConversation(conversation) } @@ -719,7 +719,7 @@ final class ConversationController { } @discardableResult - private func persist(operation: String, _ write: () throws -> Void) -> Bool { + func persist(operation: String, _ write: () throws -> Void) -> Bool { do { try write() // A successful confirmed-message write invalidates the DB-backed transcript window; bump the @@ -741,7 +741,7 @@ final class ConversationController { /// Persist operations that write confirmed messages — the ones that must bump `messageRevision`. private static let messageWriteOperations: Set = [ "upsert-messages", "apply-chat-events", "delta-batch", "load-messages", "load-older", - "send-message", "reset-resync", + "send-message", "reset-resync", "edit-message", "delete-message", ] // MARK: - Names @@ -831,6 +831,31 @@ final class ConversationController { /// re-fire through the store directly. private(set) var messageRevision = 0 + /// Forces the transcript to re-read its window. `persist(operation:)` does this for database + /// writes; an overlay change writes nothing, so it has to say so explicitly. + func bumpMessageRevision() { + messageRevision &+= 1 + } + + /// Set when a mutation needs to be reported to the person who made it. The screen presents it + /// and clears it. `nil` means there is nothing to report. + var mutationAlert: MutationAlert? + + /// A mutation the user has to be told about, because the transcript alone will not explain it. + struct MutationAlert: Equatable, Identifiable { + enum Kind: String, Equatable { + /// Another client's change won; the transcript now shows that change, not this one. + case conflict + /// The request never applied; the transcript has reverted. + case failure + } + + let action: MessageCapability + let kind: Kind + + var id: String { "\(action.rawValue)-\(kind.rawValue)" } + } + /// The transcript's bounded window with the in-memory optimistic overlay applied: every confirmed /// message from `startID` to the newest when anchored, else the newest `limit`. The DB is the source /// of the confirmed rows; the store contributes only the pending overlay. Anchoring by id means an diff --git a/FlipcashTests/ConversationMutationTests.swift b/FlipcashTests/ConversationMutationTests.swift new file mode 100644 index 000000000..d7522072f --- /dev/null +++ b/FlipcashTests/ConversationMutationTests.swift @@ -0,0 +1,167 @@ +// +// ConversationMutationTests.swift +// FlipcashTests +// +// Copyright © 2026 Code Inc. All rights reserved. +// + +import Testing +import Foundation +import FlipcashCore +@testable import Flipcash + +@MainActor +@Suite("Conversation message mutations") +struct ConversationMutationTests { + + /// The controller plus the two things a mutation test has to reach into: the transport it talks + /// to, and the database it reads sequences from. Built the same way `ConversationControllerTests` + /// builds its controller, with the database held rather than discarded. + private struct Harness { + let controller: ConversationController + let messaging: MockConversations + let database: Database + let conversationID: ConversationID + } + + private func makeHarness(selfUserID: UserID = UUID()) throws -> Harness { + let mock = MockConversations() + let database = try Database.makeTemp().database + let controller = ConversationController( + fetching: mock, messaging: mock, streaming: mock, + contactNaming: MockDMContactNaming(), + database: database, + owner: .generate()!, selfUserID: selfUserID, + typingHeartbeatInterval: .seconds(3), + incomingTypingExpiry: .seconds(10) + ) + return Harness(controller: controller, messaging: mock, database: database, conversationID: .test(1)) + } + + private func stored(_ id: UInt64, _ text: String, sender: UUID, eventSequence: UInt64) -> ConversationMessage { + ConversationMessage( + id: MessageID(value: id), senderID: sender, content: .text(text), + date: Date(timeIntervalSince1970: TimeInterval(id)), unreadSeq: id, eventSequence: eventSequence + ) + } + + @Test("Editing sends the message's stored event sequence as the expected one") + func editSendsStoredSequence() async throws { + let harness = try makeHarness() + let conversationID = harness.conversationID + let original = stored(1, "before", sender: harness.controller.selfUserID, eventSequence: 4) + try harness.database.upsertConversationMessages([original], conversationID: conversationID) + + harness.messaging.editResult = MessageMutation( + message: stored(1, "after", sender: harness.controller.selfUserID, eventSequence: 5), + isConflict: false + ) + + let outcome = await harness.controller.edit(messageID: MessageID(value: 1), in: conversationID, to: "after") + + #expect(outcome == .applied) + #expect(harness.messaging.edited.count == 1) + #expect(harness.messaging.edited.first?.expectedEventSequence == 4) + #expect(harness.messaging.edited.first?.text == "after") + } + + @Test("A successful edit persists the server's copy") + func editPersistsServerCopy() async throws { + let harness = try makeHarness() + let conversationID = harness.conversationID + try harness.database.upsertConversationMessages( + [stored(1, "before", sender: harness.controller.selfUserID, eventSequence: 4)], + conversationID: conversationID + ) + harness.messaging.editResult = MessageMutation( + message: stored(1, "after", sender: harness.controller.selfUserID, eventSequence: 5), + isConflict: false + ) + + _ = await harness.controller.edit(messageID: MessageID(value: 1), in: conversationID, to: "after") + + let persisted = try #require(try harness.database.message(id: MessageID(value: 1), conversationID: conversationID)) + #expect(persisted.content == .text("after")) + #expect(persisted.eventSequence == 5) + } + + @Test("A conflict persists the state that won and reports the conflict") + func editConflictPersistsWinner() async throws { + let harness = try makeHarness() + let conversationID = harness.conversationID + try harness.database.upsertConversationMessages( + [stored(1, "before", sender: harness.controller.selfUserID, eventSequence: 4)], + conversationID: conversationID + ) + harness.messaging.editResult = MessageMutation( + message: stored(1, "someone else won", sender: harness.controller.selfUserID, eventSequence: 6), + isConflict: true + ) + + let outcome = await harness.controller.edit(messageID: MessageID(value: 1), in: conversationID, to: "mine") + + #expect(outcome == .conflicted) + let persisted = try #require(try harness.database.message(id: MessageID(value: 1), conversationID: conversationID)) + #expect(persisted.content == .text("someone else won")) + #expect(harness.controller.mutationAlert?.kind == .conflict) + } + + @Test("A transport failure reverts the overlay and leaves the stored text alone") + func editFailureRevertsOverlay() async throws { + let harness = try makeHarness() + let conversationID = harness.conversationID + try harness.database.upsertConversationMessages( + [stored(1, "before", sender: harness.controller.selfUserID, eventSequence: 4)], + conversationID: conversationID + ) + harness.messaging.editError = ErrorEditMessage.transportFailure + + let outcome = await harness.controller.edit(messageID: MessageID(value: 1), in: conversationID, to: "after") + + #expect(outcome == .failed) + let displayed = harness.controller.windowedMessages(for: conversationID, startingAt: nil, limit: 50) + #expect(displayed.first?.content == .text("before")) + #expect(harness.controller.mutationAlert?.kind == .failure) + } + + @Test("An unconfirmed message cannot be edited — there is no sequence to send") + func editRejectsUnconfirmedMessage() async throws { + let harness = try makeHarness() + let conversationID = harness.conversationID + try harness.database.upsertConversationMessages( + [stored(1, "before", sender: harness.controller.selfUserID, eventSequence: 0)], + conversationID: conversationID + ) + + let outcome = await harness.controller.edit(messageID: MessageID(value: 1), in: conversationID, to: "after") + + #expect(outcome == .failed) + #expect(harness.messaging.edited.isEmpty) + } + + @Test("Deleting sends the stored sequence and persists the tombstone") + func deletePersistsTombstone() async throws { + let harness = try makeHarness() + let conversationID = harness.conversationID + let me = harness.controller.selfUserID + try harness.database.upsertConversationMessages( + [stored(1, "before", sender: me, eventSequence: 4)], + conversationID: conversationID + ) + harness.messaging.deleteResult = MessageMutation( + message: ConversationMessage( + id: MessageID(value: 1), senderID: me, + content: .deleted(.init(deletedBy: me, deletedAt: Date(timeIntervalSince1970: 10))), + date: Date(timeIntervalSince1970: 1), unreadSeq: 1, eventSequence: 5 + ), + isConflict: false + ) + + let outcome = await harness.controller.delete(messageID: MessageID(value: 1), in: conversationID) + + #expect(outcome == .applied) + #expect(harness.messaging.deleted.first?.expectedEventSequence == 4) + let persisted = try #require(try harness.database.message(id: MessageID(value: 1), conversationID: conversationID)) + #expect(persisted.isDeleted) + } +}