From b3be741afb2baf560845252763ff4fe378a35c6b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 00:10:34 +0000 Subject: [PATCH 01/10] fix: make the Marmot create-group form scrollable under the IME CreateGroupScreen already applied `imePaddingSafe()`, but its Column had no vertical scroll. The form (icon editor + name + description + retention picker + footer) is taller than the remaining window once the keyboard is up, so the IME padding just clipped the bottom rows with no way to reach them. Adds `verticalScroll(rememberScrollState())` to match the pattern in RelayGroupMetadataScreen, plus bottom padding so the footer is not flush against the window edge when scrolled to the end. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../loggedIn/chats/marmotGroup/CreateGroupScreen.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt index a0604b0bf2..81b054e93b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/CreateGroupScreen.kt @@ -27,6 +27,8 @@ import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll import androidx.compose.material3.AlertDialog import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField @@ -166,6 +168,11 @@ fun CreateGroupScreen( .padding(padding) .consumeWindowInsets(padding) .imePaddingSafe() + // The form is taller than the window once the IME is up, so + // `imePaddingSafe` alone just clips the retention picker and + // the footer off the bottom. Scrolling is what makes them + // reachable while the keyboard covers half the screen. + .verticalScroll(rememberScrollState()) .padding(horizontal = 16.dp), ) { Text( @@ -219,7 +226,7 @@ fun CreateGroupScreen( Text( stringRes(Res.string.marmot_create_group_footer), - modifier = Modifier.padding(top = 12.dp), + modifier = Modifier.padding(top = 12.dp, bottom = 16.dp), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, ) From 842028edca7c6473dd219a453abf778d55d8d522 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 00:50:44 +0000 Subject: [PATCH 02/10] feat: report which device owns this account's Marmot invites An inviter takes the highest created_at kind:30443 and nothing else (KeyPackageFetcher.fetchKeyPackage), and only the install holding that bundle's private keys can open the resulting Welcome. Because each install mints its own random d-tag slot, two devices signed in to one account publish KeyPackages that never replace each other, so the most recent publisher silently owns every future invite and the other device fails every Welcome with "No matching KeyPackageBundle found". Adds a "Marmot Invite Device" settings row that asks the relays which install currently owns the account's invites and offers to move them here: - MarmotManager.ownsKeyPackageEvent() delegates to the existing findBundleByEventId, deliberately the same lookup the Welcome path uses so the diagnosis cannot disagree with what happens on arrival. - AccountMarmotActions.latestKeyPackageOwner() fetches the newest KeyPackage from our publish relays and resolves the owner. - MarmotInviteDeviceDialog reports the answer and offers to publish. The check is the dialog's content, never a trigger. Republishing automatically whenever another device won would deadlock the two installs against each other, since each device's correction is the other's trigger and neither ever settles. Republishing from the same device replaces its own KeyPackage at the same d-tag, so the manual action stays idempotent. Also fixes EditGroupInfoScreen under the IME, same defect as CreateGroupScreen: imePaddingSafe() was applied but the Column had no vertical scroll, so the lower fields were clipped with no way to reach them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../amethyst/model/AccountMarmotActions.kt | 51 ++++++++ .../ui/screen/loggedIn/AccountViewModel.kt | 4 + .../chats/marmotGroup/EditGroupInfoScreen.kt | 8 ++ .../loggedIn/settings/AllSettingsScreen.kt | 118 ++++++++++++++++++ .../settings/SettingsCatalogBuilder.kt | 14 ++- amethyst/src/main/res/values/strings.xml | 4 + .../amethyst/commons/marmot/MarmotManager.kt | 15 +++ .../composeResources/values/strings.xml | 7 ++ 8 files changed, 220 insertions(+), 1 deletion(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index 86361cadd4..22abf73cac 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -36,6 +36,27 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.utils.Log import kotlin.coroutines.cancellation.CancellationException +/** + * Which install owns the newest KeyPackage currently on the account's relays. + * + * An inviter picks the highest `created_at` kind:30443 and nothing else + * ([KeyPackageFetcher.fetchKeyPackage]), and only the install holding that + * bundle's private keys can open the Welcome it produces. With the same + * account signed in twice, the two installs publish under different random + * d-tag slots, so both KeyPackages persist and the most recent publisher + * silently owns every future invite. + */ +enum class LatestKeyPackageOwner { + /** This install holds the bundle — invites land here. */ + THIS_DEVICE, + + /** A newer KeyPackage we have no private keys for — invites land elsewhere. */ + OTHER_DEVICE, + + /** Nothing published for this account, or nowhere to ask. */ + NONE, +} + /** * Marmot (MLS encrypted groups) orchestration for an [Account]: group create/ * leave/reset, member add/remove via key-package fetch, admin grant/revoke, @@ -400,6 +421,36 @@ class AccountMarmotActions( return manager.hasActiveKeyPackages() } + /** + * Ask the relays which install currently owns this account's invites. + * + * Deliberately a read, never a self-correcting one. Republishing whenever + * the answer is [LatestKeyPackageOwner.OTHER_DEVICE] would deadlock two + * installs against each other — each device's correction is the other's + * trigger, and neither ever settles — so the decision belongs to the user, + * with this as the evidence. + * + * Queries the same set we publish our own KeyPackages to, which is the + * inviter's view of us minus their own outbox: `fetchRelaysFor` unions our + * NIP-65 write set and our legacy kind:10051 with the inviter's outbox, and + * the first two are exactly [keyPackagePublishRelays]. + */ + suspend fun latestKeyPackageOwner(): LatestKeyPackageOwner { + val manager = account.marmotManager ?: return LatestKeyPackageOwner.NONE + val relays = keyPackagePublishRelays() + if (relays.isEmpty()) return LatestKeyPackageOwner.NONE + + val latest = + KeyPackageFetcher.fetchKeyPackage(account.client, account.signer.pubKey, relays) + ?: return LatestKeyPackageOwner.NONE + + val mine = manager.ownsKeyPackageEvent(latest.id) + Log.d("MarmotDbg") { + "latestKeyPackageOwner: newest KeyPackage id=${latest.id.take(8)}… createdAt=${latest.createdAt} mine=$mine" + } + return if (mine) LatestKeyPackageOwner.THIS_DEVICE else LatestKeyPackageOwner.OTHER_DEVICE + } + /** * Create a new Marmot MLS group under the CURRENT profile. * diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 6f2517432b..7175a94194 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -73,6 +73,7 @@ import com.vitorpamplona.amethyst.commons.ui.state.GenericBaseCacheAsync import com.vitorpamplona.amethyst.logTime import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.AccountSettings +import com.vitorpamplona.amethyst.model.LatestKeyPackageOwner import com.vitorpamplona.amethyst.model.UiSettingsFlow import com.vitorpamplona.amethyst.model.UrlCachedPreviewer import com.vitorpamplona.amethyst.model.privacyOptions.RoleBasedHttpClientBuilder @@ -2504,6 +2505,9 @@ class AccountViewModel( suspend fun hasPublishedKeyPackage(): Boolean = account.marmot.hasPublishedKeyPackage() + /** Which install currently owns this account's Marmot invites. See [LatestKeyPackageOwner]. */ + suspend fun latestKeyPackageOwner(): LatestKeyPackageOwner = account.marmot.latestKeyPackageOwner() + /** * Whether this account has a kind:10051 KeyPackage Relay List (MIP-00) * advertising where it publishes KeyPackages. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/EditGroupInfoScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/EditGroupInfoScreen.kt index c4498c5d7b..dd03236d16 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/EditGroupInfoScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/EditGroupInfoScreen.kt @@ -27,6 +27,8 @@ import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold @@ -157,6 +159,11 @@ fun EditGroupInfoScreen( .padding(padding) .consumeWindowInsets(padding) .imePaddingSafe() + // Same reason as CreateGroupScreen: name + description + + // avatar-url + footer is taller than what is left once the + // IME is up, so the padding alone would clip the lower + // fields with no way to reach them. + .verticalScroll(rememberScrollState()) .padding(horizontal = 16.dp), ) { Spacer(modifier = Modifier.height(8.dp)) @@ -241,6 +248,7 @@ fun EditGroupInfoScreen( text = stringRes(Res.string.marmot_edit_info_footer), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(bottom = 16.dp), ) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt index c47fa6859d..3a4aa26f90 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt @@ -37,6 +37,7 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -55,10 +56,18 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.resources.Res +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_checking +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_error +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_none +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_other +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_publish +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_this +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_title import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_action import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_body import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_title import com.vitorpamplona.amethyst.commons.resources.settings_search_no_results +import com.vitorpamplona.amethyst.model.LatestKeyPackageOwner import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.navigation.navs.EmptyNav import com.vitorpamplona.amethyst.ui.navigation.navs.INav @@ -70,6 +79,8 @@ import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import kotlin.coroutines.cancellation.CancellationException @Preview @Composable @@ -92,6 +103,7 @@ fun AllSettingsScreen( val scope = rememberCoroutineScope() var showResetMarmotDialog by remember { mutableStateOf(false) } var isResettingMarmot by remember { mutableStateOf(false) } + var showInviteDeviceDialog by remember { mutableStateOf(false) } val scrollState = rememberScrollState() val hasPrivateKey = accountViewModel.account.settings.keyPair.privKey != null @@ -101,6 +113,7 @@ fun AllSettingsScreen( // an input actually changes — not on every keystroke. `onResetMarmot` reads the volatile // `isResettingMarmot` through `rememberUpdatedState` so the memoized closure never goes stale. val onResetMarmot by rememberUpdatedState(newValue = { if (!isResettingMarmot) showResetMarmotDialog = true }) + val onMarmotInviteDevice by rememberUpdatedState(newValue = { showInviteDeviceDialog = true }) val catalog = remember(hasPrivateKey, nav, uriHandler) { buildSettingsCatalog( @@ -108,6 +121,7 @@ fun AllSettingsScreen( uriHandler = uriHandler, hasPrivateKey = hasPrivateKey, onResetMarmot = { onResetMarmot() }, + onMarmotInviteDevice = { onMarmotInviteDevice() }, ) } @@ -161,6 +175,34 @@ fun AllSettingsScreen( } } + if (showInviteDeviceDialog) { + MarmotInviteDeviceDialog( + accountViewModel = accountViewModel, + // Published from the screen's scope, not the dialog's: confirming + // closes the dialog, and a publish launched in the dialog's own + // scope would be cancelled the moment it left composition. + onConfirm = { + showInviteDeviceDialog = false + scope.launch(Dispatchers.IO) { + val successMessage = stringRes(context, R.string.marmot_invite_device_success) + try { + accountViewModel.publishMarmotKeyPackage() + launch(Dispatchers.Main) { + Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() + } + } catch (e: Exception) { + val failureMessage = + stringRes(context, R.string.marmot_invite_device_failure, e.message ?: "") + launch(Dispatchers.Main) { + Toast.makeText(context, failureMessage, Toast.LENGTH_LONG).show() + } + } + } + }, + onDismiss = { showInviteDeviceDialog = false }, + ) + } + if (showResetMarmotDialog) { ResetMarmotStateDialog( onConfirm = { @@ -278,3 +320,79 @@ private fun ResetMarmotStateDialog( }, ) } + +/** + * Reports which install currently receives this account's Marmot invites, and + * offers to move them to this one. + * + * The check is this dialog's *content*, never a trigger. An inviter always + * takes the newest kind:30443 and nothing else, so a device that silently + * republished itself back to the front whenever it lost would deadlock against + * the other install — each device's correction is the other's trigger, and + * neither ever settles. Surfacing the answer and letting the user decide is + * what keeps two signed-in devices from fighting over the account's invites. + */ +@Composable +private fun MarmotInviteDeviceDialog( + accountViewModel: AccountViewModel, + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { + var owner by remember { mutableStateOf(null) } + var checkFailed by remember { mutableStateOf(false) } + + LaunchedEffect(Unit) { + try { + owner = withContext(Dispatchers.IO) { accountViewModel.latestKeyPackageOwner() } + } catch (e: CancellationException) { + throw e + } catch (_: Exception) { + // A relay we cannot reach only costs us the diagnosis, not the + // action: publishing from here is still a valid thing to want. + checkFailed = true + } + } + + val resolved = owner + val body = + when { + checkFailed -> stringRes(Res.string.marmot_invite_device_error) + resolved == null -> stringRes(Res.string.marmot_invite_device_checking) + resolved == LatestKeyPackageOwner.THIS_DEVICE -> stringRes(Res.string.marmot_invite_device_this) + resolved == LatestKeyPackageOwner.OTHER_DEVICE -> stringRes(Res.string.marmot_invite_device_other) + else -> stringRes(Res.string.marmot_invite_device_none) + } + + AlertDialog( + onDismissRequest = onDismiss, + icon = { + Icon( + symbol = MaterialSymbols.Key, + contentDescription = null, + modifier = Modifier.size(32.dp), + ) + }, + title = { + Text( + text = stringRes(Res.string.marmot_invite_device_title), + textAlign = TextAlign.Center, + ) + }, + text = { Text(text = body) }, + confirmButton = { + Button( + onClick = onConfirm, + // Held until the check resolves so the user is never asked to + // act on an answer that has not arrived. + enabled = checkFailed || resolved != null, + ) { + Text(stringRes(Res.string.marmot_invite_device_publish)) + } + }, + dismissButton = { + TextButton(onClick = onDismiss) { + Text(stringRes(R.string.cancel)) + } + }, + ) +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt index d164a3deba..27938f508e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt @@ -30,7 +30,8 @@ import com.vitorpamplona.amethyst.ui.navigation.routes.Route /** * Assembles the full settings catalog. Not composable: actions close over [nav], - * [uriHandler], and [onResetMarmot]; conditional rows are included via [hasPrivateKey]. + * [uriHandler], [onResetMarmot] and [onMarmotInviteDevice]; conditional rows are included + * via [hasPrivateKey]. * The blank-query render of this catalog must match the legacy hardcoded screen. */ fun buildSettingsCatalog( @@ -38,6 +39,7 @@ fun buildSettingsCatalog( uriHandler: UriHandler, hasPrivateKey: Boolean, onResetMarmot: () -> Unit, + onMarmotInviteDevice: () -> Unit, ): List { // Most rows are a symbol icon + a keyword blob that navigates to a route. This local // helper collapses that shape to one line per row and makes a mismatched keyword/route @@ -84,6 +86,16 @@ fun buildSettingsCatalog( symEntry(R.string.napplet_permissions_title, MaterialSymbols.Apps, R.string.napplet_connected_apps_search_keywords, Route.ConnectedApps), symEntry(R.string.relay_auth_settings_title, MaterialSymbols.Lock, R.string.relay_auth_search_keywords, Route.RelayAuthSettings), symEntry(R.string.call_settings, MaterialSymbols.Phone, R.string.call_settings_search_keywords, Route.CallSettings), + // Opens a dialog rather than a route: the row's whole job is + // to report which device currently owns this account's Marmot + // invites, and that answer costs a relay round trip, so it is + // fetched on demand instead of on every settings render. + SettingsEntry( + titleRes = R.string.marmot_invite_device, + icon = SettingsIcon.Symbol(MaterialSymbols.Key), + keywordsRes = R.string.marmot_invite_device_search_keywords, + onClick = onMarmotInviteDevice, + ), ), ) diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 22f30b780d..b6f6a248e4 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1348,6 +1348,10 @@ legal, safety, abuse, csae, child protection Danger Zone + Marmot Invite Device + mls, marmot, keypackage, key package, invite, device, group chat + This device now receives your Marmot invites. + Failed to publish KeyPackage: %1$s Reset Marmot State Marmot state reset. Failed to reset Marmot state: %1$s diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 586f4cd0a4..14823c4eeb 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -2443,6 +2443,21 @@ class MarmotManager( */ suspend fun hasActiveKeyPackages(): Boolean = keyPackageRotationManager.hasActiveKeyPackages() + /** + * True when the private bundle behind [eventId] — the Nostr id of a + * published kind:30443 — is one this install holds, active or retained. + * + * Two installs of the same account each mint their own random d-tag slot + * ([KeyPackageRotationManager.getOrCreateSlotDTag]), so their KeyPackages + * never replace one another and both sit on relays indefinitely. An + * inviter then takes whichever has the highest `created_at`, and only the + * install holding that bundle can open the resulting Welcome. This is how + * a device answers "would an invite against that KeyPackage reach me?" — + * deliberately the same lookup the Welcome path itself performs, so the + * answer cannot disagree with what actually happens on arrival. + */ + suspend fun ownsKeyPackageEvent(eventId: HexKey): Boolean = keyPackageRotationManager.findBundleByEventId(eventId) != null + /** * Check if a specific group membership exists. */ diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index ab19800ef0..e72dde98a1 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1621,6 +1621,13 @@ User Preferences Search settings No settings found for "%1$s" + Marmot Invite Device + Asking your relays which device currently receives your Marmot invites\u2026 + This device already receives your Marmot invites. Nothing to do \u2014 publishing again is harmless but unnecessary. + Another device signed in to this account published a newer KeyPackage, so Marmot invites are going there and cannot be opened here. Publishing from this device takes over future invites; the other device keeps the groups it already joined. + No KeyPackage was found on your relays for this account. Publish one so others can invite you to Marmot groups. + Could not reach your relays to check. You can still publish from this device. + Publish from this device Reset Marmot State? This will permanently delete every Marmot group chat, message history, and MLS key on this device for the current account. Peers will not be notified and may still see you in groups until their next commit. This cannot be undone. A new KeyPackage will be published the next time the app syncs. Reset From d054a563e8dc898d7c967b646a09a1e4a3da7fa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 02:16:39 +0000 Subject: [PATCH 03/10] feat: warn on the Marmot groups screen when another device owns invites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settings row only reaches a user who already suspects the cause. The symptom shows up here: with invites landing on a second install, both tabs are empty and no new requests arrive, which is exactly what "nobody has invited me" looks like — so the failure was indistinguishable from nothing happening. MarmotInviteDeviceBanner runs latestKeyPackageOwner() when the groups screen opens and, only for OTHER_DEVICE, shows an errorContainer strip above the tabs offering to publish from this device. Styled after the AgentStreamPreviewBanner already in this package. - An unreachable relay renders nothing: a failed check is not evidence, and would otherwise accuse a device that may well be this one. - Tapping publish hides the banner optimistically, since the publish makes this device the newest KeyPackage by definition and re-asking the relays would only add a round trip; a failed publish puts the banner back. - Still a tap, never automatic, for the same reason as the settings row: an inviter takes the newest KeyPackage, so a banner that republished on sight would deadlock two installs against each other. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../marmotGroup/MarmotGroupListScreen.kt | 5 + .../marmotGroup/MarmotInviteDeviceBanner.kt | 163 ++++++++++++++++++ .../composeResources/values/strings.xml | 2 + 3 files changed, 170 insertions(+) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt index 52239ce076..fb99a7d87c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt @@ -142,6 +142,11 @@ fun MarmotGroupListScreen( }, ) { padding -> Column(modifier = Modifier.fillMaxSize().padding(padding)) { + // Above the tabs, because the state it reports is the reason both + // tabs can be empty: invites landing on another install look + // exactly like no invites at all. + MarmotInviteDeviceBanner(accountViewModel) + PrimaryTabRow(selectedTabIndex = selectedTab) { Tab( selected = selectedTab == 0, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt new file mode 100644 index 0000000000..f2d97bcc27 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt @@ -0,0 +1,163 @@ +/* + * 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.animation.AnimatedVisibility +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +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.draw.clip +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.unit.dp +import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.commons.icons.symbols.Icon +import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols +import com.vitorpamplona.amethyst.commons.resources.Res +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_banner +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_banner_dismiss +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_publish +import com.vitorpamplona.amethyst.model.LatestKeyPackageOwner +import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel +import com.vitorpamplona.amethyst.ui.stringRes +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import kotlin.coroutines.cancellation.CancellationException + +/** + * Warns, on the Marmot groups screen, that another install of this account is + * the one receiving its invites. + * + * This is the symptom's own screen. A user whose invites are landing on a + * second device sees an empty group list and no new requests — exactly what + * "nobody has invited me" looks like — so without a banner here the failure is + * indistinguishable from nothing happening, and the diagnosis sits behind a + * settings row nobody has a reason to open. + * + * Publishing stays a tap, never automatic: an inviter takes the newest + * KeyPackage, so a banner that republished itself on sight would deadlock the + * two installs against each other, each device's correction being the other's + * trigger. + */ +@Composable +fun MarmotInviteDeviceBanner( + accountViewModel: AccountViewModel, + modifier: Modifier = Modifier, +) { + var owner by remember { mutableStateOf(null) } + var dismissed by remember { mutableStateOf(false) } + val scope = rememberCoroutineScope() + val context = LocalContext.current + + LaunchedEffect(Unit) { + try { + owner = withContext(Dispatchers.IO) { accountViewModel.latestKeyPackageOwner() } + } catch (e: CancellationException) { + throw e + } catch (_: Exception) { + // A relay we cannot reach is not evidence of anything, so it stays + // silent rather than accusing a device that may well be this one. + owner = null + } + } + + AnimatedVisibility(visible = owner == LatestKeyPackageOwner.OTHER_DEVICE && !dismissed) { + Row( + modifier = + modifier + .fillMaxWidth() + .padding(horizontal = 10.dp, vertical = 4.dp) + .clip(RoundedCornerShape(8.dp)) + .background(MaterialTheme.colorScheme.errorContainer) + .padding(start = 10.dp, end = 4.dp, top = 6.dp, bottom = 6.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon( + symbol = MaterialSymbols.Warning, + contentDescription = null, + tint = MaterialTheme.colorScheme.onErrorContainer, + modifier = Modifier.size(20.dp), + ) + Text( + text = stringRes(Res.string.marmot_invite_device_banner), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onErrorContainer, + modifier = Modifier.padding(start = 8.dp).weight(1f), + ) + TextButton( + onClick = { + // Hidden optimistically: the publish makes this device the + // newest KeyPackage, so re-asking the relays to learn what + // we just did would only add a round trip. + owner = LatestKeyPackageOwner.THIS_DEVICE + scope.launch(Dispatchers.IO) { + val successMessage = stringRes(context, R.string.marmot_invite_device_success) + try { + accountViewModel.publishMarmotKeyPackage() + launch(Dispatchers.Main) { + Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() + } + } catch (e: Exception) { + val failureMessage = + stringRes(context, R.string.marmot_invite_device_failure, e.message ?: "") + launch(Dispatchers.Main) { + Toast.makeText(context, failureMessage, Toast.LENGTH_LONG).show() + // The warning was right after all, so put it back. + owner = LatestKeyPackageOwner.OTHER_DEVICE + } + } + } + }, + ) { + Text( + text = stringRes(Res.string.marmot_invite_device_publish), + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.onErrorContainer, + ) + } + IconButton(onClick = { dismissed = true }, modifier = Modifier.size(32.dp)) { + Icon( + symbol = MaterialSymbols.Close, + contentDescription = stringRes(Res.string.marmot_invite_device_banner_dismiss), + tint = MaterialTheme.colorScheme.onErrorContainer, + modifier = Modifier.size(18.dp), + ) + } + } + } +} diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index e72dde98a1..eed9024d54 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1622,6 +1622,8 @@ Search settings No settings found for "%1$s" Marmot Invite Device + Marmot invites are going to another device and cannot be opened here. + Dismiss Asking your relays which device currently receives your Marmot invites\u2026 This device already receives your Marmot invites. Nothing to do \u2014 publishing again is harmless but unnecessary. Another device signed in to this account published a newer KeyPackage, so Marmot invites are going there and cannot be opened here. Publishing from this device takes over future invites; the other device keeps the groups it already joined. From b65c508f56aa17a6ba4ae5f4602fb74aafb74915 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 13:41:14 +0000 Subject: [PATCH 04/10] polish: make the Marmot groups list read like the DM list next to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list sorted by newestMessage.createdAt() and then never showed it, putting a message count in the trailing slot instead — a number no reader asked for, leaving the ordering looking arbitrary. It now shows the timestamp, as ChatroomHeaderCompose already does for DMs. Preview line no longer goes blank. A system row keeps its state in tags and MIP-04 media keeps its in imeta, so `event.content` was empty for both; they now resolve through the detectors that already existed (MarmotAppEvent.KIND_SYSTEM, hasMip04Media/hasEncryptedMediaV2). A sender prefix is added for other people's messages, but only when metadataOrNull()?.bestName() gives a real name — toBestDisplayName() falls back to a hex stub, and "a1b2c3d4: hi" is noise, not attribution. System rows get no prefix; their caption already names the actor. The unread pill was hand-rolled here and strictly worse than the one in the next package: fixed .size(22.dp) clipped "99+", a hardcoded 11.sp ignored the reader's font scale, and it carried no content description. Extracted ConcordUnreadBadge's implementation to a shared ChatUnreadBadge and pointed both at it, rather than leaving a third copy. Concord's public API and its own plural are unchanged. Also: a "Create a group" button in the empty state, but only on the Known tab — no action fills New Requests, so a button there would be a dead end dressed as a next step; Size55dp instead of the literal; dividers between rows rather than after the last one. MarmotInviteDeviceBanner, per the same review: errorContainer -> surfaceVariant, since it is an explanation most users never see and the error palette made it the loudest thing on the screen; and its cramped single Row became message-above / action-below, which was a wrap risk at phone width with large font scales. R.plurals.marmot_message_count is now unused. Left in place rather than deleted: it is Crowdin-managed and CLAUDE.md warns off hand-editing those. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../screen/loggedIn/chats/ChatUnreadBadge.kt | 82 +++++++++++ .../marmotGroup/MarmotGroupListScreen.kt | 132 ++++++++++++------ .../marmotGroup/MarmotInviteDeviceBanner.kt | 118 +++++++++------- .../concord/ConcordUnreadBadge.kt | 52 ++----- amethyst/src/main/res/values/strings.xml | 4 + .../composeResources/values/strings.xml | 5 + 6 files changed, 257 insertions(+), 136 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/ChatUnreadBadge.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/ChatUnreadBadge.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/ChatUnreadBadge.kt new file mode 100644 index 0000000000..ac6e6067f9 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/ChatUnreadBadge.kt @@ -0,0 +1,82 @@ +/* + * 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 + +import androidx.compose.animation.animateContentSize +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.sizeIn +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.unit.dp + +/** Counts above this render as "N+" so a very busy room doesn't blow out the row. */ +const val CHAT_UNREAD_CAP = 99 + +/** + * The unread pill every chat list shares: Concord channels, Marmot groups. + * + * Sizing is `sizeIn` + padding rather than a fixed `size`, so "99+" widens the + * pill instead of being clipped by it, and the label rides `labelSmall` rather + * than a hardcoded sp so it still tracks the reader's font scale. + * + * [contentDescription] is the caller's, because the plural that reads well out + * loud is per-surface ("3 unread messages" vs "3 unread invites"); the pill + * itself is the same object everywhere. + */ +@Composable +fun ChatUnreadBadge( + count: Int, + contentDescription: String, + modifier: Modifier = Modifier, +) { + if (count <= 0) return + val label = if (count > CHAT_UNREAD_CAP) "$CHAT_UNREAD_CAP+" else count.toString() + Box( + modifier = + modifier + .semantics { this.contentDescription = contentDescription } + // Smoothly grows/shrinks as the count changes digits (1 → 2 → … → 99+) instead of + // snapping — a small touch that makes new activity feel noticed. + .animateContentSize() + .sizeIn(minWidth = 20.dp, minHeight = 20.dp) + .clip(CircleShape) + .background(MaterialTheme.colorScheme.primary) + .padding(horizontal = 6.dp, vertical = 2.dp), + contentAlignment = Alignment.Center, + ) { + Text( + text = label, + style = MaterialTheme.typography.labelSmall, + fontWeight = FontWeight.Bold, + color = MaterialTheme.colorScheme.onPrimary, + ) + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt index fb99a7d87c..9faaa3e6b7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt @@ -20,7 +20,6 @@ */ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup -import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -31,8 +30,9 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.CircleShape +import androidx.compose.material3.Button import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.HorizontalDivider @@ -52,13 +52,11 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.clip import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp -import androidx.compose.ui.unit.sp import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon @@ -66,6 +64,7 @@ import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.model.marmotGroups.MarmotGroupChatroom import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.marmot_create_group +import com.vitorpamplona.amethyst.commons.resources.marmot_empty_create_action import com.vitorpamplona.amethyst.commons.resources.marmot_group_fallback_name import com.vitorpamplona.amethyst.commons.resources.marmot_groups_title import com.vitorpamplona.amethyst.commons.resources.marmot_no_groups @@ -73,6 +72,10 @@ import com.vitorpamplona.amethyst.commons.resources.marmot_no_groups_desc import com.vitorpamplona.amethyst.commons.resources.marmot_no_invitations import com.vitorpamplona.amethyst.commons.resources.marmot_no_invitations_desc import com.vitorpamplona.amethyst.commons.resources.marmot_no_messages_yet +import com.vitorpamplona.amethyst.commons.resources.marmot_preview_group_updated +import com.vitorpamplona.amethyst.commons.resources.marmot_preview_media +import com.vitorpamplona.amethyst.commons.resources.marmot_preview_no_text +import com.vitorpamplona.amethyst.commons.resources.marmot_preview_with_sender import com.vitorpamplona.amethyst.commons.resources.marmot_tab_known import com.vitorpamplona.amethyst.commons.resources.marmot_tab_known_count import com.vitorpamplona.amethyst.commons.resources.marmot_tab_new_requests @@ -81,9 +84,16 @@ import com.vitorpamplona.amethyst.ui.navigation.bottombars.FabBottomBarPadded import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.note.NonClickableUserPictures +import com.vitorpamplona.amethyst.ui.note.elements.TimeAgoStyle +import com.vitorpamplona.amethyst.ui.note.elements.ToggleableTimeAgoText import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.ChatUnreadBadge +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.hasEncryptedMediaV2 +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.hasMip04Media import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.header.DisplayUserSetAsSubject import com.vitorpamplona.amethyst.ui.stringRes +import com.vitorpamplona.amethyst.ui.theme.Size55dp +import com.vitorpamplona.quartz.marmot.foundation.appEvents.MarmotAppEvent import com.vitorpamplona.quartz.nip01Core.core.HexKey @OptIn(ExperimentalMaterial3Api::class) @@ -209,13 +219,25 @@ fun MarmotGroupListScreen( textAlign = TextAlign.Center, modifier = Modifier.padding(top = 4.dp), ) + // Only the Known tab gets a button. There is no action + // that fills the New Requests tab — you cannot make + // someone invite you — so offering one there would be a + // dead end dressed up as a next step. + if (selectedTab == 0) { + Button( + onClick = { nav.nav(Route.CreateMarmotGroup) }, + modifier = Modifier.padding(top = 20.dp), + ) { + Text(stringRes(Res.string.marmot_empty_create_action)) + } + } } } } else { LazyColumn( modifier = Modifier.fillMaxSize(), ) { - items(visibleGroups, key = { it.first }) { (groupId, chatroom) -> + itemsIndexed(visibleGroups, key = { _, it -> it.first }) { index, (groupId, chatroom) -> MarmotGroupListItem( groupId = groupId, chatroom = chatroom, @@ -224,7 +246,11 @@ fun MarmotGroupListScreen( nav.nav(Route.MarmotGroupChat(groupId)) }, ) - HorizontalDivider() + // Between rows only: a divider under the last one draws a + // line across empty space with nothing beneath it. + if (index < visibleGroups.lastIndex) { + HorizontalDivider() + } } } } @@ -269,6 +295,35 @@ fun MarmotGroupListItem( // to ~100 entries, so counting per recomposition is cheap. val unread = chatroom.messages.count { (it.createdAt() ?: Long.MIN_VALUE) > lastReadTime } + // A preview has to survive the messages that carry no text: a system row + // keeps its state in tags and MIP-04 media keeps its in imeta, so both used + // to render as a blank second line under the group name. + val myPubKey = accountViewModel.account.signer.pubKey + val previewEvent = newestMessage?.event + val previewBody = + when { + newestMessage == null -> stringRes(Res.string.marmot_no_messages_yet) + previewEvent == null -> stringRes(Res.string.marmot_preview_no_text) + previewEvent.kind == MarmotAppEvent.KIND_SYSTEM -> stringRes(Res.string.marmot_preview_group_updated) + hasMip04Media(previewEvent) || hasEncryptedMediaV2(previewEvent) -> stringRes(Res.string.marmot_preview_media) + previewEvent.content.isNotBlank() -> previewEvent.content + else -> stringRes(Res.string.marmot_preview_no_text) + } + // Only a genuinely known name earns the prefix: `toBestDisplayName` would + // fall back to a hex stub, and "a1b2c3d4: hi" is noise, not attribution. + val senderName = + previewEvent + ?.pubKey + ?.takeIf { it != myPubKey } + ?.let { accountViewModel.getUserIfExists(it)?.metadataOrNull()?.bestName() } + val previewText = + // A system caption already names its actor, so prefixing one would say it twice. + if (senderName != null && previewEvent?.kind != MarmotAppEvent.KIND_SYSTEM) { + stringRes(Res.string.marmot_preview_with_sender, senderName, previewBody) + } else { + previewBody + } + Row( modifier = Modifier @@ -281,7 +336,7 @@ fun MarmotGroupListItem( if (memberPubkeys.isNotEmpty()) { NonClickableUserPictures( userHexList = memberPubkeys, - size = 55.dp, + size = Size55dp, accountViewModel = accountViewModel, ) } @@ -309,47 +364,36 @@ fun MarmotGroupListItem( overflow = TextOverflow.Ellipsis, ) } - if (newestMessage != null) { - Text( - text = newestMessage.event?.content ?: "", - style = MaterialTheme.typography.bodySmall, + Text( + text = previewText, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(top = 2.dp), + ) + } + // The list is sorted by newestMessage.createdAt(), so showing that time + // is what makes the ordering legible; the message count it replaced was + // a number no reader was asking for. + Column( + horizontalAlignment = Alignment.End, + verticalArrangement = Arrangement.spacedBy(4.dp), + ) { + newestMessage?.createdAt()?.let { createdAt -> + ToggleableTimeAgoText( + timestamp = createdAt, + style = TimeAgoStyle.Short, color = MaterialTheme.colorScheme.onSurfaceVariant, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.padding(top = 2.dp), - ) - } else { - Text( - text = stringRes(Res.string.marmot_no_messages_yet), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(top = 2.dp), + // The row is the tap target. A toggleable timestamp would + // swallow taps meant to open the group. + toggleable = false, ) } - } - Column(horizontalAlignment = Alignment.End) { if (unread > 0) { - Box( - modifier = - Modifier - .size(22.dp) - .clip(CircleShape) - .background(MaterialTheme.colorScheme.primary), - contentAlignment = Alignment.Center, - ) { - Text( - text = if (unread > 99) "99+" else unread.toString(), - color = MaterialTheme.colorScheme.onPrimary, - fontSize = 11.sp, - fontWeight = FontWeight.Bold, - textAlign = TextAlign.Center, - ) - } - } else { - Text( - text = pluralStringResource(R.plurals.marmot_message_count, chatroom.messages.size, chatroom.messages.size), - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, + ChatUnreadBadge( + count = unread, + contentDescription = pluralStringResource(R.plurals.marmot_unread_messages, unread, unread), ) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt index f2d97bcc27..193eb62070 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt @@ -23,6 +23,8 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup import android.widget.Toast import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -97,66 +99,78 @@ fun MarmotInviteDeviceBanner( } AnimatedVisibility(visible = owner == LatestKeyPackageOwner.OTHER_DEVICE && !dismissed) { - Row( + // Message on its own line, actions under it. All three in one Row fit + // only on a wide screen with default font scale; at phone width with + // large fonts the label and the button fought for the same space. + // + // `surfaceVariant`, not `errorContainer`: this is an explanation, and + // most users will never see it. The error palette would make it the + // loudest thing on a screen whose actual subject is the group list. + Column( modifier = modifier .fillMaxWidth() .padding(horizontal = 10.dp, vertical = 4.dp) .clip(RoundedCornerShape(8.dp)) - .background(MaterialTheme.colorScheme.errorContainer) - .padding(start = 10.dp, end = 4.dp, top = 6.dp, bottom = 6.dp), - verticalAlignment = Alignment.CenterVertically, + .background(MaterialTheme.colorScheme.surfaceVariant) + .padding(start = 10.dp, end = 4.dp, top = 8.dp, bottom = 4.dp), ) { - Icon( - symbol = MaterialSymbols.Warning, - contentDescription = null, - tint = MaterialTheme.colorScheme.onErrorContainer, - modifier = Modifier.size(20.dp), - ) - Text( - text = stringRes(Res.string.marmot_invite_device_banner), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onErrorContainer, - modifier = Modifier.padding(start = 8.dp).weight(1f), - ) - TextButton( - onClick = { - // Hidden optimistically: the publish makes this device the - // newest KeyPackage, so re-asking the relays to learn what - // we just did would only add a round trip. - owner = LatestKeyPackageOwner.THIS_DEVICE - scope.launch(Dispatchers.IO) { - val successMessage = stringRes(context, R.string.marmot_invite_device_success) - try { - accountViewModel.publishMarmotKeyPackage() - launch(Dispatchers.Main) { - Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() - } - } catch (e: Exception) { - val failureMessage = - stringRes(context, R.string.marmot_invite_device_failure, e.message ?: "") - launch(Dispatchers.Main) { - Toast.makeText(context, failureMessage, Toast.LENGTH_LONG).show() - // The warning was right after all, so put it back. - owner = LatestKeyPackageOwner.OTHER_DEVICE + Row(verticalAlignment = Alignment.Top) { + Icon( + symbol = MaterialSymbols.Warning, + contentDescription = null, + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(20.dp), + ) + Text( + text = stringRes(Res.string.marmot_invite_device_banner), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(start = 8.dp, end = 4.dp).weight(1f), + ) + IconButton(onClick = { dismissed = true }, modifier = Modifier.size(24.dp)) { + Icon( + symbol = MaterialSymbols.Close, + contentDescription = stringRes(Res.string.marmot_invite_device_banner_dismiss), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(18.dp), + ) + } + } + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.End, + ) { + TextButton( + onClick = { + // Hidden optimistically: the publish makes this device the + // newest KeyPackage, so re-asking the relays to learn what + // we just did would only add a round trip. + owner = LatestKeyPackageOwner.THIS_DEVICE + scope.launch(Dispatchers.IO) { + val successMessage = stringRes(context, R.string.marmot_invite_device_success) + try { + accountViewModel.publishMarmotKeyPackage() + launch(Dispatchers.Main) { + Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() + } + } catch (e: Exception) { + val failureMessage = + stringRes(context, R.string.marmot_invite_device_failure, e.message ?: "") + launch(Dispatchers.Main) { + Toast.makeText(context, failureMessage, Toast.LENGTH_LONG).show() + // The warning was right after all, so put it back. + owner = LatestKeyPackageOwner.OTHER_DEVICE + } } } - } - }, - ) { - Text( - text = stringRes(Res.string.marmot_invite_device_publish), - style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.onErrorContainer, - ) - } - IconButton(onClick = { dismissed = true }, modifier = Modifier.size(32.dp)) { - Icon( - symbol = MaterialSymbols.Close, - contentDescription = stringRes(Res.string.marmot_invite_device_banner_dismiss), - tint = MaterialTheme.colorScheme.onErrorContainer, - modifier = Modifier.size(18.dp), - ) + }, + ) { + Text( + text = stringRes(Res.string.marmot_invite_device_publish), + style = MaterialTheme.typography.labelMedium, + ) + } } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordUnreadBadge.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordUnreadBadge.kt index 7b321b32af..fa55d55f02 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordUnreadBadge.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordUnreadBadge.kt @@ -20,32 +20,21 @@ */ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.concord -import androidx.compose.animation.animateContentSize -import androidx.compose.foundation.background -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.sizeIn -import androidx.compose.foundation.shape.CircleShape -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.clip import androidx.compose.ui.res.pluralStringResource -import androidx.compose.ui.semantics.contentDescription -import androidx.compose.ui.semantics.semantics -import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.R - -/** Counts above this render as "N+" so a very busy channel doesn't blow out the row. */ -private const val CONCORD_UNREAD_CAP = 99 +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.CHAT_UNREAD_CAP +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.ChatUnreadBadge /** * A small pill showing an unread-message [count] (new messages since this account last read). * Renders nothing when [count] is zero, so callers can place it unconditionally. Capped at - * [CONCORD_UNREAD_CAP]+ and carries a plural content description for screen readers. + * [CHAT_UNREAD_CAP]+ and carries a plural content description for screen readers. + * + * The pill itself is [ChatUnreadBadge], shared with the other chat lists; this + * wrapper only supplies Concord's own plural so the two surfaces cannot drift + * apart visually. */ @Composable fun ConcordUnreadBadge( @@ -53,26 +42,9 @@ fun ConcordUnreadBadge( modifier: Modifier = Modifier, ) { if (count <= 0) return - val label = if (count > CONCORD_UNREAD_CAP) "$CONCORD_UNREAD_CAP+" else count.toString() - val description = pluralStringResource(R.plurals.concord_unread_messages, count, count) - Box( - modifier = - modifier - .semantics { contentDescription = description } - // Smoothly grows/shrinks as the count changes digits (1 → 2 → … → 99+) instead of - // snapping — a small touch that makes new activity feel noticed. - .animateContentSize() - .sizeIn(minWidth = 20.dp, minHeight = 20.dp) - .clip(CircleShape) - .background(MaterialTheme.colorScheme.primary) - .padding(horizontal = 6.dp, vertical = 2.dp), - contentAlignment = Alignment.Center, - ) { - Text( - text = label, - style = MaterialTheme.typography.labelSmall, - fontWeight = FontWeight.Bold, - color = MaterialTheme.colorScheme.onPrimary, - ) - } + ChatUnreadBadge( + count = count, + contentDescription = pluralStringResource(R.plurals.concord_unread_messages, count, count), + modifier = modifier, + ) } diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index b6f6a248e4..58c98afb55 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -2709,6 +2709,10 @@ Nowhere Forum + + %1$d new message + %1$d new messages + %1$d msg %1$d msgs diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index eed9024d54..2cdc1b80a9 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1621,6 +1621,11 @@ User Preferences Search settings No settings found for "%1$s" + Group updated + Attachment + Message + %1$s: %2$s + Create a group Marmot Invite Device Marmot invites are going to another device and cannot be opened here. Dismiss From 6a0d33b090f68885a94be1f03f75ac7ca0486d08 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 16:15:56 +0000 Subject: [PATCH 05/10] fix: audit findings on the Marmot invite-device work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from a review of this branch, all confirmed against the source before changing anything. 1. The success toast fired on every failure path. publishMarmotKeyPackage early-returns in silence for a read-only account or an empty relay set, and client.publish is fire-and-forget, so "This device now receives your Marmot invites" appeared even when nothing was published or a relay rejected it — from both the banner and an un-gated settings row. republishKeyPackageConfirmed() now waits for a relay OK via publishAndConfirm and returns whether one accepted; callers report that instead. A new string covers the rejected case. The startup path keeps fire-and-forget on purpose: making boot wait on relays is a worse trade. The banner also hides for read-only logins rather than offering an action that cannot work. 2. A regression from the previous commit: the media branch was tested before content.isNotBlank(), so a captioned attachment previewed as "Attachment" instead of the caption — worse than the raw event.content it replaced. Text wins now; "Attachment" is the empty-media fallback. 3. Sender names never arrived. metadataOrNull()?.bestName() is a StateFlow.value read, not a snapshot read, so a kind:0 landing after the row composed never reached it, and nothing requested that kind:0 either. Now reads through observeUserInfo, which subscribes and recomposes. 4. Entering the groups screen fanned a 30s-idle-window fetchAll across the whole write set every time, and `remember` meant a rotation re-ran it and un-dismissed the banner. The owner check takes a maxAgeSeconds and caches its answer; the passive banner reuses it for 15 minutes while the settings dialog still forces a fresh read. `dismissed` is now rememberSaveable. Also drops a dead layout.size import. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../amethyst/model/AccountMarmotActions.kt | 47 ++++++++++++++++++- .../ui/screen/loggedIn/AccountViewModel.kt | 15 +++++- .../marmotGroup/MarmotGroupListScreen.kt | 26 +++++++--- .../marmotGroup/MarmotInviteDeviceBanner.kt | 41 +++++++++++++--- .../loggedIn/settings/AllSettingsScreen.kt | 9 +++- amethyst/src/main/res/values/strings.xml | 1 + 6 files changed, 120 insertions(+), 19 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index 22abf73cac..59b12a79cc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -31,9 +31,11 @@ import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageFetcher import com.vitorpamplona.quartz.marmot.protocolCore.GroupLifecycleState import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.publishAndConfirm import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.utils.Log +import com.vitorpamplona.quartz.utils.TimeUtils import kotlin.coroutines.cancellation.CancellationException /** @@ -68,6 +70,9 @@ enum class LatestKeyPackageOwner { class AccountMarmotActions( private val account: Account, ) { + /** Last (timestamp, answer) from [latestKeyPackageOwner], for passive callers. */ + private var lastOwnerCheck: Pair? = null + /** * Resolve the relay set for a Marmot group. Prefer the relays carried in * the MLS GroupContext metadata so every member converges on the same @@ -435,11 +440,20 @@ class AccountMarmotActions( * NIP-65 write set and our legacy kind:10051 with the inviter's outbox, and * the first two are exactly [keyPackagePublishRelays]. */ - suspend fun latestKeyPackageOwner(): LatestKeyPackageOwner { + suspend fun latestKeyPackageOwner(maxAgeSeconds: Long = 0L): LatestKeyPackageOwner { val manager = account.marmotManager ?: return LatestKeyPackageOwner.NONE val relays = keyPackagePublishRelays() if (relays.isEmpty()) return LatestKeyPackageOwner.NONE + // A passive caller (the groups-screen banner) may reuse a recent answer. + // Without this, every entry to that screen fanned a REQ out across the + // whole write set — and the thing it asks about only changes when + // another device publishes, which is rare enough to cache. + val cached = lastOwnerCheck + if (maxAgeSeconds > 0 && cached != null && TimeUtils.now() - cached.first <= maxAgeSeconds) { + return cached.second + } + val latest = KeyPackageFetcher.fetchKeyPackage(account.client, account.signer.pubKey, relays) ?: return LatestKeyPackageOwner.NONE @@ -448,7 +462,36 @@ class AccountMarmotActions( Log.d("MarmotDbg") { "latestKeyPackageOwner: newest KeyPackage id=${latest.id.take(8)}… createdAt=${latest.createdAt} mine=$mine" } - return if (mine) LatestKeyPackageOwner.THIS_DEVICE else LatestKeyPackageOwner.OTHER_DEVICE + val owner = if (mine) LatestKeyPackageOwner.THIS_DEVICE else LatestKeyPackageOwner.OTHER_DEVICE + lastOwnerCheck = TimeUtils.now() to owner + return owner + } + + /** + * The user-initiated republish behind the invite-device banner and the + * settings row. Returns whether a relay actually accepted the KeyPackage. + * + * Deliberately not [publishMarmotKeyPackage]. That one is the best-effort + * startup path: it early-returns in silence for a read-only account or an + * empty relay set and then hands the event to a fire-and-forget + * `client.publish`, so a caller reporting the outcome to someone watching + * would call every one of those failures a success. + */ + suspend fun republishKeyPackageConfirmed(): Boolean { + val manager = account.marmotManager ?: return false + if (!account.isWriteable()) return false + val relays = keyPackagePublishRelays() + if (relays.isEmpty()) return false + + val event = manager.generateKeyPackageEvent(relays.toList()) + account.cache.justConsumeMyOwnEvent(event) + val accepted = account.client.publishAndConfirm(event, relays) + Log.d("MarmotDbg") { + "republishKeyPackageConfirmed: id=${event.id.take(8)}… accepted=$accepted on ${relays.size} relay(s)" + } + // The answer we just changed; a stale cache would keep the banner up. + lastOwnerCheck = if (accepted) TimeUtils.now() to LatestKeyPackageOwner.THIS_DEVICE else null + return accepted } /** diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 7175a94194..41f5429181 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -2505,8 +2505,19 @@ class AccountViewModel( suspend fun hasPublishedKeyPackage(): Boolean = account.marmot.hasPublishedKeyPackage() - /** Which install currently owns this account's Marmot invites. See [LatestKeyPackageOwner]. */ - suspend fun latestKeyPackageOwner(): LatestKeyPackageOwner = account.marmot.latestKeyPackageOwner() + /** + * Which install currently owns this account's Marmot invites. See [LatestKeyPackageOwner]. + * + * [maxAgeSeconds] lets a passive caller reuse a recent answer instead of + * fanning a REQ across the write set; 0 always asks the relays. + */ + suspend fun latestKeyPackageOwner(maxAgeSeconds: Long = 0L): LatestKeyPackageOwner = account.marmot.latestKeyPackageOwner(maxAgeSeconds) + + /** Republishes this device's KeyPackage; true only when a relay accepted it. */ + suspend fun republishKeyPackage(): Boolean = account.marmot.republishKeyPackageConfirmed() + + /** False for a read-only (pubkey-only) login, which cannot publish at all. */ + fun canPublish(): Boolean = account.isWriteable() /** * Whether this account has a kind:10051 KeyPackage Relay List (MIP-00) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt index 9faaa3e6b7..081ac4ee1f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt @@ -28,7 +28,6 @@ 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.layout.size import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.CircleShape @@ -80,6 +79,7 @@ import com.vitorpamplona.amethyst.commons.resources.marmot_tab_known import com.vitorpamplona.amethyst.commons.resources.marmot_tab_known_count import com.vitorpamplona.amethyst.commons.resources.marmot_tab_new_requests import com.vitorpamplona.amethyst.commons.resources.marmot_tab_new_requests_count +import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.observeUserInfo import com.vitorpamplona.amethyst.ui.navigation.bottombars.FabBottomBarPadded import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.Route @@ -305,17 +305,31 @@ fun MarmotGroupListItem( newestMessage == null -> stringRes(Res.string.marmot_no_messages_yet) previewEvent == null -> stringRes(Res.string.marmot_preview_no_text) previewEvent.kind == MarmotAppEvent.KIND_SYSTEM -> stringRes(Res.string.marmot_preview_group_updated) - hasMip04Media(previewEvent) || hasEncryptedMediaV2(previewEvent) -> stringRes(Res.string.marmot_preview_media) + // Text first: an attachment usually carries a caption, and showing + // "Attachment" over the words the sender actually wrote would be a + // step back from the raw `content` this replaced. previewEvent.content.isNotBlank() -> previewEvent.content + hasMip04Media(previewEvent) || hasEncryptedMediaV2(previewEvent) -> stringRes(Res.string.marmot_preview_media) else -> stringRes(Res.string.marmot_preview_no_text) } - // Only a genuinely known name earns the prefix: `toBestDisplayName` would - // fall back to a hex stub, and "a1b2c3d4: hi" is noise, not attribution. - val senderName = + // Only a genuinely known name earns the prefix: `bestName` returns null + // without metadata, and "a1b2c3d4: hi" is noise, not attribution. + // + // Read through `observeUserInfo`, not `metadataOrNull()`: the latter is a + // plain StateFlow.value read, so a kind:0 arriving after the row composed + // would never reach it — and nothing would have asked for that kind:0 + // either. This both subscribes and recomposes when it lands. + val senderUser = previewEvent ?.pubKey ?.takeIf { it != myPubKey } - ?.let { accountViewModel.getUserIfExists(it)?.metadataOrNull()?.bestName() } + ?.let { accountViewModel.getUserIfExists(it) } + val senderName = + if (senderUser != null) { + observeUserInfo(senderUser, accountViewModel).value?.info?.bestName() + } else { + null + } val previewText = // A system caption already names its actor, so prefixing one would say it twice. if (senderName != null && previewEvent?.kind != MarmotAppEvent.KIND_SYSTEM) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt index 193eb62070..6819dc31a9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt @@ -40,6 +40,7 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -61,6 +62,12 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlin.coroutines.cancellation.CancellationException +/** + * How long a passive owner check may reuse its previous answer. What it reports + * only changes when another device publishes, which no user does often. + */ +private const val OWNER_CHECK_MAX_AGE_SECONDS = 15L * 60L + /** * Warns, on the Marmot groups screen, that another install of this account is * the one receiving its invites. @@ -82,13 +89,25 @@ fun MarmotInviteDeviceBanner( modifier: Modifier = Modifier, ) { var owner by remember { mutableStateOf(null) } - var dismissed by remember { mutableStateOf(false) } + // Saveable: a rotation is not a decision to un-dismiss a warning the user + // has already read and waved away. + var dismissed by rememberSaveable { mutableStateOf(false) } val scope = rememberCoroutineScope() val context = LocalContext.current + // A read-only login cannot publish at all, so the only action this banner + // offers is impossible for it. Better to say nothing than to offer a button + // that silently does nothing. + val canPublish = accountViewModel.canPublish() LaunchedEffect(Unit) { + if (!canPublish) return@LaunchedEffect try { - owner = withContext(Dispatchers.IO) { accountViewModel.latestKeyPackageOwner() } + owner = + withContext(Dispatchers.IO) { + // Passive check: reuse a recent answer rather than fanning a + // REQ across the whole write set on every entry to this screen. + accountViewModel.latestKeyPackageOwner(maxAgeSeconds = OWNER_CHECK_MAX_AGE_SECONDS) + } } catch (e: CancellationException) { throw e } catch (_: Exception) { @@ -143,16 +162,24 @@ fun MarmotInviteDeviceBanner( ) { TextButton( onClick = { - // Hidden optimistically: the publish makes this device the - // newest KeyPackage, so re-asking the relays to learn what - // we just did would only add a round trip. + // Hidden optimistically, but only *stays* hidden once a + // relay has accepted: `republishKeyPackage` waits for the + // OK, so a rejection, a read-only account or an empty relay + // set come back false instead of being reported as the + // success they are not. owner = LatestKeyPackageOwner.THIS_DEVICE scope.launch(Dispatchers.IO) { val successMessage = stringRes(context, R.string.marmot_invite_device_success) + val rejectedMessage = stringRes(context, R.string.marmot_invite_device_rejected) try { - accountViewModel.publishMarmotKeyPackage() + val accepted = accountViewModel.republishKeyPackage() launch(Dispatchers.Main) { - Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() + if (accepted) { + Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() + } else { + Toast.makeText(context, rejectedMessage, Toast.LENGTH_LONG).show() + owner = LatestKeyPackageOwner.OTHER_DEVICE + } } } catch (e: Exception) { val failureMessage = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt index 3a4aa26f90..55449775e4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt @@ -185,10 +185,15 @@ fun AllSettingsScreen( showInviteDeviceDialog = false scope.launch(Dispatchers.IO) { val successMessage = stringRes(context, R.string.marmot_invite_device_success) + val rejectedMessage = stringRes(context, R.string.marmot_invite_device_rejected) try { - accountViewModel.publishMarmotKeyPackage() + // Waits for a relay OK. The fire-and-forget publish this + // replaced reported success for a read-only account, an + // empty relay set and an outright rejection alike. + val accepted = accountViewModel.republishKeyPackage() launch(Dispatchers.Main) { - Toast.makeText(context, successMessage, Toast.LENGTH_SHORT).show() + val message = if (accepted) successMessage else rejectedMessage + Toast.makeText(context, message, if (accepted) Toast.LENGTH_SHORT else Toast.LENGTH_LONG).show() } } catch (e: Exception) { val failureMessage = diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 58c98afb55..095f28bf96 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1352,6 +1352,7 @@ mls, marmot, keypackage, key package, invite, device, group chat This device now receives your Marmot invites. Failed to publish KeyPackage: %1$s + No relay accepted the KeyPackage. Check your relay settings and try again. Reset Marmot State Marmot state reset. Failed to reset Marmot state: %1$s From 13557a0a154fddabe3b5a9e181c266cc50c845a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 17:27:24 +0000 Subject: [PATCH 06/10] fix: second-round audit findings on the invite-device work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of these are defects in the previous commit's fixes. 1. republishKeyPackageConfirmed() minted before asking the relays, but generateCurrentProfileKeyPackage overwrites activeBundles[slot] and discards the private keys of the KeyPackage already published. A rejected republish therefore left the old KeyPackage on relays with its keys gone and every invite against it unopenable. The damage case is pressing the button when this device ALREADY owns the newest — which the UI string called "harmless". That case is now a no-op reporting success truthfully, and the string no longer claims harmlessness. When another device owns the newest, minting is still required, and the bundle it replaces was not one an inviter would have picked anyway. 2. ownsKeyPackageEvent() matched on the Nostr event id, but findBundleByEventId resolves an id to a SLOT and returns whatever bundle occupies it now, so a regenerated slot reports a stale id as still ours — hiding the warning while invites silently fail. The KDoc claimed the answer "cannot disagree with what actually happens on arrival"; it can, since the Welcome path matches findBundleByRef first. Now ownsKeyPackage(event) matches the MLS KeyPackage reference, a hash over the KeyPackage that cannot alias, falling back to the event id only for events carrying no ref tag. 3. The read-only guard was on the banner but not the settings dialog, so a pubkey-only login was told "No relay accepted the KeyPackage. Check your relay settings" for a refusal that happened at the isWriteable() check before any relay was contacted. 4. The owner-check cache was written only after a successful fetch, so the NONE result re-fanned a 30s fetchAll across the whole write set on every entry — for exactly the accounts with nothing on their relays. Not addressed here: generateKeyPackage dropping the superseded bundle is a quartz-level flaw that the startup rotation path shares. Fixing it means retaining superseded bundles in KeyPackageRotationManager's persisted snapshot, which every user's Welcome handling depends on — a maintainer decision, not a UI branch change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../amethyst/model/AccountMarmotActions.kt | 30 +++++++++++++++---- .../loggedIn/settings/AllSettingsScreen.kt | 12 ++++++-- .../amethyst/commons/marmot/MarmotManager.kt | 26 +++++++++++----- .../composeResources/values/strings.xml | 3 +- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index 59b12a79cc..b016167eaa 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -454,15 +454,21 @@ class AccountMarmotActions( return cached.second } - val latest = - KeyPackageFetcher.fetchKeyPackage(account.client, account.signer.pubKey, relays) - ?: return LatestKeyPackageOwner.NONE + val latest = KeyPackageFetcher.fetchKeyPackage(account.client, account.signer.pubKey, relays) - val mine = manager.ownsKeyPackageEvent(latest.id) + val owner = + when { + latest == null -> LatestKeyPackageOwner.NONE + manager.ownsKeyPackage(latest) -> LatestKeyPackageOwner.THIS_DEVICE + else -> LatestKeyPackageOwner.OTHER_DEVICE + } Log.d("MarmotDbg") { - "latestKeyPackageOwner: newest KeyPackage id=${latest.id.take(8)}… createdAt=${latest.createdAt} mine=$mine" + "latestKeyPackageOwner: newest KeyPackage id=${latest?.id?.take(8)}… " + + "createdAt=${latest?.createdAt} owner=$owner" } - val owner = if (mine) LatestKeyPackageOwner.THIS_DEVICE else LatestKeyPackageOwner.OTHER_DEVICE + // Cached even when nothing was found: that answer cost the same fan-out + // as any other, so leaving it uncached would re-run the whole query on + // every entry for exactly the accounts with nothing on their relays. lastOwnerCheck = TimeUtils.now() to owner return owner } @@ -483,6 +489,18 @@ class AccountMarmotActions( val relays = keyPackagePublishRelays() if (relays.isEmpty()) return false + // Minting is destructive: `generateCurrentProfileKeyPackage` overwrites + // `activeBundles[slot]`, dropping the private keys of the KeyPackage + // already on relays. That is survivable when another device owns the + // newest one (nobody was going to invite us through ours anyway), but + // doing it when we ALREADY own the newest would destroy the very keys + // the current invites depend on — for no gain, since the answer would + // not change. So that case is a no-op that truthfully reports success. + if (latestKeyPackageOwner() == LatestKeyPackageOwner.THIS_DEVICE) { + Log.d("MarmotDbg") { "republishKeyPackageConfirmed: already the newest; not minting" } + return true + } + val event = manager.generateKeyPackageEvent(relays.toList()) account.cache.justConsumeMyOwnEvent(event) val accepted = account.client.publishAndConfirm(event, relays) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt index 55449775e4..33f376ddf3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt @@ -61,6 +61,7 @@ import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_error import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_none import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_other import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_publish +import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_read_only import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_this import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_title import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_action @@ -345,8 +346,13 @@ private fun MarmotInviteDeviceDialog( ) { var owner by remember { mutableStateOf(null) } var checkFailed by remember { mutableStateOf(false) } + // A read-only login cannot publish at all. Without this the dialog would + // offer the button and then blame the relays for a refusal that happened at + // the isWriteable() check, before any relay was contacted. + val canPublish = accountViewModel.canPublish() LaunchedEffect(Unit) { + if (!canPublish) return@LaunchedEffect try { owner = withContext(Dispatchers.IO) { accountViewModel.latestKeyPackageOwner() } } catch (e: CancellationException) { @@ -361,6 +367,7 @@ private fun MarmotInviteDeviceDialog( val resolved = owner val body = when { + !canPublish -> stringRes(Res.string.marmot_invite_device_read_only) checkFailed -> stringRes(Res.string.marmot_invite_device_error) resolved == null -> stringRes(Res.string.marmot_invite_device_checking) resolved == LatestKeyPackageOwner.THIS_DEVICE -> stringRes(Res.string.marmot_invite_device_this) @@ -388,8 +395,9 @@ private fun MarmotInviteDeviceDialog( Button( onClick = onConfirm, // Held until the check resolves so the user is never asked to - // act on an answer that has not arrived. - enabled = checkFailed || resolved != null, + // act on an answer that has not arrived, and never offered at + // all to an account that cannot publish. + enabled = canPublish && (checkFailed || resolved != null), ) { Text(stringRes(Res.string.marmot_invite_device_publish)) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 14823c4eeb..3de761ffaf 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -2444,19 +2444,31 @@ class MarmotManager( suspend fun hasActiveKeyPackages(): Boolean = keyPackageRotationManager.hasActiveKeyPackages() /** - * True when the private bundle behind [eventId] — the Nostr id of a - * published kind:30443 — is one this install holds, active or retained. + * True when this install holds the private bundle for [event], a published + * kind:30443 — i.e. whether an invite issued against it could be opened here. * * Two installs of the same account each mint their own random d-tag slot * ([KeyPackageRotationManager.getOrCreateSlotDTag]), so their KeyPackages * never replace one another and both sit on relays indefinitely. An * inviter then takes whichever has the highest `created_at`, and only the - * install holding that bundle can open the resulting Welcome. This is how - * a device answers "would an invite against that KeyPackage reach me?" — - * deliberately the same lookup the Welcome path itself performs, so the - * answer cannot disagree with what actually happens on arrival. + * install holding that bundle can open the resulting Welcome. + * + * Matched on the MLS KeyPackage reference, which is a hash over the + * KeyPackage itself, and only on the Nostr event id when an event carries + * no ref tag. The id route alone is not safe to answer this question: + * [KeyPackageRotationManager.findBundleByEventId] resolves an id to a + * *slot* and then returns whatever bundle occupies that slot now, so once + * a slot has been regenerated it reports a stale id as still ours and we + * would hide a warning about invites we can no longer open. The ref cannot + * alias that way, and it is what the Welcome path matches on first. */ - suspend fun ownsKeyPackageEvent(eventId: HexKey): Boolean = keyPackageRotationManager.findBundleByEventId(eventId) != null + suspend fun ownsKeyPackage(event: KeyPackageEvent): Boolean { + val refHex = event.keyPackageRef() + if (refHex != null) { + return keyPackageRotationManager.findBundleByRef(refHex.hexToByteArray()) != null + } + return keyPackageRotationManager.findBundleByEventId(event.id) != null + } /** * Check if a specific group membership exists. diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index 2cdc1b80a9..e364ed95fd 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1630,7 +1630,8 @@ Marmot invites are going to another device and cannot be opened here. Dismiss Asking your relays which device currently receives your Marmot invites\u2026 - This device already receives your Marmot invites. Nothing to do \u2014 publishing again is harmless but unnecessary. + This device already receives your Marmot invites. Nothing to do. + This account is signed in read-only, so it cannot publish a KeyPackage. Sign in with your private key to receive Marmot invites on this device. Another device signed in to this account published a newer KeyPackage, so Marmot invites are going there and cannot be opened here. Publishing from this device takes over future invites; the other device keeps the groups it already joined. No KeyPackage was found on your relays for this account. Publish one so others can invite you to Marmot groups. Could not reach your relays to check. You can still publish from this device. From 8e921fcfacade204eb714fdaafdcf81662c7d9a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 17:48:49 +0000 Subject: [PATCH 07/10] fix: retain the KeyPackage a regenerated slot displaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit generateKeyPackage and generateCurrentProfileKeyPackage replaced activeBundles[slot] and dropped the displaced bundle's private keys — but neither does anything to the kind:30443 a previous generation was published as. That event keeps sitting on relays, so the account was left advertising a KeyPackage whose Welcomes it could no longer open. The user-initiated republish added earlier in this branch made that reachable from a button. All three install paths now route through installIntoSlotUnlocked, which moves the displaced bundle into retainedBundles keyed by the event ids it was published as. No snapshot format change: retainedBundles has been persisted since v5 and is already keyed by event id, so there is no version bump and no migration. Retention stays bounded by the two limits already in place — MAX_RETAINED_BUNDLES with oldest-first eviction, and pruning past each KeyPackage's own not_after. The displaced event ids also leave eventIdToSlot, which fixes the aliasing bug at its root rather than at the call site: findBundleByEventId resolved an id through the slot to whatever bundle occupied it now, so an older event handed out keys that cannot open the Welcome addressed to it. Covered by RegeneratedSlotRetentionTest — the event-id path, the findBundleByRef path the Welcome code tries first, and the aliasing regression. All three were confirmed to fail against the pre-fix manager before being kept. This is the first coverage this path has had. Trade worth naming: retaining unused superseded KeyPackages extends how long those private keys live, where they were previously destroyed at once. The alternative is advertising a KeyPackage we cannot open, and the bound is the one last-resort retention already accepted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../amethyst/model/AccountMarmotActions.kt | 14 +-- .../KeyPackageRotationManager.kt | 54 ++++++++- .../RegeneratedSlotRetentionTest.kt | 106 ++++++++++++++++++ 3 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/RegeneratedSlotRetentionTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index b016167eaa..148720a141 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -489,13 +489,13 @@ class AccountMarmotActions( val relays = keyPackagePublishRelays() if (relays.isEmpty()) return false - // Minting is destructive: `generateCurrentProfileKeyPackage` overwrites - // `activeBundles[slot]`, dropping the private keys of the KeyPackage - // already on relays. That is survivable when another device owns the - // newest one (nobody was going to invite us through ours anyway), but - // doing it when we ALREADY own the newest would destroy the very keys - // the current invites depend on — for no gain, since the answer would - // not change. So that case is a no-op that truthfully reports success. + // Minting no longer destroys the displaced bundle — the rotation + // manager retains it, keyed by the event id it was published as — but + // every regeneration still costs a keypair, a relay round trip, and a + // slot in the bounded retention map, where it can evict a bundle + // someone is about to invite us through. Republishing when we already + // own the newest KeyPackage buys none of that back, since the answer + // cannot change, so that case is a no-op reporting success truthfully. if (latestKeyPackageOwner() == LatestKeyPackageOwner.THIS_DEVICE) { Log.d("MarmotDbg") { "republishKeyPackageConfirmed: already the newest; not minting" } return true diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt index fa7f433319..d125828abf 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt @@ -91,9 +91,17 @@ class KeyPackageRotationManager( private val eventIdToSlot = mutableMapOf() /** - * Consumed KeyPackages we deliberately keep the private keys for, keyed by + * Published KeyPackages we deliberately keep the private keys for, keyed by * the Nostr event id (kind:30443) they were published as. * + * Two things land here. A KeyPackage a Welcome consumed, per the + * last-resort reasoning below; and a KeyPackage a slot regenerated out from + * under, which is retained for a plainer reason: regenerating replaces + * `activeBundles[slot]` but does nothing to the kind:30443 already sitting + * on relays, so dropping its keys would leave us advertising a KeyPackage + * no one can invite us through. Keeping it is what makes the advertisement + * honest. + * * A KeyPackage carrying the LastResort marker (`0x000A`) is not single-use: * OpenMLS skips `delete_key_package` for one, so MDK — which marks every * KeyPackage last-resort and caches the peer KeyPackage it resolved — will @@ -394,7 +402,7 @@ class KeyPackageRotationManager( val bundle = KeyPackageBundle(keyPackage, initKp.privateKey, encKp.privateKey, sigKp.privateKey) mutex.withLock { - activeBundles[dTagSlot] = bundle + installIntoSlotUnlocked(dTagSlot, bundle) persistUnlocked() } return bundle @@ -418,7 +426,7 @@ class KeyPackageRotationManager( ): KeyPackageBundle { val bundle = CurrentProfileGroupFactory.createKeyPackage(signer, ciphersuite = ciphersuite) mutex.withLock { - activeBundles[dTagSlot] = bundle + installIntoSlotUnlocked(dTagSlot, bundle) persistUnlocked() } return bundle @@ -578,10 +586,48 @@ class KeyPackageRotationManager( dTagSlot: String, bundle: KeyPackageBundle, ) = mutex.withLock { - activeBundles[dTagSlot] = bundle + installIntoSlotUnlocked(dTagSlot, bundle) persistUnlocked() } + /** + * Install [bundle] at [dTagSlot], retaining whatever it displaces. + * + * The displaced bundle's kind:30443 is still on relays — regenerating a + * slot publishes a NEW addressable event only once the caller sends it, and + * a send that never lands, or lands while a peer is already holding the + * older KeyPackage, leaves that older one live. So its keys move to + * [retainedBundles] rather than being dropped. + * + * Its event ids also leave [eventIdToSlot]. That index answers + * "which slot backs this id", and after this call the honest answer is + * "none" — leaving it would make [findBundleByEventId] resolve the id + * through the slot to the bundle that just *replaced* it, handing the + * Welcome path keys that cannot open the message it is holding. + * + * Caller must hold the mutex. + */ + private fun installIntoSlotUnlocked( + dTagSlot: String, + bundle: KeyPackageBundle, + ) { + val displaced = activeBundles.put(dTagSlot, bundle) + if (displaced == null) return + + val staleEventIds = eventIdToSlot.entries.filter { it.value == dTagSlot }.map { it.key } + for (staleEventId in staleEventIds) { + eventIdToSlot.remove(staleEventId) + // Re-inserted so the most recently displaced entry sorts last and + // survives eviction the longest, matching [consumeSlotUnlocked]. + retainedBundles.remove(staleEventId) + retainedBundles[staleEventId] = displaced + } + pruneExpiredRetainedUnlocked() + while (retainedBundles.size > MAX_RETAINED_BUNDLES) { + retainedBundles.remove(retainedBundles.keys.first()) + } + } + /** * Get the d-tag slots that need rotation (KeyPackage was consumed). */ diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/RegeneratedSlotRetentionTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/RegeneratedSlotRetentionTest.kt new file mode 100644 index 0000000000..4cacfb33cb --- /dev/null +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/RegeneratedSlotRetentionTest.kt @@ -0,0 +1,106 @@ +/* + * 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.quartz.marmot.mip00KeyPackages + +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertSame +import org.junit.Test + +/** + * Regenerating a slot must not strand the KeyPackage already on relays. + * + * `generateKeyPackage` replaces `activeBundles[slot]`, but it does nothing to + * the kind:30443 a previous generation was published as — that event keeps + * sitting on relays, and a peer can still invite through it. Dropping the + * displaced private keys therefore left the account advertising a KeyPackage + * whose Welcomes it could not open. + */ +class RegeneratedSlotRetentionTest { + private val identity = ByteArray(32) { 0x22 } + + @Test + fun aRegeneratedSlotKeepsTheKeysOfTheKeyPackageStillOnRelays() = + runBlocking { + val manager = KeyPackageRotationManager() + val slot = manager.getOrCreateSlotDTag("primary") + + val first = manager.generateKeyPackage(identity, slot) + manager.recordPublishedEventId(slot, FIRST_EVENT_ID) + + // The user republishes — or a consumed slot rotates — and the slot + // is minted afresh while the first KeyPackage is still published. + val second = manager.generateKeyPackage(identity, slot) + manager.recordPublishedEventId(slot, SECOND_EVENT_ID) + + val recoveredFirst = manager.findBundleByEventId(FIRST_EVENT_ID) + assertNotNull("the displaced bundle must survive regeneration", recoveredFirst) + assertSame("a Welcome against the older event must get the OLDER bundle", first, recoveredFirst) + + val recoveredSecond = manager.findBundleByEventId(SECOND_EVENT_ID) + assertSame("the newest event must still resolve to the active bundle", second, recoveredSecond) + } + + @Test + fun aRegeneratedSlotIsAlsoRecoverableByKeyPackageReference() = + runBlocking { + val manager = KeyPackageRotationManager() + val slot = manager.getOrCreateSlotDTag("primary") + + val first = manager.generateKeyPackage(identity, slot) + manager.recordPublishedEventId(slot, FIRST_EVENT_ID) + manager.generateKeyPackage(identity, slot) + + // The Welcome path matches on the KeyPackage reference before it + // falls back to the Nostr event id, so retention has to hold up + // under that lookup too. + val byRef = manager.findBundleByRef(first.keyPackage.reference()) + assertSame("the displaced bundle must be reachable by its own ref", first, byRef) + } + + @Test + fun regeneratingDoesNotLeaveTheOldEventIdPointingAtTheNewBundle() = + runBlocking { + val manager = KeyPackageRotationManager() + val slot = manager.getOrCreateSlotDTag("primary") + + val first = manager.generateKeyPackage(identity, slot) + manager.recordPublishedEventId(slot, FIRST_EVENT_ID) + val second = manager.generateKeyPackage(identity, slot) + + // The regression this guards: resolving the id through the slot + // index returned whatever now occupied the slot, so the older event + // handed out keys that cannot open the Welcome addressed to it. + val recovered = manager.findBundleByEventId(FIRST_EVENT_ID) + assertEquals( + "the old event id must not resolve to the replacement bundle", + false, + recovered === second, + ) + assertSame(first, recovered) + } + + companion object { + private val FIRST_EVENT_ID = "aa".repeat(32) + private val SECOND_EVENT_ID = "bb".repeat(32) + } +} From 94aab4013ff9d24e7926611648a03f25861793c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 18:38:37 +0000 Subject: [PATCH 08/10] feat: put Marmot Groups in the drawer, after Concord MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MarmotGroupListScreen was registered in the nav graph but nothing ever navigated to Route.MarmotGroupList — no commit in the repo's history has ever added a navigator for it, so the screen has been unreachable since it was written. Marmot groups only surfaced as rows in the unified chat rooms list, which opens MarmotGroupChat directly and skips the list entirely. Adds NavBarItem.MARMOT_GROUPS with its catalog definition and files it into the drawer's Feeds section immediately after CONCORD. One list drives both the drawer and the Side Menu settings catalog, so the row is show/hide configurable without touching either. - Icon is Lock rather than Group: the rooms list already labels a Marmot room with that symbol, so it is the signifier users have learned for these, and it keeps the row distinct from Concord's directly above. - Also filed under the chats NavBarCategory. No test requires it, but every sibling chat destination is there, and a catalog item in no category is invisible in the bottom-bar settings picker. - BottomBarFeedPreloaders gets a Unit branch, which the exhaustive `when` forced us to consider. No preload is the right answer: the list renders from marmotGroupList.rooms plus marmotManager.activeGroupIds(), and MarmotManager already owns a per-group subscription for every group it restores or joins, so a list-level REQ would duplicate those. Enum insertion is safe for existing installs: navBarItemsFromNames persists by name rather than ordinal and drops names it doesn't know. This also makes the invite-device banner and the list polish earlier in this branch reachable for the first time. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../amethyst/ui/navigation/bottombars/NavBarItem.kt | 12 ++++++++++++ .../amethyst/ui/navigation/drawer/DrawerSections.kt | 1 + .../ui/screen/loggedIn/BottomBarFeedPreloaders.kt | 6 ++++++ amethyst/src/main/res/values/strings.xml | 1 + 4 files changed, 20 insertions(+) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt index 6d563bc387..8d35d0492a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt @@ -73,6 +73,7 @@ enum class NavBarItem { PUBLIC_CHATS, RELAY_GROUPS, CONCORD, + MARMOT_GROUPS, GEOHASH_CHATS, FOLLOW_PACKS, LIVE_STREAMS, @@ -387,6 +388,16 @@ val NavBarCatalog: Map = icon = MaterialSymbols.Group, resolveRoute = { Route.Concords }, ), + NavBarItem.MARMOT_GROUPS to + NavBarItemDef( + id = NavBarItem.MARMOT_GROUPS, + labelRes = R.string.marmot_groups_title, + // Lock, not Group: the rooms list already labels a Marmot room + // with this symbol, so it is the signifier users have learned + // for these, and it keeps the row distinct from Concord's. + icon = MaterialSymbols.Lock, + resolveRoute = { Route.MarmotGroupList }, + ), NavBarItem.GEOHASH_CHATS to NavBarItemDef( id = NavBarItem.GEOHASH_CHATS, @@ -522,6 +533,7 @@ val BottomBarCategories: List = NavBarItem.PUBLIC_CHATS, NavBarItem.RELAY_GROUPS, NavBarItem.CONCORD, + NavBarItem.MARMOT_GROUPS, NavBarItem.GEOHASH_CHATS, ), ), diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerSections.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerSections.kt index d2242d7759..03745d89f7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerSections.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerSections.kt @@ -144,6 +144,7 @@ private val DrawerFeedsItems: List = NavBarItem.PUBLIC_CHATS, NavBarItem.RELAY_GROUPS, NavBarItem.CONCORD, + NavBarItem.MARMOT_GROUPS, NavBarItem.GEOHASH_CHATS, NavBarItem.CALENDARS, NavBarItem.CALENDAR_COLLECTIONS, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/BottomBarFeedPreloaders.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/BottomBarFeedPreloaders.kt index 23fa984f1c..998933b64e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/BottomBarFeedPreloaders.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/BottomBarFeedPreloaders.kt @@ -154,6 +154,12 @@ private fun PreloadFor( NavBarItem.CONCORD -> ConcordChannelSubscription(accountViewModel.dataSources().concordChannels, accountViewModel) + // The Marmot list renders from local state alone — marmotGroupList.rooms plus + // marmotManager.activeGroupIds() — and MarmotManager already owns a per-group + // subscription for every group it restores or joins. There is no list-level REQ + // to warm up, and adding one would duplicate those. + NavBarItem.MARMOT_GROUPS -> Unit + NavBarItem.FOLLOW_PACKS -> FollowPacksFilterAssemblerSubscription(accountViewModel) NavBarItem.LIVE_STREAMS -> LiveStreamsFilterAssemblerSubscription(accountViewModel) diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 095f28bf96..8c4b354f4e 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -225,6 +225,7 @@ Public Chat Marmot Group + Marmot Groups Public Chat Metadata Public chats are visible to everyone on Nostr and anyone can participate on them. They are great for open communities around specific topics. From 947de68df07d82bc81c40cbc5ada59049ec3424d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 18:44:18 +0000 Subject: [PATCH 09/10] refactor: drop the settings surface for the invite-device check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check belongs where the symptom is. A user whose Marmot invites are landing on another install sees an empty group list, not a settings screen, and the banner on the Marmot groups screen already shows exactly when the newest KeyPackage is not ours. Removes the "Marmot Invite Device" settings row, its onMarmotInviteDevice plumbing through buildSettingsCatalog, and MarmotInviteDeviceDialog. The dialog's shape was the tell: it had to say something in every state — checking, ours, theirs, none published, relays unreachable, read-only login — so it needed seven strings to the banner's one, most of them to tell the user there was nothing to do. The banner renders in the single state worth interrupting for and stays silent in the rest. Drops the nine strings only that surface used (two Android, seven shared). All were added earlier in this branch and have no translated copies in any locale, so nothing Crowdin-managed is touched. The banner's six strings and the three ViewModel methods it calls — canPublish, latestKeyPackageOwner, republishKeyPackage — stay, each now with exactly one caller. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../loggedIn/settings/AllSettingsScreen.kt | 131 ------------------ .../settings/SettingsCatalogBuilder.kt | 14 +- amethyst/src/main/res/values/strings.xml | 2 - .../composeResources/values/strings.xml | 7 - 4 files changed, 1 insertion(+), 153 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt index 33f376ddf3..c47fa6859d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt @@ -37,7 +37,6 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -56,19 +55,10 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.resources.Res -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_checking -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_error -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_none -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_other -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_publish -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_read_only -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_this -import com.vitorpamplona.amethyst.commons.resources.marmot_invite_device_title import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_action import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_body import com.vitorpamplona.amethyst.commons.resources.reset_marmot_confirm_title import com.vitorpamplona.amethyst.commons.resources.settings_search_no_results -import com.vitorpamplona.amethyst.model.LatestKeyPackageOwner import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.navigation.navs.EmptyNav import com.vitorpamplona.amethyst.ui.navigation.navs.INav @@ -80,8 +70,6 @@ import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext -import kotlin.coroutines.cancellation.CancellationException @Preview @Composable @@ -104,7 +92,6 @@ fun AllSettingsScreen( val scope = rememberCoroutineScope() var showResetMarmotDialog by remember { mutableStateOf(false) } var isResettingMarmot by remember { mutableStateOf(false) } - var showInviteDeviceDialog by remember { mutableStateOf(false) } val scrollState = rememberScrollState() val hasPrivateKey = accountViewModel.account.settings.keyPair.privKey != null @@ -114,7 +101,6 @@ fun AllSettingsScreen( // an input actually changes — not on every keystroke. `onResetMarmot` reads the volatile // `isResettingMarmot` through `rememberUpdatedState` so the memoized closure never goes stale. val onResetMarmot by rememberUpdatedState(newValue = { if (!isResettingMarmot) showResetMarmotDialog = true }) - val onMarmotInviteDevice by rememberUpdatedState(newValue = { showInviteDeviceDialog = true }) val catalog = remember(hasPrivateKey, nav, uriHandler) { buildSettingsCatalog( @@ -122,7 +108,6 @@ fun AllSettingsScreen( uriHandler = uriHandler, hasPrivateKey = hasPrivateKey, onResetMarmot = { onResetMarmot() }, - onMarmotInviteDevice = { onMarmotInviteDevice() }, ) } @@ -176,39 +161,6 @@ fun AllSettingsScreen( } } - if (showInviteDeviceDialog) { - MarmotInviteDeviceDialog( - accountViewModel = accountViewModel, - // Published from the screen's scope, not the dialog's: confirming - // closes the dialog, and a publish launched in the dialog's own - // scope would be cancelled the moment it left composition. - onConfirm = { - showInviteDeviceDialog = false - scope.launch(Dispatchers.IO) { - val successMessage = stringRes(context, R.string.marmot_invite_device_success) - val rejectedMessage = stringRes(context, R.string.marmot_invite_device_rejected) - try { - // Waits for a relay OK. The fire-and-forget publish this - // replaced reported success for a read-only account, an - // empty relay set and an outright rejection alike. - val accepted = accountViewModel.republishKeyPackage() - launch(Dispatchers.Main) { - val message = if (accepted) successMessage else rejectedMessage - Toast.makeText(context, message, if (accepted) Toast.LENGTH_SHORT else Toast.LENGTH_LONG).show() - } - } catch (e: Exception) { - val failureMessage = - stringRes(context, R.string.marmot_invite_device_failure, e.message ?: "") - launch(Dispatchers.Main) { - Toast.makeText(context, failureMessage, Toast.LENGTH_LONG).show() - } - } - } - }, - onDismiss = { showInviteDeviceDialog = false }, - ) - } - if (showResetMarmotDialog) { ResetMarmotStateDialog( onConfirm = { @@ -326,86 +278,3 @@ private fun ResetMarmotStateDialog( }, ) } - -/** - * Reports which install currently receives this account's Marmot invites, and - * offers to move them to this one. - * - * The check is this dialog's *content*, never a trigger. An inviter always - * takes the newest kind:30443 and nothing else, so a device that silently - * republished itself back to the front whenever it lost would deadlock against - * the other install — each device's correction is the other's trigger, and - * neither ever settles. Surfacing the answer and letting the user decide is - * what keeps two signed-in devices from fighting over the account's invites. - */ -@Composable -private fun MarmotInviteDeviceDialog( - accountViewModel: AccountViewModel, - onConfirm: () -> Unit, - onDismiss: () -> Unit, -) { - var owner by remember { mutableStateOf(null) } - var checkFailed by remember { mutableStateOf(false) } - // A read-only login cannot publish at all. Without this the dialog would - // offer the button and then blame the relays for a refusal that happened at - // the isWriteable() check, before any relay was contacted. - val canPublish = accountViewModel.canPublish() - - LaunchedEffect(Unit) { - if (!canPublish) return@LaunchedEffect - try { - owner = withContext(Dispatchers.IO) { accountViewModel.latestKeyPackageOwner() } - } catch (e: CancellationException) { - throw e - } catch (_: Exception) { - // A relay we cannot reach only costs us the diagnosis, not the - // action: publishing from here is still a valid thing to want. - checkFailed = true - } - } - - val resolved = owner - val body = - when { - !canPublish -> stringRes(Res.string.marmot_invite_device_read_only) - checkFailed -> stringRes(Res.string.marmot_invite_device_error) - resolved == null -> stringRes(Res.string.marmot_invite_device_checking) - resolved == LatestKeyPackageOwner.THIS_DEVICE -> stringRes(Res.string.marmot_invite_device_this) - resolved == LatestKeyPackageOwner.OTHER_DEVICE -> stringRes(Res.string.marmot_invite_device_other) - else -> stringRes(Res.string.marmot_invite_device_none) - } - - AlertDialog( - onDismissRequest = onDismiss, - icon = { - Icon( - symbol = MaterialSymbols.Key, - contentDescription = null, - modifier = Modifier.size(32.dp), - ) - }, - title = { - Text( - text = stringRes(Res.string.marmot_invite_device_title), - textAlign = TextAlign.Center, - ) - }, - text = { Text(text = body) }, - confirmButton = { - Button( - onClick = onConfirm, - // Held until the check resolves so the user is never asked to - // act on an answer that has not arrived, and never offered at - // all to an account that cannot publish. - enabled = canPublish && (checkFailed || resolved != null), - ) { - Text(stringRes(Res.string.marmot_invite_device_publish)) - } - }, - dismissButton = { - TextButton(onClick = onDismiss) { - Text(stringRes(R.string.cancel)) - } - }, - ) -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt index 27938f508e..d164a3deba 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/SettingsCatalogBuilder.kt @@ -30,8 +30,7 @@ import com.vitorpamplona.amethyst.ui.navigation.routes.Route /** * Assembles the full settings catalog. Not composable: actions close over [nav], - * [uriHandler], [onResetMarmot] and [onMarmotInviteDevice]; conditional rows are included - * via [hasPrivateKey]. + * [uriHandler], and [onResetMarmot]; conditional rows are included via [hasPrivateKey]. * The blank-query render of this catalog must match the legacy hardcoded screen. */ fun buildSettingsCatalog( @@ -39,7 +38,6 @@ fun buildSettingsCatalog( uriHandler: UriHandler, hasPrivateKey: Boolean, onResetMarmot: () -> Unit, - onMarmotInviteDevice: () -> Unit, ): List { // Most rows are a symbol icon + a keyword blob that navigates to a route. This local // helper collapses that shape to one line per row and makes a mismatched keyword/route @@ -86,16 +84,6 @@ fun buildSettingsCatalog( symEntry(R.string.napplet_permissions_title, MaterialSymbols.Apps, R.string.napplet_connected_apps_search_keywords, Route.ConnectedApps), symEntry(R.string.relay_auth_settings_title, MaterialSymbols.Lock, R.string.relay_auth_search_keywords, Route.RelayAuthSettings), symEntry(R.string.call_settings, MaterialSymbols.Phone, R.string.call_settings_search_keywords, Route.CallSettings), - // Opens a dialog rather than a route: the row's whole job is - // to report which device currently owns this account's Marmot - // invites, and that answer costs a relay round trip, so it is - // fetched on demand instead of on every settings render. - SettingsEntry( - titleRes = R.string.marmot_invite_device, - icon = SettingsIcon.Symbol(MaterialSymbols.Key), - keywordsRes = R.string.marmot_invite_device_search_keywords, - onClick = onMarmotInviteDevice, - ), ), ) diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 8c4b354f4e..6909fa82c3 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1349,8 +1349,6 @@ legal, safety, abuse, csae, child protection Danger Zone - Marmot Invite Device - mls, marmot, keypackage, key package, invite, device, group chat This device now receives your Marmot invites. Failed to publish KeyPackage: %1$s No relay accepted the KeyPackage. Check your relay settings and try again. diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index e364ed95fd..5e98a0446d 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1626,15 +1626,8 @@ Message %1$s: %2$s Create a group - Marmot Invite Device Marmot invites are going to another device and cannot be opened here. Dismiss - Asking your relays which device currently receives your Marmot invites\u2026 - This device already receives your Marmot invites. Nothing to do. - This account is signed in read-only, so it cannot publish a KeyPackage. Sign in with your private key to receive Marmot invites on this device. - Another device signed in to this account published a newer KeyPackage, so Marmot invites are going there and cannot be opened here. Publishing from this device takes over future invites; the other device keeps the groups it already joined. - No KeyPackage was found on your relays for this account. Publish one so others can invite you to Marmot groups. - Could not reach your relays to check. You can still publish from this device. Publish from this device Reset Marmot State? This will permanently delete every Marmot group chat, message history, and MLS key on this device for the current account. Peers will not be notified and may still see you in groups until their next commit. This cannot be undone. A new KeyPackage will be published the next time the app syncs. From fccdb456d853a0b209738141be2cc817d429c268 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 19:05:12 +0000 Subject: [PATCH 10/10] fix: third-round audit findings on the invite-device check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of these are defects in the previous round's fixes. 1. An offline check poisoned the cache. fetchAll returns an empty list for a device that reached no relay rather than throwing, so "nobody has published one" and "we are offline" arrive identically as NONE — and the banner's catch for unreachable relays never fires. Caching that for the full 15 minutes suppressed the warning long after the network came back. This is in direct tension with the previous round, which cached NONE precisely to stop re-fanning the query on every entry; both concerns are real, so NONE now gets a 60-second life while a definite answer keeps the full one. 2. The automatic publish paths never invalidated the cache. republishKeyPackageConfirmed did, but publishMarmotKeyPackage and publishMarmotKeyPackages — the post-Welcome rotation — did not, so the banner kept warning for up to 15 minutes after a rotation had already made this device the owner. 3. The banner's publish caught CancellationException and popped a failure toast for a banner that had already left composition, twelve lines below a LaunchedEffect that correctly rethrows it. 4. The drawer label duplicates marmot_groups_title, which commonsUI has translated into 56 locales. Not fixable in place: labelRes is an Android @StringRes shared by every catalog entry, and a Compose resource cannot be referenced there without changing the type for all of them. Documented as an accepted duplication; Crowdin manages both resource sets, so the Android copy gets translated too. 5. A comment of mine overclaimed. getUserIfExists returns null for an unseen sender, so observeUserInfo is never reached and the kind:0 the comment promised is never requested. Corrected the comment rather than the code: the LoadUser idiom would put a metadata REQ behind every row for senders the reader never asked about, which a preview prefix does not justify. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QLiEQmy9z71bEvebmUGkSP --- .../amethyst/model/AccountMarmotActions.kt | 29 +++++++++++++++++-- .../ui/navigation/bottombars/NavBarItem.kt | 6 ++++ .../marmotGroup/MarmotGroupListScreen.kt | 9 ++++-- .../marmotGroup/MarmotInviteDeviceBanner.kt | 6 ++++ 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index 148720a141..3932a29c91 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -38,6 +38,18 @@ import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.TimeUtils import kotlin.coroutines.cancellation.CancellationException +/** + * How long a [LatestKeyPackageOwner.NONE] answer may be reused. + * + * Much shorter than a definite answer's life, because NONE is ambiguous: + * `fetchAll` returns an empty list for a device that reached no relay at all + * rather than throwing, so "nobody has published one" and "we are offline" + * arrive identically. Long enough to stop repeated entries from re-fanning the + * query, short enough that the banner is not suppressed for a quarter of an + * hour after the network comes back. + */ +private const val NONE_MAX_AGE_SECONDS = 60L + /** * Which install owns the newest KeyPackage currently on the account's relays. * @@ -356,6 +368,10 @@ class AccountMarmotActions( } account.client.publish(event, relays) } + // A rotation we just published makes this device the newest owner. + // Leaving the old answer in place would keep the banner warning for + // up to its full life about a state that no longer exists. + lastOwnerCheck = null } } @@ -376,6 +392,9 @@ class AccountMarmotActions( } account.cache.justConsumeMyOwnEvent(event) account.client.publish(event, relays) + // Same as the rotation path: we have just changed who owns the newest + // KeyPackage, so any cached answer is stale by construction. + lastOwnerCheck = null } /** @@ -450,8 +469,14 @@ class AccountMarmotActions( // whole write set — and the thing it asks about only changes when // another device publishes, which is rare enough to cache. val cached = lastOwnerCheck - if (maxAgeSeconds > 0 && cached != null && TimeUtils.now() - cached.first <= maxAgeSeconds) { - return cached.second + if (maxAgeSeconds > 0 && cached != null) { + val maxAge = + if (cached.second == LatestKeyPackageOwner.NONE) { + minOf(maxAgeSeconds, NONE_MAX_AGE_SECONDS) + } else { + maxAgeSeconds + } + if (TimeUtils.now() - cached.first <= maxAge) return cached.second } val latest = KeyPackageFetcher.fetchKeyPackage(account.client, account.signer.pubKey, relays) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt index 8d35d0492a..4eadd963bb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/NavBarItem.kt @@ -391,6 +391,12 @@ val NavBarCatalog: Map = NavBarItem.MARMOT_GROUPS to NavBarItemDef( id = NavBarItem.MARMOT_GROUPS, + // Duplicates the already-translated `marmot_groups_title` in + // commonsUI's composeResources, which the screen's own title uses. + // Unavoidable here: `labelRes` is an Android @StringRes and every + // other catalog entry is one, so a Compose resource cannot be + // referenced without changing the type for all ~60 of them. Crowdin + // manages both resource sets, so this one gets translated too. labelRes = R.string.marmot_groups_title, // Lock, not Group: the rooms list already labels a Marmot room // with this symbol, so it is the signifier users have learned diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt index 081ac4ee1f..0b68fb77c3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt @@ -317,8 +317,13 @@ fun MarmotGroupListItem( // // Read through `observeUserInfo`, not `metadataOrNull()`: the latter is a // plain StateFlow.value read, so a kind:0 arriving after the row composed - // would never reach it — and nothing would have asked for that kind:0 - // either. This both subscribes and recomposes when it lands. + // would never reach it. + // + // Deliberately `getUserIfExists` rather than the `LoadUser` idiom: this only + // subscribes for a sender the cache already knows, and a sender it does not + // simply goes unprefixed. Creating a User per unknown sender would put a + // metadata REQ behind every row of a list that is mostly strangers' names + // the reader never asked for — a preview line is not worth that. val senderUser = previewEvent ?.pubKey diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt index 6819dc31a9..db221bcb6a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotInviteDeviceBanner.kt @@ -181,6 +181,12 @@ fun MarmotInviteDeviceBanner( owner = LatestKeyPackageOwner.OTHER_DEVICE } } + } catch (e: CancellationException) { + // Leaving the screen mid-publish is not a failure to + // report, and swallowing it here would break the + // scope's cancellation as well as pop a toast for a + // banner that is already gone. + throw e } catch (e: Exception) { val failureMessage = stringRes(context, R.string.marmot_invite_device_failure, e.message ?: "")