Review fix: record withheld receipts in both tracking sets

Codex P1: the manager-path withheld claim landed only in
PrivateChatManager.sentReadReceipts, while the lifecycle read pass
dedups against ChatViewModel's persisted set — enabling receipts
before the next lifecycle pass could send a receipt for a message
read while the setting was off. The withheld branch now records into
the owner's persisted set too (markReceiptHandled, wired in the
bootstrapper); test strengthened to require both sets.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack 2026-08-11 10:03:01 +02:00
parent 3e46829f32
commit cfa875459d
3 changed files with 19 additions and 5 deletions

View File

@ -252,15 +252,23 @@ final class PrivateChatManager: ObservableObject {
/// suites through the shared UserDefaults-backed setting.
var sendsReadReceipts: () -> Bool = { ReadReceiptSettings.sendReadReceipts }
/// Records a withheld receipt in the owner's persisted set too: the
/// lifecycle read pass dedups against ChatViewModel.sentReadReceipts,
/// not this manager's set, so claiming only locally would let a receipt
/// for a message read while the setting was OFF fire after re-enabling.
var markReceiptHandled: ((String) -> Void)?
private func sendReadReceipt(for message: BitchatMessage) {
guard !sentReadReceipts.contains(message.id),
let senderPeerID = message.senderPeerID else {
return
}
// Withheld receipts are still claimed below as sent: re-enabling the
// setting must never fire a retroactive burst disclosing past reads.
// Withheld receipts are still claimed as sent in BOTH tracking
// sets: re-enabling the setting must never fire a retroactive burst
// disclosing past reads, from this manager or the lifecycle pass.
guard sendsReadReceipts() else {
sentReadReceipts.insert(message.id)
markReceiptHandled?(message.id)
return
}

View File

@ -90,6 +90,9 @@ private extension ChatViewModelBootstrapper {
viewModel.privateChatManager.conversationStore = viewModel.conversations
viewModel.privateChatManager.messageRouter = viewModel.messageRouter
viewModel.privateChatManager.unifiedPeerService = viewModel.unifiedPeerService
viewModel.privateChatManager.markReceiptHandled = { [weak viewModel] messageID in
viewModel?.markReadReceiptSent(messageID)
}
viewModel.unifiedPeerService.messageRouter = viewModel.messageRouter
// Surface silent outbox drops (attempt cap, TTL expiry, overflow
// eviction) as a visible failure. The store's no-downgrade rule does

View File

@ -462,10 +462,13 @@ struct ChatViewModelServiceLifecycleTests {
#expect(!sentReadReceipt)
// ...while the chat is still marked read locally and the receipt is
// recorded as handled, so re-enabling the setting never fires a
// retroactive burst disclosing past reading activity.
// recorded as handled in BOTH tracking sets (the lifecycle pass
// dedups against the owner's persisted set, the manager against its
// own), so re-enabling the setting never fires a retroactive burst
// disclosing past reading activity from either path.
#expect(!viewModel.unreadPrivateMessages.contains(peerID))
#expect(viewModel.sentReadReceipts.contains("read-2") || viewModel.privateChatManager.sentReadReceipts.contains("read-2"))
#expect(viewModel.sentReadReceipts.contains("read-2"))
#expect(viewModel.privateChatManager.sentReadReceipts.contains("read-2"))
}
@Test @MainActor