- 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 <noreply@anthropic.com>
229 lines
11 KiB
Markdown
229 lines
11 KiB
Markdown
# Aether SvelteKit — Common Agent Mistakes (Reference)
|
||
|
||
This is the detailed mistake catalog referenced by `BOOTSTRAP__AI_Agent_Quickstart.md`.
|
||
Use it as a troubleshooting and prevention guide, not as first-pass onboarding.
|
||
|
||
Review policy: balanced curation.
|
||
- Keep: high-impact or recurring mistakes.
|
||
- Archive: one-off or lower-signal historical incidents.
|
||
|
||
---
|
||
|
||
## Active Mistakes (Curated)
|
||
|
||
### 1) IDAA content exposed publicly
|
||
**Impact:** Sev-1 privacy breach.
|
||
|
||
**What happened:** A route guard was removed on an IDAA BB route.
|
||
|
||
**Rule:** All `/idaa/` content is private. Never remove auth guards on IDAA routes.
|
||
|
||
**Verify:** Confirm route/layout auth checks still gate all IDAA pages before data load.
|
||
|
||
### 2) Object ID included in PATCH body (`data_kv`)
|
||
**Impact:** 400 errors (`Unknown column in SET`).
|
||
|
||
**What happened:** Object ID (for URL path) was also sent in `data_kv`.
|
||
|
||
**Rule:** Keep object ID in the URL only; `data_kv` contains changed fields only.
|
||
|
||
**Verify:** In `update_ae_obj__*` calls, confirm `data_kv` excludes `*_id` fields.
|
||
|
||
### 3) Coarse-store `$effect` reactivity loops
|
||
**Impact:** repeated fetches, duplicate side effects, noisy state churn.
|
||
|
||
**What happened:** `$effect` read fields from `svelte-persisted-store` stores (`$ae_loc`, `$idaa_loc`), subscribing to entire store.
|
||
|
||
**Rule:** Be minimal about coarse-store reads in `$effect`; move transient triggers to session state and keep duplicate guards in executor paths.
|
||
|
||
**Verify:** Check effect dependencies and ensure unrelated writes do not retrigger critical effects.
|
||
|
||
### 4) Dexie `.get()` used with string object ID
|
||
**Impact:** false misses (`undefined`) despite cached data.
|
||
|
||
**What happened:** `.get()` queried primary key `id`, but V3 records rely on object string IDs (e.g. `person_id`).
|
||
|
||
**Rule:** Use `.where('<obj_id>').equals(value).first()` for object lookups.
|
||
|
||
**Verify:** Search for `.get(` against object IDs and replace with indexed `where` query.
|
||
|
||
### 5) Misunderstanding `$effect` and auth gates
|
||
**Impact:** security confusion and wrong mitigations.
|
||
|
||
**What happened:** Child-component `$effect` blocks were treated as possible bypass vectors.
|
||
|
||
**Rule:** Child effects cannot bypass parent layout auth gates; pre-gate risk is in universal `+page.ts` / `+layout.ts` loads and prefetch.
|
||
|
||
**Verify:** Keep private-module data loads out of universal load files unless explicitly authenticated.
|
||
|
||
### 6) Using query `key` like `x-no-account-id: bypass`
|
||
**Impact:** dropped `x-account-id`, unexpected 403 on scoped requests.
|
||
|
||
**What happened:** `key` was treated as bypass intent and account context was stripped.
|
||
|
||
**Rule:** `key` is business data, not bypass intent. Only explicit `x-no-account-id: bypass` drops account context.
|
||
|
||
**Verify:** Check request headers for account-scoped calls and preserve `x-account-id` unless bypass is explicitly required.
|
||
|
||
### 7) Pre-stringifying `*_json` before API wrappers
|
||
**Impact:** double encoding risk and inconsistent payloads.
|
||
|
||
**What happened:** Callers stringified JSON fields manually before wrapper serialization.
|
||
|
||
**Rule:** Pass plain objects for `*_json` fields; wrappers handle serialization.
|
||
|
||
**Verify:** Remove `JSON.stringify(...)` around `cfg_json`, `data_json`, and related fields before wrapper calls.
|
||
|
||
### 8) Broad Dexie result windows silently clipped
|
||
**Impact:** “All” views show fewer rows than narrower filters.
|
||
|
||
**What happened:** page limits or API revalidation replaced local broad result sets.
|
||
|
||
**Rule:** For empty text search, local IDB result set should drive visible results; server refresh updates cache without shrinking display.
|
||
|
||
**Verify:** Compare empty-search “All” view vs narrower filter counts and inspect revalidation merge behavior.
|
||
|
||
### 9) Missing `IDB_CONTENT_VERSIONS` bump after `properties_to_save` changes
|
||
**Impact:** long-lived stale cache bugs (e.g. IDAA “no meetings found”).
|
||
|
||
**What happened:** shape persisted in IDB changed, but table content version was not bumped.
|
||
|
||
**Rule:** When persisted object shape/behavior changes, bump matching `IDB_CONTENT_VERSIONS` entry in `src/lib/stores/store_versions.ts`.
|
||
|
||
**Verify:** For relevant object changes, confirm both version increment and table-wiring path are in the same change set.
|
||
|
||
### 10) API retry loop broken by returning transient errors
|
||
**Impact:** no retries on common WiFi/network blips.
|
||
|
||
**What happened:** transient error paths returned `false` instead of throwing, bypassing retry loop.
|
||
|
||
**Rule:** Keep retry classification strict:
|
||
- transient `TypeError` and timeout aborts -> `throw`
|
||
- navigation abort -> `return false`
|
||
- deterministic 4xx -> `return false`
|
||
- 5xx -> `throw`
|
||
|
||
**Verify:** Simulate transient network failure and confirm retry/backoff attempts occur.
|
||
|
||
### 11) Account-scoped trigger fired before bootstrap account is ready
|
||
**Impact:** wrong-account API fetch and cached cross-account data.
|
||
|
||
**What happened:** trigger effects ran before account bootstrap settled, reading stale context.
|
||
|
||
**Rule:** Gate account-scoped load triggers on `$slct.account_id` readiness, not persisted `$ae_loc.account_id`.
|
||
|
||
**Verify:** Ensure trigger effects require browser + account_id + api readiness before fetch trigger assignment.
|
||
|
||
### 12) `tmp_sort_*` comparator direction inverted
|
||
**Impact:** priority ordering reversed.
|
||
|
||
**What happened:** descending comparator used with `build_tmp_sort` encoding (which expects ascending).
|
||
|
||
**Rule:** Use ascending compare for `build_tmp_sort` outputs, with documented exceptions for legacy encodings.
|
||
|
||
**Verify:** Confirm comparator direction per module encoding and avoid `collection.reverse().sortBy(...)` assumptions.
|
||
|
||
### 13) `$` sigil used on plain prop values
|
||
**Impact:** runtime `store_invalid_shape` errors.
|
||
|
||
**What happened:** child component treated normal prop values as Svelte stores.
|
||
|
||
**Rule:** In Svelte 5, props passed as values are plain values. Do not use `$prop` unless prop is an actual store.
|
||
|
||
**Verify:** In migrated components, replace `$lq__...` prop reads with plain `lq__...` prop access.
|
||
|
||
### 14) Null JSON blob fields not guarded
|
||
**Impact:** read/write crashes (`cannot access property of null`).
|
||
|
||
**What happened:** `*_json` / `*_kv_json` DB columns were null before first write.
|
||
|
||
**Rule:** Use optional chaining for reads and `?? {}` initialization before object spread writes.
|
||
|
||
**Verify:** Audit reads/writes for `cfg_json`, `data_json`, `poc_kv_json`, and similar nullable blob fields.
|
||
|
||
### 15) Service worker stale-tab behavior misunderstood
|
||
**Impact:** users run old code longer than expected, “can’t reproduce” bug reports.
|
||
|
||
**What happened:** deployment assumptions ignored SW activation lifecycle.
|
||
|
||
**Rule:** Keep SW activation behavior explicit (`skipWaiting`, `clients.claim`) and evaluate trade-offs for session-heavy flows.
|
||
|
||
**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)
|
||
|
||
These are retained as project memory but removed from the active mistake list because they are lower-signal or one-off.
|
||
|
||
1. **Bad `.d.ts` module declaration masking errors** (important incident, low recurrence recently).
|
||
2. **Launcher `file_purpose == 'admin'` filtering gap** (specific historical enum-audit miss).
|
||
3. **Deleting files with `rm`** (still a workflow rule, now maintained in bootstrap File Safety section rather than as a mistake entry).
|
||
|
||
---
|
||
|
||
## Related Docs
|
||
|
||
- `documentation/BOOTSTRAP__AI_Agent_Quickstart.md`
|
||
- `documentation/TODO__Agents.md`
|
||
- `documentation/GUIDE__SvelteKit2_Svelte5_DexieJS.md`
|
||
- `documentation/GUIDE__AE_API_V3_for_Frontend.md`
|
||
- `documentation/PROJECT__Stores_Svelte5_Migration.md`
|