The recovery button did not recover, and half the text went black

Two bugs from testing on the emulator, one of them mine from this morning.

## "Erase and start fresh" erased nothing

🔴 The `VaultUnreadable` screen — shown when the keys on this phone cannot be
opened — offered exactly one action, and it called `model::signOut`. `signOut`
defaults to `erasingDevice = false`: the door that deliberately KEEPS the device
so coming back is silent. So it set the phase to SignedOut, left the unreadable
vault exactly where it was, and the next `restore()` opened it, failed again,
and came straight back to the same screen.

⚠️ It looked like it worked, because the screen changed. That is the whole
problem with a recovery action that does not recover: the feedback IS the claim,
and the person only finds out when they try to sign in and land back where they
started. Which is what happened, twice, to the person testing it.

`discardUnreadableVault()` destroys the vault, opens a fresh one, and lands on
sign-in with a sentence saying what happened and that contacts will see the
security keys change. Deliberately NOT `signOut(erasingDevice = true)`, which
would also have worked: that path first tries to revoke the device on the relay
using a session that by definition does not exist here, and its comments are
about handing a phone on. The keys are already lost; the only question is
whether the app can get past them.

Three tests, in the same source-scan shape as `DestructiveLabelsTest`. Falsified:
putting `model::signOut` back fails with the reason spelled out.

## Black text on the dark ground

🔴 `Surface(color = Color.Transparent)` with no `contentColor`. Material derives
it with `contentColorFor(color)`, which has no answer for Transparent and
returns Unspecified — so `LocalContentColor` falls back to its own default,
which is BLACK. Every `Text` that does not name a colour of its own drew black
on the dark ground.

