Keep shell state reads safe in read-only snapshots

app/src/test/java/org/justinvisser/wolfskin/tv/DiscoverSnapshotReadTest.kt

@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-only
+package org.justinvisser.wolfskin.tv
+
+import android.app.Application
+import androidx.activity.ComponentActivity
+import androidx.compose.ui.test.junit4.createAndroidComposeRule
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.*
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.justinvisser.wolfskin.conductarr.MemoryCacheStore
+import org.justinvisser.wolfskin.design.*
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+/** No drawn screen may prewarm memory before the production Discover observer starts. */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34], application = Application::class)
+class DiscoverSnapshotReadTest {
+    @get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
+
+    @Test fun signedInHostObservesFreshShellThroughTheRealAsyncProvider() {
+        FakeConductarr().use { fake ->
+            ConductarrSession(MemoryCacheStore()).use { session ->
+                session.select("server:shared", SHARED_TOKEN, fake.url)
+                runBlocking { session.authenticate() }
+                val identity = requireNotNull(session.identity.value)
+                assertTrue(identity.signedIn)
+                val shell = ShellState(listOf(ShellPerson("shared", "Shared", "S", true)), "shared")
+                lateinit var current: DiscoverState
+                rule.setContent {
+                    current = rememberDiscoverState(shell, shell::enterDestination, {}, identity = identity, fixtures = false,
+                        loadFixture = { error("Production observer must not load fixtures") })
+                }
+                rule.waitUntil(30_000) {
+                    org.robolectric.Shadows.shadowOf(android.os.Looper.getMainLooper()).idle()
+                    current.snapshot.status == DiscoverStatus.Ready
+                }
+                rule.waitForIdle()
+                rule.runOnIdle {
+                    assertTrue(fake.calls.any { it.url.encodedPath.endsWith("/feed") })
+                    assertEquals(0, current.focusedRow)
+                    assertEquals(0, current.column)
+                    current.focus(current.firstShelf)
+                }
+                rule.waitForIdle()
+                rule.runOnIdle {
+                    assertNotNull(current.focusedLane)
+                    assertEquals(current.firstShelf, current.focusedRow)
+                    assertEquals(0, current.column)
+                }
+            }
+        }
+    }
+}

docker/README.md

@@ -78,6 +78,13 @@ resolves consistently in native captures; the approved JSON weights stay unchang
 Nav, picker names, labels and pill weights use that same mapping. Nav label colour
 fades for 200 ms ease-out; the blob retains its critically damped spring.
 
+Shell view accessors are read-only, including the first read from Discover's
+`snapshotFlow`. Construction and explicit person/navigation events retain root
+and destination memory; reading an unknown destination returns an unsaved default.
+Per-person focus, cards and scrollers still survive route changes, while reselecting
+the current root resets it. JVM regressions exercise both read-only snapshots and
+the signed-in asynchronous Discover host without a screen prewarming its memory.
+
 Settings pills have a separate 1 dp stroke (2 px on the stage), not a focus ring.
 The stroke is inset within the fixed 20 dp / 40 px height; horizontal border-box
 padding includes it. The 6 px dash / 4 px gap follows the browser's 3:2 stroke-width

wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/ShellState.kt

@@ -57,6 +57,7 @@ class ShellState(
         people = value
         if (selected != null) person = selected
         pickerIndex = people.indexOf(person).coerceAtLeast(0)
+        preparePersonMemory()
     }
     var person by mutableStateOf(people.firstOrNull { it.key == initialPerson } ?: people.first())
         private set
