From 4e6377b628cf4cfff402dd1384bb36b26bc53f39 Mon Sep 17 00:00:00 2001 From: justinzeus Date: Fri, 6 Nov 2026 10:50:56 +0100 Subject: [PATCH] Keep shell state reads safe in read-only snapshots --- .../wolfskin/tv/DiscoverSnapshotReadTest.kt | 56 +++++++++++++++ docker/README.md | 7 ++ .../wolfskin/design/ShellState.kt | 23 +++++- .../wolfskin/design/ShellSnapshotReadTest.kt | 70 +++++++++++++++++++ .../wolfskin/design/SnapshotReadAuditTest.kt | 46 ++++++++++++ 5 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/org/justinvisser/wolfskin/tv/DiscoverSnapshotReadTest.kt create mode 100644 wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/ShellSnapshotReadTest.kt create mode 100644 wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/SnapshotReadAuditTest.kt diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/DiscoverSnapshotReadTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/DiscoverSnapshotReadTest.kt new file mode 100644 index 00000000..cd7d4208 --- /dev/null +++ b/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() + + @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) + } + } + } + } +} diff --git a/docker/README.md b/docker/README.md index aeba3db5..db9676ad 100644 --- a/docker/README.md +++ b/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 diff --git a/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/ShellState.kt b/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/ShellState.kt index 3e82a286..4afd8415 100644 --- a/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/ShellState.kt +++ b/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(null) private val personRequests = mutableMapOf() - 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(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 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 diff --git a/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/ShellSnapshotReadTest.kt b/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/ShellSnapshotReadTest.kt new file mode 100644 index 00000000..a1762334 --- /dev/null +++ b/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) + } + } +} diff --git a/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/SnapshotReadAuditTest.kt b/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/SnapshotReadAuditTest.kt new file mode 100644 index 00000000..ab5fdd4d --- /dev/null +++ b/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 = 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*\\(") +}