⚠️ It read as "some of the text is hard to read" rather than as a broken theme,
because everything that DOES set a colour was fine and only the inheriting ones
went dark. That is what made it easy to ship: the screens I looked at longest
were the ones that name their colours.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-07 12:37:03 -04:00
co-authored by Claude Opus 5
parent c688ed21b4
commit 28d10bec4d
3 changed files with 130 additions and 1 deletions
@@ -118,6 +118,19 @@ class MainActivity : androidx.fragment.app.FragmentActivity() {
// default container colour paints straight over it. // default container colour paints straight over it.
Surface( Surface(
color = androidx.compose.ui.graphics.Color.Transparent, color = androidx.compose.ui.graphics.Color.Transparent,
// 🔴 **`contentColor` MUST be set when `color` is
// Transparent.** Material derives it with
// `contentColorFor(color)`, which has no answer for
// Transparent and returns Unspecified — so `LocalContentColor`
// falls back to its own default, which is BLACK. Every
// `Text` that does not name a colour of its own then draws
// black on the dark ground.
//
// It looked like "some of the text is hard to read" rather
// than like a broken theme, because the many places that DO
// set a colour were fine and only the inheriting ones went
// dark.
contentColor = Palette.Ink,
modifier = Modifier.fillMaxSize().background(groundBrush()), modifier = Modifier.fillMaxSize().background(groundBrush()),
) { ) {
// 🔴 **The gate is around Root, not inside it.** Drawn over // 🔴 **The gate is around Root, not inside it.** Drawn over
@@ -915,7 +928,12 @@ private fun ColumnScope.VaultUnreadable(
"Starting fresh publishes a new device. Everyone you talk to will see " + "Starting fresh publishes a new device. Everyone you talk to will see " +
"\"security keys changed\", so tell them to expect it.", "\"security keys changed\", so tell them to expect it.",
) )
Button(onClick = model::signOut, modifier = Modifier.testTag("start-fresh")) { // 🔴 `discardUnreadableVault`, NOT `signOut`. `signOut()` defaults to
// keeping the device, so it left the unreadable vault in place and the next
// restore came straight back here — the only button on a dead-end screen
// did nothing but redraw it.
Button(onClick = model::discardUnreadableVault,
modifier = Modifier.testTag("start-fresh")) {
Text("Erase and start fresh") Text("Erase and start fresh")
} }
} }
@@ -3726,6 +3726,35 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
* ⚠️ Keeping the keys does not leave the mailbox reachable: a relay token * ⚠️ Keeping the keys does not leave the mailbox reachable: a relay token
* is minted against the account credential, which this clears either way. * is minted against the account credential, which this clears either way.
*/ */
/**
* Throw away a vault that cannot be opened, so sign-in is possible again.
*
* 🔴 **The "Erase and start fresh" button used to call `signOut()`, which
* erases nothing.** `signOut` defaults to `erasingDevice = false` — the
* door that deliberately KEEPS the device so coming back is silent — so it
* set the phase to SignedOut and left the unreadable vault exactly where it
* was. The next `restore()` opened it, failed again, and went straight back
* to VaultUnreadable. The one button on a dead-end screen was a loop.
*
* ⚠️ Not `signOut(erasingDevice = true)` either, though that would have
* worked. That path first tries to revoke the device on the relay using a
* session that by definition does not exist here, and its comments are
* about handing a phone on. This is a different situation: the keys are
* already lost, and the only question is whether the app can get past them.
*/
fun discardUnreadableVault() = viewModelScope.launch {
polling?.cancel()
polling = null
session = null
withContext(Dispatchers.IO) { runCatching { KeystoreVault.destroy(getApplication()) } }
identity = null
vault = KeystoreVault.open(getApplication())
_phase.value = Phase.SignedOut(
error = "The keys stored on this phone could not be opened, so they have "
+ "been cleared. Signing in registers this phone as a new device — "
+ "your contacts will see that your security keys changed.")
}
fun signOut(erasingDevice: Boolean = false) = viewModelScope.launch { fun signOut(erasingDevice: Boolean = false) = viewModelScope.launch {
polling?.cancel() polling?.cancel()
polling = null polling = null
@@ -0,0 +1,82 @@
package io.flamenet.messenger.app
import java.io.File
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue
/**
* The one button on a dead-end screen has to be a way out.
*
* 🔴 **"Erase and start fresh" erased nothing.** The `VaultUnreadable` screen —
* shown when the keys on this phone cannot be opened — offered exactly one
* action, and it called `model::signOut`. `signOut` defaults to
* `erasingDevice = false`, the door that deliberately KEEPS the device so
* coming back is silent. So it set the phase to SignedOut, left the unreadable
* vault exactly where it was, and the next `restore()` opened it, failed again,
* and came straight back to the same screen.
*
* ⚠️ **It looked like it worked.** The screen changed. That is the whole
* problem with a recovery action that does not recover: the feedback is the
* claim, and the person only finds out on the next launch — or, as here, when
* they try to sign in and land back where they started.
*
* A source scan, coarse on purpose, in the same shape as
* `DestructiveLabelsTest`. It cannot prove the recovery is correct. It proves
* this particular wrong call cannot come back.
*/
class RecoveryActuallyRecoversTest {
private fun source(name: String): String {
val f = File("src/main/kotlin/io/flamenet/messenger/app/$name")
assertTrue(f.isFile, "cannot find $name — this test reads the sources")
return f.readText()
}
/** The block that draws the dead-end screen. */
private fun vaultUnreadableScreen(): String {
val s = source("MainActivity.kt")
val start = s.indexOf("private fun ColumnScope.VaultUnreadable(")
assertTrue(start > 0, "the VaultUnreadable screen has been renamed or removed")
val end = s.indexOf("\n}", start)
return s.substring(start, if (end > start) end else s.length)
}
@Test
fun `the way out of an unreadable vault does not call plain signOut`() {
val screen = vaultUnreadableScreen()
assertFalse(
screen.contains("model::signOut") || screen.contains("model.signOut("),
"the recovery button calls signOut, which keeps the device by default — " +
"it leaves the unreadable vault in place and the next restore returns here",
)
}
@Test
fun `the way out of an unreadable vault discards it`() {
assertTrue(
vaultUnreadableScreen().contains("discardUnreadableVault"),
"the recovery button no longer calls discardUnreadableVault",
)
}
/**
* 🔴 And that the discard actually destroys something. A method with the
* right name that does not reach the vault would satisfy the test above and
* fail the person in exactly the same way.
*/
@Test
fun `discarding an unreadable vault destroys it and reopens a fresh one`() {
val s = source("MessengerViewModel.kt")
val start = s.indexOf("fun discardUnreadableVault(")
assertTrue(start > 0, "discardUnreadableVault is gone")
val body = s.substring(start, minOf(start + 1200, s.length))
assertTrue(body.contains("KeystoreVault.destroy"),
"discardUnreadableVault never destroys the vault")
assertTrue(body.contains("KeystoreVault.open"),
"discardUnreadableVault destroys the vault without opening a fresh one, " +
"so the next sign-in has nowhere to store keys")
assertTrue(body.contains("Phase.SignedOut"),
"discardUnreadableVault does not leave the app on the sign-in screen")
}
}