mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-25 02:25:20 +00:00
DMs to unreachable peers use store-and-forward instead of failing instantly (#1415)
* DMs to unreachable peers route through store-and-forward; drop stale reachability gates
Field-found: sendPrivateMessage pre-judged reachability and marked the
message failed without ever calling the router, so the retained outbox,
courier deposits, and bridge drops never ran — a DM composed after a
peer's reachability window lapsed was dead on arrival while an identical
one sent a minute earlier delivered. Messages now always route; a live
path earns "sent", everything else stays "sending" until the router
reports carried/delivered or expires it as failed.
A sweep for sibling gates found and fixed:
- startPrivateChat refused offline non-mutual favorites ("mutual favorite
required for offline messaging") — store-and-forward only needs the
recipient's noise key, so the gate and its string are gone.
- markPrivateMessagesAsRead skipped the whole receipt pass for peers
without a stored Nostr key, starving mesh-connected non-favorites; the
router picks the transport now (sentReadReceipts dedups the parallel
PrivateChatManager path).
- DM/geoDM blocked notices and the unknown-group error posted to the
active public timeline; they now land in the thread they're about via
addLocalPrivateSystemMessage.
- Banned copy: literal "user" fallbacks in code become "anon" (the
default-nickname convention), and es/fr/it/pt translations of four keys
still saying usuario/utilisateur/utente/usuário now say person. Orphaned
keys removed (system.dm.unreachable, content.delivery.reason.unreachable,
system.chat.requires_favorite).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Deflake BLEServiceCoreTests: longTimeout for positive waits
CI flaked on duplicatePacket_isDeduped: the first message landed after the
5s wait expired on a loaded runner (the test's later count==1 assertions
passed, proving late delivery, not lost delivery). All four positive waits
in the file now use TestConstants.longTimeout, which exists for exactly
this — waitUntil returns as soon as the condition holds, so passing runs
never pay the longer ceiling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fix read-receipt test contamination; baseline Periphery noiseKey flake
Two CI failures, both environmental:
- markReadReceiptSent_returnsFalseOnSecondCall flaked because every test
ChatViewModel shared one per-process read-receipts scratch suite; the
lifecycle test persists the same "read-1" ID (a path the receipt-gate
removal now reaches), and test order decided who saw whose state. Each
instance now gets its own UUID-suffixed suite.
- Periphery intermittently misses the loadFromDisk read of
PrekeyBundleStore.StoredBundle.noiseKey and fails --strict with a false
"assign-only" finding (recurrence of a known flake). Its USR is
baselined; an in-source periphery:ignore can't work because strict mode
flags it as superfluous on runs where the indexer gets it right.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
jack
Claude Fable 5
parent
dc99d641c8
commit
55d51ba084
@@ -32,7 +32,7 @@ struct BLEServiceCoreTests {
|
||||
ble._test_handlePacket(packet, fromPeerID: sender, signingPublicKey: signingKey)
|
||||
let receivedFirst = await TestHelpers.waitUntil(
|
||||
{ delegate.publicMessagesSnapshot().count == 1 },
|
||||
timeout: TestConstants.defaultTimeout
|
||||
timeout: TestConstants.longTimeout
|
||||
)
|
||||
#expect(receivedFirst)
|
||||
|
||||
@@ -188,7 +188,7 @@ struct BLEServiceCoreTests {
|
||||
|
||||
let rebound = await TestHelpers.waitUntil(
|
||||
{ ble._test_centralBinding(centralUUID) == newPeerID },
|
||||
timeout: TestConstants.defaultTimeout
|
||||
timeout: TestConstants.longTimeout
|
||||
)
|
||||
#expect(rebound)
|
||||
|
||||
@@ -197,7 +197,7 @@ struct BLEServiceCoreTests {
|
||||
let peerIDs = ble.currentPeerSnapshots().map(\.peerID)
|
||||
return peerIDs.contains(newPeerID) && !peerIDs.contains(oldPeerID)
|
||||
},
|
||||
timeout: TestConstants.defaultTimeout
|
||||
timeout: TestConstants.longTimeout
|
||||
)
|
||||
#expect(retired)
|
||||
}
|
||||
@@ -374,7 +374,7 @@ struct BLEServiceCoreTests {
|
||||
|
||||
let reachedBudget = await TestHelpers.waitUntil(
|
||||
{ outbound.count(ofType: .pong) >= budget },
|
||||
timeout: TestConstants.defaultTimeout
|
||||
timeout: TestConstants.longTimeout
|
||||
)
|
||||
#expect(reachedBudget)
|
||||
// Give any over-budget pong a chance to surface, then confirm the
|
||||
|
||||
@@ -328,7 +328,7 @@ struct ChatLifecycleCoordinatorContextTests {
|
||||
#expect(context.ownerLevelReadPasses == [peerID])
|
||||
}
|
||||
@Test @MainActor
|
||||
func markPrivateMessagesAsRead_routesReceiptsOnlyForNostrReachableFavorites() {
|
||||
func markPrivateMessagesAsRead_routesReceiptsForFavoritesAndNonFavorites() {
|
||||
let context = MockChatLifecycleContext()
|
||||
let coordinator = ChatLifecycleCoordinator(context: context)
|
||||
let noiseKey = Data(repeating: 0xAB, count: 32)
|
||||
@@ -351,12 +351,15 @@ struct ChatLifecycleCoordinatorContextTests {
|
||||
#expect(context.routedReadReceipts.map(\.peerID) == [peerID])
|
||||
#expect(context.sentReadReceipts.contains("in-1"))
|
||||
|
||||
// No favorite relationship (no Nostr key): the receipt pass is skipped.
|
||||
// No favorite relationship: receipts still route — the router picks
|
||||
// whatever transport can reach the peer (mesh included). Gating on a
|
||||
// stored Nostr key silently starved mesh-connected non-favorites.
|
||||
let otherKey = Data(repeating: 0xCD, count: 32)
|
||||
let otherPeer = PeerID(hexData: otherKey)
|
||||
context.privateChats[otherPeer] = [makePrivateMessage(id: "in-2", senderPeerID: otherPeer)]
|
||||
coordinator.markPrivateMessagesAsRead(from: otherPeer)
|
||||
#expect(context.routedReadReceipts.map(\.messageID) == ["in-1"])
|
||||
#expect(context.routedReadReceipts.map(\.messageID) == ["in-1", "in-2"])
|
||||
#expect(context.sentReadReceipts.contains("in-2"))
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -234,6 +234,12 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC
|
||||
meshOnlySystemMessages.append(content)
|
||||
}
|
||||
|
||||
private(set) var privateSystemMessages: [(content: String, peerID: PeerID)] = []
|
||||
|
||||
func addLocalPrivateSystemMessage(_ content: String, to peerID: PeerID) {
|
||||
privateSystemMessages.append((content, peerID))
|
||||
}
|
||||
|
||||
static let dummyIdentity = NostrIdentity(
|
||||
privateKey: Data(repeating: 0x11, count: 32),
|
||||
publicKey: Data(repeating: 0x22, count: 32),
|
||||
@@ -680,19 +686,21 @@ struct ChatPrivateConversationCoordinatorContextTests {
|
||||
#expect(context.systemMessages.isEmpty)
|
||||
}
|
||||
|
||||
/// Field-found: pre-judging reachability here marked the message failed
|
||||
/// without ever routing it, so the router's retained outbox, courier
|
||||
/// deposits, and bridge drops never got a chance. A fully unreachable
|
||||
/// non-favorite must still be routed and stay "sending" (the router's
|
||||
/// callbacks later move it to carried/delivered or expire it as failed).
|
||||
@Test @MainActor
|
||||
func sendPrivateMessage_failsWhenOfflineWithoutMutualFavorite() async {
|
||||
func sendPrivateMessage_routesAndStaysSendingWhenOfflineWithoutMutualFavorite() async {
|
||||
let context = MockChatPrivateConversationContext()
|
||||
let coordinator = ChatPrivateConversationCoordinator(context: context)
|
||||
let peerID = PeerID(hexData: Data(repeating: 0xCD, count: 32))
|
||||
|
||||
coordinator.sendPrivateMessage("hello?", to: peerID)
|
||||
|
||||
#expect(context.routedPrivateMessages.isEmpty)
|
||||
#expect(context.systemMessages.count == 1)
|
||||
guard case .failed = context.privateChats[peerID]?.first?.deliveryStatus else {
|
||||
Issue.record("expected .failed delivery status")
|
||||
return
|
||||
}
|
||||
#expect(context.routedPrivateMessages.map(\.content) == ["hello?"])
|
||||
#expect(context.privateChats[peerID]?.first?.deliveryStatus == .sending)
|
||||
#expect(context.systemMessages.isEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,8 +71,11 @@ struct ChatViewModelPrivateChatExtensionTests {
|
||||
// Check MockTransport implementation... it might need update or verification
|
||||
}
|
||||
|
||||
/// An unreachable recipient no longer means instant failure: the message
|
||||
/// is routed anyway so the router's outbox/courier/bridge machinery can
|
||||
/// deliver it, and it stays "sending" until a router callback resolves it.
|
||||
@Test @MainActor
|
||||
func sendPrivateMessage_unreachable_setsFailedStatus() async {
|
||||
func sendPrivateMessage_unreachable_staysSendingForStoreAndForward() async {
|
||||
let (viewModel, _) = makeTestableViewModel()
|
||||
let validHex = "0102030405060708090a0b0c0d0e0f100102030405060708090a0b0c0d0e0f10"
|
||||
let peerID = PeerID(str: validHex)
|
||||
@@ -80,11 +83,7 @@ struct ChatViewModelPrivateChatExtensionTests {
|
||||
viewModel.sendPrivateMessage("Hello", to: peerID)
|
||||
|
||||
#expect(viewModel.privateChats[peerID]?.count == 1)
|
||||
let status = viewModel.privateChats[peerID]?.last?.deliveryStatus
|
||||
#expect({
|
||||
if case .failed = status { return true }
|
||||
return false
|
||||
}())
|
||||
#expect(viewModel.privateChats[peerID]?.last?.deliveryStatus == .sending)
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
@@ -780,8 +779,10 @@ struct ChatViewModelGeoDMTests {
|
||||
#expect(isFailed(status: viewModel.privateChats[convKey]?.last?.deliveryStatus))
|
||||
}
|
||||
|
||||
/// The blocked notice belongs in the DM thread the person is typing in,
|
||||
/// not on the active location-channel timeline.
|
||||
@Test @MainActor
|
||||
func sendGeohashDM_blockedRecipient_marksFailedAndAddsSystemMessage() async {
|
||||
func sendGeohashDM_blockedRecipient_marksFailedAndAddsSystemMessageInThread() async {
|
||||
let (viewModel, _) = makeTestableViewModel()
|
||||
let geohash = "u4pruydq"
|
||||
let recipientHex = "0000000000000000000000000000000000000000000000000000000000000003"
|
||||
@@ -793,9 +794,11 @@ struct ChatViewModelGeoDMTests {
|
||||
|
||||
viewModel.sendGeohashDM("hello", to: convKey)
|
||||
|
||||
#expect(viewModel.privateChats[convKey]?.count == 1)
|
||||
#expect(isFailed(status: viewModel.privateChats[convKey]?.last?.deliveryStatus))
|
||||
#expect(viewModel.messages.contains(where: { $0.sender == "system" }))
|
||||
let thread = viewModel.privateChats[convKey] ?? []
|
||||
#expect(thread.count == 2)
|
||||
#expect(isFailed(status: thread.first?.deliveryStatus))
|
||||
#expect(thread.last?.sender == "system")
|
||||
#expect(!viewModel.messages.contains(where: { $0.sender == "system" }))
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
|
||||
Reference in New Issue
Block a user