From 8f0dbf3d1dbadb0bf88bb364905c8f5b14a3ceac Mon Sep 17 00:00:00 2001 From: Scott Idem Date: Tue, 16 Jun 2026 15:12:11 -0400 Subject: [PATCH] docs: capture today's Pres Mgmt config sync lessons for future agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BOOTSTRAP__AI_Agent_Quickstart.md: new category 9 under Common Mistakes pointing to today's incident (local/remote config sync pitfalls) - REFERENCE__Common_Agent_Mistakes.md: three new entries — 16) local "shadow field" silently bypasses an admin-synced master field 17) SWR await after a write does not mean dependent caches are fresh 18) conditional/stateful sync gates are effectively undebuggable - PROJECT__Stores_Svelte5_Migration.md: updated the canonical Migration Pattern example to stamp __version in the serializer (the old example silently didn't, which is exactly what caused the leads_loc/pres_mgmt_loc bugs fixed today); flagged badges_loc/launcher_loc/events_auth_loc as not yet fixed (dormant, not harmful) and idaa_loc as the next migration that should get this from day one - TODO__Agents.md: cross-referenced today's Pres Mgmt config sync overhaul in the LCI October restoration section for context, without touching the open items in that list Co-Authored-By: Claude Sonnet 4.6 --- .../BOOTSTRAP__AI_Agent_Quickstart.md | 4 ++ .../PROJECT__Stores_Svelte5_Migration.md | 39 ++++++++++++- .../REFERENCE__Common_Agent_Mistakes.md | 57 +++++++++++++++++++ documentation/TODO__Agents.md | 10 ++++ 4 files changed, 107 insertions(+), 3 deletions(-) diff --git a/documentation/BOOTSTRAP__AI_Agent_Quickstart.md b/documentation/BOOTSTRAP__AI_Agent_Quickstart.md index bb17c367..9982ff53 100644 --- a/documentation/BOOTSTRAP__AI_Agent_Quickstart.md +++ b/documentation/BOOTSTRAP__AI_Agent_Quickstart.md @@ -300,6 +300,10 @@ Read this section first, then open the reference doc when your task touches one 6. **Network reliability** — retry classification in `api_*_object.ts` and timeout behavior. 7. **JSON field safety** — `*_json` null reads/writes and wrapper serialization behavior. 8. **Service worker rollout behavior** — stale-tab symptoms, activation expectations, and trade-offs. +9. **Local/remote config sync** — shadow fields that silently bypass an admin-synced master + field, SWR-await-after-write races, and stateful/conditional sync gates that desync local + state from history rather than current config. See the Pres Mgmt Config sync overhaul + (2026-06-16) in `PROJECT__AE_Events_PressMgmt_Config_Cleanup.md` for the full incident. The reference doc also includes a short archive list for older, less-relevant historical incidents. diff --git a/documentation/PROJECT__Stores_Svelte5_Migration.md b/documentation/PROJECT__Stores_Svelte5_Migration.md index 17532187..2e9da99f 100644 --- a/documentation/PROJECT__Stores_Svelte5_Migration.md +++ b/documentation/PROJECT__Stores_Svelte5_Migration.md @@ -72,19 +72,49 @@ export const badges_loc_defaults: BadgesLocState = { ... }; ``` ### 2. Store file (`*.svelte.ts`) + +**Updated 2026-06-16 — stamp `__version` in the serializer.** The original pattern below (bare +`JSON.stringify`) looked fine but has a real failure mode: `store_versions.ts`'s +`_check_and_wipe()` compares a `__version` field *inside* the stored JSON, and a bare +`JSON.stringify` never writes one. Two consequences were found in production code: +`pres_mgmt_loc`'s version bump was silently a no-op (never wired into `_check_and_wipe` at +all), and `leads_loc`'s *was* wired in, so `parsed?.__version` was always `undefined` — +always failing the comparison — wiping `ae_leads_loc` on **every single page load**. Both +fixed 2026-06-16. `badges_loc`, `launcher_loc`, and `events_auth_loc` still use the old bare +pattern below and are **not yet wired into `_check_and_wipe`** — not actively harmful (the +wipe never runs), but apply this fix before wiring any of them in. + ```ts import { PersistedState } from 'runed'; import { badges_loc_defaults } from './ae_events_stores__badges_defaults'; +import { AE_BADGES_LOC_VERSION } from './store_versions'; export const badges_loc = new PersistedState('ae_badges_loc', badges_loc_defaults, { serializer: { - serialize: JSON.stringify, - // Merge with defaults so new fields added after first session get their defaults. - deserialize: (raw: string) => ({ ...badges_loc_defaults, ...JSON.parse(raw) }) + // Stamp __version on every write so store_versions.ts's _check_and_wipe() can + // detect a breaking schema change and clear stale browsers on next load. This + // import also guarantees store_versions.ts's wipe side-effect runs before this + // PersistedState reads from localStorage (ES module execution order). + serialize: (value) => + JSON.stringify({ ...value, __version: AE_BADGES_LOC_VERSION }), + // Merge with defaults so new fields added after first session get their defaults, + // and strip __version back out so it doesn't pollute the typed state object. + deserialize: (raw: string) => { + const { __version, ...stored } = JSON.parse(raw); + return { ...badges_loc_defaults, ...stored }; + } } }); ``` +Then in `store_versions.ts`'s startup block: +```ts +_check_and_wipe('ae_badges_loc', AE_BADGES_LOC_VERSION); +``` + +See `ae_events_stores__pres_mgmt.svelte.ts` and `ae_events_stores__leads.svelte.ts` for the +two real, currently-correct examples of this pattern. + ### 3. Consumer syntax ```ts // Import (note .svelte extension, not .svelte.ts): @@ -116,6 +146,9 @@ badges_loc.current = { ...badges_loc_defaults }; 1. **`idaa_loc` → PersistedState** — Highest priority for IDAA stability. Promotes `novi_uuid/verified`, `archives`, `bb`, `recovery_meetings` sub-objects to their own stores following the same pattern. Primary benefit: eliminates the IDAA "Access Denied" corruption from `ae_loc` bootstrap writes. + Use the `__version`-stamping serializer from the start (see Migration Pattern above) and wire + it into `_check_and_wipe()` immediately — `idaa_loc` holds auth state, so a real wipe-on-schema- + change matters more here than anywhere else this pattern has been applied so far. 2. **`ae_loc` → PersistedState** — Largest scope (~every route in the app). Defer until after `idaa_loc` is done. Consider extracting `auth_loc` (the identity/permission fields) as the diff --git a/documentation/REFERENCE__Common_Agent_Mistakes.md b/documentation/REFERENCE__Common_Agent_Mistakes.md index 1c642ad4..3c794b38 100644 --- a/documentation/REFERENCE__Common_Agent_Mistakes.md +++ b/documentation/REFERENCE__Common_Agent_Mistakes.md @@ -150,6 +150,63 @@ Review policy: balanced curation. **Verify:** After deploy, validate that long-lived tabs pick up new SW behavior as intended. +### 16) Local "shadow field" silently bypasses the admin-synced master field +**Impact:** an admin config toggle appears to do nothing — the UI updates fine when a local +per-browser preference is clicked, but the actual admin setting has zero effect, at any +permission level. Looks like a permissions bug; isn't one. + +**What happened:** A list/table column's visibility prop was computed from only a local-only, +never-synced preference field (e.g. `hide__session_li_location_field`), while a +similarly-named, admin-synced field (`hide__session_location`) existed and was correctly +synced — just never read at that particular render call site. Found twice in the same +session (Pres Mgmt POC column, then Location column). + +**Rule:** When a remote config field and a local-only preference field could plausibly both +affect the same visible thing, combine them explicitly (`admin_field || local_field`) at +*every* call site that renders that thing — don't assume one supersedes the other, and don't +assume fixing it once means every other render site is also fixed. + +**Verify:** `grep` every consumer of the field name (and near-miss siblings, e.g. `_li_`/`_field` +suffixed variants) before trusting that a config toggle does what its label says. + +### 17) SWR `await` after a write does not mean dependent caches are fresh +**Impact:** a value you just saved appears stale or "one save behind" immediately after +saving — looks like inverted/random behavior when toggling a boolean back and forth, since +being one step behind on a 2-state toggle looks exactly like being inverted. + +**What happened:** A save handler `await`ed `load_ae_obj_id__event()` (SWR) and assumed that +meant Dexie now had the fresh record. In reality the fast path returns the stale cache +immediately and refreshes Dexie via a non-awaited background fetch. A *different* page's own +sync `$effect` read Dexie before that background fetch landed, re-syncing from the stale +record and overwriting a value that had just been set correctly moments earlier. + +**Rule:** After a write whose result other reactive code depends on, don't rely on an awaited +SWR reload to mean downstream caches are updated — for any call with a populated cache, it +almost never is. Update dependent local state directly from the data you already have (what +you just saved), not by waiting on a refetch. See the "try_cache: false Bug" section of +`GUIDE__SvelteKit2_Svelte5_DexieJS.md` for the related, equally counter-intuitive case where +disabling caching *also* disables the very write you wanted. + +### 18) Conditional ("locked") sync gates are effectively undebuggable +**Impact:** a field's local value silently depends on the *history* of an unrelated flag's +state across past syncs, not its current value — looks like inconsistent behavior tied to +permission level, browser, or test sequence, none of which is the actual cause. + +**What happened:** `sync_config__event_pres_mgmt()` only applied a subset of fields when a +separate `lock_config` flag was `true` *at that exact sync call*. Toggling `lock_config` off +even briefly during testing froze those fields at stale values in every browser that synced +during that window, with nothing in the UI indicating staleness. Removed entirely — every +event already had it set to `true` in production; the conditional gated nothing real. + +**Rule:** Avoid making one config field's sync conditional on another field's *current* +value unless that's a genuine, deliberate design choice — and if it is, surface the +dependency in the UI (disable the inputs, show a warning), don't bury it in a silent +function-level gate. + +**Verify:** Before adding `if (some_flag) { ...many field syncs... }`, check whether removing +the gate (sync unconditionally) loses any real behavior — query production data for whether +the gate is ever actually in the "off" state before assuming it needs to exist. + --- ## Archived Historical Items (Pruned) diff --git a/documentation/TODO__Agents.md b/documentation/TODO__Agents.md index 94e67200..0e6ade95 100644 --- a/documentation/TODO__Agents.md +++ b/documentation/TODO__Agents.md @@ -10,6 +10,16 @@ These features regressed over the last 6 months and must be working before the LCI conference. Reference commit for original working implementation: `bb993a102`. +**2026-06-16:** Pres Mgmt's config sync architecture was overhauled the same day this list's +LCI event data was used for live testing — `lock_config` removed (was causing "sometimes +works" reports tied to save history, not current settings), POC/Location list-table column +bugs fixed (admin setting was being silently ignored), QR display fixed to match the intended +global-default-with-trusted-override design, and the Config page got a documentation pass +(title tooltips, POC settings split into its own section). None of this touches the open +items below, but anyone picking those up should know the sync layer underneath them is in a +meaningfully different (and better-understood) state now. Full incident log: +`PROJECT__AE_Events_PressMgmt_Config_Cleanup.md`. + ### Session POC (Champion/Moderator) — `session_view.svelte` **Root cause of visible bugs:** The POC section is placed *below* the session hero card as a