From 74e84c073fa7236aaf3c8e737998336fba60afb3 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 11:49:33 +1200 Subject: [PATCH 01/19] Fix web history sync: serialize updates, await traversals, replace-vs-push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WebHistoryPlugin's sync loop had three bugs that made browser back navigation jumpy on wasmJs: 1. Updates arriving while a sync job was in flight were silently dropped, desyncing the in-memory history mirror from the real session history — after which a single browser back could traverse multiple app screens. Lifecycle callbacks and popstate events now feed a single serial processor channel and are never dropped. 2. history.go() is asynchronous but was followed by delay(1) and a time-based listener-disable for echo suppression. The traversal's popstate echo usually arrived after suppression lifted and, when state comparison failed (e.g. during exit animations), was handled as a second user back. Traversals now await their popstate echo and consume it explicitly (with a timeout fallback). 3. Any state not present in the mirror was unconditionally pushState-ed, so full-backstack replacements (loading gate -> home, section truncate-and-open) accreted stale browser entries — making screens like a startup/loading destination reachable via back. The previously-dead isSubset helper is now wired up: a state is pushed only when the previous state is a prefix of it; otherwise the current entry is replaced. Also: blind history.back() retry replaced with a bounded step-past loop, popstate listener removed and processor cancelled on detach, and the unused isNewState/collectInstructionIds helpers removed. Co-Authored-By: Claude Fable 5 --- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 347 ++++++++++-------- 1 file changed, 198 insertions(+), 149 deletions(-) diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index c8bb8359..fe15d82b 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -13,10 +13,12 @@ import dev.enro.path.getPathFromNavigationKey import dev.enro.platform.EnroLog import dev.enro.plugin.NavigationPlugin import kotlinx.browser.window +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job -import kotlinx.coroutines.delay +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeout import kotlinx.coroutines.yield @@ -32,17 +34,47 @@ import org.w3c.dom.events.Event // // Nested URL routing is a known future direction; see docs/ghpages/docs/platform/web.md // for the model we ship in beta. +// +// Synchronisation model: every input (destination lifecycle callback or browser +// popstate) is enqueued onto a single serial processor, so updates are never dropped +// and the in-memory mirror of browser history can't silently diverge from the real +// session history. History traversals the plugin itself initiates (`history.go`) +// are awaited via their popstate echo, which is consumed before it can be mistaken +// for a user-initiated back/forward. @ExperimentalEnroApi internal class WebHistoryPlugin( private val window: Window, private val rootContainer: ContainerContext, ) : NavigationPlugin() { - private var activeHistoryJob: Job? = null - private var eventListenerEnabled = true - private val eventListener: (Event) -> Unit = { - if (eventListenerEnabled && it is PopStateEvent) { - updateHistoryState(it) + private val scope = CoroutineScope(Dispatchers.Main) + + /** + * Serial work queue. `null` means "the backstack changed, re-sync browser + * history"; a [PopStateEvent] means "the browser navigated, re-sync the + * backstack". Processing strictly in order is what keeps [historyStates] + * truthful — the previous implementation dropped events that arrived while + * a sync was in flight, which desynced the mirror and made a single + * browser back traverse multiple app screens. + */ + private val events = Channel(capacity = Channel.UNLIMITED) + + /** + * Set while the plugin is awaiting the popstate echo of its own + * `history.go()` call — see [traverse]. The next popstate completes it and + * is consumed instead of being enqueued as user navigation. + */ + private var pendingTraversal: CompletableDeferred? = null + + private val eventListener: (Event) -> Unit = { event -> + if (event is PopStateEvent) { + val traversal = pendingTraversal + if (traversal != null) { + pendingTraversal = null + traversal.complete(Unit) + } else { + events.trySend(event) + } } } @@ -50,24 +82,37 @@ internal class WebHistoryPlugin( private val historyStates = mutableListOf() private var historyIndex = -1 // Index of the current state in historyStates + private val processor: Job + init { window.addEventListener("popstate", eventListener) + processor = scope.launch { + for (event in events) { + when (event) { + null -> syncFromBackstack() + else -> syncFromPopState(event) + } + } + } } override fun onAttached(controller: EnroController) {} - override fun onDetached(controller: EnroController) {} + override fun onDetached(controller: EnroController) { + window.removeEventListener("popstate", eventListener) + processor.cancel() + } override fun onOpened(navigationHandle: NavigationHandle<*>) { - updateHistoryState() + events.trySend(null) } override fun onActive(navigationHandle: NavigationHandle<*>) { - updateHistoryState() + events.trySend(null) } override fun onClosed(navigationHandle: NavigationHandle<*>) { - updateHistoryState() + events.trySend(null) } /** @@ -91,136 +136,152 @@ internal class WebHistoryPlugin( return window.location.pathname + window.location.search } + /** + * Calls `history.go(delta)` and suspends until the browser delivers the + * resulting popstate, consuming that echo. `history.go` is asynchronous — + * the previous implementation `delay(1)`-ed and hoped, which raced the + * traversal (corrupting the history position) and let the echo arrive + * after suppression was lifted, where it was processed as a second + * user back. The timeout is a safety valve for browsers that elide the + * event (e.g. a no-op traversal at a history boundary). + */ + private suspend fun traverse(delta: Int) { + if (delta == 0) return + val deferred = CompletableDeferred() + pendingTraversal = deferred + window.history.go(delta) + try { + withTimeout(TRAVERSAL_TIMEOUT_MS) { deferred.await() } + } catch (t: TimeoutCancellationException) { + EnroLog.warn("WebHistoryPlugin: history traversal ($delta) produced no popstate within ${TRAVERSAL_TIMEOUT_MS}ms") + pendingTraversal = null + } + } + @OptIn(ExperimentalWasmJsInterop::class) - private fun updateHistoryState( - event: PopStateEvent? = null, - ) { - val container = rootContainer - eventListenerEnabled = false - if (activeHistoryJob != null) { - return + private fun decodeState(state: JsAny): ContainerNode? { + return runCatching { + EnroController.jsonConfiguration.decodeFromString(state.toString()) + }.getOrNull() + } + + /** + * The browser navigated (user back/forward): drive the backstack to match + * the entry's recorded state. When a recorded state can't be applied (an + * interceptor or EmptyBehavior refuses the close, or the app rewrote the + * backstack concurrently), step past it — bounded, rather than blind-firing + * `history.back()` and re-entering through the listener. + */ + @OptIn(ExperimentalWasmJsInterop::class) + private suspend fun syncFromPopState(event: PopStateEvent) { + // popstate without a state payload (manual address-bar edit, cross-origin + // nav). Under root-only routing we can't safely restore a sensible app + // state from URL alone — no-op and let the user reload if they want the + // URL to take effect. + var poppedState = event.state?.let(::decodeState) ?: return + + var attempts = 0 + while (attempts < MAX_TRAVERSAL_ATTEMPTS) { + attempts++ + val currentState = createNodeFor(rootContainer) + if (currentState == poppedState) break + applyNodeFor(rootContainer, poppedState) + if (createNodeFor(rootContainer) == poppedState) break + // The recorded state didn't take — step one entry further back and + // try that one instead. + traverse(-1) + poppedState = window.history.state?.let(::decodeState) ?: return } - activeHistoryJob = CoroutineScope(Dispatchers.Main).launch { - val currentState = createNodeFor(container) - val serializedCurrentState = EnroController.jsonConfiguration - .encodeToString(currentState) - .toJsString() - - val windowState = window.history.state?.let { - EnroController.jsonConfiguration - .decodeFromString(it.toString()) + + val poppedIndex = historyStates.indexOfFirst { it == poppedState } + if (poppedIndex != -1) { + historyIndex = poppedIndex + } else { + historyStates.add(poppedState) + historyIndex = historyStates.lastIndex + } + } + + /** + * The backstack changed (open/active/close): mirror it into browser history. + */ + @OptIn(ExperimentalWasmJsInterop::class) + private suspend fun syncFromBackstack() { + val currentState = createNodeFor(rootContainer) + val serializedCurrentState = EnroController.jsonConfiguration + .encodeToString(currentState) + .toJsString() + + val windowState = window.history.state?.let(::decodeState) + + val isInit = historyStates.isEmpty() && historyIndex == -1 + val isNoOp = windowState != null && windowState == currentState + val closeIndex = historyStates.indexOfLast { it == currentState } + + when { + isInit -> { + historyStates.add(currentState) + historyIndex = 0 + window.history.replaceState(serializedCurrentState, "", computeUrl()) } - if (event != null && event.state != null) { - val poppedState = EnroController.jsonConfiguration - .decodeFromString(event.state.toString()) - if (currentState != poppedState) { - applyNodeFor(container, poppedState) - val updatedState = createNodeFor(container) - if (updatedState != poppedState) { - window.history.back() - return@launch - } - val poppedIndex = historyStates.indexOfFirst { it == poppedState } - if (poppedIndex != -1) { - historyIndex = poppedIndex - } else { - historyStates.add(poppedState) - historyIndex = historyStates.lastIndex - } + isNoOp -> { + if (closeIndex >= 0) { + historyIndex = closeIndex + historyStates[historyIndex] = currentState } - } else if (event != null) { - // popstate fired without a state payload (manual address-bar edit, - // cross-origin nav). Under root-only routing we can't safely restore - // a sensible app state from URL alone — no-op and let the user - // reload if they want the URL to take effect. - return@launch - } else { // Not a popstate event (opened, active, closed, init) - val isInit = historyStates.isEmpty() && historyIndex == -1 - val isNoOp = windowState != null && windowState == currentState - - val closeIndex = historyStates.indexOfLast { it == currentState } - val isClose = closeIndex >= 0 - - when { - isInit -> { - historyStates.add(currentState) - historyIndex = 0 - window.history.replaceState( - serializedCurrentState, - "example", - computeUrl(), - ) - } - - isNoOp -> { - val windowIndex = historyStates.indexOfLast { it == currentState } - historyIndex = windowIndex - historyStates[historyIndex] = currentState - window.history.replaceState( - serializedCurrentState, - "example", - computeUrl(), - ) - } - - isClose -> { - // when the target state is a close, we need to pop back to that element in the history - val previousIndex = historyIndex - historyIndex = closeIndex - historyStates[historyIndex] = currentState - val goDelta = closeIndex - previousIndex - if (closeIndex == 0) { - window.history.go(goDelta) - window.history.replaceState( - serializedCurrentState, - "example", - computeUrl(), - ) - } else { - window.history.go(goDelta - 1) - delay(1) - window.history.pushState( - serializedCurrentState, - "example", - computeUrl(), - ) - } - } - else -> { - val currentIndex = historyStates.indexOfLast { it == currentState } - if (currentIndex < 0) { - historyStates.subList(historyIndex + 1, historyStates.size).clear() - historyStates.add(currentState) - historyIndex = historyStates.lastIndex - window.history.pushState( - serializedCurrentState, - "example", - computeUrl(), - ) - } else { - val previousIndex = historyIndex - historyIndex = currentIndex - historyStates[historyIndex] = currentState - window.history.go(previousIndex - currentIndex) - delay(1) - window.history.pushState( - serializedCurrentState, - "example", - computeUrl(), - ) - } - } + window.history.replaceState(serializedCurrentState, "", computeUrl()) + } + + closeIndex >= 0 -> { + // The current state exists earlier in the history: this is a close, + // pop back to that entry. + val previousIndex = historyIndex + historyIndex = closeIndex + historyStates[historyIndex] = currentState + if (closeIndex == 0) { + traverse(closeIndex - previousIndex) + window.history.replaceState(serializedCurrentState, "", computeUrl()) + } else { + // Land one short of the target and push it fresh: pruning the + // browser's forward entries so forward can't resurrect screens + // the app has closed. (Not possible at index 0 — there's no + // entry before it to land on.) + traverse(closeIndex - previousIndex - 1) + window.history.pushState(serializedCurrentState, "", computeUrl()) + // The push destroyed the browser's forward entries — drop them + // from the mirror too. + historyStates.subList(historyIndex + 1, historyStates.size).clear() } } - delay(1) - }.apply { - invokeOnCompletion { - activeHistoryJob = null - eventListenerEnabled = true + + else -> { + // A state we haven't seen. Forward navigation (push) only when the + // previous state is a prefix of the new one — i.e. entries were + // added on top of what was already there. Anything else (a root + // reset such as loading → home, or a truncate-and-open section + // switch) REPLACES the current entry: the state it overwrites is + // no longer reachable in the app and must not survive as a browser + // back target. + val previous = historyStates.getOrNull(historyIndex) + val isPush = previous == null || isSubset(old = currentState, new = previous) + historyStates.subList(historyIndex + 1, historyStates.size).clear() + if (isPush) { + historyStates.add(currentState) + historyIndex = historyStates.lastIndex + window.history.pushState(serializedCurrentState, "", computeUrl()) + } else { + historyStates[historyIndex] = currentState + window.history.replaceState(serializedCurrentState, "", computeUrl()) + } } } } + + private companion object { + const val TRAVERSAL_TIMEOUT_MS = 250L + const val MAX_TRAVERSAL_ATTEMPTS = 10 + } } @@ -320,24 +381,12 @@ internal suspend fun applyNodeFor( } } -internal fun isNewState(old: ContainerNode, new: ContainerNode): Boolean { - val oldInstructions = collectInstructionIds(old).toSet() - val newInstructions = collectInstructionIds(new).toSet() - - // If the new tree has instructions not in the old tree, it's a new state - if (newInstructions.any { it !in oldInstructions }) { - return true - } - - // Check if the new tree is a subset of the old tree - return !isSubset(old, new) -} - -internal fun collectInstructionIds(node: ContainerNode): List { - val instructions = node.backstack.map { it.id } - return instructions + node.children.flatMap { collectInstructionIds(it) } -} - +/** + * True when [new] is a prefix-subset of [old] — i.e. [new] contains no entries + * that aren't already in [old], in the same order from the root. Used to + * distinguish a genuine forward push (the previous state is a subset of the + * next) from a replacement (entries were swapped out in a single transition). + */ internal fun isSubset(old: ContainerNode, new: ContainerNode): Boolean { fun isNodeSubset(oldNode: ContainerNode, newNode: ContainerNode): Boolean { if (oldNode.containerKey != newNode.containerKey) { @@ -400,4 +449,4 @@ public fun InstallWebHistoryPlugin( } ) } -} \ No newline at end of file +} From fb9dd67a05b973bd279be60f52284357c216b57e Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 13:12:19 +1200 Subject: [PATCH 02/19] WebHistoryPlugin: isolate per-event failures in the sync processor The serial processor loop introduced in the previous commit had no exception isolation: one throwing sync (serializer, interceptor, path computation) ended the for-loop permanently, after which browser back updated the URL natively but the app never restored state. Each event is now handled in its own try/catch (CancellationException rethrown) and logged via EnroLog.error so the underlying failure is visible. decodeState failures are also logged instead of silently ignored. Co-Authored-By: Claude Fable 5 --- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index fe15d82b..122ce09f 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -13,6 +13,7 @@ import dev.enro.path.getPathFromNavigationKey import dev.enro.platform.EnroLog import dev.enro.plugin.NavigationPlugin import kotlinx.browser.window +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -88,9 +89,20 @@ internal class WebHistoryPlugin( window.addEventListener("popstate", eventListener) processor = scope.launch { for (event in events) { - when (event) { - null -> syncFromBackstack() - else -> syncFromPopState(event) + try { + when (event) { + null -> syncFromBackstack() + else -> syncFromPopState(event) + } + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + // One failed sync must not kill history handling for the + // rest of the session — without this, a single throwing + // serializer/interceptor/path computation would end the + // processor loop and browser back would go silent while + // the URL keeps changing natively. + EnroLog.error("WebHistoryPlugin: history sync failed", t) } } } @@ -162,6 +174,8 @@ internal class WebHistoryPlugin( private fun decodeState(state: JsAny): ContainerNode? { return runCatching { EnroController.jsonConfiguration.decodeFromString(state.toString()) + }.onFailure { t -> + EnroLog.warn("WebHistoryPlugin: failed to decode history state (ignoring entry): ${t.message}") }.getOrNull() } From 635ffddba82c22c3e21895615ea6d7b7eed2dbd8 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 13:33:23 +1200 Subject: [PATCH 03/19] WebHistoryPlugin: restore from URL when an entry's state can't decode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit History entries persist on a browser tab across app builds, so a serialization-format change (or a metadata value that doesn't round-trip — e.g. kotlinx silently encodes polymorphic value classes as bare literals it then cannot decode) leaves entries whose recorded state is unreadable. Previously these no-opped: back updated the URL natively but the app never moved. Undecodable entries now fall back to resolving the entry's URL through the controller's path bindings (single-entry, same semantics as a cold-load deep link) and self-heal by overwriting the entry's state with the freshly serialized equivalent. Co-Authored-By: Claude Fable 5 --- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index 122ce09f..8563e544 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -9,6 +9,7 @@ import dev.enro.annotations.ExperimentalEnroApi import dev.enro.context.ContainerContext import dev.enro.controller.createNavigationModule import dev.enro.emptyBackstack +import dev.enro.path.getBackstackFromPath import dev.enro.path.getPathFromNavigationKey import dev.enro.platform.EnroLog import dev.enro.plugin.NavigationPlugin @@ -192,7 +193,9 @@ internal class WebHistoryPlugin( // nav). Under root-only routing we can't safely restore a sensible app // state from URL alone — no-op and let the user reload if they want the // URL to take effect. - var poppedState = event.state?.let(::decodeState) ?: return + val rawState = event.state ?: return + var poppedState = decodeState(rawState) + ?: return restoreFromUrl() var attempts = 0 while (attempts < MAX_TRAVERSAL_ATTEMPTS) { @@ -204,7 +207,9 @@ internal class WebHistoryPlugin( // The recorded state didn't take — step one entry further back and // try that one instead. traverse(-1) - poppedState = window.history.state?.let(::decodeState) ?: return + val nextRaw = window.history.state ?: return + poppedState = decodeState(nextRaw) + ?: return restoreFromUrl() } val poppedIndex = historyStates.indexOfFirst { it == poppedState } @@ -216,6 +221,45 @@ internal class WebHistoryPlugin( } } + /** + * Fallback for a history entry whose recorded state can't be decoded — + * typically an entry written by an older build of the app whose + * serialization no longer matches (stale tab history survives deploys), + * or a metadata value that doesn't round-trip. Resolves the entry's URL + * through the controller's path bindings instead — degraded (single + * entry, same semantics as a cold-load deep link) but functional — and + * self-heals the entry by overwriting its unreadable state with the + * freshly serialized equivalent so the next visit decodes normally. + */ + @OptIn(ExperimentalWasmJsInterop::class) + private suspend fun restoreFromUrl() { + val fallback = rootContainer.controller.getBackstackFromPath(currentUrl()) + if (fallback == null) { + EnroLog.warn( + "WebHistoryPlugin: history entry state was unreadable and its URL " + + "('${currentUrl()}') has no path binding — leaving app state unchanged" + ) + return + } + applyNodeFor(rootContainer, ContainerNode( + containerKey = rootContainer.container.key, + backstack = fallback, + children = emptyList(), + )) + val currentState = createNodeFor(rootContainer) + val serializedCurrentState = EnroController.jsonConfiguration + .encodeToString(currentState) + .toJsString() + window.history.replaceState(serializedCurrentState, "", computeUrl()) + val index = historyStates.indexOfFirst { it == currentState } + if (index != -1) { + historyIndex = index + } else { + historyStates.add(currentState) + historyIndex = historyStates.lastIndex + } + } + /** * The backstack changed (open/active/close): mirror it into browser history. */ From 5a1eac24f4b6d26ab0e16ff0bd16d017bbfb07b4 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 13:53:00 +1200 Subject: [PATCH 04/19] Add BoxedValueClassSerializer for polymorphic value classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kotlinx encodes a polymorphic value class as its bare literal (no discriminator can attach to a non-object body) and then cannot decode it back — an asymmetric round-trip that silently produces persisted state that can't restore. BoxedValueClassSerializer gives a value class an object-shaped single-field envelope ({"type": fqn, "value": ...}) so the discriminator attaches and round-trips become symmetric; the valueClassSubclass helper registers it in a polymorphic(Any) block. The debug-mode metadata verification in EnroController now also does a full encode->decode round-trip instead of only checking a serializer exists, so any non-restorable type fails at metadata-set time with an error naming the type and pointing at valueClassSubclass. Co-Authored-By: Claude Fable 5 --- .../BoxedValueClassSerializer.kt | 93 +++++++++++++++++++ .../kotlin/dev/enro/EnroController.kt | 21 +++++ .../BoxedValueClassSerializerTests.kt | 84 +++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt create mode 100644 enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt diff --git a/enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt b/enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt new file mode 100644 index 00000000..6a11a8c3 --- /dev/null +++ b/enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt @@ -0,0 +1,93 @@ +package dev.enro.serialization + +import kotlinx.serialization.KSerializer +import kotlinx.serialization.descriptors.SerialDescriptor +import kotlinx.serialization.descriptors.buildClassSerialDescriptor +import kotlinx.serialization.encoding.CompositeDecoder +import kotlinx.serialization.encoding.Decoder +import kotlinx.serialization.encoding.Encoder +import kotlinx.serialization.encoding.decodeStructure +import kotlinx.serialization.encoding.encodeStructure +import kotlinx.serialization.modules.PolymorphicModuleBuilder + +/** + * Gives a value class an object-shaped serialized form so it can participate + * in polymorphic serialization. + * + * A `@JvmInline value class` normally serializes as its underlying primitive + * (e.g. a bare JSON string). That representation has nowhere to carry a class + * discriminator, and kotlinx's polymorphic JSON encoding silently degrades to + * the bare literal on encode — which it then cannot decode ("Expected + * JsonObject, but had JsonLiteral as the serialized body"). Any value class + * stored as a polymorphic `Any` (for example in `NavigationKey.Metadata`, or + * as a navigation result) would round-trip lossily: written fine, unreadable + * on restore. + * + * Wrapping the value class's serializer in [BoxedValueClassSerializer] gives + * it a single-field envelope: + * + * ```json + * {"type": "com.example.CampaignId", "value": "abc-123"} + * ``` + * + * which the discriminator can attach to, restoring symmetric round-trips. + * Registration uses the same serial name as the underlying serializer, so the + * discriminator value is the value class's own serial name. + * + * This only affects polymorphic contexts — value classes used as plain typed + * properties keep their efficient bare-literal form. + * + * Register via [valueClassSubclass]: + * + * ```kotlin + * serializersModule(SerializersModule { + * polymorphic(Any::class) { + * valueClassSubclass(CampaignId.serializer()) + * } + * }) + * ``` + */ +public class BoxedValueClassSerializer( + private val inner: KSerializer, +) : KSerializer { + + override val descriptor: SerialDescriptor = buildClassSerialDescriptor( + serialName = inner.descriptor.serialName, + ) { + element("value", inner.descriptor) + } + + override fun serialize(encoder: Encoder, value: T) { + encoder.encodeStructure(descriptor) { + encodeSerializableElement(descriptor, 0, inner, value) + } + } + + override fun deserialize(decoder: Decoder): T { + return decoder.decodeStructure(descriptor) { + var result: T? = null + while (true) { + when (val index = decodeElementIndex(descriptor)) { + 0 -> result = decodeSerializableElement(descriptor, 0, inner) + CompositeDecoder.DECODE_DONE -> break + else -> error("Unexpected index $index while decoding ${descriptor.serialName}") + } + } + requireNotNull(result) { + "Missing 'value' element while decoding ${descriptor.serialName}" + } + } + } +} + +/** + * Registers [T] as a polymorphic subclass using a [BoxedValueClassSerializer] + * around [serializer], so the value class survives polymorphic round-trips. + * See [BoxedValueClassSerializer] for why plain `subclass(serializer)` is not + * safe for value classes. + */ +public inline fun PolymorphicModuleBuilder.valueClassSubclass( + serializer: KSerializer, +) { + subclass(T::class, BoxedValueClassSerializer(serializer)) +} diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt index cf9f5bc9..36c418e5 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt @@ -13,6 +13,7 @@ import dev.enro.controller.repository.PluginRepository import dev.enro.controller.repository.SerializerRepository import dev.enro.controller.repository.ViewModelRepository import dev.enro.serialization.wrapForSerialization +import kotlinx.serialization.PolymorphicSerializer import kotlinx.serialization.json.Json @Stable @@ -87,6 +88,26 @@ public class EnroController { if (!hasSerializer) { error("Object of type ${value::class} could not be added to NavigationKey.Metadata, make sure to register the serializer with the NavigationController.") } + // A registered serializer isn't enough: some shapes encode but + // can't be decoded — kotlinx silently encodes a polymorphic + // value class as its bare literal, which it then refuses to + // read back ("Expected JsonObject, but had JsonLiteral"). + // Round-trip the value so a non-restorable type fails here, + // at set time in debug, naming the type — instead of + // producing persisted state that can't restore. + val roundTrip = runCatching { + val json = controller.serializers.jsonConfiguration + val serializer = PolymorphicSerializer(Any::class) + json.decodeFromString(serializer, json.encodeToString(serializer, wrapped)) + } + roundTrip.onFailure { failure -> + error( + "Object of type ${value::class} could not be added to NavigationKey.Metadata: " + + "it does not survive a serialization round-trip (${failure.message}). " + + "If it is a value class, register it with valueClassSubclass / " + + "BoxedValueClassSerializer instead of subclass." + ) + } } } } diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt new file mode 100644 index 00000000..02dde9ab --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt @@ -0,0 +1,84 @@ +package dev.enro.serialization + +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.PolymorphicSerializer +import kotlinx.serialization.Serializable +import kotlinx.serialization.builtins.MapSerializer +import kotlinx.serialization.builtins.serializer +import kotlinx.serialization.json.ClassDiscriminatorMode +import kotlinx.serialization.json.Json +import kotlinx.serialization.modules.SerializersModule +import kotlinx.serialization.modules.polymorphic +import kotlinx.serialization.modules.subclass +import kotlin.jvm.JvmInline +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +@Serializable +@JvmInline +internal value class TestValueId(val value: String) + +class BoxedValueClassSerializerTests { + + @OptIn(ExperimentalSerializationApi::class) + private fun json(builder: kotlinx.serialization.modules.PolymorphicModuleBuilder.() -> Unit): Json { + return Json { + serializersModule = SerializersModule { + polymorphic(Any::class) { builder() } + } + classDiscriminatorMode = ClassDiscriminatorMode.ALL_JSON_OBJECTS + ignoreUnknownKeys = true + } + } + + private val polymorphicAny = PolymorphicSerializer(Any::class) + private val anyMapSerializer = MapSerializer(String.serializer(), polymorphicAny) + + @Test + fun boxedValueClassRoundTripsPolymorphically() { + val json = json { valueClassSubclass(TestValueId.serializer()) } + val original: Any = TestValueId("abc-123") + + val encoded = json.encodeToString(polymorphicAny, original) + assertTrue( + encoded.contains("\"value\":\"abc-123\""), + "expected boxed envelope, was: $encoded", + ) + + val decoded = json.decodeFromString(polymorphicAny, encoded) + assertEquals(original, decoded) + } + + @Test + fun boxedValueClassRoundTripsInsidePolymorphicMap() { + val json = json { valueClassSubclass(TestValueId.serializer()) } + val original: Map = mapOf("id" to TestValueId("abc-123")) + + val encoded = json.encodeToString(anyMapSerializer, original) + val decoded = json.decodeFromString(anyMapSerializer, encoded) + + assertEquals(original, decoded) + } + + /** + * Documents the kotlinx behaviour this serializer exists to work around: + * a plain `subclass(...)` registration encodes a polymorphic value class + * as its bare literal, which then fails to decode. If this test starts + * failing because the decode succeeds, kotlinx has fixed the asymmetry + * upstream and [BoxedValueClassSerializer] may no longer be necessary. + */ + @Test + fun plainSubclassRegistrationDoesNotRoundTrip() { + val json = json { subclass(TestValueId.serializer()) } + val original: Any = TestValueId("abc-123") + + val encoded = json.encodeToString(polymorphicAny, original) + assertEquals("\"abc-123\"", encoded, "bare literal, no discriminator") + + assertFailsWith { + json.decodeFromString(polymorphicAny, encoded) + } + } +} From d06969cd5271b4ddab8b72a17d64123c809b877b Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 14:12:39 +1200 Subject: [PATCH 05/19] WebHistoryPlugin: verify serialized history state round-trips at write time kotlinx can encode shapes it cannot decode, which writes history entries that silently fail to restore. Decode-verify every state at write time and log the failure with the full payload, so the offending shape is diagnosable at the source instead of surfacing later as a dead back button on a stale entry. Co-Authored-By: Claude Fable 5 --- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index 8563e544..b2ca3056 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -266,9 +266,21 @@ internal class WebHistoryPlugin( @OptIn(ExperimentalWasmJsInterop::class) private suspend fun syncFromBackstack() { val currentState = createNodeFor(rootContainer) - val serializedCurrentState = EnroController.jsonConfiguration - .encodeToString(currentState) - .toJsString() + val serializedJson = EnroController.jsonConfiguration.encodeToString(currentState) + // Encode-side verification: kotlinx can encode shapes it cannot decode + // (e.g. polymorphic value classes degrade to bare literals), which + // would silently write history entries that can't restore. Catch that + // at write time, with the payload, so the offending shape is + // diagnosable instead of surfacing later as a dead back button. + runCatching { + EnroController.jsonConfiguration.decodeFromString(serializedJson) + }.onFailure { t -> + EnroLog.error( + "WebHistoryPlugin: serialized history state failed round-trip verification — " + + "this entry will not restore on browser back. ${t.message}\nState: $serializedJson" + ) + } + val serializedCurrentState = serializedJson.toJsString() val windowState = window.history.state?.let(::decodeState) From 33694611fbacf7c100b8f5cf8751b7b450a4c9f3 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 15:20:13 +1200 Subject: [PATCH 06/19] WebHistoryPlugin: strip metadata from unverifiable history states + diagnose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the serialized state fails round-trip verification, log the live in-memory metadata (key names + value classes — the serialized JSON mangles the offending entry, the in-memory map has the truth) and write a metadata-stripped state instead. Instance ids and keys survive, which is all back/forward restoration strictly requires, so browser back keeps working even while an unserializable metadata value exists. Co-Authored-By: Claude Fable 5 --- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 72 ++++++++++++++----- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index b2ca3056..7cb6a49f 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -5,6 +5,8 @@ import dev.enro.EnroController import dev.enro.NavigationBackstack import dev.enro.NavigationContainer import dev.enro.NavigationHandle +import dev.enro.NavigationKey +import dev.enro.asBackstack import dev.enro.annotations.ExperimentalEnroApi import dev.enro.context.ContainerContext import dev.enro.controller.createNavigationModule @@ -247,9 +249,7 @@ internal class WebHistoryPlugin( children = emptyList(), )) val currentState = createNodeFor(rootContainer) - val serializedCurrentState = EnroController.jsonConfiguration - .encodeToString(currentState) - .toJsString() + val serializedCurrentState = serializeForHistory(currentState).toJsString() window.history.replaceState(serializedCurrentState, "", computeUrl()) val index = historyStates.indexOfFirst { it == currentState } if (index != -1) { @@ -263,24 +263,60 @@ internal class WebHistoryPlugin( /** * The backstack changed (open/active/close): mirror it into browser history. */ + /** + * Serializes [state] for storage in `history.state`, verifying the result + * actually decodes. kotlinx can encode shapes it cannot decode (e.g. + * polymorphic value classes degrade to bare literals), which would + * silently write history entries that can't restore. When verification + * fails, the live (pre-serialization) metadata is logged — the JSON + * mangles the offending entry, but the in-memory map still has the real + * key and value type — and a metadata-stripped state is written instead: + * instance ids and keys are preserved, which is all back/forward + * restoration strictly requires, at the cost of metadata-dependent + * behaviour (e.g. result channels) for restored entries. + */ + @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") + @OptIn(dev.enro.annotations.AdvancedEnroApi::class) + private fun serializeForHistory(state: ContainerNode): String { + val serialized = EnroController.jsonConfiguration.encodeToString(state) + val verification = runCatching { + EnroController.jsonConfiguration.decodeFromString(serialized) + } + if (verification.isSuccess) return serialized + + val metadataDescription = state.backstack.joinToString { instance -> + val entries = instance.metadata.map.entries.joinToString { (key, value) -> + "$key=${value::class.simpleName}" + } + "${instance.key::class.simpleName}[$entries]" + } + EnroLog.error( + "WebHistoryPlugin: serialized history state failed round-trip verification " + + "(${verification.exceptionOrNull()?.message}). In-memory metadata by instance: " + + "$metadataDescription. Falling back to a metadata-stripped history entry." + ) + + val sanitized = state.copy( + backstack = state.backstack + .map { instance -> instance.copy(metadata = NavigationKey.Metadata()) } + .asBackstack(), + ) + val sanitizedSerialized = EnroController.jsonConfiguration.encodeToString(sanitized) + val sanitizedVerification = runCatching { + EnroController.jsonConfiguration.decodeFromString(sanitizedSerialized) + } + if (sanitizedVerification.isSuccess) return sanitizedSerialized + EnroLog.error( + "WebHistoryPlugin: metadata-stripped state also failed verification: " + + "${sanitizedVerification.exceptionOrNull()?.message}" + ) + return serialized + } + @OptIn(ExperimentalWasmJsInterop::class) private suspend fun syncFromBackstack() { val currentState = createNodeFor(rootContainer) - val serializedJson = EnroController.jsonConfiguration.encodeToString(currentState) - // Encode-side verification: kotlinx can encode shapes it cannot decode - // (e.g. polymorphic value classes degrade to bare literals), which - // would silently write history entries that can't restore. Catch that - // at write time, with the payload, so the offending shape is - // diagnosable instead of surfacing later as a dead back button. - runCatching { - EnroController.jsonConfiguration.decodeFromString(serializedJson) - }.onFailure { t -> - EnroLog.error( - "WebHistoryPlugin: serialized history state failed round-trip verification — " + - "this entry will not restore on browser back. ${t.message}\nState: $serializedJson" - ) - } - val serializedCurrentState = serializedJson.toJsString() + val serializedCurrentState = serializeForHistory(currentState).toJsString() val windowState = window.history.state?.let(::decodeState) From b607ea8a97c9b9e5804d2c10fd5c9d8d967c9d30 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 15:34:00 +1200 Subject: [PATCH 07/19] Fix discriminator leak corrupting Instance.metadata in history state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of unreadable web-history entries: with ClassDiscriminatorMode.ALL_JSON_OBJECTS, kotlinx defers each discriminator write until the next beginStructure — and a value-class field never opens a structure. The pending discriminator for an inline field (e.g. a typed-id value class on a NavigationKey) therefore leaked into the next object that opened: Instance.metadata, which gained a bogus {"type": ""} entry and failed to decode with 'Expected JsonObject, but had JsonLiteral'. Every instance whose key carried a value-class field was affected, even with empty metadata. jsonConfiguration now uses the default POLYMORPHIC discriminator mode, which writes discriminators exactly where polymorphic deserialization reads them (Instance.key, metadata values) and nowhere else. Pinned by HistoryStateSerializationTests. WebHistoryPlugin's write-time verification is now a hard error instead of degrading: a state that can't restore is never written (and metadata is never stripped — silently losing result-channel wiring is worse than failing loudly). The error carries the live in-memory metadata and the serialized payload for diagnosis. Co-Authored-By: Claude Fable 5 --- .../repository/SerializerRepository.kt | 11 ++- .../HistoryStateSerializationTests.kt | 87 +++++++++++++++++++ .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 43 +++------ 3 files changed, 109 insertions(+), 32 deletions(-) create mode 100644 enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt index 33d892ed..6cb70158 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt @@ -45,7 +45,16 @@ internal class SerializerRepository { var jsonConfiguration: Json = Json { serializersModule = this@SerializerRepository.serializersModule - classDiscriminatorMode = kotlinx.serialization.json.ClassDiscriminatorMode.ALL_JSON_OBJECTS + // Deliberately the default (POLYMORPHIC) discriminator mode, NOT + // ALL_JSON_OBJECTS: kotlinx defers the discriminator write until + // the next beginStructure, and a value-class field never opens + // one — so under ALL_JSON_OBJECTS the pending discriminator for an + // inline field (e.g. a typed-id value class on a NavigationKey) + // leaks into whatever object opens next (Instance.metadata, in + // practice), corrupting the serialized form so it can't decode. + // POLYMORPHIC mode writes discriminators exactly where polymorphic + // deserialization needs them (Instance.key, metadata values) and + // nowhere else. Covered by HistoryStateSerializationTests. ignoreUnknownKeys = true } private set diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt new file mode 100644 index 00000000..560ed725 --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt @@ -0,0 +1,87 @@ +package dev.enro.serialization + +import dev.enro.NavigationKey +import dev.enro.asInstance +import dev.enro.controller.repository.SerializerRepository +import kotlinx.serialization.PolymorphicSerializer +import kotlinx.serialization.Serializable +import kotlinx.serialization.modules.SerializersModule +import kotlinx.serialization.modules.polymorphic +import kotlinx.serialization.modules.subclass +import kotlin.jvm.JvmInline +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse + +@Serializable +@JvmInline +internal value class HistoryTestId(val value: String) + +@Serializable +internal data class HistoryTestKey( + val id: HistoryTestId, + val name: String = "", +) : NavigationKey + +/** + * Regression tests for the serialized form of [NavigationKey.Instance] under + * the controller's json configuration. + * + * Under `ClassDiscriminatorMode.ALL_JSON_OBJECTS`, kotlinx defers each + * discriminator write until the next `beginStructure` — and a value-class + * field never opens a structure, so the pending discriminator for an inline + * field (e.g. a typed-id value class on a NavigationKey) leaked into the next + * object that opened: in practice `Instance.metadata`, which then failed to + * decode ("Expected JsonObject, but had JsonLiteral"). These tests pin the + * fixed (default, POLYMORPHIC) discriminator mode: instances whose keys carry + * value-class fields must round-trip, with no discriminator leakage. + */ +class HistoryStateSerializationTests { + + private fun repository(): SerializerRepository { + return SerializerRepository().apply { + registerSerializersModule( + SerializersModule { + polymorphic(NavigationKey::class) { + subclass(HistoryTestKey.serializer()) + } + } + ) + } + } + + private val instanceSerializer = + NavigationKey.Instance.serializer(PolymorphicSerializer(NavigationKey::class)) + + @Test + fun instanceWithValueClassKeyFieldRoundTrips() { + val json = repository().jsonConfiguration + val instance = HistoryTestKey(id = HistoryTestId("abc-123"), name = "name").asInstance() + + val encoded = json.encodeToString(instanceSerializer, instance) + val decoded = json.decodeFromString(instanceSerializer, encoded) + + assertEquals(instance.id, decoded.id) + assertEquals(instance.key, decoded.key) + } + + @Test + fun valueClassFieldDiscriminatorDoesNotLeakIntoMetadata() { + val json = repository().jsonConfiguration + val instance = HistoryTestKey(id = HistoryTestId("abc-123")).asInstance() + + val encoded = json.encodeToString(instanceSerializer, instance) + + // The leak's signature: the inline field's pending discriminator lands + // inside the next-opened object, so metadata gains a bogus + // {"type": "...HistoryTestId"} entry. + assertFalse( + encoded.contains("\"metadata\":{\"type\""), + "discriminator leaked into metadata: $encoded", + ) + assertFalse( + encoded.contains("HistoryTestId\""), + "inline field's class name should not appear anywhere in: $encoded", + ) + } +} diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index 7cb6a49f..dd09da0d 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -5,8 +5,6 @@ import dev.enro.EnroController import dev.enro.NavigationBackstack import dev.enro.NavigationContainer import dev.enro.NavigationHandle -import dev.enro.NavigationKey -import dev.enro.asBackstack import dev.enro.annotations.ExperimentalEnroApi import dev.enro.context.ContainerContext import dev.enro.controller.createNavigationModule @@ -265,18 +263,16 @@ internal class WebHistoryPlugin( */ /** * Serializes [state] for storage in `history.state`, verifying the result - * actually decodes. kotlinx can encode shapes it cannot decode (e.g. - * polymorphic value classes degrade to bare literals), which would - * silently write history entries that can't restore. When verification - * fails, the live (pre-serialization) metadata is logged — the JSON - * mangles the offending entry, but the in-memory map still has the real - * key and value type — and a metadata-stripped state is written instead: - * instance ids and keys are preserved, which is all back/forward - * restoration strictly requires, at the cost of metadata-dependent - * behaviour (e.g. result channels) for restored entries. + * actually decodes. kotlinx can encode shapes it cannot decode (see the + * discriminator-mode note on SerializerRepository.jsonConfiguration), and + * writing a state that can't restore would silently break browser back + * for the entry. That is a hard error — degrading (e.g. stripping + * metadata) would silently lose data such as result-channel wiring, which + * is worse than failing loudly. The error includes the live in-memory + * metadata: the serialized form mangles the offending entry, but the + * in-memory map still has the real keys and value types. */ @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") - @OptIn(dev.enro.annotations.AdvancedEnroApi::class) private fun serializeForHistory(state: ContainerNode): String { val serialized = EnroController.jsonConfiguration.encodeToString(state) val verification = runCatching { @@ -290,27 +286,12 @@ internal class WebHistoryPlugin( } "${instance.key::class.simpleName}[$entries]" } - EnroLog.error( + error( "WebHistoryPlugin: serialized history state failed round-trip verification " + - "(${verification.exceptionOrNull()?.message}). In-memory metadata by instance: " + - "$metadataDescription. Falling back to a metadata-stripped history entry." + "(${verification.exceptionOrNull()?.message}). This entry would not restore on " + + "browser back, so it has NOT been written to history. In-memory metadata by " + + "instance: $metadataDescription. State: $serialized" ) - - val sanitized = state.copy( - backstack = state.backstack - .map { instance -> instance.copy(metadata = NavigationKey.Metadata()) } - .asBackstack(), - ) - val sanitizedSerialized = EnroController.jsonConfiguration.encodeToString(sanitized) - val sanitizedVerification = runCatching { - EnroController.jsonConfiguration.decodeFromString(sanitizedSerialized) - } - if (sanitizedVerification.isSuccess) return sanitizedSerialized - EnroLog.error( - "WebHistoryPlugin: metadata-stripped state also failed verification: " + - "${sanitizedVerification.exceptionOrNull()?.message}" - ) - return serialized } @OptIn(ExperimentalWasmJsInterop::class) From 5b321b5bf5fa47a4927d9b3dc07ef3f599535db6 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 16:19:08 +1200 Subject: [PATCH 08/19] Keep ALL_JSON_OBJECTS; route history serialization through the tree encoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The discriminator leak that corrupted Instance.metadata is specific to kotlinx's STREAMING encoder: under ClassDiscriminatorMode.ALL_JSON_OBJECTS it defers each discriminator write until the next beginStructure, and a value-class field never opens one — so under polymorphic dispatch (Instance.key) the pending discriminator for an inline field leaks into the next object that opens. The TREE encoder (encodeToJsonElement) does not share the deferral and produces clean, decodable output for the same configuration. WebHistoryPlugin now encodes history state via the tree encoder, transparently fixing the corruption for all consumers without changing jsonConfiguration or any public API. The write-time round-trip verification remains as a hard error (never degrade or strip data — a state that can't restore is never written). The public jsonConfiguration accessor documents the streaming-encoder caveat. HistoryStateSerializationTests pin the tree-encode round-trip and include a documenting test that fails when kotlinx fixes the streaming encoder, signalling the workaround can be retired. Also removes the BoxedValueClassSerializer/valueClassSubclass additions and the metadata round-trip debug check from earlier on this branch — superseded: the corruption was never caused by polymorphic value-class metadata values, and nothing registers such values today. Co-Authored-By: Claude Fable 5 --- .../BoxedValueClassSerializer.kt | 93 ------------------- .../kotlin/dev/enro/EnroController.kt | 34 +++---- .../repository/SerializerRepository.kt | 11 +-- .../BoxedValueClassSerializerTests.kt | 84 ----------------- .../HistoryStateSerializationTests.kt | 57 ++++++++---- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 33 +++++-- 6 files changed, 78 insertions(+), 234 deletions(-) delete mode 100644 enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt delete mode 100644 enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt diff --git a/enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt b/enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt deleted file mode 100644 index 6a11a8c3..00000000 --- a/enro-common/src/commonMain/kotlin/dev/enro/serialization/BoxedValueClassSerializer.kt +++ /dev/null @@ -1,93 +0,0 @@ -package dev.enro.serialization - -import kotlinx.serialization.KSerializer -import kotlinx.serialization.descriptors.SerialDescriptor -import kotlinx.serialization.descriptors.buildClassSerialDescriptor -import kotlinx.serialization.encoding.CompositeDecoder -import kotlinx.serialization.encoding.Decoder -import kotlinx.serialization.encoding.Encoder -import kotlinx.serialization.encoding.decodeStructure -import kotlinx.serialization.encoding.encodeStructure -import kotlinx.serialization.modules.PolymorphicModuleBuilder - -/** - * Gives a value class an object-shaped serialized form so it can participate - * in polymorphic serialization. - * - * A `@JvmInline value class` normally serializes as its underlying primitive - * (e.g. a bare JSON string). That representation has nowhere to carry a class - * discriminator, and kotlinx's polymorphic JSON encoding silently degrades to - * the bare literal on encode — which it then cannot decode ("Expected - * JsonObject, but had JsonLiteral as the serialized body"). Any value class - * stored as a polymorphic `Any` (for example in `NavigationKey.Metadata`, or - * as a navigation result) would round-trip lossily: written fine, unreadable - * on restore. - * - * Wrapping the value class's serializer in [BoxedValueClassSerializer] gives - * it a single-field envelope: - * - * ```json - * {"type": "com.example.CampaignId", "value": "abc-123"} - * ``` - * - * which the discriminator can attach to, restoring symmetric round-trips. - * Registration uses the same serial name as the underlying serializer, so the - * discriminator value is the value class's own serial name. - * - * This only affects polymorphic contexts — value classes used as plain typed - * properties keep their efficient bare-literal form. - * - * Register via [valueClassSubclass]: - * - * ```kotlin - * serializersModule(SerializersModule { - * polymorphic(Any::class) { - * valueClassSubclass(CampaignId.serializer()) - * } - * }) - * ``` - */ -public class BoxedValueClassSerializer( - private val inner: KSerializer, -) : KSerializer { - - override val descriptor: SerialDescriptor = buildClassSerialDescriptor( - serialName = inner.descriptor.serialName, - ) { - element("value", inner.descriptor) - } - - override fun serialize(encoder: Encoder, value: T) { - encoder.encodeStructure(descriptor) { - encodeSerializableElement(descriptor, 0, inner, value) - } - } - - override fun deserialize(decoder: Decoder): T { - return decoder.decodeStructure(descriptor) { - var result: T? = null - while (true) { - when (val index = decodeElementIndex(descriptor)) { - 0 -> result = decodeSerializableElement(descriptor, 0, inner) - CompositeDecoder.DECODE_DONE -> break - else -> error("Unexpected index $index while decoding ${descriptor.serialName}") - } - } - requireNotNull(result) { - "Missing 'value' element while decoding ${descriptor.serialName}" - } - } - } -} - -/** - * Registers [T] as a polymorphic subclass using a [BoxedValueClassSerializer] - * around [serializer], so the value class survives polymorphic round-trips. - * See [BoxedValueClassSerializer] for why plain `subclass(serializer)` is not - * safe for value classes. - */ -public inline fun PolymorphicModuleBuilder.valueClassSubclass( - serializer: KSerializer, -) { - subclass(T::class, BoxedValueClassSerializer(serializer)) -} diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt index 36c418e5..59faceb2 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt @@ -13,7 +13,6 @@ import dev.enro.controller.repository.PluginRepository import dev.enro.controller.repository.SerializerRepository import dev.enro.controller.repository.ViewModelRepository import dev.enro.serialization.wrapForSerialization -import kotlinx.serialization.PolymorphicSerializer import kotlinx.serialization.json.Json @Stable @@ -88,26 +87,6 @@ public class EnroController { if (!hasSerializer) { error("Object of type ${value::class} could not be added to NavigationKey.Metadata, make sure to register the serializer with the NavigationController.") } - // A registered serializer isn't enough: some shapes encode but - // can't be decoded — kotlinx silently encodes a polymorphic - // value class as its bare literal, which it then refuses to - // read back ("Expected JsonObject, but had JsonLiteral"). - // Round-trip the value so a non-restorable type fails here, - // at set time in debug, naming the type — instead of - // producing persisted state that can't restore. - val roundTrip = runCatching { - val json = controller.serializers.jsonConfiguration - val serializer = PolymorphicSerializer(Any::class) - json.decodeFromString(serializer, json.encodeToString(serializer, wrapped)) - } - roundTrip.onFailure { failure -> - error( - "Object of type ${value::class} could not be added to NavigationKey.Metadata: " + - "it does not survive a serialization round-trip (${failure.message}). " + - "If it is a value class, register it with valueClassSubclass / " + - "BoxedValueClassSerializer instead of subclass." - ) - } } } } @@ -119,6 +98,19 @@ public class EnroController { return instance ?: error("EnroController has not been installed") } + /** + * The controller's Json configuration, including all registered + * serializers and `ClassDiscriminatorMode.ALL_JSON_OBJECTS`. + * + * Caution: when serializing polymorphic content whose concrete + * classes contain value-class fields, prefer + * `encodeToJsonElement(...)` over `encodeToString(...)`. kotlinx's + * streaming encoder defers discriminator writes until the next + * `beginStructure`, and a value-class field never opens one — the + * pending discriminator leaks into the next-opened object, producing + * JSON that cannot be decoded. The tree encoder does not share the + * bug. See HistoryStateSerializationTests in enro-runtime. + */ public val jsonConfiguration: Json get() { val instance = requireInstance() return instance.serializers.jsonConfiguration diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt index 6cb70158..33d892ed 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt @@ -45,16 +45,7 @@ internal class SerializerRepository { var jsonConfiguration: Json = Json { serializersModule = this@SerializerRepository.serializersModule - // Deliberately the default (POLYMORPHIC) discriminator mode, NOT - // ALL_JSON_OBJECTS: kotlinx defers the discriminator write until - // the next beginStructure, and a value-class field never opens - // one — so under ALL_JSON_OBJECTS the pending discriminator for an - // inline field (e.g. a typed-id value class on a NavigationKey) - // leaks into whatever object opens next (Instance.metadata, in - // practice), corrupting the serialized form so it can't decode. - // POLYMORPHIC mode writes discriminators exactly where polymorphic - // deserialization needs them (Instance.key, metadata values) and - // nowhere else. Covered by HistoryStateSerializationTests. + classDiscriminatorMode = kotlinx.serialization.json.ClassDiscriminatorMode.ALL_JSON_OBJECTS ignoreUnknownKeys = true } private set diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt deleted file mode 100644 index 02dde9ab..00000000 --- a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/BoxedValueClassSerializerTests.kt +++ /dev/null @@ -1,84 +0,0 @@ -package dev.enro.serialization - -import kotlinx.serialization.ExperimentalSerializationApi -import kotlinx.serialization.PolymorphicSerializer -import kotlinx.serialization.Serializable -import kotlinx.serialization.builtins.MapSerializer -import kotlinx.serialization.builtins.serializer -import kotlinx.serialization.json.ClassDiscriminatorMode -import kotlinx.serialization.json.Json -import kotlinx.serialization.modules.SerializersModule -import kotlinx.serialization.modules.polymorphic -import kotlinx.serialization.modules.subclass -import kotlin.jvm.JvmInline -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertFailsWith -import kotlin.test.assertTrue - -@Serializable -@JvmInline -internal value class TestValueId(val value: String) - -class BoxedValueClassSerializerTests { - - @OptIn(ExperimentalSerializationApi::class) - private fun json(builder: kotlinx.serialization.modules.PolymorphicModuleBuilder.() -> Unit): Json { - return Json { - serializersModule = SerializersModule { - polymorphic(Any::class) { builder() } - } - classDiscriminatorMode = ClassDiscriminatorMode.ALL_JSON_OBJECTS - ignoreUnknownKeys = true - } - } - - private val polymorphicAny = PolymorphicSerializer(Any::class) - private val anyMapSerializer = MapSerializer(String.serializer(), polymorphicAny) - - @Test - fun boxedValueClassRoundTripsPolymorphically() { - val json = json { valueClassSubclass(TestValueId.serializer()) } - val original: Any = TestValueId("abc-123") - - val encoded = json.encodeToString(polymorphicAny, original) - assertTrue( - encoded.contains("\"value\":\"abc-123\""), - "expected boxed envelope, was: $encoded", - ) - - val decoded = json.decodeFromString(polymorphicAny, encoded) - assertEquals(original, decoded) - } - - @Test - fun boxedValueClassRoundTripsInsidePolymorphicMap() { - val json = json { valueClassSubclass(TestValueId.serializer()) } - val original: Map = mapOf("id" to TestValueId("abc-123")) - - val encoded = json.encodeToString(anyMapSerializer, original) - val decoded = json.decodeFromString(anyMapSerializer, encoded) - - assertEquals(original, decoded) - } - - /** - * Documents the kotlinx behaviour this serializer exists to work around: - * a plain `subclass(...)` registration encodes a polymorphic value class - * as its bare literal, which then fails to decode. If this test starts - * failing because the decode succeeds, kotlinx has fixed the asymmetry - * upstream and [BoxedValueClassSerializer] may no longer be necessary. - */ - @Test - fun plainSubclassRegistrationDoesNotRoundTrip() { - val json = json { subclass(TestValueId.serializer()) } - val original: Any = TestValueId("abc-123") - - val encoded = json.encodeToString(polymorphicAny, original) - assertEquals("\"abc-123\"", encoded, "bare literal, no discriminator") - - assertFailsWith { - json.decodeFromString(polymorphicAny, encoded) - } - } -} diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt index 560ed725..e071455f 100644 --- a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt @@ -12,6 +12,7 @@ import kotlin.jvm.JvmInline import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertTrue @Serializable @JvmInline @@ -24,17 +25,22 @@ internal data class HistoryTestKey( ) : NavigationKey /** - * Regression tests for the serialized form of [NavigationKey.Instance] under - * the controller's json configuration. + * Pins the serialization strategy for persisted navigation state (browser + * history, deep-link tooling) under the controller's json configuration. * - * Under `ClassDiscriminatorMode.ALL_JSON_OBJECTS`, kotlinx defers each - * discriminator write until the next `beginStructure` — and a value-class - * field never opens a structure, so the pending discriminator for an inline - * field (e.g. a typed-id value class on a NavigationKey) leaked into the next - * object that opened: in practice `Instance.metadata`, which then failed to - * decode ("Expected JsonObject, but had JsonLiteral"). These tests pin the - * fixed (default, POLYMORPHIC) discriminator mode: instances whose keys carry - * value-class fields must round-trip, with no discriminator leakage. + * kotlinx's STREAMING encoder has a bug under + * `ClassDiscriminatorMode.ALL_JSON_OBJECTS`: it defers each discriminator + * write until the next `beginStructure`, and a value-class field never opens + * one — so under polymorphic dispatch (`Instance.key`) the pending + * discriminator for an inline field (e.g. a typed-id value class on a + * NavigationKey) leaks into the next object that opens (`Instance.metadata` + * in practice), producing JSON that fails to decode with "Expected + * JsonObject, but had JsonLiteral". The TREE encoder (`encodeToJsonElement`) + * does not share the deferral and produces clean output for the same + * configuration, so persisted state must be encoded through the tree + * encoder. [streamingEncoderLeaksDiscriminator] documents the upstream bug: + * when it starts failing, kotlinx has fixed the streaming encoder and the + * tree-encode workaround can be retired. */ class HistoryStateSerializationTests { @@ -54,11 +60,11 @@ class HistoryStateSerializationTests { NavigationKey.Instance.serializer(PolymorphicSerializer(NavigationKey::class)) @Test - fun instanceWithValueClassKeyFieldRoundTrips() { + fun treeEncodedInstanceWithValueClassKeyFieldRoundTrips() { val json = repository().jsonConfiguration val instance = HistoryTestKey(id = HistoryTestId("abc-123"), name = "name").asInstance() - val encoded = json.encodeToString(instanceSerializer, instance) + val encoded = json.encodeToJsonElement(instanceSerializer, instance).toString() val decoded = json.decodeFromString(instanceSerializer, encoded) assertEquals(instance.id, decoded.id) @@ -66,15 +72,12 @@ class HistoryStateSerializationTests { } @Test - fun valueClassFieldDiscriminatorDoesNotLeakIntoMetadata() { + fun treeEncodedInstanceDoesNotLeakDiscriminatorIntoMetadata() { val json = repository().jsonConfiguration val instance = HistoryTestKey(id = HistoryTestId("abc-123")).asInstance() - val encoded = json.encodeToString(instanceSerializer, instance) + val encoded = json.encodeToJsonElement(instanceSerializer, instance).toString() - // The leak's signature: the inline field's pending discriminator lands - // inside the next-opened object, so metadata gains a bogus - // {"type": "...HistoryTestId"} entry. assertFalse( encoded.contains("\"metadata\":{\"type\""), "discriminator leaked into metadata: $encoded", @@ -84,4 +87,24 @@ class HistoryStateSerializationTests { "inline field's class name should not appear anywhere in: $encoded", ) } + + /** + * Documents the upstream kotlinx streaming-encoder bug that forces the + * tree-encode strategy. If this test FAILS, kotlinx has fixed the + * deferred-discriminator leak — the tree-encode workaround in + * WebHistoryPlugin (and this test) can then be removed. + */ + @Test + fun streamingEncoderLeaksDiscriminator() { + val json = repository().jsonConfiguration + val instance = HistoryTestKey(id = HistoryTestId("abc-123")).asInstance() + + val encoded = json.encodeToString(instanceSerializer, instance) + + assertTrue( + encoded.contains("\"metadata\":{\"type\":\"dev.enro.serialization.HistoryTestId\""), + "kotlinx appears to have fixed the streaming-encoder discriminator leak — " + + "the tree-encode workaround can be retired. Encoded: $encoded", + ) + } } diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index dd09da0d..3cb6a8f4 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -263,18 +263,33 @@ internal class WebHistoryPlugin( */ /** * Serializes [state] for storage in `history.state`, verifying the result - * actually decodes. kotlinx can encode shapes it cannot decode (see the - * discriminator-mode note on SerializerRepository.jsonConfiguration), and - * writing a state that can't restore would silently break browser back - * for the entry. That is a hard error — degrading (e.g. stripping - * metadata) would silently lose data such as result-channel wiring, which - * is worse than failing loudly. The error includes the live in-memory - * metadata: the serialized form mangles the offending entry, but the - * in-memory map still has the real keys and value types. + * actually decodes. + * + * Encoding goes through the TREE encoder (`encodeToJsonElement`) rather + * than the streaming encoder (`encodeToString`): under + * `ClassDiscriminatorMode.ALL_JSON_OBJECTS` the streaming encoder defers + * each discriminator write until the next `beginStructure`, and a + * value-class field never opens one — so under polymorphic dispatch + * (`Instance.key`) the pending discriminator for an inline field leaks + * into the next object that opens (`Instance.metadata` in practice), + * producing JSON that cannot be decoded. The tree encoder does not share + * the deferral and produces clean, decodable output for the same + * configuration. See HistoryStateSerializationTests; the workaround can + * be removed when the upstream kotlinx streaming-encoder bug is fixed. + * + * Verification failure is a hard error — writing a state that can't + * restore would silently break browser back for the entry, and degrading + * (e.g. stripping metadata) would silently lose data such as + * result-channel wiring, which is worse than failing loudly. The error + * includes the live in-memory metadata: the serialized form mangles the + * offending entry, but the in-memory map still has the real keys and + * value types. */ @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") private fun serializeForHistory(state: ContainerNode): String { - val serialized = EnroController.jsonConfiguration.encodeToString(state) + val serialized = EnroController.jsonConfiguration + .encodeToJsonElement(ContainerNode.serializer(), state) + .toString() val verification = runCatching { EnroController.jsonConfiguration.decodeFromString(serialized) } From ab66421a34711aed28d1294e037d27cdaa3044b1 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 16:27:38 +1200 Subject: [PATCH 09/19] Pin savedstate round-trip for instances with value-class key fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The savedstate path (rememberNavigationContainer, FlowResultManager, enroSaver) uses androidx.savedstate serialization with ALL_OBJECTS — a different encoder from kotlinx's streaming JSON. Verifies it does NOT share the deferred-discriminator leak documented in HistoryStateSerializationTests, so container restoration is unaffected. Co-Authored-By: Claude Fable 5 --- .../SavedStateSerializationTests.kt | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt new file mode 100644 index 00000000..442b9835 --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt @@ -0,0 +1,52 @@ +package dev.enro.serialization + +import androidx.savedstate.serialization.decodeFromSavedState +import androidx.savedstate.serialization.encodeToSavedState +import dev.enro.NavigationKey +import dev.enro.asInstance +import dev.enro.controller.repository.SerializerRepository +import kotlinx.serialization.PolymorphicSerializer +import kotlinx.serialization.modules.SerializersModule +import kotlinx.serialization.modules.polymorphic +import kotlinx.serialization.modules.subclass +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * The savedstate analogue of [HistoryStateSerializationTests]: container + * backstacks (rememberNavigationContainer), flow results, and enroSaver all + * round-trip through androidx.savedstate serialization with + * `ClassDiscriminatorMode.ALL_OBJECTS`. This pins that an Instance whose key + * carries a value-class field survives that round-trip — i.e. that the + * androidx SavedState encoder does NOT share the kotlinx streaming-JSON + * encoder's deferred-discriminator leak (see HistoryStateSerializationTests). + */ +class SavedStateSerializationTests { + + private fun repository(): SerializerRepository { + return SerializerRepository().apply { + registerSerializersModule( + SerializersModule { + polymorphic(NavigationKey::class) { + subclass(HistoryTestKey.serializer()) + } + } + ) + } + } + + private val instanceSerializer = + NavigationKey.Instance.serializer(PolymorphicSerializer(NavigationKey::class)) + + @Test + fun instanceWithValueClassKeyFieldRoundTripsThroughSavedState() { + val configuration = repository().savedStateConfiguration + val instance = HistoryTestKey(id = HistoryTestId("abc-123"), name = "name").asInstance() + + val saved = encodeToSavedState(instanceSerializer, instance, configuration) + val decoded = decodeFromSavedState(instanceSerializer, saved, configuration) + + assertEquals(instance.id, decoded.id) + assertEquals(instance.key, decoded.key) + } +} From f4af4ef28e85099e9efcbdd04f76fb39234b8d07 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 16:41:47 +1200 Subject: [PATCH 10/19] Switch jsonConfiguration to POLYMORPHIC discriminator mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALL_JSON_OBJECTS is unusable with kotlinx 1.11 for realistic NavigationKey shapes under polymorphic dispatch — BOTH encoders fail: - STREAMING: a value-class field's deferred discriminator leaks into the next-opened object (corrupting Instance.metadata so it can't decode), and collection fields produce outright INVALID JSON (a 'type' key:value pair inside an array, with the wrong class name). - TREE (encodeToJsonElement, the previous workaround): collection fields crash the encoder with NumberFormatException — the deferred discriminator tag is applied inside a list context where it's parsed as an array index. Surfaced by any destination with Set/List fields (e.g. an entity-picker popup with allowedTypes/excludeIds). POLYMORPHIC mode (the kotlinx default) writes discriminators exactly where polymorphic deserialization reads them (Instance.key, metadata values) and handles all of these shapes correctly. WebHistoryPlugin returns to plain encodeToString (the tree-encode workaround is obsolete and was itself broken for collection fields), keeping the write-time round-trip verification as a hard error. HistoryStateSerializationTests pins the POLYMORPHIC round-trip for the full problem shape (value class + enum set + value-class set) and keeps documenting tests asserting both ALL_JSON_OBJECTS failures, so a kotlinx upgrade that fixes them is detected and the mode can be revisited. Co-Authored-By: Claude Fable 5 --- .../kotlin/dev/enro/EnroController.kt | 15 +-- .../repository/SerializerRepository.kt | 14 ++- .../HistoryStateSerializationTests.kt | 110 +++++++++++------- .../kotlin/dev/enro/ui/WebHistoryPlugin.kt | 21 +--- 4 files changed, 93 insertions(+), 67 deletions(-) diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt index 59faceb2..a0faef12 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt @@ -100,16 +100,11 @@ public class EnroController { /** * The controller's Json configuration, including all registered - * serializers and `ClassDiscriminatorMode.ALL_JSON_OBJECTS`. - * - * Caution: when serializing polymorphic content whose concrete - * classes contain value-class fields, prefer - * `encodeToJsonElement(...)` over `encodeToString(...)`. kotlinx's - * streaming encoder defers discriminator writes until the next - * `beginStructure`, and a value-class field never opens one — the - * pending discriminator leaks into the next-opened object, producing - * JSON that cannot be decoded. The tree encoder does not share the - * bug. See HistoryStateSerializationTests in enro-runtime. + * serializers. Uses the default (POLYMORPHIC) class-discriminator + * mode — `ALL_JSON_OBJECTS` is incompatible with kotlinx 1.11 for + * keys containing value-class or collection fields; see the note on + * SerializerRepository.jsonConfiguration and + * HistoryStateSerializationTests in enro-runtime. */ public val jsonConfiguration: Json get() { val instance = requireInstance() diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt index 33d892ed..cb0dc2f8 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt @@ -45,7 +45,19 @@ internal class SerializerRepository { var jsonConfiguration: Json = Json { serializersModule = this@SerializerRepository.serializersModule - classDiscriminatorMode = kotlinx.serialization.json.ClassDiscriminatorMode.ALL_JSON_OBJECTS + // Deliberately the default (POLYMORPHIC) discriminator mode. + // ALL_JSON_OBJECTS is unusable with kotlinx 1.11 for realistic + // NavigationKey shapes: under polymorphic dispatch, the STREAMING + // encoder leaks a value-class field's deferred discriminator into + // the next-opened object (corrupting Instance.metadata), and emits + // INVALID JSON for collection fields (a "type" key:value pair + // inside an array); the TREE encoder crashes outright + // (NumberFormatException) on collection fields. POLYMORPHIC mode + // writes discriminators exactly where polymorphic deserialization + // reads them (Instance.key, metadata values) and handles all of + // these shapes correctly. See HistoryStateSerializationTests, + // which also documents the ALL_JSON_OBJECTS failures so a kotlinx + // upgrade that fixes them is detected. ignoreUnknownKeys = true } private set diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt index e071455f..97e7a22d 100644 --- a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt @@ -3,44 +3,57 @@ package dev.enro.serialization import dev.enro.NavigationKey import dev.enro.asInstance import dev.enro.controller.repository.SerializerRepository +import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.PolymorphicSerializer import kotlinx.serialization.Serializable +import kotlinx.serialization.json.ClassDiscriminatorMode +import kotlinx.serialization.json.Json import kotlinx.serialization.modules.SerializersModule import kotlinx.serialization.modules.polymorphic import kotlinx.serialization.modules.subclass import kotlin.jvm.JvmInline import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertTrue @Serializable @JvmInline internal value class HistoryTestId(val value: String) +@Serializable +internal enum class HistoryTestType { NPC, LOCATION } + @Serializable internal data class HistoryTestKey( val id: HistoryTestId, val name: String = "", + val types: Set = emptySet(), + val excludeIds: Set = emptySet(), ) : NavigationKey /** - * Pins the serialization strategy for persisted navigation state (browser - * history, deep-link tooling) under the controller's json configuration. + * Pins the serialization of persisted navigation state (browser history, + * deep-link tooling) under the controller's json configuration, which uses + * the default (POLYMORPHIC) class-discriminator mode. + * + * `ClassDiscriminatorMode.ALL_JSON_OBJECTS` cannot be used: with kotlinx + * 1.11, realistic NavigationKey shapes break BOTH encoders under polymorphic + * dispatch — * - * kotlinx's STREAMING encoder has a bug under - * `ClassDiscriminatorMode.ALL_JSON_OBJECTS`: it defers each discriminator - * write until the next `beginStructure`, and a value-class field never opens - * one — so under polymorphic dispatch (`Instance.key`) the pending - * discriminator for an inline field (e.g. a typed-id value class on a - * NavigationKey) leaks into the next object that opens (`Instance.metadata` - * in practice), producing JSON that fails to decode with "Expected - * JsonObject, but had JsonLiteral". The TREE encoder (`encodeToJsonElement`) - * does not share the deferral and produces clean output for the same - * configuration, so persisted state must be encoded through the tree - * encoder. [streamingEncoderLeaksDiscriminator] documents the upstream bug: - * when it starts failing, kotlinx has fixed the streaming encoder and the - * tree-encode workaround can be retired. + * * STREAMING: a value-class field's discriminator write is deferred until + * the next `beginStructure`, which an inline field never opens, so the + * pending discriminator leaks into the next-opened object + * (`Instance.metadata` in practice) and the output fails to decode with + * "Expected JsonObject, but had JsonLiteral". Collection fields produce + * outright INVALID JSON (a `"type"` key:value pair inside an array). + * * TREE (`encodeToJsonElement`): collection fields crash the encoder with + * `NumberFormatException: For input string: "type"` (the deferred + * discriminator is applied inside a list context, where the tag is parsed + * as an array index). + * + * The documenting tests below assert those failures still exist: when one + * starts failing, kotlinx has fixed the corresponding bug and the + * POLYMORPHIC-mode constraint can be revisited. */ class HistoryStateSerializationTests { @@ -59,52 +72,69 @@ class HistoryStateSerializationTests { private val instanceSerializer = NavigationKey.Instance.serializer(PolymorphicSerializer(NavigationKey::class)) + private val fullShapeKey = HistoryTestKey( + id = HistoryTestId("abc-123"), + name = "name", + types = setOf(HistoryTestType.NPC, HistoryTestType.LOCATION), + excludeIds = setOf(HistoryTestId("excluded")), + ) + @Test - fun treeEncodedInstanceWithValueClassKeyFieldRoundTrips() { + fun instanceWithValueClassAndCollectionFieldsRoundTrips() { val json = repository().jsonConfiguration - val instance = HistoryTestKey(id = HistoryTestId("abc-123"), name = "name").asInstance() + val instance = fullShapeKey.asInstance() - val encoded = json.encodeToJsonElement(instanceSerializer, instance).toString() + val encoded = json.encodeToString(instanceSerializer, instance) val decoded = json.decodeFromString(instanceSerializer, encoded) assertEquals(instance.id, decoded.id) assertEquals(instance.key, decoded.key) } + @OptIn(ExperimentalSerializationApi::class) + private fun allJsonObjectsJson(): Json = Json(from = repository().jsonConfiguration) { + classDiscriminatorMode = ClassDiscriminatorMode.ALL_JSON_OBJECTS + } + + /** + * Documents the upstream kotlinx streaming-encoder bug. If this test + * FAILS, kotlinx has fixed the deferred-discriminator leak for inline + * fields and ALL_JSON_OBJECTS may be viable again (check the other + * documenting tests too). + */ @Test - fun treeEncodedInstanceDoesNotLeakDiscriminatorIntoMetadata() { - val json = repository().jsonConfiguration + fun allJsonObjectsStreamingEncoderLeaksValueClassDiscriminator() { + val json = allJsonObjectsJson() val instance = HistoryTestKey(id = HistoryTestId("abc-123")).asInstance() - val encoded = json.encodeToJsonElement(instanceSerializer, instance).toString() + val encoded = json.encodeToString(instanceSerializer, instance) - assertFalse( - encoded.contains("\"metadata\":{\"type\""), - "discriminator leaked into metadata: $encoded", - ) - assertFalse( - encoded.contains("HistoryTestId\""), - "inline field's class name should not appear anywhere in: $encoded", + assertTrue( + encoded.contains("\"metadata\":{\"type\":\"dev.enro.serialization.HistoryTestId\""), + "kotlinx appears to have fixed the streaming-encoder discriminator leak — " + + "ALL_JSON_OBJECTS may be viable again. Encoded: $encoded", ) } /** - * Documents the upstream kotlinx streaming-encoder bug that forces the - * tree-encode strategy. If this test FAILS, kotlinx has fixed the - * deferred-discriminator leak — the tree-encode workaround in - * WebHistoryPlugin (and this test) can then be removed. + * Documents the upstream kotlinx tree-encoder bug. If this test FAILS, + * kotlinx has fixed the collection-field discriminator crash and + * ALL_JSON_OBJECTS may be viable again (check the other documenting + * tests too). */ @Test - fun streamingEncoderLeaksDiscriminator() { - val json = repository().jsonConfiguration - val instance = HistoryTestKey(id = HistoryTestId("abc-123")).asInstance() + fun allJsonObjectsTreeEncoderFailsOnCollectionFields() { + val json = allJsonObjectsJson() + val instance = fullShapeKey.asInstance() - val encoded = json.encodeToString(instanceSerializer, instance) + val result = runCatching { + json.encodeToJsonElement(instanceSerializer, instance) + } assertTrue( - encoded.contains("\"metadata\":{\"type\":\"dev.enro.serialization.HistoryTestId\""), - "kotlinx appears to have fixed the streaming-encoder discriminator leak — " + - "the tree-encode workaround can be retired. Encoded: $encoded", + result.isFailure, + "kotlinx appears to have fixed the tree-encoder collection-field crash — " + + "ALL_JSON_OBJECTS may be viable again. Encoded: ${result.getOrNull()}", ) } } diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index 3cb6a8f4..ecbb24d8 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -263,19 +263,10 @@ internal class WebHistoryPlugin( */ /** * Serializes [state] for storage in `history.state`, verifying the result - * actually decodes. - * - * Encoding goes through the TREE encoder (`encodeToJsonElement`) rather - * than the streaming encoder (`encodeToString`): under - * `ClassDiscriminatorMode.ALL_JSON_OBJECTS` the streaming encoder defers - * each discriminator write until the next `beginStructure`, and a - * value-class field never opens one — so under polymorphic dispatch - * (`Instance.key`) the pending discriminator for an inline field leaks - * into the next object that opens (`Instance.metadata` in practice), - * producing JSON that cannot be decoded. The tree encoder does not share - * the deferral and produces clean, decodable output for the same - * configuration. See HistoryStateSerializationTests; the workaround can - * be removed when the upstream kotlinx streaming-encoder bug is fixed. + * actually decodes. Encode-and-decode-back is cheap insurance against + * serialization shapes kotlinx mishandles (see the discriminator-mode + * note on SerializerRepository.jsonConfiguration for the class of bug + * this guards against). * * Verification failure is a hard error — writing a state that can't * restore would silently break browser back for the entry, and degrading @@ -287,9 +278,7 @@ internal class WebHistoryPlugin( */ @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") private fun serializeForHistory(state: ContainerNode): String { - val serialized = EnroController.jsonConfiguration - .encodeToJsonElement(ContainerNode.serializer(), state) - .toString() + val serialized = EnroController.jsonConfiguration.encodeToString(state) val verification = runCatching { EnroController.jsonConfiguration.decodeFromString(serialized) } From 0e50c9cb3b6d2caad437657b93efcbaa30e77aa1 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 16:52:46 +1200 Subject: [PATCH 11/19] Changelog for web history sync rewrite + POLYMORPHIC discriminator mode Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a3738c..1b2b1cac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,40 @@ ## 3.0.0-beta02 (Unreleased) +### Web browser history (WasmJS) + +* Rewrote `WebHistoryPlugin`'s history synchronisation. Updates are now + processed on a serial queue instead of being dropped while a sync was in + flight (the cause of a single browser back traversing multiple screens), + self-initiated `history.go()` traversals await and consume their popstate + echo instead of racing a `delay(1)` (the cause of double-pops), and + full-backstack replacements (e.g. a loading gate resolving to home, or a + section switch) now `replaceState` instead of `pushState`, so transient + screens can no longer survive as browser back targets. +* History entries whose recorded state can't be decoded (e.g. written by an + older build of an app — tab history survives deploys) now restore through + the entry's URL via the `@NavigationPath` bindings instead of doing + nothing, and self-heal the entry's stored state. +* Serialized history state is verified to round-trip at write time; a state + that can't restore is a hard error rather than a silently broken back + button later. + +### Serialization + +* `EnroController.jsonConfiguration` now uses kotlinx's default + (`POLYMORPHIC`) class-discriminator mode instead of `ALL_JSON_OBJECTS`. + With kotlinx 1.11, `ALL_JSON_OBJECTS` breaks both Json encoders for + realistic `NavigationKey` shapes under polymorphic dispatch: the streaming + encoder leaks a value-class field's deferred discriminator into the + next-opened object (corrupting `Instance.metadata` so persisted state + could not be decoded) and emits invalid JSON for collection fields, while + the tree encoder crashes on collection fields. `POLYMORPHIC` mode writes + discriminators exactly where polymorphic deserialization reads them and + handles all of these shapes correctly. `HistoryStateSerializationTests` + documents the upstream kotlinx failures so a fixed kotlinx release is + detected. SavedState serialization (`ALL_OBJECTS`, androidx encoder) is + unaffected and covered by `SavedStateSerializationTests`. + ### Predictive back * Predictive back on iOS now starts smoothly and tracks the gesture From 278fba5d046d8cf12c4d25e7e5bed9651c9c7d6a Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 16:57:06 +1200 Subject: [PATCH 12/19] Link upstream kotlinx issue #3022 from discriminator-mode comments Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 5 +++-- .../src/commonMain/kotlin/dev/enro/EnroController.kt | 5 +++-- .../dev/enro/controller/repository/SerializerRepository.kt | 3 ++- .../dev/enro/serialization/HistoryStateSerializationTests.kt | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2b1cac..e29056bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,8 +32,9 @@ the tree encoder crashes on collection fields. `POLYMORPHIC` mode writes discriminators exactly where polymorphic deserialization reads them and handles all of these shapes correctly. `HistoryStateSerializationTests` - documents the upstream kotlinx failures so a fixed kotlinx release is - detected. SavedState serialization (`ALL_OBJECTS`, androidx encoder) is + documents the upstream kotlinx failures + ([Kotlin/kotlinx.serialization#3022](https://github.com/Kotlin/kotlinx.serialization/issues/3022)) + so a fixed kotlinx release is detected. SavedState serialization (`ALL_OBJECTS`, androidx encoder) is unaffected and covered by `SavedStateSerializationTests`. ### Predictive back diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt index a0faef12..6eb738fa 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt @@ -102,8 +102,9 @@ public class EnroController { * The controller's Json configuration, including all registered * serializers. Uses the default (POLYMORPHIC) class-discriminator * mode — `ALL_JSON_OBJECTS` is incompatible with kotlinx 1.11 for - * keys containing value-class or collection fields; see the note on - * SerializerRepository.jsonConfiguration and + * keys containing value-class or collection fields + * (https://github.com/Kotlin/kotlinx.serialization/issues/3022); see + * the note on SerializerRepository.jsonConfiguration and * HistoryStateSerializationTests in enro-runtime. */ public val jsonConfiguration: Json get() { diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt index cb0dc2f8..a3d12a12 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt @@ -57,7 +57,8 @@ internal class SerializerRepository { // reads them (Instance.key, metadata values) and handles all of // these shapes correctly. See HistoryStateSerializationTests, // which also documents the ALL_JSON_OBJECTS failures so a kotlinx - // upgrade that fixes them is detected. + // upgrade that fixes them is detected. Upstream: + // https://github.com/Kotlin/kotlinx.serialization/issues/3022 ignoreUnknownKeys = true } private set diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt index 97e7a22d..020023fe 100644 --- a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt @@ -53,7 +53,8 @@ internal data class HistoryTestKey( * * The documenting tests below assert those failures still exist: when one * starts failing, kotlinx has fixed the corresponding bug and the - * POLYMORPHIC-mode constraint can be revisited. + * POLYMORPHIC-mode constraint can be revisited. Upstream issue: + * https://github.com/Kotlin/kotlinx.serialization/issues/3022 */ class HistoryStateSerializationTests { From c66ee05e3ad6cc6572d4bd253af2308fd48c417f Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 16:57:33 +1200 Subject: [PATCH 13/19] Move web-history/serialization changelog entries to 3.0.0-beta03 3.0.0-beta02 has been released; these changes land in the next version. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e29056bc..a80c72de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 3.0.0-beta02 (Unreleased) +## 3.0.0-beta03 (Unreleased) ### Web browser history (WasmJS) @@ -37,6 +37,8 @@ so a fixed kotlinx release is detected. SavedState serialization (`ALL_OBJECTS`, androidx encoder) is unaffected and covered by `SavedStateSerializationTests`. +## 3.0.0-beta02 + ### Predictive back * Predictive back on iOS now starts smoothly and tracks the gesture From 316002d33aa11be7a5ee23f4993007d098c4f57b Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 17:16:57 +1200 Subject: [PATCH 14/19] Move SavedStateSerializationTests to desktopTest On the androidHostTest target, the Bundle-backed SavedState implementation is not representative of a real device: ANY polymorphic Instance round-trip through encodeToSavedState/decodeFromSavedState fails there ('No valid saved state was found for the key, including keys with no value-class fields at all (verified with a plain-String control key). The serialization logic the test pins is common code; desktop's Map-backed SavedState exercises it without the unrepresentative host-Bundle layer. Device-faithful Android coverage would need an instrumented test. Co-Authored-By: Claude Fable 5 --- .../serialization/SavedStateSerializationTests.kt | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) rename enro-runtime/src/{commonTest => desktopTest}/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt (75%) diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt b/enro-runtime/src/desktopTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt similarity index 75% rename from enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt rename to enro-runtime/src/desktopTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt index 442b9835..3ef2fd36 100644 --- a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt +++ b/enro-runtime/src/desktopTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt @@ -19,7 +19,17 @@ import kotlin.test.assertEquals * `ClassDiscriminatorMode.ALL_OBJECTS`. This pins that an Instance whose key * carries a value-class field survives that round-trip — i.e. that the * androidx SavedState encoder does NOT share the kotlinx streaming-JSON - * encoder's deferred-discriminator leak (see HistoryStateSerializationTests). + * encoder's deferred-discriminator leak (see HistoryStateSerializationTests + * and https://github.com/Kotlin/kotlinx.serialization/issues/3022). + * + * Lives in desktopTest rather than commonTest: the serialization logic under + * test is common, but on the androidHostTest target the Bundle-backed + * SavedState implementation is not representative of a real device — ANY + * polymorphic Instance round-trip fails there ("No valid saved state was + * found for the key 'key'"), including keys with no value classes at all. + * Desktop's Map-backed SavedState exercises the same common serialization + * code without the unrepresentative host-Bundle layer; device-faithful + * Android coverage would need an instrumented test. */ class SavedStateSerializationTests { From 8636a02bfa0614848d064049cdf8f0b837993f75 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 17:18:36 +1200 Subject: [PATCH 15/19] Revert "Move SavedStateSerializationTests to desktopTest" This reverts commit 316002d33aa11be7a5ee23f4993007d098c4f57b. --- .../serialization/SavedStateSerializationTests.kt | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) rename enro-runtime/src/{desktopTest => commonTest}/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt (75%) diff --git a/enro-runtime/src/desktopTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt similarity index 75% rename from enro-runtime/src/desktopTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt rename to enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt index 3ef2fd36..442b9835 100644 --- a/enro-runtime/src/desktopTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt @@ -19,17 +19,7 @@ import kotlin.test.assertEquals * `ClassDiscriminatorMode.ALL_OBJECTS`. This pins that an Instance whose key * carries a value-class field survives that round-trip — i.e. that the * androidx SavedState encoder does NOT share the kotlinx streaming-JSON - * encoder's deferred-discriminator leak (see HistoryStateSerializationTests - * and https://github.com/Kotlin/kotlinx.serialization/issues/3022). - * - * Lives in desktopTest rather than commonTest: the serialization logic under - * test is common, but on the androidHostTest target the Bundle-backed - * SavedState implementation is not representative of a real device — ANY - * polymorphic Instance round-trip fails there ("No valid saved state was - * found for the key 'key'"), including keys with no value classes at all. - * Desktop's Map-backed SavedState exercises the same common serialization - * code without the unrepresentative host-Bundle layer; device-faithful - * Android coverage would need an instrumented test. + * encoder's deferred-discriminator leak (see HistoryStateSerializationTests). */ class SavedStateSerializationTests { From 4a100beb880eb8479185e5e4ec95f1c7c6253d44 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 17:31:06 +1200 Subject: [PATCH 16/19] Run framework-touching host tests under Robolectric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Android host-test target compiles commonTest against android.jar stubs, and isReturnDefaultValues makes stubbed framework methods return defaults silently — savedstate's Bundle round-trip 'succeeds' on write and returns null on read ('No valid saved state was found for the key ...'). Any polymorphic Instance round-trip through encodeToSavedState/decodeFromSavedState failed this way on the host target, including keys with no value-class fields. Adds dev.enro.test.platform.RobolectricHostTest, an expect/actual base class that runs the test under RobolectricTestRunner on the Android host target (no-op everywhere else), with Robolectric on the host-test classpath. SavedStateSerializationTests extends it and now passes on testAndroidHostTest with a functional Bundle — also confirming that real-device container state restoration round-trips polymorphic instances correctly. The SceneHarnessSmokeTest/SceneIntegrationTests/BackstackSavedStateTests exclusions are left as-is; they may be removable with the same pattern. Co-Authored-By: Claude Fable 5 --- enro-runtime/build.gradle.kts | 10 +++++++++- .../dev/enro/test/platform/RobolectricHostTest.kt | 7 +++++++ .../serialization/SavedStateSerializationTests.kt | 3 ++- .../dev/enro/test/platform/RobolectricHostTest.kt | 15 +++++++++++++++ .../dev/enro/test/platform/RobolectricHostTest.kt | 3 +++ .../dev/enro/test/platform/RobolectricHostTest.kt | 3 +++ .../dev/enro/test/platform/RobolectricHostTest.kt | 3 +++ 7 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt create mode 100644 enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt create mode 100644 enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt create mode 100644 enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt create mode 100644 enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt diff --git a/enro-runtime/build.gradle.kts b/enro-runtime/build.gradle.kts index 19f7d557..c79968d5 100644 --- a/enro-runtime/build.gradle.kts +++ b/enro-runtime/build.gradle.kts @@ -14,8 +14,11 @@ kotlin { // Return default values for unmocked Android framework methods // rather than throwing "Method ... not mocked" — defensive against // any test path that brushes a stub from android.jar (e.g. - // savedstate's Bundle). + // savedstate's Bundle). Tests that genuinely exercise framework + // classes should extend dev.enro.test.platform.RobolectricHostTest + // to run under Robolectric on this target instead of the stubs. isReturnDefaultValues = true + isIncludeAndroidResources = true } } } @@ -57,6 +60,11 @@ kotlin { implementation(project(":enro-test")) implementation(libs.compose.uiTest) } + val androidHostTest by getting { + dependencies { + implementation(libs.testing.robolectric) + } + } androidMain.dependencies { implementation(libs.androidx.core) implementation(libs.androidx.appcompat) diff --git a/enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..760d98c0 --- /dev/null +++ b/enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,7 @@ +package dev.enro.test.platform + +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +actual abstract class RobolectricHostTest actual constructor() diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt index 442b9835..36494319 100644 --- a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt @@ -5,6 +5,7 @@ import androidx.savedstate.serialization.encodeToSavedState import dev.enro.NavigationKey import dev.enro.asInstance import dev.enro.controller.repository.SerializerRepository +import dev.enro.test.platform.RobolectricHostTest import kotlinx.serialization.PolymorphicSerializer import kotlinx.serialization.modules.SerializersModule import kotlinx.serialization.modules.polymorphic @@ -21,7 +22,7 @@ import kotlin.test.assertEquals * androidx SavedState encoder does NOT share the kotlinx streaming-JSON * encoder's deferred-discriminator leak (see HistoryStateSerializationTests). */ -class SavedStateSerializationTests { +class SavedStateSerializationTests : RobolectricHostTest() { private fun repository(): SerializerRepository { return SerializerRepository().apply { diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..fd90d35c --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,15 @@ +package dev.enro.test.platform + +/** + * Base class for common tests that touch Android framework classes (Bundle, + * SavedState) at runtime. + * + * On the Android host-test target, framework classes come from android.jar + * stubs, and `isReturnDefaultValues = true` makes them silently return + * defaults — e.g. `Bundle.getBundle(...)` returns null for a value that was + * "written" moments earlier, which surfaces as confusing downstream errors + * ("No valid saved state was found for the key ..."). Extending this class + * runs the test under Robolectric on that target, providing functional + * framework implementations; on every other target it is a no-op. + */ +expect abstract class RobolectricHostTest() diff --git a/enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..9ad2514b --- /dev/null +++ b/enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,3 @@ +package dev.enro.test.platform + +actual abstract class RobolectricHostTest actual constructor() diff --git a/enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..9ad2514b --- /dev/null +++ b/enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,3 @@ +package dev.enro.test.platform + +actual abstract class RobolectricHostTest actual constructor() diff --git a/enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..9ad2514b --- /dev/null +++ b/enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,3 @@ +package dev.enro.test.platform + +actual abstract class RobolectricHostTest actual constructor() From 5214db3bc61edf4c8b32e07556dc46fd2eb9f177 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 17:44:26 +1200 Subject: [PATCH 17/19] CI: cancel superseded PR runs; release: changelog-driven release notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ci.yml gains a concurrency group keyed by PR number (falling back to ref): a new push to a PR cancels the in-flight run for the superseded commit. Pushes to main are never cancelled, so every main commit keeps its complete CI record. The changelog now stacks changes under a standing '## Unreleased' header. The updateVersion task stamps that header with the version and date at release time, inserts a fresh empty '## Unreleased' above it, and writes the released section's body to build/release-notes.md — failing before any file is written when the Unreleased section is empty. release.yml commits CHANGELOG.md alongside version.properties and attaches the extracted notes to the GitHub release via 'gh release create --notes-file' (the 'changes' input becomes an optional prefix). Also modernises the workflow's actions: checkout@v4, setup-java@v4, gradle/actions/setup-gradle@v4, add-and-commit@v9, and the archived create-release@v1 replaced with the gh CLI. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 7 +++++ .github/workflows/release.yml | 53 ++++++++++++++++++++++------------- build.gradle.kts | 38 +++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c73f2af4..69a3015c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,13 @@ on: - main workflow_dispatch: +# Superseded PR runs are wasted compute: when a new push lands on a PR, cancel +# the in-flight run for the previous commit. Pushes to main are never +# cancelled — every main commit keeps its complete CI record for bisecting. +concurrency: + group: ci-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: ci-linux: name: CI / Linux (Android, Desktop, wasmJs) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4f381c3c..75555558 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,8 +7,8 @@ on: required: true default: '' changes: - description: 'Release notes' - required: true + description: 'Optional extra notes, prepended above the CHANGELOG "Unreleased" content in the GitHub release' + required: false default: '' jobs: release: @@ -16,16 +16,20 @@ jobs: runs-on: macos-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 + with: + # add-and-commit pushes the release commit using the checkout + # credentials, so check out with the publish token. + token: ${{ secrets.PUBLISH_GITHUB_TOKEN }} - name: Set up JDK 21 - uses: actions/setup-java@v3.9.0 + uses: actions/setup-java@v4 with: distribution: 'zulu' java-version: 21 - name: Setup gradle - uses: gradle/gradle-build-action@v2 + uses: gradle/actions/setup-gradle@v4 - name: Install gpg secret key run: cat <(echo -e "${{ secrets.PUBLISH_SIGNING_KEY_LITERAL }}") | gpg --batch --import @@ -36,8 +40,13 @@ jobs: - name: Export gpg secret key run: sudo gpg --export-secret-keys --pinentry-mode loopback --passphrase=${{ secrets.PUBLISH_SIGNING_KEY_PASSWORD }} ${{ secrets.PUBLISH_SIGNING_KEY_ID }} > ${{ secrets.PUBLISH_SIGNING_KEY_LOCATION }} - - name: Update Version Name - run: ./gradlew updateVersion -PversionName=${{ github.event.inputs.versionname }} + # Stamps version.properties AND the changelog: the standing + # "## Unreleased" section becomes "## ()", a fresh empty + # "## Unreleased" is inserted above it, and the released section's body + # is written to build/release-notes.md for the Create Release step. + # Fails (before writing anything) if the Unreleased section is empty. + - name: Update Version Name + Changelog + run: ./gradlew updateVersion -PversionName=${{ inputs.versionname }} - name: Publish Release env: @@ -51,21 +60,25 @@ jobs: run: ./gradlew publishAllPublicationsToMavenCentralRepository -PmavenCentralUsername="${{ secrets.PUBLISH_SONATYPE_USER }}" -PmavenCentralPassword="${{ secrets.PUBLISH_SONATYPE_PASSWORD }}" --no-parallel --stacktrace --continue --exclude-task :enro-common:publishJsPublicationToMavenCentralRepository - name: Update Repo - uses: EndBug/add-and-commit@v5 - env: - GITHUB_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} + uses: EndBug/add-and-commit@v9 with: - add: "./version.properties" - message: ${{ format('Released {0}', github.event.inputs.versionname) }} + add: | + version.properties + CHANGELOG.md + message: ${{ format('Released {0}', inputs.versionname) }} push: true + - name: Prepare release notes + env: + EXTRA_NOTES: ${{ inputs.changes }} + run: | + if [ -n "$EXTRA_NOTES" ]; then + printf '%s\n\n' "$EXTRA_NOTES" | cat - build/release-notes.md > build/release-notes-final.md + else + cp build/release-notes.md build/release-notes-final.md + fi + - name: Create Release - uses: actions/create-release@v1 env: - GITHUB_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} - with: - tag_name: ${{ github.event.inputs.versionname }} - release_name: Release ${{ github.event.inputs.versionname }} - body: ${{ github.event.inputs.changes }} - draft: false - prerelease: false + GH_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} + run: gh release create "${{ inputs.versionname }}" --title "Release ${{ inputs.versionname }}" --notes-file build/release-notes-final.md diff --git a/build.gradle.kts b/build.gradle.kts index ddc5051c..aadc913f 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -115,6 +115,44 @@ tasks.register("updateVersion") { error("The versionName '$versionName' is the current versionName") } + // ------------------------------------------------------------------ + // Changelog: changes stack under a standing "## Unreleased" header. + // Releasing stamps that header with the version (and date), inserts a + // fresh empty "## Unreleased" above it, and writes the released + // section's body to build/release-notes.md for the release workflow + // to attach to the GitHub release. All validation happens before any + // file is written, so a failed release leaves the repo untouched. + // ------------------------------------------------------------------ + val changelogFile = rootProject.file("CHANGELOG.md") + val changelog = changelogFile.readText() + val unreleasedHeader = "## Unreleased" + val headerIndex = changelog.indexOf(unreleasedHeader) + if (headerIndex == -1) { + error("CHANGELOG.md must contain an '## Unreleased' section to release from") + } + val bodyStart = headerIndex + unreleasedHeader.length + val nextHeaderIndex = changelog.indexOf("\n## ", bodyStart) + .let { if (it == -1) changelog.length else it } + val releaseNotesBody = changelog.substring(bodyStart, nextHeaderIndex).trim() + if (releaseNotesBody.isEmpty()) { + error( + "The 'Unreleased' section of CHANGELOG.md is empty — " + + "add release notes before releasing $versionName" + ) + } + + val releaseNotesFile = rootProject.layout.buildDirectory.file("release-notes.md").get().asFile + releaseNotesFile.parentFile.mkdirs() + releaseNotesFile.writeText(releaseNotesBody + "\n") + + val releaseDate = java.time.LocalDate.now() + changelogFile.writeText( + changelog.replaceFirst( + unreleasedHeader, + "## Unreleased\n\n## $versionName ($releaseDate)", + ) + ) + versionPropertiesFile.writeText("versionName=$versionName\nversionCode=$versionCode") } } From 6233e2502003899f9f52b66ed086cd49cfa1a7a5 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 17:44:55 +1200 Subject: [PATCH 18/19] Changelog: standing Unreleased header for the release automation Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a80c72de..55e11cab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 3.0.0-beta03 (Unreleased) +## Unreleased ### Web browser history (WasmJS) From 3816144519d1fcde18bb2605b783ead3d72e7e63 Mon Sep 17 00:00:00 2001 From: Isaac Udy Date: Thu, 11 Jun 2026 17:48:09 +1200 Subject: [PATCH 19/19] =?UTF-8?q?Release:=20drop=20the=20optional=20notes?= =?UTF-8?q?=20input=20=E2=80=94=20the=20changelog=20is=20the=20single=20so?= =?UTF-8?q?urce?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .github/workflows/release.yml | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 75555558..7defbd19 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,10 +6,6 @@ on: description: 'Version Name' required: true default: '' - changes: - description: 'Optional extra notes, prepended above the CHANGELOG "Unreleased" content in the GitHub release' - required: false - default: '' jobs: release: name: Release @@ -68,17 +64,7 @@ jobs: message: ${{ format('Released {0}', inputs.versionname) }} push: true - - name: Prepare release notes - env: - EXTRA_NOTES: ${{ inputs.changes }} - run: | - if [ -n "$EXTRA_NOTES" ]; then - printf '%s\n\n' "$EXTRA_NOTES" | cat - build/release-notes.md > build/release-notes-final.md - else - cp build/release-notes.md build/release-notes-final.md - fi - - name: Create Release env: GH_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} - run: gh release create "${{ inputs.versionname }}" --title "Release ${{ inputs.versionname }}" --notes-file build/release-notes-final.md + run: gh release create "${{ inputs.versionname }}" --title "Release ${{ inputs.versionname }}" --notes-file build/release-notes.md