@@ -73,7 +74,7 @@ class ShellState(
     var requestsVisible by mutableStateOf(false)
     var toast by mutableStateOf<String?>(null)
     private val personRequests = mutableMapOf<String, ComingInState>()
-    val requests: ComingInState get() = personRequests.getOrPut(person.key) { ComingInState() }
+    val requests: ComingInState get() = personRequests.getValue(person.key)
     private var requestReturnRoute: String? = null
     var handOff by mutableStateOf<String?>(null)
     var settingsRow by mutableStateOf(0)
@@ -85,17 +86,30 @@ class ShellState(
     var contentFocus: ShellContentFocus? = null
     var inputRevision = 0L
         private set
+
+    init { preparePersonMemory() }
+
+    private fun preparePersonMemory() {
+        personRequests.getOrPut(person.key) { ComingInState() }
+        tabs.forEach(::retainView)
+        retainView(route)
+    }
+
+    private fun retainView(destination: String) {
+        views.getOrPut(person.key to destination) { RememberedView() }
+    }
     val tabs: List<String> get() = if (person.hasAnime) ANIME_TABS else BASE_TABS
     val currentTab: Int get() = tabs.indexOf(rootRoute).coerceAtLeast(0)
     val view: RememberedView get() = rememberedView(route)
     val navSavedAt: String? get() = if (requestsVisible && requests.snapshot.status == ComingInStatus.Unavailable) requests.snapshot.savedAt ?: view.savedAt else view.savedAt
 
-    /** Screen options share the same per-person memory as their root's focus and scroller. */
+    /** Reads never register defaults: snapshotFlow evaluates accessors in a read-only snapshot. */
     fun rememberedView(destination: String): RememberedView =
-        views.getOrPut(person.key to destination) { RememberedView() }
+        views[person.key to destination] ?: RememberedView()
 
     /** External intents and upstream pops remain authoritative over the screen. */
     fun enterDestination(destination: String) {
+        retainView(destination)
         route = destination
         if (destination in tabs) rootRoute = destination
         handOff = null
@@ -159,6 +173,7 @@ class ShellState(
                     pickerVisible = false
                     navFocused = false
                     settingsRow = 0
+                    retainView("settings")
                     route = "settings"
                     onAction(ShellAction.Destination(route))
                 }
@@ -187,6 +202,7 @@ class ShellState(
         rootRoute = "home"
         requestReturnRoute = null
         requestsVisible = false
+        preparePersonMemory()
         views[person.key to route] = RememberedView()
         onAction(ShellAction.Person(selected))
     }
@@ -226,6 +242,7 @@ class ShellState(
             } else {
                 val target = tabs[navIndex]
                 requestReturnRoute = null
+                retainView(target)
                 if (target == route) views[person.key to target] = RememberedView()
                 route = target
                 rootRoute = target

wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/ShellSnapshotReadTest.kt

@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-only
+package org.justinvisser.wolfskin.design
+
+import android.app.Application
+import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.snapshotFlow
+import androidx.compose.runtime.snapshots.Snapshot
+import androidx.compose.ui.test.junit4.createComposeRule
+import kotlinx.coroutines.flow.first
+import org.junit.Assert.*
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+/** A first read must work without composition prewarming the person's route memory. */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34], application = Application::class)
+class ShellSnapshotReadTest {
+    @get:Rule val rule = createComposeRule()
+
+    @Test fun unseenViewCanBeReadInAReadOnlySnapshot() {
+        val shell = ShellState(fixturePeople(), "justin")
+        val snapshot = Snapshot.takeSnapshot()
+        try {
+            snapshot.enter {
+                assertEquals(0, shell.view.row)
+                assertTrue(shell.view.cards.isEmpty())
+                assertNull(shell.view.savedAt)
+                assertEquals(0, shell.rememberedView("never-seen").row)
+                assertNotSame(shell.rememberedView("never-seen"), shell.rememberedView("never-seen"))
+            }
+        } finally { snapshot.dispose() }
+    }
+
+    @Test fun launchedSnapshotFlowReadsDefaultsAndThenRetainsExplicitNavigationMemory() {
+        val shell = ShellState(fixturePeople(), "justin")
+        var observed: RememberedView? = null
+        rule.setContent {
+            LaunchedEffect(shell) { observed = snapshotFlow { shell.view }.first() }
+        }
+        rule.runOnIdle {
+            val home = requireNotNull(observed)
+            assertEquals(0, home.row)
+            assertTrue(home.cards.isEmpty())
+            home.row = 3
+            home.cards["shelf"] = 2
+            shell.enterDestination("movies")
+            assertEquals(0, shell.view.row)
+            shell.view.row = 5
+            shell.enterDestination("home")
+            assertSame(home, shell.view)
+            assertEquals(3, shell.view.row)
+            assertEquals(2, shell.view.cards["shelf"])
+            shell.enterDestination("movies")
+            assertEquals(5, shell.view.row)
+            shell.focusPerson(1)
+            shell.key(ShellKey.Ok)
+            shell.enterDestination("movies")
+            assertEquals(0, shell.view.row)
+            shell.view.row = 7
+            shell.pickerVisible = true
+            shell.focusPerson(0)
+            shell.key(ShellKey.Ok)
+            shell.enterDestination("movies")
+            assertEquals(5, shell.view.row)
+        }
+    }
+}

wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/SnapshotReadAuditTest.kt

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+package org.justinvisser.wolfskin.design
+
+import java.io.File
+import org.junit.Assert.*
+import org.junit.Test
+
+/** Guard known state writers; runtime tests cover transitive property reads. */
+class SnapshotReadAuditTest {
+    @Test fun snapshotObserversDoNotCallKnownStateWriters() {
+        val roots = listOf(File("src/main/java"), File("../app/src/wolfskin"))
+        val violations = roots.flatMap { root ->
+            assertTrue("Missing owned source root: $root", root.isDirectory)
+            root.walkTopDown().filter { it.isFile && it.extension == "kt" }.flatMap { file ->
+                observerBodies(file.readText()).filter { writingCalls.containsMatchIn(it) }.map { "$file: $it" }
+            }.toList()
+        }
+        assertTrue(violations.joinToString("\n"), violations.isEmpty())
+    }
+
+    @Test fun shellReadAccessorDoesNotInsertMemory() {
+        val source = File("src/main/java/org/justinvisser/wolfskin/design/ShellState.kt").readText()
+        val accessor = source.substringAfter("fun rememberedView(").substringBefore("/**")
+        assertFalse("rememberedView must not insert into snapshot state", Regex("getOrPut|\\.put\\s*\\(|\\]\\s*=").containsMatchIn(accessor))
+    }
+
+    @Test fun knownWriterIsRejectedWithoutRejectingItsCollector() {
+        val unsafe = observerBodies("snapshotFlow { state.focus(2) }").single()
+        assertTrue(writingCalls.containsMatchIn(unsafe))
+        val safe = observerBodies("snapshotFlow { state.focusedLane?.id to state.column }.collect { state.focus(2) }").single()
+        assertFalse(writingCalls.containsMatchIn(safe))
+    }
+
+    private fun observerBodies(source: String): List<String> = Regex("snapshotFlow\\s*\\{").findAll(source).map { match ->
+        val start = match.range.last + 1
+        var depth = 1
+        var end = start
+        while (end < source.length && depth > 0) {
+            when (source[end]) { '{' -> depth++; '}' -> depth-- }
+            end++
+        }
+        source.substring(start, end - 1)
+    }.toList()
+
+    private val writingCalls = Regex("\\b(?:retainView|preparePersonMemory|enterDestination|focus|updateSnapshot|updateLanes|invalidateLanes|updateRatings|recordCenter|getOrPut|put|putAll|clear|remove)\\s*\\(")
+}