From b97732643bb59ebe1255991601403e1b4974dbc5 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 20:27:34 +0200 Subject: [PATCH] fix: put proposals in the room that signs, and open the queue when one row is not all of it The Proposals button sat behind `mlsGroupState == null`, so it was offered in NIP-17 rooms and withheld from Marmot ones. That is the wrong way round rather than a gap: FrostSigningManager signs "in its #admins room", and an #admins room is a Marmot group. **Where a group actually proposes.** `FrostSigningManager.broadcast` writes a signing message as an unprocessed `MarmotInnerEvent`, which `NotaryViewModel` MLS-encrypts and broadcasts as a kind:445 -- "the same path every other event in a Marmot group takes, which is why this needs no transport of its own". A NIP-17 room holds no MLS state for that path to use, and since 9107b81 `encryptAndSendMarmotInnerEvent` throws `MarmotMissingChatGroupException` on exactly that rather than silently doing nothing. So the button was shown in the one kind of room where a proposal cannot leave the device, and hidden in the kind where every proposal this app has made actually lives. The Shared Key button beside it keeps the gate, and the comment above it keeps its wording, because for a ceremony the gate is right: ChillDKG runs over NIP-17 because it has to -- its participants are not yet a Marmot group, and its whole purpose is to produce the key one would be keyed on -- so a room that already has an MLS tree is not a room a ceremony can run in. The two buttons stand next to each other and answer different questions; only one of them is about the transport the group already has. **Not gated on whether the room can sign.** A plain DM now shows Proposals too, and opening it says "This group has not been asked to sign anything yet." Gating on `FrostSigningRepository.canSign` would mean threading that repository into `ChatRoomDetailScreen` for one button's visibility, and the Shared Key button it sits beside is not gated either -- a room with no ceremony behind it still offers to hold one. An empty list is a truthful answer to a tap; an absent button is no answer at all to a member wondering where the proposals went, which is the complaint this commit starts from. **One row is not the queue.** Tapping a signing line in the transcript went straight to that session, and the list was reached only when the line predated chat rows naming their session. That is right while the reader has one decision outstanding and wrong the moment they have two: the second proposal is not in the transcript beside the first -- it may be pages up, or have arrived while they were reading -- so answering the one they tapped and leaving looks exactly like being done. A group proposing a chapter and the translation that depends on it is two sessions at once, which is the case `ProposalListScreen` exists for; the transcript was still handing over one of them and calling it the answer. **Only for a row that is itself waiting.** `hidesOtherDecisions` asks two things rather than one: that the tapped session is waiting on this member, and that it is not the only one. A line about something the group has already signed still opens that session directly. A reader who taps history is asking to see what was signed, and meeting that with the queue would be substituting a general answer for a specific request -- the same fault as the one being fixed, pointing the other way. **Where the answer comes from.** `ChatMessageListViewModel` observes `observeSessionsForChatRoom` and keeps the ids of the sessions `FrostSigningManager.isAwaitingApproval` calls pending. That is the same question `ProposalListUIState.waitingForYou` asks, deliberately, so the transcript and the list cannot come to different views about which proposals still have a decision in them. Observed rather than read once, because a proposal arrives while a room is open as often as before it is opened, and one answered on another device stops being owed with nothing happening here at all. Held as state rather than queried at the tap: a navigation callback is not suspend, and there is nowhere inside one to put a query. The read happens in the click lambda rather than during composition, so a proposal arriving moves where the next tap goes without recomposing the transcript to do it. That took the repository through `MantraNavHost` -> `ChatRoomMessagingScreen` -> `ChatMessageListViewModel.factory`; the screen's preview takes `NO_OP_FROST_SIGNING_REPOSITORY`, which already answers with an empty list. **Not covered, deliberately.** `ChatRoomMessagingScreen` calls `chatMessageListViewModel.initiate()` inside `key(true) { }` rather than a `LaunchedEffect`, so it re-runs on recomposition and launches a fresh collector each time. The three observers already there carry that exposure; `observeProposalsAwaitingYou` joins them rather than diverging from them, and since each collector writes the same value from the same Room flow the cost is duplicate collection, not a wrong answer. Fixing it changes how this screen starts all of its work, which is every observer's business rather than this one's. Nothing counts the outstanding decisions anywhere a reader can see them before tapping. A signing line still says only whether the line it sits on has been answered, and the room list says nothing. A badge wants this count somewhere it outlives one open room, which is its own change. No tests. Both changes are navigation decisions taken in composables -- `ChatRoomDetailScreen`'s visibility gate and `ChatRoomMessagingScreen`'s route choice -- and there is no UI test harness here to press a button in; the one piece with a seam, `isAwaitingApproval`, is already what the proposal list is tested through. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 808 tests pass, 511 jvm and 297 android, unchanged from before the change. That a Marmot room now offers the list and that a second waiting proposal redirects the tap are read from the code, not asserted -- both want the app on a device with a group that has a key. Co-Authored-By: Claude Opus 5 --- .../ui/composable/ChatRoomDetailScreen.kt | 51 +++++++++------- .../ui/composable/ChatRoomMessagingScreen.kt | 26 +++++--- .../ui/composable/navigation/MantraNavHost.kt | 1 + .../ui/view/model/ChatMessageListViewModel.kt | 60 ++++++++++++++++++- 4 files changed, 105 insertions(+), 33 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt index 8cb0ed7c..79932dde 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt @@ -367,33 +367,38 @@ fun ChatRoomDetailScreen( Text("Shared Key") } } + } - // Next to the key, because that is what they sign with. - // The transcript carries a proposal past as it happens; - // this is where a member goes to find one that has - // scrolled away, or to see what the group has signed. - item { - TextButton( - onClick = { - onNavigateToRoute.invoke( - ProposalListRoute( - activeUserPublicKey = activeUserPublicKey, - chatRoomId = chatRoomId - ) + // Every room, not only the one that holds the key. A + // ceremony needs a NIP-17 group, but a signing message is + // a marmot inner event -- see `FrostSigningManager.broadcast` + // -- so the room a group actually proposes in is the MLS + // one, which is the last place this should be missing from. + // + // The transcript carries a proposal past as it happens; + // this is where a member goes to find one that has scrolled + // away, or to see what the group has signed. + item { + TextButton( + onClick = { + onNavigateToRoute.invoke( + ProposalListRoute( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId ) - } - ) { - Icon( - Icons.Default.Draw, - contentDescription = "Proposals" ) - - Spacer( - modifier = Modifier.width(10.dp) - ) - - Text("Proposals") } + ) { + Icon( + Icons.Default.Draw, + contentDescription = "Proposals" + ) + + Spacer( + modifier = Modifier.width(10.dp) + ) + + Text("Proposals") } } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt index b023c243..83c55c4e 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt @@ -44,6 +44,7 @@ import androidx.lifecycle.viewmodel.compose.viewModel import press.mantra.compose.database.model.ChatRoom import press.mantra.compose.database.model.intermdiate.LocalChatRoom import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.FrostSigningRepository import press.mantra.compose.repository.NostrRepository import press.mantra.compose.ui.composable.navigation.routes.ChatRoomDetailRoute import press.mantra.compose.ui.composable.navigation.routes.DkgRitualRoute @@ -65,6 +66,7 @@ fun ChatRoomMessagingScreen( initialChatRoomMessagingUIState: ChatRoomMessagingUIState = ChatRoomMessagingUIState.Loading, nostrRepository: NostrRepository, chatRepository: ChatRepository, + frostSigningRepository: FrostSigningRepository, onNavigateToRouteAndPopUpInclusive: (Route) -> Unit, onNavigateToRoute: (Route) -> Unit, ) { @@ -100,7 +102,8 @@ fun ChatRoomMessagingScreen( factory = press.mantra.compose.ui.view.model.ChatMessageListViewModel.factory( localChatRoom = chatRoomDetailUIState.localChatRoom, nostrRepository = nostrRepository, - chatRepository = chatRepository + chatRepository = chatRepository, + frostSigningRepository = frostSigningRepository ) ) @@ -149,13 +152,21 @@ fun ChatRoomMessagingScreen( ) }, onOpenSigning = { sessionId -> - // A line written before chat rows named their - // session cannot say which proposal it is about, - // and the room may have several. The list is the - // honest answer: it shows all of them with their - // own state, rather than guessing at one. + // Two reasons to open the queue instead of the one + // proposal. A line written before chat rows named + // their session cannot say which proposal it is + // about, and the room may have several -- the list + // is the honest answer there, showing all of them + // with their own state rather than guessing at one. + // + // The other is that the proposal they tapped is + // not the only one waiting on them, in which case + // it is not the whole of what is being asked and a + // screen showing only it would say it was. onNavigateToRoute.invoke( - if (sessionId == null) { + if (sessionId == null || + chatMessageListViewModel.hidesOtherDecisions(sessionId) + ) { ProposalListRoute( activeUserPublicKey = activeUserPublicKey, chatRoomId = chatRoomId @@ -440,6 +451,7 @@ private fun ChatRoomMessagingScreenPreview() { ), nostrRepository = NostrRepository.NO_OP_NOSTR_REPOSITORY, chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, + frostSigningRepository = FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY, onNavigateToRouteAndPopUpInclusive = {}, onNavigateToRoute = {} ) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt index c1e74835..dd08385b 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt @@ -746,6 +746,7 @@ fun MantraNavHost( relayHint = route.relayHint, nostrRepository = databaseNostrRepository, chatRepository = databaseChatRepository, + frostSigningRepository = databaseFrostSigningRepository, onNavigateToRouteAndPopUpInclusive = { chatRoomDetailRoute -> navController.navigate( route = chatRoomDetailRoute 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..c88916b9 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 @@ -73,9 +73,11 @@ import press.mantra.compose.database.model.intermdiate.LocalChatRoom import press.mantra.compose.database.model.types.SynchronizationFilter import press.mantra.compose.extensions.shortened import press.mantra.compose.extensions.toFormattedTimeAndDateString +import press.mantra.compose.managers.FrostSigningManager import press.mantra.compose.nostr.Nip17Filters import press.mantra.compose.nostr.Relays import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.FrostSigningRepository import press.mantra.compose.repository.NostrRepository import press.mantra.compose.ui.composable.widgets.profile.ProfileColor import press.mantra.compose.ui.view.state.ChatMessageListUIState @@ -91,7 +93,8 @@ class ChatMessageListViewModel( initialChatMessageListUIState: ChatMessageListUIState, val localChatRoom: LocalChatRoom, val nostrRepository: NostrRepository, - val chatRepository: ChatRepository + val chatRepository: ChatRepository, + val frostSigningRepository: FrostSigningRepository ): ViewModel() { val logger = Logger.withTag(TAG) var chatMessageListUIState: ChatMessageListUIState by mutableStateOf(initialChatMessageListUIState) @@ -114,6 +117,33 @@ class ChatMessageListViewModel( var openMessageActionsFor: Long? by mutableStateOf(null) private set + /** + * The sessions still waiting on this member to agree to sign, by id. + * + * Held rather than asked for, because it is read at the moment of a tap -- + * see [hidesOtherDecisions] -- and that is not a moment that can wait on a + * query. + */ + var proposalsAwaitingYou: Set by mutableStateOf(emptySet()) + private set + + /** + * Whether opening [sessionId] by itself would hide other decisions the room + * is waiting on this member for. + * + * A signing line names one proposal, and opening it is right while that is + * the only thing being asked of them. It stops being right the moment there + * are two: the second is not in the transcript beside the first -- it may be + * pages up, or have arrived while they were reading -- so answering the one + * they tapped and leaving would look exactly like being done. + * + * Only for a proposal that is itself waiting on them. A line about something + * the group has already signed is history, and a reader who taps one is + * asking to see that, not to be handed the queue. + */ + fun hidesOtherDecisions(sessionId: String): Boolean = + sessionId in proposalsAwaitingYou && proposalsAwaitingYou.size > 1 + fun startDirectMessage(participant: Participant) { logger.d("Arming a direct message to ${participant.participantPublicKey}") directMessageRecipient = participant @@ -157,9 +187,31 @@ class ChatMessageListViewModel( logger.d("init") scheduleSynchronization() observeChatRoomFeed() + observeProposalsAwaitingYou() askForGroupHistory() } + /** + * Watches which of the room's proposals are waiting on this member. + * + * Observed rather than read once: a proposal arrives while the room is + * open as often as before it is opened, and one answered on another device + * stops being owed without anything happening here. Asked of the manager, + * the way the proposal list asks, so the two cannot disagree about which + * proposals still have a decision in them. + */ + private fun observeProposalsAwaitingYou() { + viewModelScope.launch(Dispatchers.IO) { + frostSigningRepository + .observeSessionsForChatRoom(localChatRoom.chatRoom.id) + .collect { sessions -> + proposalsAwaitingYou = sessions.filter { + FrostSigningManager.isAwaitingApproval(it.session, it.items) + }.map { it.session.id }.toSet() + } + } + } + /** * Ask the group for its signed record, if this device holds none of it. * @@ -643,14 +695,16 @@ class ChatMessageListViewModel( initialChatMessageListUIState: ChatMessageListUIState = ChatMessageListUIState.Loading, localChatRoom: LocalChatRoom, nostrRepository: NostrRepository, - chatRepository: ChatRepository + chatRepository: ChatRepository, + frostSigningRepository: FrostSigningRepository ): ViewModelProvider.Factory = viewModelFactory { initializer { ChatMessageListViewModel( initialChatMessageListUIState = initialChatMessageListUIState, localChatRoom = localChatRoom, nostrRepository = nostrRepository, - chatRepository = chatRepository + chatRepository = chatRepository, + frostSigningRepository = frostSigningRepository ) } }