fix(stores): close persistence fallbacks flagged in PR #358 review

CHANGELOG.md

@@ -12,14 +12,20 @@ packages, and tooling contracts may change before a stable release.
   (`store cart ui.CartState = ui.NewCartState() persist "local"`). The generated
   store runtime hydrates from localStorage/sessionStorage on load, re-hydrates on
   SPA navigation (stores first declared on a later route are picked up on content
-  swap), writes the store's declared fields on change, mirrors cross-tab writes
+  swap, and a store first declared without persistence adopts a later route's
+  persist config and restores its saved value, so persistence never depends on
+  navigation order), writes the store's declared fields on change, mirrors cross-tab writes
   through the `storage` event, exposes `window.__gowdkStores.clear(name)` to drop
   a persisted store, and discards persisted state whose embedded schema hash no longer
   matches the store's shape (so a struct change never restores stale data).
   Only the store's own fields persist — never component state, props, or computed
   values. New diagnostics: `page_store_persist_scope_invalid` (error),
-  `page_store_persist_secret_field` (warning), and
-  `page_store_persist_key_conflict` (warning). Persistence is a JS-island/store
+  `page_store_persist_secret_field` (warning, raised for nested secret-resembling
+  fields such as `Profile.Token`, not only top-level fields),
+  `page_store_persist_key_conflict` (warning), and
+  `page_store_persist_scope_conflict` (warning, when the same store name is
+  persisted under different `local`/`session` scopes across pages and would
+  otherwise let navigation order decide the backend). Persistence is a JS-island/store
   runtime feature; WASM islands do not yet participate in page stores.
 - M4 Go interop is complete for the current 0.x surface: a user can see why a Go
   function or type did or did not bind. `gowdk inspect go-bindings` emits a

docs/language/components.md

@@ -293,9 +293,11 @@ component state, props, or computed values. The compiler embeds a hash of the
 store's struct shape; when the shape changes, stale persisted state is discarded
 rather than restored, so a struct change never crashes on old data. Because
 browser storage is readable by any script on the origin, persisting a field whose
-name resembles a secret (`token`, `password`, `secret`, `auth`, …) is a warning:
-keep credentials and trusted authorization state server-side. An unknown scope is
-rejected — see `gowdk explain page_store_persist_scope_invalid`.
+name resembles a secret (`token`, `password`, `secret`, `auth`, …) is a warning —
+including a nested field such as `Profile.Token`, because persistence writes the
+whole value of each top-level field: keep credentials and trusted authorization
+state server-side. An unknown scope is rejected — see
+`gowdk explain page_store_persist_scope_invalid`.
 
 Persisted stores also sync across tabs: when one tab writes, other tabs on the
 origin mirror the value through the browser `storage` event. To drop a persisted
@@ -304,7 +306,11 @@ store (for example after checkout or logout), call
 the store to its build-time init value. If two pages persist a store with the
 same name but different shapes, they share one storage key and discard each
 other's data on navigation; the compiler warns with
-`page_store_persist_key_conflict`.
+`page_store_persist_key_conflict`. If they share the same shape but declare
+different `local`/`session` scopes, the runtime keeps whichever scope initialized
+first and the compiler warns with `page_store_persist_scope_conflict`. A store
+first reached on a route that does not persist it still adopts persistence when a
+later route declares it, restoring the saved value regardless of navigation order.
 
 Persistence survives SPA navigation: when the client runtime swaps page content
 it re-scans store seeds, so a store first declared on a later client-side route

docs/language/syntax.md

@@ -499,7 +499,9 @@ The scope must be `"local"` (localStorage) or `"session"` (sessionStorage); any
 other value is rejected with `page_store_persist_scope_invalid`. Persisted store
 state is keyed by store name, restores over the build-time init value on load,
 is discarded when the store's struct shape changes, and warns when a persisted
-field name resembles a secret.
+field name resembles a secret (nested fields included). Declaring the same store
+name with a different `local`/`session` scope across pages warns with
+`page_store_persist_scope_conflict`.
 
 Client blocks can declare limited DOM refs for safe methods:
 

