diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt index 6b82cd43..2539c488 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt @@ -21,6 +21,7 @@ import press.mantra.compose.database.model.Participant import press.mantra.compose.exceptions.MarmotMissingChatGroupException import press.mantra.compose.database.model.intermdiate.LocalChatRoom import press.mantra.compose.extensions.exporterSecret +import press.mantra.compose.extensions.shortened import press.mantra.compose.extensions.toHex import press.mantra.compose.managers.MarmotInboundManager.EPOCH_RETENTION_WINDOW import press.mantra.compose.nostr.MarmotDelivery @@ -231,6 +232,12 @@ abstract class MarmotOutboundDao( }.onFailure { logger.e("Failed to invite $peerPublicKey to ${localChatRoom.chatRoom.id}", it) notAdded.add(peerPublicKey) + announceInviteFailed( + chatRoomId = localChatRoom.chatRoom.id, + userPublicKey = localChatRoom.chatRoom.userPublicKey, + peerPublicKey = peerPublicKey, + reason = it.message, + ) } } @@ -287,9 +294,30 @@ abstract class MarmotOutboundDao( ) ) + // One line each, as `inviteMember` writes for the invites it makes -- this + // path does its own commit and never goes through it. Written before the + // Welcomes rather than after, so a delivery that fails has an invite to be + // read against instead of a failure on its own. + peers.forEach { (peerPublicKey, _) -> + announceMembership( + chatRoomId = localChatRoom.chatRoom.id, + userPublicKey = localChatRoom.chatRoom.userPublicKey, + messageType = ChatMessage.TYPE_MEMBER_INVITED, + content = "Invited ${memberName(peerPublicKey)} to the group", + ) + } + val welcomeBytes = commitResult.welcomeBytes if (welcomeBytes == null) { logger.e("Batched commit for ${localChatRoom.chatRoom.id} produced no welcome") + peers.forEach { (peerPublicKey, _) -> + announceInviteFailed( + chatRoomId = localChatRoom.chatRoom.id, + userPublicKey = localChatRoom.chatRoom.userPublicKey, + peerPublicKey = peerPublicKey, + reason = "the group produced no invitation to send", + ) + } return peers.map { it.first } } @@ -300,17 +328,20 @@ abstract class MarmotOutboundDao( val notAdded = mutableListOf() peers.forEach { (peerPublicKey, peerKeyPackage) -> - runCatching { - deliveryWelcome( - nostrGroupId = localChatRoom.chatRoom.id, - userPublicKey = localChatRoom.chatRoom.userPublicKey, - welcomeBytes = welcomeBytes, - peerKeyPackageEventId = peerKeyPackage.id, - relays = relays, - createdAt = Clock.System.now() - ) - }.onFailure { - logger.e("Failed to deliver the welcome to $peerPublicKey", it) + // `deliveryWelcome` swallows what it catches and reports it as a + // transcript line instead, so its answer is what says whether this peer + // was reached -- a runCatching here would see nothing to catch. + val delivered = deliveryWelcome( + nostrGroupId = localChatRoom.chatRoom.id, + userPublicKey = localChatRoom.chatRoom.userPublicKey, + welcomeBytes = welcomeBytes, + peerKeyPackageEventId = peerKeyPackage.id, + relays = relays, + createdAt = Clock.System.now() + ) + + if (!delivered) { + logger.e("Failed to deliver the welcome to $peerPublicKey") notAdded.add(peerPublicKey) } } @@ -318,6 +349,100 @@ abstract class MarmotOutboundDao( return notAdded } + /** + * One line in the room's transcript, about a membership change. + * + * The same shape as `ChronicleManager.announce`, and for the same reason: this + * is the device saying what it just did, not an event anybody sent. Content is + * a whole sentence, so nothing prefixes a name to it -- see + * [ChatMessage.MEMBERSHIP_TYPES]. + */ + private suspend fun announceMembership( + chatRoomId: HexKey, + userPublicKey: HexKey, + messageType: String, + content: String, + ) { + database.chatMessageDao().upsert( + ChatMessage( + content = content, + messageType = messageType, + chatRoomId = chatRoomId, + senderPublicKey = userPublicKey, + isUserMessage = true, + giftWrapPayloadId = null, + marmotGroupEventId = null, + marmotInnerEventId = null, + ) + ) + } + + /** + * What to call an invitee on a membership line. + * + * Written into the content rather than resolved at render time, because these + * lines are about something that happened once and the AUTHORED-set machinery + * that follows a rename is for lines with an actor. A member being invited has + * a Profile row by construction -- Participant.participantPublicKey is a + * foreign key onto it -- so the fallback is for the delivery path, which runs + * long after the invite and reads its peer back out of a key package. + */ + private suspend fun memberName(publicKey: HexKey): String = + database.profileDao().getProfileByPublicKey(publicKey)?.humanReadableNameOrPubkey() + ?: publicKey.shortened() + + /** + * The Welcome for an invite made earlier has gone out. + * + * Written by the deferred delivery site only -- `DatabaseNostrRepository`, once + * a relay has acknowledged the commit. An invite into a group that was still + * just its creator sends its Welcome in the same breath as the invite, so its + * [ChatMessage.TYPE_MEMBER_INVITED] line already says this and a second one + * would only be the same second told twice. See [ChatMessage.MEMBERSHIP_TYPES]. + */ + suspend fun announceInviteSent( + chatRoomId: HexKey, + userPublicKey: HexKey, + peerKeyPackageEventId: HexKey, + ) { + val peerPublicKey = database.marmotKeyPackageDao() + .getMarmotKeyPackageById(peerKeyPackageEventId)?.publicKey + + announceMembership( + chatRoomId = chatRoomId, + userPublicKey = userPublicKey, + messageType = ChatMessage.TYPE_MEMBER_INVITE_SENT, + content = peerPublicKey?.let { "Sent ${memberName(it)} their invitation" } + ?: "Sent the invitation", + ) + } + + /** + * An invite did not make it, and nothing else will say so. + * + * The reason is put in the line because it is the only copy the user gets: the + * screen that asked for the invite is gone by the time the Welcome is + * delivered, and the failures that happen before it closes take the whole + * transaction -- and the invite line inside it -- down with them. See + * `DatabaseChatRepository.inviteMember`. + */ + suspend fun announceInviteFailed( + chatRoomId: HexKey, + userPublicKey: HexKey, + peerPublicKey: HexKey?, + reason: String?, + ) { + val who = peerPublicKey?.let { "${memberName(it)}'s invitation" } ?: "the invitation" + val why = reason?.takeIf { it.isNotBlank() }?.let { ": $it" } ?: "" + + announceMembership( + chatRoomId = chatRoomId, + userPublicKey = userPublicKey, + messageType = ChatMessage.TYPE_MEMBER_INVITE_FAILED, + content = "Couldn't send $who$why", + ) + } + private suspend fun inviteMember( nostrGroupId: HexKey, mlsGroup: MlsGroup, @@ -357,6 +482,20 @@ abstract class MarmotOutboundDao( "KeyPackage credential identity does not match memberPubKey" } + // Say so now, not when the Welcome eventually goes out. On the deferred path + // that is a relay round trip away and may never happen at all, and until this + // line existed the room showed nothing whatsoever in the meantime -- an invite + // that was queued, one that failed to reach the wire and one that was never + // made all looked identical from the transcript. Inside the caller's + // transaction, so an invite that does not survive `addMember` leaves no claim + // that it did. + announceMembership( + chatRoomId = nostrGroupId, + userPublicKey = userPublicKey, + messageType = ChatMessage.TYPE_MEMBER_INVITED, + content = "Invited ${memberName(peerPublicKey)} to the group", + ) + val retainedBefore = mlsGroup.retainedSecrets() val commitResult = mlsGroup.addMember( peerKeyPackage.tlsEncodedMarmotKeyPackage @@ -462,6 +601,16 @@ abstract class MarmotOutboundDao( } } + /** + * Put the invitee's Welcome on the outbound queue. + * + * Returns whether it got there. The caller cannot see that any other way -- the + * body swallows what it catches, deliberately, because on the deferred path this + * runs from a relay acknowledgement with no invite screen left to fail back to. + * What replaced reporting nothing at all is the [ChatMessage.TYPE_MEMBER_INVITE_FAILED] + * line written below, which outlives the screen and is the only account of a + * Welcome that never went out. + */ suspend fun deliveryWelcome( nostrGroupId: HexKey, userPublicKey: HexKey, @@ -469,7 +618,7 @@ abstract class MarmotOutboundDao( peerKeyPackageEventId: HexKey, relays: List, createdAt: Instant - ) { + ): Boolean { try { val welcomeBase64 = Base64.encode( source = welcomeBytes @@ -507,22 +656,14 @@ abstract class MarmotOutboundDao( ) ) + // The invite's own transcript line is not written here. It used to be, and + // that put nothing in the room for an invite into an established group -- + // where this runs a relay round trip later, if at all, and never at all if + // the ack does not come. It also hung off the two lookups below, so a + // missing key package or profile cost the line and not just the name. + // `inviteMember` writes it when the invite is made instead; what this + // function still owes the room is the failure below. database.marmotKeyPackageDao().getMarmotKeyPackageById(peerKeyPackageEventId)?.let { marmotKeyPackage -> - database.profileDao().getProfileByPublicKey(marmotKeyPackage.publicKey)?.let { profile -> - // Save chatMessage for the invite... - database.chatMessageDao().upsert( - ChatMessage( - content = "Invited ${profile.humanReadableNameOrPubkey() ?: "participant"} to chat", // use profile.humanReadable... - chatRoomId = nostrGroupId, - senderPublicKey = userPublicKey, - isUserMessage = true, // TODO: This is information message... - giftWrapPayloadId = welcomeEventId, - marmotGroupEventId = null, - marmotInnerEventId = null - ) - ) - } - // The group's signed work, offered to the member being invited. // Nothing else will ever show it to them: MLS gives a joiner no // history, and a group-signed event never travels -- every device @@ -550,8 +691,26 @@ abstract class MarmotOutboundDao( recipient = marmotKeyPackage.publicKey, ) } + + return true } catch (e: Throwable) { logger.e("Failed to deliver welcome:", e) + + // The room's only account of this. Its own runCatching because the + // failure being reported may well be the database, and a throw from + // here would be one the callers on the immediate path do not expect -- + // taking the invite's transaction down over a line about it. + runCatching { + announceInviteFailed( + chatRoomId = nostrGroupId, + userPublicKey = userPublicKey, + peerPublicKey = database.marmotKeyPackageDao() + .getMarmotKeyPackageById(peerKeyPackageEventId)?.publicKey, + reason = e.message, + ) + }.onFailure { logger.e("Failed to record the undelivered welcome:", it) } + + return false } } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt index e657249c..cab0c0d6 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt @@ -357,6 +357,49 @@ data class ChatMessage( TYPE_CHRONICLE_RECEIVED, ) + /** + * Adding a member to the group, as lines in the group's chat. + * + * An invite is two separate things, and in an established group they can be + * minutes apart: the membership change this device commits, and the Welcome + * that lets the invitee in going on the wire. The second waits on a relay + * acknowledging the commit -- see docs/marmot-membership.md -- and it is the + * one that can fail after the screen that asked for it has closed, which + * leaves the transcript as the only place able to say so. + * + * So [TYPE_MEMBER_INVITED] is written the moment the invite is made, and + * [TYPE_MEMBER_INVITE_SENT] only where the Welcome is delivered later than + * that. Where the two happen together -- a group that is still only its + * creator has nobody to inform, so its Welcome goes out immediately -- there + * is one line, because there was one event. + * + * Written by the inviter's device, for itself. None of these travels: what + * the group receives is the commit, and each member's transcript is written + * from what it made of that. A member watching from the side sees nothing + * here, correctly. + * + * Content is a whole sentence rather than a predicate, so these stay out of + * the AUTHORED sets and nothing prefixes a name to them. The invitee's name + * is written into the content the way the chronicle's is, which does not + * follow a rename and is the accepted cost for a line about a thing that + * happened once. + */ + const val TYPE_MEMBER_INVITED = "memberInvited" + const val TYPE_MEMBER_INVITE_SENT = "memberInviteSent" + const val TYPE_MEMBER_INVITE_FAILED = "memberInviteFailed" + + /** + * Every membership line, for the one check the transcript dispatches on. + * + * A type missing from here renders as a chat bubble -- silently, and looking + * exactly like the inviter having said "Invited Bob to the group". + */ + val MEMBERSHIP_TYPES = setOf( + TYPE_MEMBER_INVITED, + TYPE_MEMBER_INVITE_SENT, + TYPE_MEMBER_INVITE_FAILED, + ) + const val TYPE_UNDECRYPTABLE_OUTER_LAYER = "undecryptableOuterLayer" const val TYPE_PENDING_COMMIT = "pendingCommit" diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt index 80b04c00..51f0087d 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt @@ -168,11 +168,30 @@ class DatabaseChatRepository( peerPublicKey: HexKey, peerKeyPackage: MarmotKeyPackage ) { - database.marmotOutboundDao().inviteMemberToChatRoom( - localChatRoom = localChatRoom, - peerPublicKey = peerPublicKey, - peerKeyPackage = peerKeyPackage - ) + try { + database.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peerPublicKey, + peerKeyPackage = peerKeyPackage + ) + } catch (e: Throwable) { + // Outside the transaction that just went down, which is the point: the + // invite line `inviteMember` writes went with it, so an invite refused + // for want of MLS state or over a mismatched credential left the room + // with nothing at all to show. The caller still gets the throw and + // still puts its own message on the invite screen -- this is the copy + // that is still there tomorrow. + logger.e("Failed to invite $peerPublicKey to ${localChatRoom.chatRoom.id}", e) + runCatching { + database.marmotOutboundDao().announceInviteFailed( + chatRoomId = localChatRoom.chatRoom.id, + userPublicKey = localChatRoom.chatRoom.userPublicKey, + peerPublicKey = peerPublicKey, + reason = e.message, + ) + }.onFailure { logger.e("Failed to record the refused invite:", it) } + throw e + } } override suspend fun addMembers( localChatRoom: LocalChatRoom, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseNostrRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseNostrRepository.kt index 54ec9490..a827c7eb 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseNostrRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseNostrRepository.kt @@ -439,15 +439,45 @@ class DatabaseNostrRepository( database.chatRoomDao().findChatRoomById(marmotCommitResult.chatRoomId)?.let { localChatRoom -> // TODO: Update status of participant Invitation.PENDING -> Invitation.SENT logger.d("localChatRoom: $localChatRoom") - marmotCommitResult.welcomeBytes?.let { welcomeBytes -> - logger.d("welcomeBytes: ${welcomeBytes.toHex()}") - database.marmotOutboundDao().deliveryWelcome( - nostrGroupId = marmotCommitResult.chatRoomId, + val welcomeBytes = marmotCommitResult.welcomeBytes + if (welcomeBytes == null) { + // The commit was acknowledged and there is nothing to let + // the invitee in with, so this invite is over. Nobody is + // waiting on the answer by now -- the screen that asked + // closed when the commit was made -- which is why it goes + // in the room rather than to a caller. + database.marmotOutboundDao().announceInviteFailed( + chatRoomId = marmotCommitResult.chatRoomId, + userPublicKey = marmotCommitResult.userPublicKey, + peerPublicKey = database.marmotKeyPackageDao() + .getMarmotKeyPackageById(marmotCommitResult.peerKeyPackageEventId) + ?.publicKey, + reason = "the commit carried no invitation", + ) + return@let + } + + logger.d("welcomeBytes: ${welcomeBytes.toHex()}") + val delivered = database.marmotOutboundDao().deliveryWelcome( + nostrGroupId = marmotCommitResult.chatRoomId, + userPublicKey = marmotCommitResult.userPublicKey, + welcomeBytes = welcomeBytes, + peerKeyPackageEventId = marmotCommitResult.peerKeyPackageEventId, + createdAt = marmotCommitResult.createdAt, + relays = Relays.DefaultDMRelayList.map { it.url } // TODO: Get localChatRoom relays... + ) + + // This is the deferred delivery site, and the only one where + // the Welcome goes out later than the invite that made it -- + // so it is the only one that owes the room a second line. An + // invite whose room was still just its creator sent its + // Welcome in the same breath and said so once already. A + // failure needs nothing here: `deliveryWelcome` files its own. + if (delivered) { + database.marmotOutboundDao().announceInviteSent( + chatRoomId = marmotCommitResult.chatRoomId, userPublicKey = marmotCommitResult.userPublicKey, - welcomeBytes = welcomeBytes, peerKeyPackageEventId = marmotCommitResult.peerKeyPackageEventId, - createdAt = marmotCommitResult.createdAt, - relays = Relays.DefaultDMRelayList.map { it.url } // TODO: Get localChatRoom relays... ) } } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt index 66067b1b..4a72aabf 100755 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt @@ -424,6 +424,24 @@ class ChatMessageListViewModel( return@items } + // Inviting somebody is nobody's words either, and + // for a while it was not in the room at all: the + // line was written when the Welcome went out, which + // on the deferred path is a relay round trip away + // and may never happen. Passed as answered and + // settled for the same reason the chronicle's are + // -- these report rather than ask, so they stay in + // the quiet tint and offer nothing to review. + if (localChatMessage.chatMessage.messageType in ChatMessage.MEMBERSHIP_TYPES) { + RitualNotice( + localChatMessage = localChatMessage, + isAnswered = true, + isSettled = true, + onClick = {} + ) + return@items + } + // Signing lines are the same kind of thing and get // the same treatment -- nobody said them either -- // but they lead somewhere else, because what a @@ -745,6 +763,13 @@ private fun RitualNotice( ChatMessage.TYPE_CHRONICLE_SENT -> Icons.Default.Upload ChatMessage.TYPE_CHRONICLE_RECEIVED -> Icons.Default.Download + // An invite made and an invite sent are two separate steps on the deferred + // path, so they get separate icons -- the whole reason both lines exist is + // to be able to see that the first happened and the second did not. + ChatMessage.TYPE_MEMBER_INVITED -> Icons.Default.PersonAdd + ChatMessage.TYPE_MEMBER_INVITE_SENT -> Icons.Default.Upload + ChatMessage.TYPE_MEMBER_INVITE_FAILED -> Icons.Default.ErrorOutline + else -> Icons.Default.PanTool } @@ -763,7 +788,8 @@ private fun RitualNotice( val tint = when { chatMessage.messageType == ChatMessage.TYPE_DKG_FAILED || - chatMessage.messageType == ChatMessage.TYPE_FROST_FAILED -> + chatMessage.messageType == ChatMessage.TYPE_FROST_FAILED || + chatMessage.messageType == ChatMessage.TYPE_MEMBER_INVITE_FAILED -> MaterialTheme.colorScheme.error isRequest -> MaterialTheme.colorScheme.primary else -> MaterialTheme.colorScheme.onSurfaceVariant diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt index 37693973..f8698d79 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/dao/MarmotOutboundDaoJvmTest.kt @@ -6,6 +6,7 @@ import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import kotlinx.coroutines.runBlocking import press.mantra.compose.database.MantraDatabase import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatMessage import press.mantra.compose.database.model.ChatRoom import press.mantra.compose.database.model.MarmotKeyPackage import press.mantra.compose.database.model.NostrEvent @@ -51,25 +52,36 @@ class MarmotOutboundDaoJvmTest { private val user = KeyPair().pubKey.toHexKey() private val peer = KeyPair().pubKey.toHexKey() + private val secondPeer = KeyPair().pubKey.toHexKey() private val roomId = "b".repeat(64) /** - * A room with `mlsGroupState = null` -- exactly the shape a room restored from an inbound - * gift wrap has, which is the case the guard exists for. + * A member the database knows by name. Every invitee needs one -- + * Participant.participantPublicKey is a foreign key onto Profile -- and the membership + * lines read the name back out of it. */ - private suspend fun seedStatelessRoom(): LocalChatRoom { - val nostrEventId = "c".repeat(64) + private suspend fun seedProfile(publicKey: String, name: String, nostrEventId: String) { db.nostrEventDao().upsert( NostrEvent( id = nostrEventId, - pubKey = user, + pubKey = publicKey, kind = 0, tags = emptyArray(), content = "{}", sig = "0".repeat(128), ) ) - db.profileDao().upsert(Profile(publicKey = user, userName = "user", nostrEventId = nostrEventId)) + db.profileDao().upsert( + Profile(publicKey = publicKey, userName = name, nostrEventId = nostrEventId) + ) + } + + /** + * A room with `mlsGroupState = null` -- exactly the shape a room restored from an inbound + * gift wrap has, which is the case the guard exists for. + */ + private suspend fun seedStatelessRoom(): LocalChatRoom { + seedProfile(user, "user", "c".repeat(64)) val chatRoom = ChatRoom( id = roomId, userPublicKey = user, @@ -100,18 +112,7 @@ class MarmotOutboundDaoJvmTest { */ private suspend fun seedMlsRoom(): LocalChatRoom { val stateless = seedStatelessRoom() - val peerEventId = "e".repeat(64) - db.nostrEventDao().upsert( - NostrEvent( - id = peerEventId, - pubKey = peer, - kind = 0, - tags = emptyArray(), - content = "{}", - sig = "0".repeat(128), - ) - ) - db.profileDao().upsert(Profile(publicKey = peer, userName = "peer", nostrEventId = peerEventId)) + seedProfile(peer, "peer", "e".repeat(64)) val mlsGroup = MlsGroup.create( identity = user.hexToByteArray(), initialExtensions = listOf( @@ -265,4 +266,110 @@ class MarmotOutboundDaoJvmTest { assertEquals(1, retained.size, "the pre-commit epoch was not retained") assertEquals(0L, retained.single().epoch, "a freshly created group is at epoch 0") } + + // ---- What the room is told about it ----------------------------------- + + private suspend fun transcript() = db.chatMessageDao().getChatMessagesByChatRoomId(roomId) + .map { it.chatMessage } + + /** + * The invite has to reach the transcript, and as a membership line rather than a chat + * bubble. It used to be written by `deliveryWelcome` -- which meant it depended on the + * invitee's key package and profile both being found, and neither is here, so the room + * was told nothing at all about an invite that succeeded. + */ + @Test + fun `inviting a member puts a line in the room`() = runBlocking { + val localChatRoom = seedMlsRoom() + + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = marmotKeyPackageFor(peer), + ) + + val line = transcript().singleOrNull { it.messageType == ChatMessage.TYPE_MEMBER_INVITED } + assertNotNull(line, "the invite left no line in the room") + assertTrue(line.content.contains("peer"), "the line does not name the invitee: ${line.content}") + } + + /** + * The case the line exists for. A group that already has members defers its Welcome until + * a relay acknowledges the commit -- see docs/marmot-membership.md -- so an invite made + * here is on nothing but a promise, and until the line moved to invite time the room said + * nothing whatsoever in the meantime and nothing ever if the ack never came. + */ + @Test + fun `an invite into an established group is in the room before its welcome goes out`() = runBlocking { + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = seedMlsRoom(), + peerPublicKey = peer, + peerKeyPackage = marmotKeyPackageFor(peer), + ) + seedProfile(secondPeer, "second", "f".repeat(64)) + + // Re-read: the first invite advanced the epoch and persisted new state. + val established = assertNotNull(db.chatRoomDao().findChatRoomById(roomId)) + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = established, + peerPublicKey = secondPeer, + peerKeyPackage = marmotKeyPackageFor(secondPeer), + ) + + val invited = transcript().filter { it.messageType == ChatMessage.TYPE_MEMBER_INVITED } + assertEquals(2, invited.size, "the deferred invite left no line in the room") + assertTrue( + invited.any { it.content.contains("second") }, + "the second invitee is not named: ${invited.map { it.content }}", + ) + + // Nothing has acknowledged the commit, so the Welcome has not gone out and nothing + // may claim it has. That absence is what makes a stuck invite visible. + assertTrue( + transcript().none { it.messageType == ChatMessage.TYPE_MEMBER_INVITE_SENT }, + "a welcome was reported sent before any relay acknowledged the commit", + ) + } + + /** + * Membership lines are not somebody's words, and the transcript decides that from the + * type alone. One missing from [ChatMessage.MEMBERSHIP_TYPES] renders as a chat bubble, + * looking exactly like the inviter having said "Invited peer to the group". + */ + @Test + fun `the invite line is a membership line and not a bubble`() = runBlocking { + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = seedMlsRoom(), + peerPublicKey = peer, + peerKeyPackage = marmotKeyPackageFor(peer), + ) + + assertTrue( + transcript().all { it.messageType in ChatMessage.MEMBERSHIP_TYPES }, + "an invite wrote something the transcript would render as a bubble: " + + transcript().map { it.messageType }, + ) + } + + /** + * A refused invite must not leave the room claiming one was made. The line is written + * inside the caller's transaction precisely so that it goes down with everything else the + * refused invite touched -- what tells the user instead is + * `DatabaseChatRepository.inviteMember`, from outside it. + */ + @Test + fun `a refused invite leaves no claim that one was made`() = runBlocking { + runCatching { + db.marmotOutboundDao().inviteMemberToChatRoom( + localChatRoom = seedStatelessRoom(), + peerPublicKey = peer, + peerKeyPackage = keyPackage(), + ) + } + + assertTrue( + transcript().none { it.messageType == ChatMessage.TYPE_MEMBER_INVITED }, + "the room was told about an invite that was refused", + ) + } } diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/repository/DatabaseChatRepositoryJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/repository/DatabaseChatRepositoryJvmTest.kt new file mode 100644 index 00000000..4bfc224c --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/database/repository/DatabaseChatRepositoryJvmTest.kt @@ -0,0 +1,126 @@ +package press.mantra.compose.database.repository + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatMessage +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.MarmotKeyPackage +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.exceptions.MarmotMissingChatGroupException +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * What the room is told when an invite does not happen. + * + * `MarmotOutboundDao.inviteMemberToChatRoom` writes the invite's transcript line inside its + * own transaction, which is right: an invite that does not survive `addMember` must not leave + * the room claiming one was made. But it means a refusal takes the only account of itself + * down with it, and the screen that asked has closed by the time anybody looks -- so the + * repository is where a failure has to be written from, outside the transaction that rolled + * back. + */ +class DatabaseChatRepositoryJvmTest { + + private val db: MantraDatabase = getRoomDatabase( + Room.inMemoryDatabaseBuilder() + ) + + @AfterTest + fun closeDb() = db.close() + + private val user = KeyPair().pubKey.toHexKey() + private val peer = KeyPair().pubKey.toHexKey() + private val roomId = "b".repeat(64) + + private val repository = DatabaseChatRepository( + database = db, + scope = CoroutineScope(Dispatchers.Unconfined), + ) + + private suspend fun seedProfile(publicKey: String, name: String, nostrEventId: String) { + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = publicKey, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert( + Profile(publicKey = publicKey, userName = name, nostrEventId = nostrEventId) + ) + } + + /** A room restored from an inbound gift wrap: no MLS state, so nothing to invite into. */ + private suspend fun seedStatelessRoom(): LocalChatRoom { + seedProfile(user, "user", "c".repeat(64)) + seedProfile(peer, "peer", "e".repeat(64)) + val chatRoom = ChatRoom( + id = roomId, + userPublicKey = user, + subject = "a restored room", + description = null, + mlsGroupState = null, + ) + db.chatRoomDao().upsert(chatRoom) + return LocalChatRoom(chatRoom = chatRoom) + } + + private fun keyPackage() = MarmotKeyPackage( + id = "d".repeat(64), + publicKey = peer, + tlsEncodedMarmotKeyPackage = ByteArray(0), + ) + + @Test + fun `a refused invite is written into the room it was refused for`() = runBlocking { + val localChatRoom = seedStatelessRoom() + + assertFailsWith { + repository.inviteMember( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = keyPackage(), + ) + } + + val line = db.chatMessageDao().getChatMessagesByChatRoomId(roomId) + .map { it.chatMessage } + .singleOrNull { it.messageType == ChatMessage.TYPE_MEMBER_INVITE_FAILED } + + assertNotNull(line, "the refused invite left no line in the room") + assertTrue(line.content.contains("peer"), "the line does not name the invitee: ${line.content}") + } + + /** + * The caller still has to hear about it. The transcript line is the copy that is there + * tomorrow; the throw is what puts a message on the invite screen today, and swallowing it + * would pop the user back to the chat as though the invite had gone out. + */ + @Test + fun `a refused invite still reaches the caller`() = runBlocking { + val localChatRoom = seedStatelessRoom() + + assertFailsWith { + repository.inviteMember( + localChatRoom = localChatRoom, + peerPublicKey = peer, + peerKeyPackage = keyPackage(), + ) + } + } +} diff --git a/docs/marmot-membership.md b/docs/marmot-membership.md index c6feeb21..46a783b3 100644 --- a/docs/marmot-membership.md +++ b/docs/marmot-membership.md @@ -129,6 +129,37 @@ The group state is persisted after `commit()` and before any Welcome is delivere so a crash between the two leaves the group at the epoch the Welcomes describe rather than one behind it. +## What the room is told + +An invite writes three kinds of line into the room's transcript, none of which +travels — see `ChatMessage.MEMBERSHIP_TYPES`. They exist because the deferral +above is invisible from anywhere else: the screen that asks for an invite closes +the moment the commit is made, and everything that can still go wrong goes wrong +after that. + +| line | written by | says | +|-----------------------------|-----------------------------------------|---------------------------------------| +| `TYPE_MEMBER_INVITED` | `inviteMember`, `addMembersToChatRoom` | the membership change was made | +| `TYPE_MEMBER_INVITE_SENT` | `DatabaseNostrRepository` | the deferred Welcome went on the wire | +| `TYPE_MEMBER_INVITE_FAILED` | `deliveryWelcome`, `DatabaseChatRepository` | it did not, and why | + +The invite line is written **when the invite is made**, inside the caller's +transaction. Both halves of that matter: written any later and an invite waiting +on an ack that never comes leaves the room showing nothing, which is what this +looked like before; written outside the transaction and an invite that does not +survive `addMember` leaves the room claiming one was made. + +Only the immediate branch's Welcome goes out in the same breath as the invite, so +only the deferred branch owes a second line. Where the two are one event, one line +is the whole truth — and `createdAt` is stored to the second, so a second line +would be one the transcript could not reliably order after the first anyway. + +A refusal is the awkward case, because rolling the transaction back is right and +takes the account of it with it. `DatabaseChatRepository.inviteMember` catches, +writes the failed line from outside the transaction, and rethrows — the throw is +what puts a message on the invite screen now, the line is what is still there +tomorrow. + ## Other things that bite **Sequential invites each advance the epoch.** Where they still happen — the @@ -140,9 +171,12 @@ error. **A member with no published key package cannot be added.** A Marmot invite needs the invitee's `MarmotKeyPackage`. Both call sites look it up with a timeout and -collect the ones that failed. Today that only reaches the log — the -`TODO: Update status of participant Invitation.PENDING -> Invitation.SENT` at the -Welcome delivery site is the same gap seen from the other end. +collect the ones that failed. That is still only a log and a returned list — +nothing puts it in the room, because it happens before there is an invite to write +a line about. The `TODO: Update status of participant Invitation.PENDING -> +Invitation.SENT` at the Welcome delivery site is the same gap seen from the other +end: the transcript lines above say what happened, but a Participant row still +carries no state saying where its invite got to. **`deliveryWelcome` uses `Relays.DefaultDMRelayList`, not the room's relays.** There is a `TODO: Get localChatRoom relays...` on the ack-triggered call site.