Fix web browser-history sync (WasmJS) + switch Json discriminator mode to POLYMORPHIC#220
Merged
Conversation
…-push 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…e 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 <noreply@anthropic.com>
…iagnose 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 <noreply@anthropic.com>
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": "<value class fqn>"} 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 <noreply@anthropic.com>
…ncoder 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3.0.0-beta02 has been released; these changes land in the next version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This reverts commit 316002d.
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…source Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes discovered while debugging jumpy/broken browser back-navigation in a production WasmJS app (reglyph), plus the test coverage that pins them.
1.
WebHistoryPluginsync rewriteThe history synchronisation had three concurrency bugs:
history.go()races / echo double-backs. Traversals were followed bydelay(1)and time-based listener disabling; the traversal's own popstate echo usually arrived after suppression lifted and was processed as a second user back. Self-initiated traversals now await and explicitly consume their popstate echo.pushState-ed, so full-backstack replacements (loading gate → home, section truncate-and-open) accreted stale browser entries — making transient screens (e.g. a startup/loading destination) reachable via back. The previously-deadisSubsethelper is now wired up: push only when the previous state is a prefix of the new one, otherwisereplaceState.Also: bounded step-past loop instead of the blind
history.back()retry; listener/processor cleanup on detach; URL fallback — entries whose recorded state can't be decoded (tab history survives app deploys) restore through their URL via the@NavigationPathbindings and self-heal the entry's stored state; and write-time round-trip verification — a state that can't restore is a hard error at write time, never a silently dead back button later.2.
jsonConfiguration:ALL_JSON_OBJECTS→ defaultPOLYMORPHICmodeThe verification above immediately caught silent state corruption, which turned out to be upstream kotlinx (1.11.0) bugs in both Json encoders under
ClassDiscriminatorMode.ALL_JSON_OBJECTSwith polymorphic dispatch:Instance.metadata, undecodable)Set/List)"type"key:value pair inside an array, wrong class name)NumberFormatException: For input string: "type")Root mechanism: kotlinx defers each discriminator write until the next
beginStructure, and value-class fields never open one; the pending tag then lands in whatever opens next (a sibling object, or a list context where it's parsed as an array index). Since typed-id value classes and collection params are bread-and-butterNavigationKeyshapes,ALL_JSON_OBJECTSis effectively unusable until kotlinx fixes this.POLYMORPHIC(the kotlinx default) writes discriminators exactly where polymorphic deserialization reads them (Instance.key, metadata values) and handles all of these shapes correctly. Old persisted states with extratypefields still decode (ignoreUnknownKeys); states corrupted by the leak are handled by the URL fallback.Tests
HistoryStateSerializationTests— pins thePOLYMORPHICround-trip for the full problem shape (value class + enum set + value-class set), plus documenting tests that assert the upstream kotlinx failures still exist — when a kotlinx upgrade fixes them, those tests fail and the mode constraint can be revisited.SavedStateSerializationTests— pins that the savedstate path (androidx encoder,ALL_OBJECTS) does not share the bug, so container restoration is unaffected.Upstream issue: Kotlin/kotlinx.serialization#3022 (open since June 2025) — the collection-field failures here match its reported shape, and the repro in these tests adds the polymorphic-dispatch + value-class-field variants.
Changelog
Added under 3.0.0-beta03 (Unreleased): web history sync rewrite + serialization mode change.
🤖 Generated with Claude Code