From c4f6469a5fab8d407a9bfc0886beffcc2b43de0e Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Fri, 25 Sep 2026 16:51:01 -0400 Subject: [PATCH] feat(cordn): make the discovery list on New Cordn group usable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A discovery run against a public relay returned 55 coordinators as a flat list of identical-looking rows. Each showed a name and one line of text, and that line was broken. **"Last announced Aug 7 ago".** `timeAgoNoDot` answers with a span for something recent and an absolute date for anything past a month, and one template served both. Every stale row read as nonsense. The date branch now gets a template with no "ago" in it. **Rows that could not be told apart.** The reference server ships as "My coordinator", so a dozen rows carried that name and nothing else. A pubkey prefix is appended to the names that actually collide, so the common case stays clean — "My coordinator · b9d8fac7" next to a plain "cordn-net". **Nothing about where or what.** The relays were already in `DiscoveredCoordinator` and simply never drawn; rows now show the hosts (two, then a count) and the coordinator's own `about` when it published one, which turns out to be the most telling line on the page: "Cordn coordinator running in a browser tab; key package quota 32 per identity". **55 rows deep.** Anything that has not announced in a month is folded behind "Show 55 that stopped announcing", with a line saying why it matters — creating a group on a coordinator that has gone away fails at the first call. Live ones are capped at eight with "Show all N". On group count: there is no such number to show. The eleven coordinator tools have no group listing and should not — a coordinator enumerating its groups to a stranger would leak exactly what the exposure card promises it does not. Liveness, relays and the advertised surface are the honest substitutes. **Not a LazyColumn**, which was my first instinct and is wrong here: this screen is a form, and a lazy list disposes what scrolls off, which for the text fields below would throw away focus and IME state mid-typing. Capping the rows bounds composition without putting a form inside a recycler. Also capitalises the last two stragglers, the screen title and the picker's call to action. Co-Authored-By: Claude Opus 5 (1M context) --- .../cordnGroup/CordnCreateGroupScreen.kt | 167 ++++++++++++++++-- amethyst/src/main/res/values/strings.xml | 12 +- .../amethyst/CordnCoordinatorNameTest.kt | 60 +++++++ .../composeResources/values/strings.xml | 2 +- 4 files changed, 228 insertions(+), 13 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/CordnCoordinatorNameTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnCreateGroupScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnCreateGroupScreen.kt index 55bdce8966..1521934ada 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnCreateGroupScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnCreateGroupScreen.kt @@ -39,6 +39,7 @@ import androidx.compose.material3.RadioButton import androidx.compose.material3.Scaffold import androidx.compose.material3.Switch import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -49,11 +50,13 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.cordn.CoordinatorConfig import com.vitorpamplona.amethyst.commons.cordn.CordnCoordinatorDiscovery +import com.vitorpamplona.amethyst.commons.cordn.DiscoveredCoordinator import com.vitorpamplona.amethyst.commons.cordn.GroupExposure import com.vitorpamplona.amethyst.commons.cordn.ui.CordnExposureCard import com.vitorpamplona.amethyst.commons.icons.symbols.Icon @@ -67,8 +70,11 @@ import com.vitorpamplona.amethyst.ui.note.timeAgoNoDot import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.quartz.cordn.spec01GroupMetadata.CordnGroupMetadata +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.nip19Bech32.decodePublicKeyAsHexOrNull +import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.launch /** @@ -146,6 +152,8 @@ fun CordnCreateGroupScreen( // is what commits to it, and `createGroup` opens the session. var discovered by remember { mutableStateOf(null) } var discovering by remember { mutableStateOf(false) } + var showStale by remember { mutableStateOf(false) } + var showAllLive by remember { mutableStateOf(false) } val discoverFailed = stringRes(R.string.cordn_coordinators_discover_failed) // Offers this account already holds are not offers; they are the choices @@ -196,23 +204,84 @@ fun CordnCreateGroupScreen( label = coordinator.label ?: coordinator.pubKey.take(16), selected = selected == coordinator.pubKey, onSelect = { selected = coordinator.pubKey }, + relays = relayLabel(coordinator.relays), ) } - offers.forEach { offer -> + // Split on staleness rather than listing everything flat. A run + // against a public relay returns a long tail of coordinators that + // last announced months ago, and creating a group on one that has + // gone away fails at the first call -- so the live ones come first + // and the rest sit behind a count. + val cutoff = TimeUtils.now() - TimeUtils.ONE_MONTH + val live = offers.filter { it.announcedAt >= cutoff } + val stale = offers.filter { it.announcedAt < cutoff } + val names = offers.map { it.displayName() } + + // Bounded rather than lazy. The obvious fix for a long list is a + // LazyColumn, and it is the wrong one here: this screen is a form, + // and a lazy list disposes what scrolls off it -- which for the + // text fields below would throw away focus and IME state mid-typing. + // Capping the rows keeps composition bounded without putting a form + // inside a recycler. + val shownLive = if (showAllLive) live else live.take(LIVE_PREVIEW) + + shownLive.forEach { offer -> CoordinatorChoice( // Its own word for itself, and only that: nothing here has // verified the name a coordinator announces. - label = offer.surface.name?.takeIf { it.isNotBlank() } ?: offer.pubKey.take(16), + label = disambiguate(offer.displayName(), offer.pubKey, names), selected = selected == offer.pubKey, onSelect = { selected = offer.pubKey }, - // Staleness decides whether this choice can work at all: a - // coordinator that announced itself two years ago and went - // away takes the group creation down with it. - detail = stringRes(R.string.cordn_coordinators_discover_seen, timeAgoNoDot(offer.announcedAt).trim()), + detail = announcedLabel(offer.announcedAt), + relays = relayLabel(offer.relays), + about = offer.surface.about?.takeIf { it.isNotBlank() }, ) } + if (live.size > LIVE_PREVIEW) { + TextButton(onClick = { showAllLive = !showAllLive }) { + Text( + if (showAllLive) { + stringRes(R.string.cordn_coordinators_show_fewer) + } else { + stringRes(R.string.cordn_coordinators_show_all, live.size) + }, + ) + } + } + + if (stale.isNotEmpty()) { + TextButton(onClick = { showStale = !showStale }) { + Text( + if (showStale) { + stringRes(R.string.cordn_coordinators_hide_older) + } else { + stringRes(R.string.cordn_coordinators_show_older, stale.size) + }, + ) + } + } + + if (showStale && stale.isNotEmpty()) { + Text( + text = stringRes(R.string.cordn_coordinators_stale_note), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + stale.forEach { offer -> + CoordinatorChoice( + label = disambiguate(offer.displayName(), offer.pubKey, names), + selected = selected == offer.pubKey, + onSelect = { selected = offer.pubKey }, + detail = announcedLabel(offer.announcedAt), + relays = relayLabel(offer.relays), + about = offer.surface.about?.takeIf { it.isNotBlank() }, + dimmed = true, + ) + } + } + CoordinatorChoice( label = stringRes(R.string.cordn_create_coordinator_new), selected = selected == null, @@ -395,25 +464,103 @@ private fun CoordinatorChoice( selected: Boolean, onSelect: () -> Unit, detail: String? = null, + /** Where it answers. A coordinator has no address beyond its pubkey (§8.5). */ + relays: String? = null, + /** Its own sentence about itself, if it published one. */ + about: String? = null, + /** Dimmed for a coordinator that stopped announcing long ago. */ + dimmed: Boolean = false, ) { + val fade = if (dimmed) 0.6f else 1f Row( - modifier = Modifier.fillMaxWidth().selectable(selected = selected, onClick = onSelect), + modifier = Modifier.fillMaxWidth().selectable(selected = selected, onClick = onSelect).padding(vertical = 2.dp), verticalAlignment = Alignment.CenterVertically, ) { RadioButton(selected = selected, onClick = onSelect) - Column { - Text(label, style = MaterialTheme.typography.bodyMedium) + Column(Modifier.weight(1f)) { + Text( + text = label, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface.copy(alpha = fade), + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + relays?.let { + Text( + text = it, + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = fade), + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + about?.let { + Text( + text = it, + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = fade), + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + } detail?.let { Text( text = it, style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, + color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = fade), ) } } } } +/** How many live coordinators a discovery run shows before it asks. */ +private const val LIVE_PREVIEW = 8 + +/** Its announced name, or the key when it published none. */ +private fun DiscoveredCoordinator.displayName(): String = surface.name?.takeIf { it.isNotBlank() } ?: pubKey.take(16) + +/** + * How long ago a coordinator last announced, in words that stay words. + * + * `timeAgoNoDot` answers with a span for something recent and an absolute date + * for anything past a month, and the caller used one template for both — so + * every stale coordinator on the discovery list read "Last announced Aug 7 + * ago". The date branch gets a template with no "ago" in it. + */ +@Composable +private fun announcedLabel(announcedAt: Long): String = + if (TimeUtils.now() - announcedAt > TimeUtils.ONE_MONTH) { + stringRes(R.string.cordn_coordinators_discover_seen_on, timeAgoNoDot(announcedAt).trim()) + } else { + stringRes(R.string.cordn_coordinators_discover_seen, timeAgoNoDot(announcedAt).trim()) + } + +/** The hosts it answers on, the first two and a count of the rest. */ +@Composable +private fun relayLabel(relays: List): String? { + if (relays.isEmpty()) return null + val hosts = relays.map { it.url.substringAfter("://").trim('/') }.distinct() + val shown = hosts.take(2).joinToString(", ") + return if (hosts.size <= 2) { + shown + } else { + stringRes(R.string.cordn_coordinators_relays_more, shown, hosts.size - 2) + } +} + +/** + * Names are the coordinator's own word for itself and collide constantly — the + * reference server ships as "My coordinator", so a discovery run returns a + * dozen rows with that name and nothing to tell them apart. A pubkey prefix is + * added only to the ones that actually clash, so the common case stays clean. + */ +internal fun disambiguate( + name: String, + pubKey: HexKey, + allNames: List, +): String = if (allNames.count { it == name } > 1) "$name \u00b7 ${pubKey.take(8)}" else name + /** * The coordinator this screen would create against, or null while the form * cannot name one. diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index d1690e6ea6..ef0badda0b 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -365,7 +365,7 @@ Members This group has no admins: everyone can add, remove and rename, permanently. Technical details - New cordn group + New Cordn group A cordn group is ordered by one coordinator you choose. Creating the group happens on this device — the coordinator only learns the group exists once you invite someone or send a message. Coordinator Find coordinators @@ -403,7 +403,15 @@ Could not read announcements Nobody is announcing on your relays. Nothing found, and %1$d of your relays did not answer. - Last announced %1$s ago + Announced %1$s ago + Last announced %1$s + %1$s + %1$s +%2$d more + Show all %1$d + Show fewer + Show %1$d that stopped announcing + Hide the ones that stopped announcing + These have not announced in over a month. Creating a group on one that has gone away will fail. Add someone Name, npub or name@domain They can only be added if they published a key package to this coordinator. diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/CordnCoordinatorNameTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/CordnCoordinatorNameTest.kt new file mode 100644 index 0000000000..339fa80822 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/CordnCoordinatorNameTest.kt @@ -0,0 +1,60 @@ +/* + * 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 + +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.cordnGroup.disambiguate +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Telling two coordinators apart on the discovery list. + * + * An announced name is the coordinator's own word for itself and collides + * constantly: the reference server ships as "My coordinator", so a run against + * a public relay comes back with a dozen rows carrying that name and nothing + * else to separate them. + */ +class CordnCoordinatorNameTest { + @Test + fun `a name nobody else uses is left alone`() { + val names = listOf("cordn-net", "My coordinator") + + assertEquals("cordn-net", disambiguate("cordn-net", "aabbccdd11223344", names)) + } + + @Test + fun `a colliding name gains its key`() { + val names = listOf("My coordinator", "My coordinator", "cordn-net") + + assertEquals("My coordinator · aabbccdd", disambiguate("My coordinator", "aabbccdd11223344", names)) + } + + @Test + fun `two that collide get different suffixes`() { + val names = listOf("My coordinator", "My coordinator") + + val first = disambiguate("My coordinator", "aaaaaaaa11112222", names) + val second = disambiguate("My coordinator", "bbbbbbbb33334444", names) + + assertEquals("My coordinator · aaaaaaaa", first) + assertEquals("My coordinator · bbbbbbbb", second) + } +} diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index 105c2c0bd5..0b4831993c 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -5391,7 +5391,7 @@ Encrypted group chat ordered by a coordinator you pick. Coordinated Best for teams that want one reliable ordering of the conversation. - Create cordn group + Create Cordn group End-to-end encrypted with MLS — the coordinator can never read a message. One agreed order for everyone, so history cannot be reshuffled by a bad clock. The coordinator learns who is in the group and when you talk, even though it cannot read what you say.