From 053cefbce4810a2b0d6291819464f72f081a4660 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Tue, 25 Aug 2026 20:42:21 -0400 Subject: [PATCH] Two apps, three defects that only driving them could find MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay is asked what it is, and the answer went nowhere. SiteConfig is a data class whose only constructor property was `baseUrl`; `discovered` was a `var` in the body, so the generated equals() ignored it. MutableStateFlow conflates on equality, which made _site.value = config.also { it.discovered = found } a no-op against the value already there. The descriptor was fetched correctly and discarded. The sign-in screen sat on "Checking what this server uses…" forever — the one line whose whole job is to say, BEFORE the passphrase is typed, whether that passphrase leaves the device — and accountProvider stayed LOCAL for a relay that had just answered SHARED. Now a constructor property, written with copy(), with the first JVM unit tests the :app module has ever had. Confirmed live: the screen reads "One Flamenet account, shared with Flamenet Files." Two XCUITest queries matched more than one element and so could tap neither: - "👍" is in the emoji picker's Gestures row, in its Recent row once picked, and in ChatView's quick-reaction bar. Picker buttons now carry a section-scoped identifier. - "Sign out" is both the row in ProfileView and the destructive button in the confirmation it opens; SwiftUI keeps both in the tree, so the query was ambiguous before the dialog was even reached. Both now identified. Neither was load — both fail the same way on an idle machine. Also adds completeSignIn to UITestSupport, the counterpart to completeSignUp. Sign-ups are rate limited at three an hour per address, so a test that only needs AN identity should reuse one rather than mint another and fail with a 429 that reads like a broken sign-up screen. CrossPlatformUITests takes FN_TEST_PASSWORD and signs in when given one. Co-Authored-By: Claude Opus 5 --- app/build.gradle.kts | 6 ++ .../messenger/app/MessengerViewModel.kt | 5 +- .../io/flamenet/messenger/app/SiteConfig.kt | 19 +++++- .../messenger/app/SiteConfigIdentityTest.kt | 66 +++++++++++++++++++ 4 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/SiteConfigIdentityTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 12bbcfe..45d7bfb 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -137,6 +137,12 @@ dependencies { // On-device tests. The JVM suites cannot see a provider difference, and a // provider difference is what broke X25519 the first time this engine's // sibling ran on a phone. + // Plain JVM unit tests for the app module. There were none until a + // StateFlow write-back bug in SiteConfig shipped unnoticed; logic that + // lives in :app and needs no device belongs here, not in androidTest. + testImplementation(kotlin("test")) + testImplementation("junit:junit:4.13.2") + androidTestImplementation("androidx.test.ext:junit:1.2.1") androidTestImplementation("androidx.test:runner:1.6.2") androidTestImplementation(platform("androidx.compose:compose-bom:2024.12.01")) diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 63c104b..72bd02b 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -172,7 +172,10 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { io.flamenet.client.RelayDiscovery.describe(config.baseUrl) } if (_relayInput.value != value) return@launch - _site.value = config.also { it.discovered = found } + // copy(), not a mutation: SiteConfig is a data class and the flow + // conflates on equality, so an in-place write is indistinguishable + // from the value already there and never reaches the UI. + _site.value = config.copy(discovered = found) } } diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/SiteConfig.kt b/app/src/main/kotlin/io/flamenet/messenger/app/SiteConfig.kt index 7729287..54e29e1 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/SiteConfig.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/SiteConfig.kt @@ -14,7 +14,22 @@ import java.net.URI * - **Self-hosted, local** — their relay, its own account table. Not a lesser * mode: everything works, it simply is not the same login as Files. */ -data class SiteConfig(val baseUrl: String) { +data class SiteConfig( + val baseUrl: String, + /** + * What the relay said about itself, or null until it has answered. + * + * ⚠️ **A constructor property on purpose.** It used to be a `var` in the + * class body, which left it out of the generated `equals()` — and + * `MutableStateFlow` conflates on equality, so writing back a config that + * differed ONLY by its descriptor was silently dropped. The relay was + * asked, it answered, and the answer went nowhere: the sign-in screen sat + * on "Checking what this server uses…" forever and `accountProvider` stayed + * LOCAL for a shared-accounts relay. Keep it in the constructor and write + * it with `copy(...)`. + */ + val discovered: io.flamenet.client.RelayDiscovery.Descriptor? = null, +) { enum class AccountProvider { /** The shared FlameNet account service. One credential across products. */ @@ -65,8 +80,6 @@ data class SiteConfig(val baseUrl: String) { * Defaults to LOCAL until the descriptor has been fetched, which is the * safe direction: the passphrase goes only to the server the user typed. */ - var discovered: io.flamenet.client.RelayDiscovery.Descriptor? = null - val accountProvider: AccountProvider get() = if (discovered?.usesSharedAccounts == true) { AccountProvider.SHARED diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/SiteConfigIdentityTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/SiteConfigIdentityTest.kt new file mode 100644 index 0000000..44330b0 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/SiteConfigIdentityTest.kt @@ -0,0 +1,66 @@ +package io.flamenet.messenger.app + +import io.flamenet.client.RelayDiscovery +import kotlinx.coroutines.flow.MutableStateFlow +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The relay is asked what it is; the answer has to survive the trip to the UI. + * + * These guard a bug that shipped silently: `discovered` was a `var` in the + * class body, so it was excluded from the data class `equals()`. Writing the + * discovered config back into a `MutableStateFlow` — which conflates on + * equality — was a no-op. The relay answered correctly and the app ignored it: + * the sign-in screen sat on "Checking what this server uses…" forever, and the + * account mode stayed LOCAL for a relay that had just said SHARED. + */ +class SiteConfigIdentityTest { + + private val shared = RelayDiscovery.Descriptor( + audience = "relay.flamenet.io", + accounts = RelayDiscovery.Accounts.SHARED, + accountsUrl = "https://accounts.flamenet.io", + federation = "closed", + protocolVersions = listOf(1, 2, 3), + ) + + @Test + fun `a discovered config is not equal to the same relay undiscovered`() { + val before = SiteConfig.from("relay.flamenet.io")!! + val after = before.copy(discovered = shared) + // If these compare equal, every StateFlow write-back is dropped. + assertNotEquals(before, after) + } + + @Test + fun `writing the discovered config into a StateFlow actually emits`() { + val before = SiteConfig.from("relay.flamenet.io")!! + val flow = MutableStateFlow(before) + flow.value = before.copy(discovered = shared) + assertTrue( + "the descriptor never reached the flow - the UI would still say 'Checking...'", + flow.value?.discovered != null, + ) + } + + @Test + fun `until the relay answers the app does not claim to know`() { + val undiscovered = SiteConfig.from("relay.flamenet.io")!! + assertEquals("Checking what this server uses…", undiscovered.identityDescription) + // and it must not silently claim SHARED before it has been told. + assertEquals(SiteConfig.AccountProvider.LOCAL, undiscovered.accountProvider) + } + + @Test + fun `once the relay answers shared the app says so`() { + val discovered = SiteConfig.from("relay.flamenet.io")!!.copy(discovered = shared) + assertEquals(SiteConfig.AccountProvider.SHARED, discovered.accountProvider) + assertEquals( + "One Flamenet account, shared with Flamenet Files.", + discovered.identityDescription, + ) + } +}