From 65c56d0035a6831c3eaf991d22b2b42fbcf9eedb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 19:28:25 +0000 Subject: [PATCH 1/3] feat: inline member management on Marmot group info screen - Fold the Remove Member screen into the info screen: each member row now has an inline remove icon that opens a confirmation dialog. - Fold the Add Member screen into the info screen: a search field with a user-suggestion list lives directly above the member list, so adding a member never requires leaving the screen. - Move Leave Group out of the scrolling body into a top-bar action so it is reachable regardless of member count. - Shrink the relay strip (35dp tiles with a small activity dot) and tuck it next to the group header; drop the "Relays" label and the MLS epoch readout, which were visual clutter. - Route the chat screen's Add Member button to the info screen and delete the two standalone screens and their routes. - Add headless interop test 17: A creates a group, adds B, removes B, re-adds B, and verifies B can both receive and send messages. Guards the reported regression where a re-added member came back without a usable leaf and silently lost the ability to post. https://claude.ai/code/session_01JaeqZwVNLKvUUvXWRzPmRP --- .../amethyst/ui/navigation/AppNavigation.kt | 4 - .../amethyst/ui/navigation/routes/Routes.kt | 8 - .../chats/marmotGroup/AddMemberScreen.kt | 306 -------------- .../marmotGroup/MarmotGroupChatScreen.kt | 2 +- .../marmotGroup/MarmotGroupInfoScreen.kt | 385 ++++++++++++------ .../chats/marmotGroup/RemoveMemberScreen.kt | 250 ------------ tools/marmot-interop/headless/tests-manage.sh | 89 ++++ .../marmot-interop/marmot-interop-headless.sh | 1 + 8 files changed, 359 insertions(+), 686 deletions(-) delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/AddMemberScreen.kt delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/RemoveMemberScreen.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt index f1ec9ca6e..a1e79eb3f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt @@ -75,13 +75,11 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.bookmarkgroups.list.metadat import com.vitorpamplona.amethyst.ui.screen.loggedIn.bookmarkgroups.membershipManagement.ArticleBookmarkListManagementScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.bookmarkgroups.membershipManagement.PostBookmarkListManagementScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.bookmarkgroups.old.OldBookmarkListScreen -import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.AddMemberScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.CreateGroupScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.EditGroupInfoScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.MarmotGroupChatScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.MarmotGroupInfoScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.MarmotGroupListScreen -import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.RemoveMemberScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.ChatroomByAuthorScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.ChatroomScreen import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.send.NewGroupDMScreen @@ -345,8 +343,6 @@ fun BuildNavigation( composableFromEndArgs { MarmotGroupInfoScreen(it.nostrGroupId, accountViewModel, nav) } composableFromBottom { CreateGroupScreen(accountViewModel, nav) } - composableFromBottomArgs { AddMemberScreen(it.nostrGroupId, accountViewModel, nav) } - composableFromBottomArgs { RemoveMemberScreen(it.nostrGroupId, accountViewModel, nav) } composableFromBottomArgs { EditGroupInfoScreen(it.nostrGroupId, accountViewModel, nav) } composableFromEndArgs { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt index 7844f28fd..f94f5ec5b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt @@ -370,14 +370,6 @@ sealed class Route { @Serializable object CreateMarmotGroup : Route() - @Serializable data class MarmotGroupAddMember( - val nostrGroupId: String, - ) : Route() - - @Serializable data class MarmotGroupRemoveMember( - val nostrGroupId: String, - ) : Route() - @Serializable data class MarmotGroupEditInfo( val nostrGroupId: String, ) : Route() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/AddMemberScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/AddMemberScreen.kt deleted file mode 100644 index 4c163e8b7..000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/AddMemberScreen.kt +++ /dev/null @@ -1,306 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup - -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.consumeWindowInsets -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.imePadding -import androidx.compose.foundation.layout.padding -import androidx.compose.material3.HorizontalDivider -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.OutlinedTextField -import androidx.compose.material3.Scaffold -import androidx.compose.material3.Text -import androidx.compose.material3.TextButton -import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateListOf -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.runtime.setValue -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.style.TextOverflow -import androidx.compose.ui.unit.dp -import androidx.lifecycle.compose.collectAsStateWithLifecycle -import com.vitorpamplona.amethyst.commons.model.nip05DnsIdentifiers.Nip05State -import com.vitorpamplona.amethyst.model.User -import com.vitorpamplona.amethyst.ui.navigation.navs.INav -import com.vitorpamplona.amethyst.ui.navigation.topbars.ActionTopBar -import com.vitorpamplona.amethyst.ui.note.UserPicture -import com.vitorpamplona.amethyst.ui.note.creators.userSuggestions.ShowUserSuggestionList -import com.vitorpamplona.amethyst.ui.note.creators.userSuggestions.UserSuggestionState -import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel -import com.vitorpamplona.amethyst.ui.theme.SuggestionListDefaultHeightPage -import com.vitorpamplona.quartz.nip01Core.core.HexKey -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch - -@Composable -fun AddMemberScreen( - nostrGroupId: HexKey, - accountViewModel: AccountViewModel, - nav: INav, -) { - var searchInput by remember { mutableStateOf("") } - val selectedUsers = remember { mutableStateListOf() } - var statusMessage by remember { mutableStateOf(null) } - var isError by remember { mutableStateOf(false) } - var isAdding by remember { mutableStateOf(false) } - val scope = rememberCoroutineScope() - - val userSuggestions = - remember { - UserSuggestionState(accountViewModel.account, accountViewModel.nip05ClientBuilder()) - } - - DisposableEffect(Unit) { - onDispose { userSuggestions.reset() } - } - - Scaffold( - topBar = { - ActionTopBar( - postRes = com.vitorpamplona.amethyst.R.string.add, - onCancel = { nav.popBack() }, - onPost = { - if (selectedUsers.isEmpty()) { - statusMessage = "No users selected" - isError = true - return@ActionTopBar - } - isAdding = true - isError = false - val usersToAdd = selectedUsers.toList() - scope.launch(Dispatchers.IO) { - var successCount = 0 - val failures = mutableListOf>() - for ((index, user) in usersToAdd.withIndex()) { - statusMessage = - "Adding ${user.toBestDisplayName()} (${index + 1}/${usersToAdd.size})..." - isError = false - try { - val result = - accountViewModel.addMarmotGroupMember( - nostrGroupId, - user.pubkeyHex, - ) - if (result.startsWith("Success")) { - successCount++ - } else { - failures.add(user to result.removePrefix("Error: ")) - } - } catch (e: Exception) { - failures.add(user to (e.message ?: "unknown error")) - } - } - - if (failures.isEmpty()) { - nav.popBack() - } else { - statusMessage = - buildString { - if (successCount > 0) { - append("Added $successCount. ") - } - append("Failed: ") - append( - failures.joinToString("; ") { (user, reason) -> - "${user.toBestDisplayName()} ($reason)" - }, - ) - } - isError = true - // Keep failed users in the list for retry, drop successful ones - val failedUsers = failures.map { it.first }.toSet() - selectedUsers.clear() - selectedUsers.addAll(failedUsers) - isAdding = false - } - } - }, - isActive = { !isAdding && selectedUsers.isNotEmpty() }, - ) - }, - ) { padding -> - Column( - modifier = - Modifier - .padding(padding) - .consumeWindowInsets(padding) - .imePadding(), - ) { - // Selected users list - if (selectedUsers.isNotEmpty()) { - selectedUsers.toList().forEachIndexed { index, user -> - SelectedUserRow( - user = user, - accountViewModel = accountViewModel, - nav = nav, - enabled = !isAdding, - onClear = { - selectedUsers.remove(user) - statusMessage = null - isError = false - }, - ) - if (index < selectedUsers.lastIndex) { - HorizontalDivider() - } - } - HorizontalDivider() - } - - // Search field - OutlinedTextField( - value = searchInput, - onValueChange = { newValue -> - searchInput = newValue - if (!isError) statusMessage = null - if (newValue.length > 2) { - userSuggestions.processCurrentWord(newValue) - } else { - userSuggestions.reset() - } - }, - label = { Text("Search users") }, - placeholder = { Text("Name, npub, or NIP-05") }, - modifier = - Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp), - singleLine = true, - enabled = !isAdding, - ) - - if (statusMessage != null) { - Text( - text = statusMessage!!, - modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp), - style = MaterialTheme.typography.bodySmall, - color = - if (isError) { - MaterialTheme.colorScheme.error - } else { - MaterialTheme.colorScheme.primary - }, - ) - } - - Spacer(modifier = Modifier.height(4.dp)) - - // User suggestion list - if (!isAdding && searchInput.length > 2) { - ShowUserSuggestionList( - userSuggestions = userSuggestions, - onSelect = { user -> - if (selectedUsers.none { it.pubkeyHex == user.pubkeyHex }) { - selectedUsers.add(user) - } - searchInput = "" - userSuggestions.reset() - }, - accountViewModel = accountViewModel, - modifier = SuggestionListDefaultHeightPage, - onEmpty = { - Text( - "They must have published a KeyPackage (kind:30443) to be added.", - modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - }, - ) - } - } - } -} - -@Composable -private fun SelectedUserRow( - user: User, - accountViewModel: AccountViewModel, - nav: INav, - enabled: Boolean, - onClear: () -> Unit, -) { - Row( - modifier = - Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp, vertical = 8.dp), - verticalAlignment = Alignment.CenterVertically, - ) { - UserPicture( - userHex = user.pubkeyHex, - size = 40.dp, - accountViewModel = accountViewModel, - nav = nav, - ) - Column( - modifier = Modifier.weight(1f).padding(start = 12.dp), - ) { - Text( - text = user.toBestDisplayName(), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.Bold, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) - UserSecondaryLine(user) - } - TextButton(onClick = onClear, enabled = enabled) { - Text("Remove") - } - } -} - -@Composable -private fun UserSecondaryLine(user: User) { - val nip05StateMetadata by user.nip05State().flow.collectAsStateWithLifecycle() - - val text = - when (val state = nip05StateMetadata) { - is Nip05State.Exists -> { - val name = state.nip05.name - if (name == "_") state.nip05.domain else "$name@${state.nip05.domain}" - } - - else -> { - user.pubkeyDisplayHex() - } - } - - Text( - text = text, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt index 2dc1d6932..334cd7a35 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt @@ -86,7 +86,7 @@ fun MarmotGroupChatScreen( } }, actions = { - IconButton(onClick = { nav.nav(Route.MarmotGroupAddMember(nostrGroupId)) }) { + IconButton(onClick = { nav.nav(Route.MarmotGroupInfo(nostrGroupId)) }) { Icon( imageVector = Icons.Default.GroupAdd, contentDescription = "Add Member", diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt index ea2fe7886..914eabb66 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt @@ -32,8 +32,10 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn @@ -43,7 +45,6 @@ import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.automirrored.filled.ExitToApp import androidx.compose.material.icons.filled.Edit -import androidx.compose.material.icons.filled.GroupAdd import androidx.compose.material.icons.filled.PersonRemove import androidx.compose.material3.AlertDialog import androidx.compose.material3.ExperimentalMaterial3Api @@ -51,11 +52,13 @@ import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -77,9 +80,12 @@ import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.note.RenderRelayIcon import com.vitorpamplona.amethyst.ui.note.UserPicture +import com.vitorpamplona.amethyst.ui.note.creators.userSuggestions.ShowUserSuggestionList +import com.vitorpamplona.amethyst.ui.note.creators.userSuggestions.UserSuggestionState import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.rooms.LoadUser -import com.vitorpamplona.amethyst.ui.theme.LargeRelayIconModifier +import com.vitorpamplona.amethyst.ui.theme.MediumRelayIconModifier +import com.vitorpamplona.amethyst.ui.theme.SuggestionListDefaultHeightChat import com.vitorpamplona.amethyst.ui.theme.allGoodColor import com.vitorpamplona.amethyst.ui.theme.placeholderText import com.vitorpamplona.amethyst.ui.theme.ripple24dp @@ -108,10 +114,24 @@ fun MarmotGroupInfoScreen( val members by chatroom.members.collectAsStateWithLifecycle() var showLeaveDialog by remember { mutableStateOf(false) } var isLeaving by remember { mutableStateOf(false) } + var memberToRemove by remember { mutableStateOf(null) } + var addSearchInput by remember { mutableStateOf("") } + var addStatus by remember { mutableStateOf(null) } + var isAddError by remember { mutableStateOf(false) } + var isAdding by remember { mutableStateOf(false) } val scope = rememberCoroutineScope() val myPubkey = accountViewModel.account.signer.pubKey val context = LocalContext.current + val userSuggestions = + remember { + UserSuggestionState(accountViewModel.account, accountViewModel.nip05ClientBuilder()) + } + + DisposableEffect(Unit) { + onDispose { userSuggestions.reset() } + } + Scaffold( topBar = { TopAppBar( @@ -131,10 +151,14 @@ fun MarmotGroupInfoScreen( contentDescription = "Edit Group Info", ) } - IconButton(onClick = { nav.nav(Route.MarmotGroupAddMember(nostrGroupId)) }) { + IconButton( + onClick = { showLeaveDialog = true }, + enabled = !isLeaving, + ) { Icon( - imageVector = Icons.Default.GroupAdd, - contentDescription = "Add Member", + imageVector = Icons.AutoMirrored.Filled.ExitToApp, + contentDescription = "Leave Group", + tint = MaterialTheme.colorScheme.error, ) } }, @@ -155,46 +179,90 @@ fun MarmotGroupInfoScreen( .fillMaxWidth() .padding(16.dp), ) { - Text( - text = displayName ?: "Group ${nostrGroupId.take(8)}...", - style = MaterialTheme.typography.headlineSmall, - fontWeight = FontWeight.Bold, - ) - if (!groupDescription.isNullOrEmpty()) { - Text( - text = groupDescription!!, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(top = 4.dp), - ) - } - Text( - text = "${members.size} members", - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(top = 4.dp), - ) - if (groupRelays.isNotEmpty()) { - GroupRelayList( - relayUrls = groupRelays, - relayActivity = relayActivity, - accountViewModel = accountViewModel, - nav = nav, - ) - } - val epoch = accountViewModel.account.marmotManager?.groupEpoch(nostrGroupId) - if (epoch != null) { - Text( - text = "Epoch: $epoch", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(top = 2.dp), - ) + Row(verticalAlignment = Alignment.CenterVertically) { + Column(modifier = Modifier.weight(1f)) { + Text( + text = displayName ?: "Group ${nostrGroupId.take(8)}...", + style = MaterialTheme.typography.headlineSmall, + fontWeight = FontWeight.Bold, + ) + if (!groupDescription.isNullOrEmpty()) { + Text( + text = groupDescription!!, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 4.dp), + ) + } + Text( + text = "${members.size} members", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 4.dp), + ) + } + if (groupRelays.isNotEmpty()) { + GroupRelayStrip( + relayUrls = groupRelays, + relayActivity = relayActivity, + accountViewModel = accountViewModel, + nav = nav, + ) + } } } HorizontalDivider() } + // Inline add-member search + item { + AddMemberInline( + nostrGroupId = nostrGroupId, + searchInput = addSearchInput, + onSearchInputChange = { value -> + addSearchInput = value + if (!isAddError) addStatus = null + if (value.length > 2) { + userSuggestions.processCurrentWord(value) + } else { + userSuggestions.reset() + } + }, + userSuggestions = userSuggestions, + statusMessage = addStatus, + isError = isAddError, + isAdding = isAdding, + accountViewModel = accountViewModel, + onAdd = { user -> + isAdding = true + isAddError = false + addStatus = "Adding ${user.toBestDisplayName()}..." + val targetPubkey = user.pubkeyHex + val targetName = user.toBestDisplayName() + scope.launch(Dispatchers.IO) { + try { + val result = accountViewModel.addMarmotGroupMember(nostrGroupId, targetPubkey) + if (result.startsWith("Success")) { + addStatus = null + isAddError = false + addSearchInput = "" + userSuggestions.reset() + } else { + addStatus = "Failed to add $targetName: ${result.removePrefix("Error: ")}" + isAddError = true + } + } catch (e: Exception) { + addStatus = "Failed to add $targetName: ${e.message ?: "unknown error"}" + isAddError = true + } finally { + isAdding = false + } + } + }, + ) + HorizontalDivider() + } + // Members section header item { Text( @@ -205,61 +273,21 @@ fun MarmotGroupInfoScreen( ) } - // Member list + // Member list with inline remove button items(members, key = { it.leafIndex }) { member -> + val isMe = member.pubkey == myPubkey + val isAdmin = member.pubkey in adminPubkeys MemberRow( member = member, - isMe = member.pubkey == myPubkey, - isAdmin = member.pubkey in adminPubkeys, + isMe = isMe, + isAdmin = isAdmin, + canRemove = !isMe, accountViewModel = accountViewModel, nav = nav, + onRemoveClick = { memberToRemove = member }, ) HorizontalDivider() } - - // Group actions section - item { - HorizontalDivider(modifier = Modifier.padding(top = 8.dp)) - Row( - modifier = - Modifier - .fillMaxWidth() - .clickable { nav.nav(Route.MarmotGroupRemoveMember(nostrGroupId)) } - .padding(horizontal = 16.dp, vertical = 16.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(12.dp), - ) { - Icon( - imageVector = Icons.Default.PersonRemove, - contentDescription = "Remove Member", - ) - Text( - text = "Remove Member", - style = MaterialTheme.typography.bodyLarge, - ) - } - HorizontalDivider() - Row( - modifier = - Modifier - .fillMaxWidth() - .clickable { showLeaveDialog = true } - .padding(horizontal = 16.dp, vertical = 16.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(12.dp), - ) { - Icon( - imageVector = Icons.AutoMirrored.Filled.ExitToApp, - contentDescription = "Leave Group", - tint = MaterialTheme.colorScheme.error, - ) - Text( - text = "Leave Group", - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.error, - ) - } - } } } @@ -290,6 +318,96 @@ fun MarmotGroupInfoScreen( onDismiss = { showLeaveDialog = false }, ) } + + memberToRemove?.let { member -> + ConfirmRemoveMemberDialog( + memberPubkey = member.pubkey, + accountViewModel = accountViewModel, + onConfirm = { + memberToRemove = null + scope.launch(Dispatchers.IO) { + try { + accountViewModel.removeMarmotGroupMember(nostrGroupId, member.leafIndex) + launch(Dispatchers.Main) { + Toast + .makeText(context, "Member removed", Toast.LENGTH_SHORT) + .show() + } + } catch (e: Exception) { + launch(Dispatchers.Main) { + Toast + .makeText( + context, + "Failed to remove member: ${e.message}", + Toast.LENGTH_LONG, + ).show() + } + } + } + }, + onDismiss = { memberToRemove = null }, + ) + } +} + +@Composable +private fun AddMemberInline( + nostrGroupId: HexKey, + searchInput: String, + onSearchInputChange: (String) -> Unit, + userSuggestions: UserSuggestionState, + statusMessage: String?, + isError: Boolean, + isAdding: Boolean, + accountViewModel: AccountViewModel, + onAdd: (com.vitorpamplona.amethyst.model.User) -> Unit, +) { + Column(modifier = Modifier.fillMaxWidth()) { + OutlinedTextField( + value = searchInput, + onValueChange = onSearchInputChange, + label = { Text("Add member") }, + placeholder = { Text("Name, npub, or NIP-05") }, + modifier = + Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 8.dp), + singleLine = true, + enabled = !isAdding, + ) + + if (statusMessage != null) { + Text( + text = statusMessage, + modifier = Modifier.padding(horizontal = 16.dp, vertical = 4.dp), + style = MaterialTheme.typography.bodySmall, + color = + if (isError) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.primary + }, + ) + } + + if (!isAdding && searchInput.length > 2) { + Spacer(modifier = Modifier.height(4.dp)) + ShowUserSuggestionList( + userSuggestions = userSuggestions, + onSelect = { user -> onAdd(user) }, + accountViewModel = accountViewModel, + modifier = SuggestionListDefaultHeightChat, + onEmpty = { + Text( + "They must have published a KeyPackage (kind:30443) to be added.", + modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + }, + ) + } + } } @Composable @@ -297,15 +415,17 @@ fun MemberRow( member: GroupMemberInfo, isMe: Boolean, isAdmin: Boolean, + canRemove: Boolean, accountViewModel: AccountViewModel, nav: INav, + onRemoveClick: () -> Unit, ) { Row( modifier = Modifier .fillMaxWidth() .clickable { nav.nav(Route.Profile(member.pubkey)) } - .padding(horizontal = 16.dp, vertical = 10.dp), + .padding(start = 16.dp, end = 8.dp, top = 10.dp, bottom = 10.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp), ) { @@ -332,6 +452,15 @@ fun MemberRow( ) } } + if (canRemove) { + IconButton(onClick = onRemoveClick) { + Icon( + imageVector = Icons.Default.PersonRemove, + contentDescription = "Remove Member", + tint = MaterialTheme.colorScheme.error, + ) + } + } } } @@ -360,12 +489,41 @@ fun LeaveGroupDialog( ) } +@Composable +private fun ConfirmRemoveMemberDialog( + memberPubkey: HexKey, + accountViewModel: AccountViewModel, + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Remove Member") }, + text = { + LoadUser(baseUserHex = memberPubkey, accountViewModel = accountViewModel) { user -> + val name = user?.toBestDisplayName() ?: "${memberPubkey.take(16)}..." + Text("Are you sure you want to remove \"$name\" from this group?") + } + }, + confirmButton = { + TextButton(onClick = onConfirm) { + Text("Remove", color = MaterialTheme.colorScheme.error) + } + }, + dismissButton = { + TextButton(onClick = onDismiss) { + Text("Cancel") + } + }, + ) +} + /** Window (in seconds) within which a relay is considered actively carrying this group's traffic. */ private const val RELAY_ACTIVITY_WINDOW_SECS = 7L * 24 * 60 * 60 @OptIn(ExperimentalLayoutApi::class) @Composable -fun GroupRelayList( +fun GroupRelayStrip( relayUrls: List, relayActivity: Map, accountViewModel: AccountViewModel, @@ -378,29 +536,22 @@ fun GroupRelayList( if (normalized.isEmpty()) return - Column(modifier = Modifier.padding(top = 8.dp)) { - Text( - text = "Relays", - style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - FlowRow( - modifier = Modifier.padding(top = 4.dp), - horizontalArrangement = Arrangement.spacedBy(8.dp), - verticalArrangement = Arrangement.spacedBy(8.dp), - ) { - val nowSeconds = System.currentTimeMillis() / 1000L - normalized.forEach { (raw, relay) -> - val lastSeen = relayActivity[relay] - val isActive = lastSeen != null && (nowSeconds - lastSeen) <= RELAY_ACTIVITY_WINDOW_SECS - GroupRelayTile( - relay = relay, - fallbackUrl = raw, - isActive = isActive, - accountViewModel = accountViewModel, - nav = nav, - ) - } + FlowRow( + modifier = Modifier.padding(start = 8.dp), + horizontalArrangement = Arrangement.spacedBy(4.dp), + verticalArrangement = Arrangement.spacedBy(4.dp), + ) { + val nowSeconds = System.currentTimeMillis() / 1000L + normalized.forEach { (raw, relay) -> + val lastSeen = relayActivity[relay] + val isActive = lastSeen != null && (nowSeconds - lastSeen) <= RELAY_ACTIVITY_WINDOW_SECS + GroupRelayTile( + relay = relay, + fallbackUrl = raw, + isActive = isActive, + accountViewModel = accountViewModel, + nav = nav, + ) } } } @@ -421,7 +572,7 @@ fun GroupRelayTile( val clickableModifier = remember(relay) { Modifier - .size(55.dp) + .size(35.dp) .combinedClickable( indication = ripple24dp, interactionSource = MutableInteractionSource(), @@ -444,7 +595,7 @@ fun GroupRelayTile( loadProfilePicture = accountViewModel.settings.showProfilePictures(), pingInMs = 0, loadRobohash = accountViewModel.settings.isNotPerformanceMode(), - iconModifier = LargeRelayIconModifier, + iconModifier = MediumRelayIconModifier, ) val dotColor = @@ -458,15 +609,15 @@ fun GroupRelayTile( modifier = Modifier .align(Alignment.BottomEnd) - .size(12.dp) + .size(8.dp) .clip(CircleShape) .background(MaterialTheme.colorScheme.surface) - .padding(2.dp), + .padding(1.dp), ) { Box( modifier = Modifier - .size(8.dp) + .size(6.dp) .clip(CircleShape) .background(dotColor), ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/RemoveMemberScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/RemoveMemberScreen.kt deleted file mode 100644 index 9afb0ebb0..000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/RemoveMemberScreen.kt +++ /dev/null @@ -1,250 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup - -import android.widget.Toast -import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.automirrored.filled.ArrowBack -import androidx.compose.material.icons.filled.PersonRemove -import androidx.compose.material3.AlertDialog -import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.HorizontalDivider -import androidx.compose.material3.Icon -import androidx.compose.material3.IconButton -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Scaffold -import androidx.compose.material3.Text -import androidx.compose.material3.TextButton -import androidx.compose.material3.TopAppBar -import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.runtime.setValue -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.text.style.TextOverflow -import androidx.compose.ui.unit.dp -import androidx.lifecycle.compose.collectAsStateWithLifecycle -import com.vitorpamplona.amethyst.commons.marmot.GroupMemberInfo -import com.vitorpamplona.amethyst.ui.navigation.navs.INav -import com.vitorpamplona.amethyst.ui.note.UserPicture -import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel -import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.rooms.LoadUser -import com.vitorpamplona.quartz.nip01Core.core.HexKey -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch - -@OptIn(ExperimentalMaterial3Api::class) -@Composable -fun RemoveMemberScreen( - nostrGroupId: HexKey, - accountViewModel: AccountViewModel, - nav: INav, -) { - val chatroom = - remember(nostrGroupId) { - accountViewModel.account.marmotGroupList.getOrCreateGroup(nostrGroupId) - } - val members by chatroom.members.collectAsStateWithLifecycle() - var memberToRemove by remember { mutableStateOf(null) } - var isRemoving by remember { mutableStateOf(false) } - val scope = rememberCoroutineScope() - val myPubkey = accountViewModel.account.signer.pubKey - val context = LocalContext.current - - Scaffold( - topBar = { - TopAppBar( - navigationIcon = { - IconButton(onClick = { nav.popBack() }) { - Icon( - imageVector = Icons.AutoMirrored.Filled.ArrowBack, - contentDescription = "Back", - ) - } - }, - title = { Text("Remove Member") }, - ) - }, - ) { padding -> - val removableMembers = members.filter { it.pubkey != myPubkey } - - if (removableMembers.isEmpty()) { - Column( - modifier = - Modifier - .fillMaxSize() - .padding(padding) - .padding(16.dp), - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally, - ) { - Text( - text = "No members to remove", - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } - } else { - LazyColumn( - modifier = - Modifier - .fillMaxSize() - .padding(padding), - ) { - item { - Text( - text = "Select a member to remove", - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(horizontal = 16.dp, vertical = 12.dp), - ) - } - - items(removableMembers, key = { it.leafIndex }) { member -> - RemovableMemberRow( - member = member, - accountViewModel = accountViewModel, - nav = nav, - onRemoveClick = { memberToRemove = member }, - ) - HorizontalDivider() - } - } - } - } - - if (memberToRemove != null) { - val member = memberToRemove!! - ConfirmRemoveMemberDialog( - memberPubkey = member.pubkey, - accountViewModel = accountViewModel, - onConfirm = { - memberToRemove = null - isRemoving = true - scope.launch(Dispatchers.IO) { - try { - accountViewModel.removeMarmotGroupMember(nostrGroupId, member.leafIndex) - isRemoving = false - launch(Dispatchers.Main) { - Toast - .makeText(context, "Member removed", Toast.LENGTH_SHORT) - .show() - } - nav.popBack() - } catch (e: Exception) { - isRemoving = false - launch(Dispatchers.Main) { - Toast - .makeText( - context, - "Failed to remove member: ${e.message}", - Toast.LENGTH_LONG, - ).show() - } - } - } - }, - onDismiss = { memberToRemove = null }, - ) - } -} - -@Composable -private fun RemovableMemberRow( - member: GroupMemberInfo, - accountViewModel: AccountViewModel, - nav: INav, - onRemoveClick: () -> Unit, -) { - Row( - modifier = - Modifier - .fillMaxWidth() - .clickable(onClick = onRemoveClick) - .padding(horizontal = 16.dp, vertical = 10.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(12.dp), - ) { - UserPicture( - userHex = member.pubkey, - size = 36.dp, - accountViewModel = accountViewModel, - nav = nav, - ) - Column(modifier = Modifier.weight(1f)) { - LoadUser(baseUserHex = member.pubkey, accountViewModel = accountViewModel) { user -> - Text( - text = user?.toBestDisplayName() ?: "${member.pubkey.take(16)}...", - style = MaterialTheme.typography.bodyMedium, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) - } - } - Icon( - imageVector = Icons.Default.PersonRemove, - contentDescription = "Remove", - tint = MaterialTheme.colorScheme.error, - ) - } -} - -@Composable -private fun ConfirmRemoveMemberDialog( - memberPubkey: HexKey, - accountViewModel: AccountViewModel, - onConfirm: () -> Unit, - onDismiss: () -> Unit, -) { - AlertDialog( - onDismissRequest = onDismiss, - title = { Text("Remove Member") }, - text = { - LoadUser(baseUserHex = memberPubkey, accountViewModel = accountViewModel) { user -> - val name = user?.toBestDisplayName() ?: "${memberPubkey.take(16)}..." - Text("Are you sure you want to remove \"$name\" from this group?") - } - }, - confirmButton = { - TextButton(onClick = onConfirm) { - Text("Remove", color = MaterialTheme.colorScheme.error) - } - }, - dismissButton = { - TextButton(onClick = onDismiss) { - Text("Cancel") - } - }, - ) -} diff --git a/tools/marmot-interop/headless/tests-manage.sh b/tools/marmot-interop/headless/tests-manage.sh index fd14c49f9..b4a293c8e 100644 --- a/tools/marmot-interop/headless/tests-manage.sh +++ b/tools/marmot-interop/headless/tests-manage.sh @@ -155,6 +155,95 @@ test_08_admin_promote_demote() { fi } +test_17_readd_after_remove() { + banner "Test 17 — Re-add after remove restores B's leaf" + local id="17 re-add after remove" + + # Scenario reported on the info screen: A creates a group, adds B, + # removes B, then adds B again. Before the fix B rejoined without a + # usable own_leaf, so B could see metadata but could neither decrypt + # new posts nor sign messages. This test catches that regression by + # driving a full round-trip (A→B and B→A) after the re-add. + + local out gid mls_gid + out=$(amy_json marmot group create --name "Interop-17-readd") || { + record_result "$id" fail "amy create failed"; return + } + gid=$(printf '%s' "$out" | jq -r '.group_id') + mls_gid=$(printf '%s' "$out" | jq -r '.mls_group_id') + [[ -n "$gid" && -n "$mls_gid" ]] || { + record_result "$id" fail "missing ids from create"; return + } + + # A adds B. + amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { + record_result "$id" fail "initial add B failed"; return + } + wait_for_invite B 60 >/dev/null || { + record_result "$id" fail "B never received first invite"; return + } + wn_b groups accept "$mls_gid" >/dev/null 2>&1 || true + + # Confirm the baseline works: A → B and B → A both deliver. + amy_json marmot message send "$gid" "pre-remove hello" >/dev/null || { + record_result "$id" fail "A send before remove failed"; return + } + wait_for_message B "$mls_gid" "pre-remove hello" 90 || { + record_result "$id" fail "B missed pre-remove hello"; return + } + wn_b messages send "$mls_gid" "pre-remove pong" >/dev/null 2>&1 || true + amy_json marmot await message "$gid" --match "pre-remove pong" --timeout 60 >/dev/null || { + record_result "$id" fail "A missed B's pre-remove pong"; return + } + + # A removes B. + amy_json marmot group remove "$gid" "$B_NPUB" >/dev/null || { + record_result "$id" fail "remove B failed"; return + } + # Let the Remove commit propagate to B's wnd. + local deadline=$(( $(date +%s) + 120 )) removed=0 + while [[ $(date +%s) -lt $deadline ]]; do + if ! wn_b --json groups members "$mls_gid" 2>/dev/null \ + | jq -e --arg p "$B_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' \ + >/dev/null 2>&1; then + removed=1; break + fi + sleep 3 + done + [[ "$removed" -eq 1 ]] || warn "B still appears in members after remove — continuing" + + # B republishes its KP so the next add can find a fresh one. + wn_b keys publish >/dev/null 2>&1 || true + sleep 3 + + # A re-adds B. + amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { + record_result "$id" fail "re-add B failed"; return + } + wait_for_invite B 60 >/dev/null || { + record_result "$id" fail "B never received second invite"; return + } + wn_b groups accept "$mls_gid" >/dev/null 2>&1 || true + + # Regression probes: after re-add B must have a usable leaf again. + # (1) A→B: proves B can decrypt new group messages. + amy_json marmot message send "$gid" "post-readd hello" >/dev/null || { + record_result "$id" fail "A send after re-add failed"; return + } + wait_for_message B "$mls_gid" "post-readd hello" 120 || { + record_result "$id" fail "B didn't decrypt post-readd message (missing own_leaf?)"; return + } + # (2) B→A: proves B can still sign/commit — the exact breakage reported. + wn_b messages send "$mls_gid" "post-readd pong" >/dev/null 2>&1 || { + record_result "$id" fail "B send after re-add failed (likely no leaf)"; return + } + if amy_json marmot await message "$gid" --match "post-readd pong" --timeout 120 >/dev/null; then + record_result "$id" pass + else + record_result "$id" fail "A never saw B's post-readd pong (B couldn't type)" + fi +} + test_11_leave_group() { banner "Test 11 — Leave group" local id="11 leave group" diff --git a/tools/marmot-interop/marmot-interop-headless.sh b/tools/marmot-interop/marmot-interop-headless.sh index df2bd8308..1baa6365f 100755 --- a/tools/marmot-interop/marmot-interop-headless.sh +++ b/tools/marmot-interop/marmot-interop-headless.sh @@ -123,3 +123,4 @@ test_13_keypackage_rotation test_14_wn_removes_a test_15_wn_member_leaves test_16_wn_keypackage_rotation +test_17_readd_after_remove From a10fdb934a4a59b429b75e4431ce0df69a925f4e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 19:36:44 +0000 Subject: [PATCH 2/3] fix: route Leave Group back to the Messages screen After leaving a Marmot group the user was dropped on the group list; send them to the main Messages tab instead, which is the correct starting point for picking another conversation. https://claude.ai/code/session_01JaeqZwVNLKvUUvXWRzPmRP --- .../screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt index 914eabb66..3ef3f350f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt @@ -301,7 +301,7 @@ fun MarmotGroupInfoScreen( try { accountViewModel.leaveMarmotGroup(nostrGroupId) accountViewModel.account.marmotGroupList.removeGroup(nostrGroupId) - nav.nav(Route.MarmotGroupList) + nav.nav(Route.Message) } catch (e: Exception) { isLeaving = false launch(Dispatchers.Main) { From 32b0f41be9cd9a144ef28bd0f75659667b2a102d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 19:52:30 +0000 Subject: [PATCH 3/3] test: move re-add-after-remove regression to Quartz MlsGroup test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug (re-added user rejoins without a usable own_leaf, so they can neither decrypt new group messages nor send) reproduces entirely with two in-process MlsGroup instances — no relay or second Nostr client needed. A pure Quartz unit test runs in seconds and catches the regression deterministically, where the bash interop version depended on whitenoise-rs, wnd daemons and a local relay. - quartz: add MlsGroupLifecycleTest.testReaddAfterRemove_RejoinerCanEncryptAndDecrypt covering create → add → round-trip → remove → re-add with a fresh KeyPackage → round-trip again, asserting leafIndex and both encrypt/decrypt directions on the rejoined instance. - interop: drop test_17_readd_after_remove from the headless suite and its registration, since the unit test supersedes it. https://claude.ai/code/session_01JaeqZwVNLKvUUvXWRzPmRP --- .../marmot/mls/MlsGroupLifecycleTest.kt | 56 ++++++++++++ tools/marmot-interop/headless/tests-manage.sh | 89 ------------------- .../marmot-interop/marmot-interop-headless.sh | 1 - 3 files changed, 56 insertions(+), 90 deletions(-) diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt index bfd27bce6..0240d2cec 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt @@ -27,6 +27,7 @@ import kotlin.test.assertContentEquals import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertTrue /** * End-to-end lifecycle tests for MlsGroup covering the full protocol flow: @@ -298,6 +299,61 @@ class MlsGroupLifecycleTest { ) } + /** + * Regression: removing a member and re-adding them must produce a fully- + * functional group instance on the rejoiner's side. In the originally + * reported bug, the re-added user came back with no usable own_leaf — + * the UI showed the group but the user could neither decrypt new + * messages nor send any. This exercises the full loop with a fresh + * KeyPackage on re-add and asserts both directions still work. + */ + @Test + fun testReaddAfterRemove_RejoinerCanEncryptAndDecrypt() { + val alice = MlsGroup.create("alice".encodeToByteArray()) + + // First add: Bob joins. + val firstBundle = createStandaloneKeyPackage("bob") + val firstAdd = alice.addMember(firstBundle.keyPackage.toTlsBytes()) + val bob1 = MlsGroup.processWelcome(firstAdd.welcomeBytes!!, firstBundle) + + // Baseline round-trip works before the remove. + val hello = "hello before remove".encodeToByteArray() + assertContentEquals(hello, bob1.decrypt(alice.encrypt(hello)).content) + val pong = "pong before remove".encodeToByteArray() + assertContentEquals(pong, alice.decrypt(bob1.encrypt(pong)).content) + + // Alice removes Bob. bob1 is now stale (left-behind, no commit to apply). + alice.removeMember(bob1.leafIndex) + assertEquals(1, alice.memberCount) + + // Re-add: Bob publishes a fresh KeyPackage, Alice adds it, Bob processes + // the new Welcome from scratch. This is the exact flow driven by the + // info screen's inline "Add member" after an earlier removal. + val secondBundle = createStandaloneKeyPackage("bob") + val secondAdd = alice.addMember(secondBundle.keyPackage.toTlsBytes()) + val bob2 = MlsGroup.processWelcome(secondAdd.welcomeBytes!!, secondBundle) + + assertEquals(alice.epoch, bob2.epoch, "Rejoiner must be at Alice's epoch") + assertEquals(2, alice.memberCount) + assertEquals(2, bob2.memberCount) + + // Own-leaf sanity: the rejoiner must actually hold a leaf in the tree. + assertTrue(bob2.leafIndex >= 0, "Rejoiner must have a valid leafIndex after re-add") + + // The bug surfaces here — if bob2 has no usable leaf / keying material, + // one of these two round-trips fails. + val afterHello = "hello after re-add".encodeToByteArray() + val decrypted = bob2.decrypt(alice.encrypt(afterHello)) + assertContentEquals(afterHello, decrypted.content, "Rejoiner could not decrypt") + assertEquals(0, decrypted.senderLeafIndex, "Sender should still be Alice at leaf 0") + + val afterPong = "pong after re-add".encodeToByteArray() + val bobEncrypted = bob2.encrypt(afterPong) + val aliceSees = alice.decrypt(bobEncrypted) + assertContentEquals(afterPong, aliceSees.content, "Rejoiner could not send (no usable leaf)") + assertEquals(bob2.leafIndex, aliceSees.senderLeafIndex) + } + // ----------------------------------------------------------------------- // 8. Signing key rotation (Update proposal) // ----------------------------------------------------------------------- diff --git a/tools/marmot-interop/headless/tests-manage.sh b/tools/marmot-interop/headless/tests-manage.sh index b4a293c8e..fd14c49f9 100644 --- a/tools/marmot-interop/headless/tests-manage.sh +++ b/tools/marmot-interop/headless/tests-manage.sh @@ -155,95 +155,6 @@ test_08_admin_promote_demote() { fi } -test_17_readd_after_remove() { - banner "Test 17 — Re-add after remove restores B's leaf" - local id="17 re-add after remove" - - # Scenario reported on the info screen: A creates a group, adds B, - # removes B, then adds B again. Before the fix B rejoined without a - # usable own_leaf, so B could see metadata but could neither decrypt - # new posts nor sign messages. This test catches that regression by - # driving a full round-trip (A→B and B→A) after the re-add. - - local out gid mls_gid - out=$(amy_json marmot group create --name "Interop-17-readd") || { - record_result "$id" fail "amy create failed"; return - } - gid=$(printf '%s' "$out" | jq -r '.group_id') - mls_gid=$(printf '%s' "$out" | jq -r '.mls_group_id') - [[ -n "$gid" && -n "$mls_gid" ]] || { - record_result "$id" fail "missing ids from create"; return - } - - # A adds B. - amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { - record_result "$id" fail "initial add B failed"; return - } - wait_for_invite B 60 >/dev/null || { - record_result "$id" fail "B never received first invite"; return - } - wn_b groups accept "$mls_gid" >/dev/null 2>&1 || true - - # Confirm the baseline works: A → B and B → A both deliver. - amy_json marmot message send "$gid" "pre-remove hello" >/dev/null || { - record_result "$id" fail "A send before remove failed"; return - } - wait_for_message B "$mls_gid" "pre-remove hello" 90 || { - record_result "$id" fail "B missed pre-remove hello"; return - } - wn_b messages send "$mls_gid" "pre-remove pong" >/dev/null 2>&1 || true - amy_json marmot await message "$gid" --match "pre-remove pong" --timeout 60 >/dev/null || { - record_result "$id" fail "A missed B's pre-remove pong"; return - } - - # A removes B. - amy_json marmot group remove "$gid" "$B_NPUB" >/dev/null || { - record_result "$id" fail "remove B failed"; return - } - # Let the Remove commit propagate to B's wnd. - local deadline=$(( $(date +%s) + 120 )) removed=0 - while [[ $(date +%s) -lt $deadline ]]; do - if ! wn_b --json groups members "$mls_gid" 2>/dev/null \ - | jq -e --arg p "$B_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' \ - >/dev/null 2>&1; then - removed=1; break - fi - sleep 3 - done - [[ "$removed" -eq 1 ]] || warn "B still appears in members after remove — continuing" - - # B republishes its KP so the next add can find a fresh one. - wn_b keys publish >/dev/null 2>&1 || true - sleep 3 - - # A re-adds B. - amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { - record_result "$id" fail "re-add B failed"; return - } - wait_for_invite B 60 >/dev/null || { - record_result "$id" fail "B never received second invite"; return - } - wn_b groups accept "$mls_gid" >/dev/null 2>&1 || true - - # Regression probes: after re-add B must have a usable leaf again. - # (1) A→B: proves B can decrypt new group messages. - amy_json marmot message send "$gid" "post-readd hello" >/dev/null || { - record_result "$id" fail "A send after re-add failed"; return - } - wait_for_message B "$mls_gid" "post-readd hello" 120 || { - record_result "$id" fail "B didn't decrypt post-readd message (missing own_leaf?)"; return - } - # (2) B→A: proves B can still sign/commit — the exact breakage reported. - wn_b messages send "$mls_gid" "post-readd pong" >/dev/null 2>&1 || { - record_result "$id" fail "B send after re-add failed (likely no leaf)"; return - } - if amy_json marmot await message "$gid" --match "post-readd pong" --timeout 120 >/dev/null; then - record_result "$id" pass - else - record_result "$id" fail "A never saw B's post-readd pong (B couldn't type)" - fi -} - test_11_leave_group() { banner "Test 11 — Leave group" local id="11 leave group" diff --git a/tools/marmot-interop/marmot-interop-headless.sh b/tools/marmot-interop/marmot-interop-headless.sh index 1baa6365f..df2bd8308 100755 --- a/tools/marmot-interop/marmot-interop-headless.sh +++ b/tools/marmot-interop/marmot-interop-headless.sh @@ -123,4 +123,3 @@ test_13_keypackage_rotation test_14_wn_removes_a test_15_wn_member_leaves test_16_wn_keypackage_rotation -test_17_readd_after_remove