Stop throwing away a sign-up that finished, and the app sends through a bridge
ci / image (pull_request) Successful in 8s
ci / secret-sweep (pull_request) Successful in 13s
ci / vectors-head (pull_request) Successful in 4m32s
ci / unit-tests (pull_request) Successful in 9m59s

The on-device test now passes. The app, on an emulator, with an obfs4
bridge in force, signs up on relay1, publishes its device, looks up a
contact, adds it and sends a message. Six obfs4 handshakes counted by the
app, five of them before the send, and the relay logged
POST /e2e/messages 200.

What was in the way was not the bridge.

🔴 **signIn discarded work that had already succeeded.** It ran
`withTimeoutOrNull(60s) { withContext(IO) { authenticate(...) } }`.
`authenticate` is blocking socket work, so cancelling the coroutine does
not stop it: the timeout fired, the screen said "The server did not
answer. It may be down, or the address may be wrong", and the
registration carried on and finished behind it. One run left the account
`andb7564513` on relay1 with a published identity and a device while the
person was told the server was unreachable. The relay answered sign-up in
397 ms and the app's next call came 115 seconds later.

Reproduced with no bridge (`-e direct true`), so this was never a bridge
defect. The work now runs in its own `async`, the first 60 seconds still
report, as "Still waiting for the server" rather than as a failure, and a
late success is adopted instead of dropped. After 180 more seconds it
gives up and cancels the work, so a success nobody is waiting for cannot
surprise somebody who moved on.

Spanish for the new string is in values-es.

Two things the test learned to stop believing:

- An outgoing line in the thread is not delivery. The app renders a
  message as soon as it is composed, and a run that asserted only the
  text reported OK while the relay never saw a POST at all. Delivery is
  now the absence of both the `pending` and `undelivered` markers.
- `Obfs4Settings` on main takes `add`, not `save`, and returns
  `AddResult`. The branch this was first written against had the older
  single-line API.

InstrumentedSuitesCanSignUpTest now accepts `typeIfPresent("birth-year")`
as well as the direct call. relay1 does not ask for a birth year, so a
suite driving it has to fill that field only when the relay puts it on
screen. The rule it enforces is unchanged.

