docs(openspec): archive persistence-corruption-guard and promote its spec
Close the SDD cycle for the data-loss shielding change: verified pass with warnings (0 critical, 10/10 scenarios with named tests, W1 fixed post-verify), three stacked work units shipped plus the path-resolution fix. The persistence-resilience capability spec is promoted to openspec/specs/.
This commit is contained in:
@@ -0,0 +1,38 @@
|
||||
# Exploration: persistence-corruption-guard
|
||||
|
||||
Source: full-app silent-failure audit (2026-07-11, engram `audit/2026-07-full-app-bug-classes` #2348). All mechanisms verified by reading live code; this change covers audit CRITICAL-1 and CRITICAL-4 (same architecture, three subsystems).
|
||||
|
||||
## The defect architecture (repeated 3×)
|
||||
|
||||
**Corrupt/failed read → silently coerced to empty state → next write persists the empty state → permanent user-data loss, zero signal.**
|
||||
|
||||
### Instance A — Alarms (worst: auto-persists with NO user action)
|
||||
- `ServicioAlarmas._parsear()` (`lib/servicios/servicio_alarmas.dart:70-110`) wraps `jsonDecode` + per-entry `AlarmaMusical.fromJson` for the WHOLE list in one `catch (_) { return empty config; }` — no logging.
|
||||
- `AlarmaMusical.fromJson` (`lib/modelos/alarma_musical.dart:147`) does `json['id'] as String` with no fallback (every other field has `?? default`) — ONE malformed/legacy entry throws and blanks the entire list.
|
||||
- `_configActual()` caches the original corrupted raw string in `_cacheRaw` (L66) but the EMPTY config in `_cache`.
|
||||
- `recalcularTodas()` (L201-220) — called as the second line of `EstadoAlarmas.inicializar()` on every cold start AND every 60s from the refresh timer — computes `nuevoRaw` from the empty list, finds it differs from `_cacheRaw` (the corrupted original), and calls `_guardar()`, overwriting disk with the empty list. **One bad read → all alarms permanently gone, automatically.**
|
||||
|
||||
### Instance B — Custom stations
|
||||
- `EstadoRadio._cargarEmisorasCustom()` (`lib/estado/estado_radio.dart:503-521`): `catch (_) { _emisorasCustom = []; }` — not even a log. Any read failure (corruption, future `Emisora.fromMap` schema break) silently empties the user's custom stations.
|
||||
- Next `agregarEmisoraCustom`/`eliminarEmisoraCustom` → `_guardarEmisorasCustom()` (L523-528) persists the near-empty list → **every previously saved custom station destroyed the first time the user adds or removes one.**
|
||||
|
||||
### Instance C — EQ presets / device names
|
||||
- `ServicioEcualizador._leerMapa` / `_leerMapaStrings` / `_leerPresetPrincipal` / `_leerPresetsPorEmisora` (`lib/servicios/servicio_ecualizador.dart:245,266,288,308`): identical catch-to-empty shape for per-station/per-device/matrix presets and device names.
|
||||
- Any `guardarXxx` call read-modify-writes through the corrupted-then-empty state, propagating the loss to disk on the user's next EQ tweak.
|
||||
|
||||
## Recommended fix shape
|
||||
|
||||
1. **Per-entry tolerant parsing** everywhere a LIST/MAP of entries is decoded: decode the container; parse each entry individually inside its own try; skip corrupt entries (log id/index + reason via debugPrint/developer.log); keep every valid entry. A single bad entry must never blank its siblings.
|
||||
2. **Entry identity rule**: an entry whose `id` is missing/invalid is skipped (never fabricate an id — ghosts are worse than a skipped entry).
|
||||
3. **Degraded-read guard (the load-bearing invariant)**: when the TOP-LEVEL decode fails (whole string unparseable), the in-memory state may be empty for the session BUT automatic persistence must be suppressed — `recalcularTodas()` (and any other write not initiated by an explicit user mutation) must NOT write while the last read was degraded. An explicit user mutation (save/delete alarm, add/remove station, EQ tweak) re-establishes write authority (user intent wins). Track per-subsystem (e.g. `_lecturaDegradada` flag set by the failed read, cleared on successful read or explicit mutation).
|
||||
4. **Logging**: every skipped entry and every degraded read logs with enough context to diagnose from logcat (`[PluriWave]` prefix convention).
|
||||
|
||||
## Testability (all Dart-testable with existing harnesses)
|
||||
- Seed SharedPreferences mock with: (a) one corrupt entry among valid ones → valid entries survive load, corrupt one skipped, `recalcularTodas()` does NOT rewrite the raw string destructively; (b) fully corrupt raw string → empty in-memory, `recalcularTodas()` writes NOTHING; explicit `guardarAlarma()` afterwards DOES write (user-intent rule).
|
||||
- Same pattern for `estado_radio` custom stations and `servicio_ecualizador` map readers.
|
||||
- Existing suites: `test/servicios/servicio_alarmas_*`, `test/estado/estado_radio_*`(check exact names), `test/servicios/servicio_ecualizador_test.dart` (20 cases) — extend, do not regress.
|
||||
|
||||
## Risks
|
||||
- `recalcularTodas()` legitimately rewrites when occurrences advance — the degraded guard must ONLY suppress writes when the last read failed, not change normal recalc behavior (locked by existing tests).
|
||||
- `_cacheRaw`/`_cache` coherence: fix must keep the raw-vs-parsed cache comparison meaningful after per-entry skips (a skipped entry means parsed != raw forever → would re-trigger save every cycle; decide: normalize raw after successful degraded-entry load via ONE explicit, logged, non-destructive rewrite of the SURVIVING entries — this is a design-phase decision, flag it).
|
||||
- EQ maps: colon-delimited matrix keys (`station:device`) must round-trip unchanged for surviving entries.
|
||||
Reference in New Issue
Block a user