internal/buildgen/runtime_assets.go

@@ -138,7 +138,24 @@ func storeRuntimeSource() string {
   };
 
   registry.init = (name, state, persist) => {
-    if (!name || registry.stores[name]) return;
+    if (!name) return;
+    if (registry.stores[name]) {
+      // The store already exists (for example SPA navigation reached a later
+      // route that declares the same store). If this declaration adds
+      // persistence and we are not persisting yet, adopt it and restore the
+      // saved value so persistence does not depend on which route loaded first.
+      // A conflicting scope is kept first-wins and reported at build time by
+      // page_store_persist_scope_conflict, so navigation cannot thrash storage.
+      if (persist && persist.scope && persist.key && persist.version && !registry.persist[name]) {
+        registry.persist[name] = persist;
+        const restored = readPersisted(persist, registry.fields[name]);
+        if (restored) {
+          registry.stores[name] = Object.assign({}, registry.stores[name] || {}, restored);
+          notify(name);
+        }
+      }
+      return;
+    }
     const seed = Object.assign(Object.create(null), state || {});
     registry.fields[name] = Object.keys(seed);
     registry.seeds[name] = Object.assign(Object.create(null), seed);

internal/buildgen/store_persist_runtime_test.go

@@ -163,6 +163,37 @@ r.hydrate();
 assert.equal(r.get("prefs").Theme, "dark", "hydrate() picks up a store first seen on a later route");
 assert.equal(r.get("cart").Count, 0, "re-hydrate leaves an existing store untouched");
 
+// 9. Adopting persistence across SPA navigation: a store first declared WITHOUT
+// persistence must pick up a later route's persist config (and restore the saved
+// value) instead of silently staying unpersisted, so persistence does not depend
+// on which route loaded first.
+localStorage.setItem("gowdk:store:wishlist", JSON.stringify({ v: "w1", s: { Items: 2 } }));
+// First navigation declares wishlist without persistence (no data-gowdk-persist).
+global.document.querySelectorAll = () => [{
+  getAttribute(name) { return name === "data-gowdk-store" ? "wishlist" : null; },
+  get textContent() { return '{"Items":0}'; }
+}];
+r.hydrate();
+assert.equal(r.get("wishlist").Items, 0, "unpersisted first declaration uses the seed, not storage");
+// Second navigation declares the same store as persist "local".
+let adopted = null;
+r.subscribe("wishlist", (next) => { adopted = next; });
+global.document.querySelectorAll = () => [{
+  getAttribute(name) {
+    if (name === "data-gowdk-store") return "wishlist";
+    if (name === "data-gowdk-persist") return "local";
+    if (name === "data-gowdk-persist-key") return "gowdk:store:wishlist";
+    if (name === "data-gowdk-persist-version") return "w1";
+    return null;
+  },
+  get textContent() { return '{"Items":0}'; }
+}];
+r.hydrate();
+assert.equal(r.get("wishlist").Items, 2, "re-hydrate adopts persistence and restores the saved value");
+assert.ok(adopted && adopted.Items === 2, "adopting persistence notifies subscribers");
+r.set("wishlist", { Items: 7 });
+assert.equal(JSON.parse(localStorage.getItem("gowdk:store:wishlist")).s.Items, 7, "after adoption, set() writes through to storage");
+
 console.log("OK");
 `
 }

internal/compiler/validate_page.go

@@ -3,6 +3,7 @@ package compiler
 import (
 	"fmt"
 	"os"
+	"sort"
 	"strings"
 
 	"github.com/cssbruno/gowdk"
@@ -419,22 +420,48 @@ func validateStorePersist(page gwdkir.Page, store gwdkir.Store, resolved gotypes
 		}}
 	}
 	var diagnostics []ValidationError
-	for _, field := range resolved.Fields {
-		if !looksLikeSecretFieldName(field.Name) {
-			continue
-		}
+	// Persistence writes the whole value of each top-level field, so a nested
+	// field such as Profile.Token reaches browser storage too. Scan every field
+	// path the resolver recorded (top-level and nested), not just the top level.
+	for _, path := range secretResemblingFieldPaths(resolved) {
 		diagnostics = append(diagnostics, ValidationError{
 			Code:     "page_store_persist_secret_field",
 			PageID:   page.ID,
 			Source:   page.Source,
 			Span:     firstSpan(store.Span, page.Spans.Page),
 			Severity: SeverityWarning,
-			Message:  fmt.Sprintf("page %s store %q persists field %q, which resembles a secret; %s browser storage is readable by any script on this origin", page.ID, store.Name, field.Name, store.Persist),
+			Message:  fmt.Sprintf("page %s store %q persists field %q, which resembles a secret; %s browser storage is readable by any script on this origin", page.ID, store.Name, path, store.Persist),
 		})
 	}
 	return diagnostics
 }
 
+// secretResemblingFieldPaths returns the resolved struct's field paths (top-level
+// and nested) whose leaf name resembles a secret, deduplicated and sorted for a
+// stable diagnostic order. Slice/array markers ("[]") are stripped so a path
+// reads like Tags.Token rather than Tags[].Token.
+func secretResemblingFieldPaths(resolved gotypes.Struct) []string {
+	seen := map[string]bool{}
+	var paths []string
+	for raw := range resolved.FieldTypes {
+		path := strings.ReplaceAll(raw, "[]", "")
+		leaf := path
+		if index := strings.LastIndex(path, "."); index >= 0 {
+			leaf = path[index+1:]
+		}
+		if leaf == "" || !looksLikeSecretFieldName(leaf) {
+			continue
+		}
+		if seen[path] {
+			continue
+		}
+		seen[path] = true
+		paths = append(paths, path)
+	}
+	sort.Strings(paths)
+	return paths
+}
+
 // looksLikeSecretFieldName flags field names that commonly hold credentials or
 // trusted authorization state, which the store contract already forbids from
 // browser-visible state and which persistence would write to disk.

internal/compiler/validate_page_persist_test.go

@@ -1,6 +1,7 @@
 package compiler
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/cssbruno/gowdk"
@@ -88,6 +89,52 @@ func TestValidatePageWarnsOnPersistedSecretField(t *testing.T) {
 	}
 }
 
+func TestValidatePageWarnsOnNestedPersistedSecretField(t *testing.T) {
+	// ProfileState has no top-level secret-like field, but nests Account.Token.
+	// Persistence writes the whole Account value, so the nested secret must be
+	// flagged even though the top-level scan alone would miss it.
+	page := persistedStorePage("profile", "ProfileState", "NewProfileState", "local")
+	diagnostics := ValidatePage(gowdk.Config{}, irPage(page))
+	diagnostic := firstDiagnostic(diagnostics, "page_store_persist_secret_field")
+	if diagnostic == nil {
+		t.Fatalf("missing nested page_store_persist_secret_field diagnostic: %#v", diagnostics)
+	}
+	if !strings.Contains(diagnostic.Message, "Account.Token") {
+		t.Fatalf("expected the nested field path Account.Token in the message, got %q", diagnostic.Message)
+	}
+	if diagnostic.Severity != SeverityWarning {
+		t.Fatalf("nested secret field should be a warning, got severity %v", diagnostic.Severity)
+	}
+	if ValidationErrors(diagnostics).HasErrors() {
+		t.Fatalf("nested secret field warning should not fail the build: %#v", diagnostics)
+	}
+}
+
+func TestValidateWarnsOnPersistedStoreScopeConflict(t *testing.T) {
+	// Same store name and shape but different persist scopes share one storage
+	// key; the effective scope then depends on navigation order.
+	app := appFixture{Pages: []gwdkir.Page{
+		persistedStorePageNamed("shop", "/shop", "cart", "CounterState", "NewCounterState", "local"),
+		persistedStorePageNamed("checkout", "/checkout", "cart", "CounterState", "NewCounterState", "session"),
+	}}
+	diagnostics := ValidateProgramReport(gowdk.Config{}, app.program(gowdk.Config{}))
+	d := firstDiagnostic(diagnostics, "page_store_persist_scope_conflict")
+	if d == nil {
+		t.Fatalf("missing page_store_persist_scope_conflict: %#v", diagnostics)
+	}
+	if d.Severity != SeverityWarning {
+		t.Fatalf("scope conflict should be a warning, got %v", d.Severity)
+	}
+	// A mismatched-shape conflict is reported separately; same shape must not
+	// also raise a key conflict.
+	if other := firstDiagnostic(diagnostics, "page_store_persist_key_conflict"); other != nil {
+		t.Fatalf("same-shape scope conflict must not also raise a key conflict: %#v", other)
+	}
+	if ValidationErrors(diagnostics).HasErrors() {
+		t.Fatalf("scope conflict alone must not fail the build: %#v", diagnostics)
+	}
+}
+
 func TestValidatePageDoesNotPersistCheckUnpersistedStore(t *testing.T) {
 	// A non-persisted store with a secret-looking field must stay silent.
 	page := persistedStorePage("session", "SessionState", "NewSessionState", "")

internal/compiler/validate_stores.go

@@ -36,21 +36,42 @@ func validatePersistedStoreConflicts(pages []gwdkir.Page) []ValidationError {
 				seen[store.Name] = seenStore{page: page, store: store, signature: signature}
 				continue
 			}
-			if prior.signature == signature {
-				continue // same shape sharing one key across routes is intended
+			if prior.signature != signature {
+				diagnostics = append(diagnostics, ValidationError{
+					Code:     "page_store_persist_key_conflict",
+					PageID:   page.ID,
+					Source:   page.Source,
+					Span:     firstSpan(store.Span, page.Spans.Page),
+					Severity: SeverityWarning,
+					Related:  relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted here on page %s", prior.store.Name, prior.page.ID)),
+					Message: fmt.Sprintf(
+						"persisted store %q has different shapes on pages %s and %s but shares browser storage key %q; navigating between them discards each other's saved data. Rename one store or give them matching shapes",
+						store.Name, prior.page.ID, page.ID, "gowdk:store:"+store.Name,
+					),
+				})
+				continue
 			}
-			diagnostics = append(diagnostics, ValidationError{
-				Code:     "page_store_persist_key_conflict",
-				PageID:   page.ID,
-				Source:   page.Source,
-				Span:     firstSpan(store.Span, page.Spans.Page),
-				Severity: SeverityWarning,
-				Related:  relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted here on page %s", prior.store.Name, prior.page.ID)),
-				Message: fmt.Sprintf(
-					"persisted store %q has different shapes on pages %s and %s but shares browser storage key %q; navigating between them discards each other's saved data. Rename one store or give them matching shapes",
-					store.Name, prior.page.ID, page.ID, "gowdk:store:"+store.Name,
-				),
-			})
+			if prior.store.Persist != store.Persist {
+				// Same name and shape but different persist scopes (local vs
+				// session) still share one browser storage key. The runtime
+				// registry keeps the scope of whichever route initialized first,
+				// so the effective backend depends on navigation history. Flag
+				// it instead of silently letting nav order decide.
+				diagnostics = append(diagnostics, ValidationError{
+					Code:     "page_store_persist_scope_conflict",
+					PageID:   page.ID,
+					Source:   page.Source,
+					Span:     firstSpan(store.Span, page.Spans.Page),
+					Severity: SeverityWarning,
+					Related:  relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted with scope %q here on page %s", prior.store.Name, prior.store.Persist, prior.page.ID)),
+					Message: fmt.Sprintf(
+						"persisted store %q uses scope %q on page %s but scope %q on page %s while sharing browser storage key %q; only one scope wins and which one depends on navigation order. Use the same persist scope on both pages or rename one store",
+						store.Name, prior.store.Persist, prior.page.ID, store.Persist, page.ID, "gowdk:store:"+store.Name,
+					),
+				})
+				continue
+			}
+			// Same name, shape, and scope is intentional cross-route sharing.
 		}
 	}
 	return diagnostics

internal/diagnostics/explain.go

@@ -41,6 +41,21 @@ store cart admin.AuditState = admin.NewAuditState() persist "local"`,
 store cart  ui.CartState     = ui.NewCartState()     persist "local"
 // pages/admin.page.gwdk
 store audit admin.AuditState = admin.NewAuditState() persist "local"`,
+	},
+	"page_store_persist_scope_conflict": {
+		Details: "Two pages declare a persisted store with the same name and the same struct shape but different persist scopes (one local, one session). Persistence is keyed by store name (gowdk:store:<name>), so they share one storage slot, but local uses window.localStorage and session uses window.sessionStorage. The runtime keeps whichever scope initialized first, so the effective backend — and whether the value survives a browser restart — depends on which route the user visited first. Use the same scope on both pages, or rename one store.",
+		NextSteps: []string{
+			"Declare the same persist scope (both \"local\" or both \"session\") wherever the store is persisted.",
+			"Rename one of the stores so each persisted store owns its own scope and storage key.",
+		},
+		Invalid: `// pages/shop.page.gwdk
+store cart ui.CartState = ui.NewCartState() persist "local"
+// pages/checkout.page.gwdk  (same shape, different scope)
+store cart ui.CartState = ui.NewCartState() persist "session"`,
+		Fixed: `// pages/shop.page.gwdk
+store cart ui.CartState = ui.NewCartState() persist "local"
+// pages/checkout.page.gwdk
+store cart ui.CartState = ui.NewCartState() persist "local"`,
 	},
 	"page_store_persist_scope_invalid": {
 		Details: "A page store may opt into browser persistence with persist \"local\" or persist \"session\". local uses window.localStorage (survives a browser restart); session uses window.sessionStorage (survives reload and SPA navigation, cleared when the tab closes). No other scope is supported.",

internal/diagnostics/registry.go

@@ -131,6 +131,7 @@ var Registry = []Code{
 	{Code: "package_must_be_first", Area: "packages", Stability: StabilityStable, Severity: SeverityError, Summary: "package declaration is not the first non-comment declaration"},
 	{Code: "page_store_error", Area: "stores", Stability: StabilityExperimental, Severity: SeverityError, Summary: "page store type or initializer is invalid"},
 	{Code: "page_store_persist_key_conflict", Area: "stores", Stability: StabilityExperimental, Severity: SeverityWarning, Summary: "persisted stores share a storage key but have different shapes"},
+	{Code: "page_store_persist_scope_conflict", Area: "stores", Stability: StabilityExperimental, Severity: SeverityWarning, Summary: "persisted stores share a storage key but use different scopes"},
 	{Code: "page_store_persist_scope_invalid", Area: "stores", Stability: StabilityExperimental, Severity: SeverityError, Summary: "page store persist scope is not \"local\" or \"session\""},
 	{Code: "page_store_persist_secret_field", Area: "stores", Stability: StabilityExperimental, Severity: SeverityWarning, Summary: "persisted page store field resembles a secret"},
 	{Code: "parse_error", Area: "parser", Stability: StabilityStable, Severity: SeverityError, Summary: "parser rejected source without a more specific code"},

testfixture/islands/islands.go

@@ -51,6 +51,21 @@ type SessionState struct {
 	Open  bool
 }
 
+// Credentials nests a secret-resembling field. It exercises the nested
+// persisted-store secret-field warning and should not be persisted in practice.
+type Credentials struct {
+	Token string
+	Label string
+}
+
+// ProfileState carries a nested Credentials value, so persisting it would write
+// the nested Token to browser storage even though no top-level field looks like
+// a secret.
+type ProfileState struct {
+	Name    string
+	Account Credentials
+}
+
 func NewCounterState() CounterState {
 	return CounterState{Count: 1, Open: false}
 }
@@ -59,6 +74,10 @@ func NewSessionState() SessionState {
 	return SessionState{}
 }
 
+func NewProfileState() ProfileState {
+	return ProfileState{}
+}
+
 func NewOtherState() OtherState {
 	return OtherState{Name: "other"}
 }