Gate: `:app:testDebugUnitTest` is 671 tests with 267 failures on this
machine, identical with and without this change, measured by stashing it
and re-running. 266 of those are
`IllegalArgumentException: Unsupported class file major version` from a
JDK 26 toolchain, which is this laptop rather than the repository. The
one real failure was the guard above, and it passes now: 3 tests, 0
failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-10-14 16:24:51 -04:00
co-authored by Claude Opus 5
parent adf44db7ca
commit 4469df0bf3
5 changed files with 83 additions and 22 deletions
@@ -14,7 +14,6 @@ import androidx.compose.ui.test.printToString
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import androidx.test.rule.GrantPermissionRule
import io.flamenet.obfs4.BridgeLineResult
import io.flamenet.obfs4.Obfs4Stats
import org.junit.Assume.assumeTrue
import org.junit.Before
@@ -92,15 +91,14 @@ class BridgeMessageTest {
)
handle = "andb" + (System.currentTimeMillis() % 100_000_000)
// The same call the Network screen's save button makes, so this is the
// app's own setting path and not a test-only hook.
// The same call the Network screen's own button makes, so this is the
// app's setting path and not a test-only hook.
val settings = Obfs4Settings(InstrumentationRegistry.getInstrumentation().targetContext)
if (direct) {
settings.clear()
} else {
val saved = settings.save(bridgeLine!!)
if (saved !is BridgeLineResult.Ok) {
throw AssertionError("the bridge line was refused: $saved")
settings.clear()
if (!direct) {
val added = settings.add(bridgeLine!!)
if (added !is Obfs4Settings.AddResult.Added) {
throw AssertionError("the bridge line was refused: ${added.message}")
}
}
@@ -148,11 +146,29 @@ class BridgeMessageTest {
compose.onNodeWithTag("draft").performTextInput(text)
compose.onNodeWithTag("send").performClick()
// The line appears only once at least one of the peer's devices took
// it, so seeing it is evidence of delivery.
// The line appearing is NOT delivery. The app renders an outgoing
// message as soon as it is composed: a run that asserted only this
// reported OK while the relay never saw a POST /e2e/messages at all.
awaitNode(hasText(text, substring = true), 90_000)
compose.onNode(hasText(text, substring = true)).assertIsDisplayed()
// Delivery is the absence of both markers. The row carries `pending`
// while the message is still queued and `undelivered` when it failed,
// so a row with neither is one the relay accepted.
val deadline = System.currentTimeMillis() + 150_000
var queued = true
while (System.currentTimeMillis() < deadline) {
settle(1_000)
if (nodesWithTag("undelivered").isNotEmpty()) {
throw AssertionError("the message is marked undelivered. On screen: ${visibleText()}")
}
if (nodesWithTag("pending").isEmpty()) {
queued = false
break
}
}
assert(!queued) { "the message was still pending after 150 seconds" }
// The counter is the whole evidence, so say what it was at each
// stage rather than only that it moved.
val afterSend = Obfs4Stats.completedHandshakes()
@@ -33,6 +33,7 @@ import io.flamenet.e2e.Label
import io.flamenet.e2e.Vault
import io.flamenet.e2e.X25519
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
@@ -570,15 +571,41 @@ class MessengerViewModel(
_phase.value = Phase.SignedOut(busy = true)
viewModelScope.launch {
// A sign-in that cannot finish has to SAY so (android#19).
val result =
withTimeoutOrNull(SIGN_IN_TIMEOUT_MS) {
withContext(Dispatchers.IO) {
runCatching { authenticate(config, handle, passphrase, register, email) }
}
} ?: Result.failure(
IllegalStateException(str(R.string.core_server_did_not_answer)),
)
// 🔴 The work runs in its own job so the timeout cannot throw away
// a sign-up that actually completed.
//
// It used to be `withTimeoutOrNull { withContext(IO) { ... } }`.
// `authenticate` is blocking socket work, so cancelling the
// coroutine does not stop it: on a slow network the timeout fired,
// the screen said the server did not answer, and the registration
// carried on and finished. One emulator run left the account
// `andb7564513` on relay1 with a published identity and a device,
// and the person looking at the phone was told the server was
// unreachable. The relay's log showed signup 200 in 397 ms and the
// next call 115 seconds later.
val work =
async(Dispatchers.IO) {
runCatching { authenticate(config, handle, passphrase, register, email) }
}
// A sign-in that cannot finish has to SAY so (android#19), so the
// first window still reports, but as "still trying" rather than as
// a failure, and the work keeps running behind it.
var result = withTimeoutOrNull(SIGN_IN_TIMEOUT_MS) { work.await() }
if (result == null) {
_phase.value = Phase.SignedOut(busy = true, error = str(R.string.core_still_trying))
result =
withTimeoutOrNull(SIGN_IN_PATIENCE_MS) { work.await() }
?: run {
// Now it really has not finished. Stop waiting, and
// stop the work too, so a later success cannot
// surprise somebody who has moved on.
work.cancel()
Result.failure(
IllegalStateException(str(R.string.core_server_did_not_answer)),
)
}
}
result.fold(
onSuccess = {
@@ -5242,10 +5269,20 @@ class MessengerViewModel(
companion object {
/**
* How long sign-in may take before it reports failure (android#19).
* How long sign-in may take before it says anything (android#19).
*/
private const val SIGN_IN_TIMEOUT_MS = 60_000L
/**
* How much longer it keeps waiting after saying so.
*
* A sign-up on a slow link, or through an obfs4 bridge on an emulator,
* has taken 115 seconds between the relay accepting the account and
* the app's next call. Giving up at 60 and discarding the result left
* people with an account they were told did not exist.
*/
private const val SIGN_IN_PATIENCE_MS = 180_000L
/** How long to wait for a relay to describe itself before saying it did not. */
private const val RELAY_PROBE_TIMEOUT_MS = 15_000L
@@ -6,6 +6,7 @@
<resources>
<string name="core_signed_in_on_this_device">Sesión iniciada en este dispositivo.</string>
<string name="core_not_a_server_address">Eso no parece la dirección de un servidor.</string>
<string name="core_still_trying">Seguimos esperando al servidor. Puede tardar un minuto con una conexión lenta o a través de un puente.</string>
<string name="core_server_did_not_answer">El servidor no respondió. Puede que esté caído o que la dirección esté mal: revísala e inténtalo de nuevo.</string>
<string name="core_registered_with_prekeys">Registro hecho, con %1$d claves previas de un solo uso publicadas.</string>
<string name="core_look_the_person_up_first">Primero busca a la persona.</string>
+1
View File
@@ -6,6 +6,7 @@
<resources>
<string name="core_signed_in_on_this_device">Signed in on this device.</string>
<string name="core_not_a_server_address">That doesn\'t look like a server address.</string>
<string name="core_still_trying">Still waiting for the server. This can take a minute on a slow connection or through a bridge.</string>
<string name="core_server_did_not_answer">The server did not answer. It may be down, or the address may be wrong, check it and try again.</string>
<string name="core_registered_with_prekeys">Registered with %1$d one-time prekeys published.</string>
<string name="core_look_the_person_up_first">Look the person up first.</string>
@@ -36,8 +36,14 @@ class InstrumentedSuitesCanSignUpTest {
for (f in suites()) {
val body = f.readText()
if (!body.contains("""onNodeWithTag("submit").performClick()""")) continue
// Either the direct call or a helper that names the tag. relay1
// does not ask for a birth year at all, so a suite driving it has
// to fill the field only when the relay put it on screen, and
// `typeIfPresent("birth-year", ...)` is that. The rule is still
// "the age gate is handled", which is what android#21 was about.
assertTrue(
body.contains("""onNodeWithTag("birth-year")"""),
body.contains("""onNodeWithTag("birth-year")""") ||
body.contains("""typeIfPresent("birth-year"""),
"${f.name} taps submit without filling birth-year. Submit is disabled " +
"until the age gate has four digits and performClick on a disabled " +
"node does nothing, so the run fails later looking for a screen the " +