Files
pluriwave/openspec/changes/alarm-system-overhaul/design.md
T
Javier Bautista Fernández 29f7d54e85
Build & Deploy PluriWave / Análisis de código (push) Successful in 25s
Build & Deploy PluriWave / Build APK + AAB release (push) Successful in 2m25s
fix(alarm): fail-safe alarm system overhaul (SDD alarm-system-overhaul, slice A)
Root-cause fix for the unstoppable-alarm incident (alarm rang 15 minutes,
only uninstall silenced it) plus systematic hardening of every stop path.

Native (Kotlin):
- Verified stop: stopActiveAlarm now derives its result from the real
  post-teardown state (companion instance + synchronous stopEverything +
  activeRingingId check) instead of reporting unconditional success.
- Atomic teardown: every stop path (stop action, notification button,
  snooze, missed, onDestroy, startForeground failure) funnels through one
  stopEverything() covering audio, wakelock, notification, foreground
  state and firing-record cleanup; player.release() guarded.
- Bounded ringing: 10-minute auto-silence armed via AlarmManager fires a
  FIRED->MISSED transition with a localized missed-alarm notification;
  repeating alarms keep their native rearm, deleted alarms never produce
  ghost MISSED notifications.
- Durable firing record with onStartCommand re-validation (resurrection
  guard) and boot-time stale cleanup; firing records cleared on every
  refuse/mismatch/cancel path.
- New notification-only dismissal channel (dismissAlarmNotificationOnly)
  so UI-level dedup can never kill a live ring's audio.

Flutter (Dart):
- Stop/disable/edit/delete of a ringing alarm always attempt to silence
  it; on native-query failure the stop falls back toward silence via the
  id-scoped legacy stop.
- Verified-stop results surface failures: the ringing screen keeps
  dismiss-by-design on success, but on a verified failure it stays up
  with a persistent force-stop banner (guarded against double-dismiss)
  and auto-dismisses if the ring ends externally (missed/notification).
- Missed events sync alarm bookkeeping without opening the ringing UI.
- 4 new l10n keys translated across all 13 locales (ARB guard green).

550 tests green, analyzer clean. Reviewed in 3 adversarial 4-lens rounds
(2 deterministic + 1 refuter-corroborated critical fixed); formal
gentle-ai receipt waived by maintainer authorization (correction scope
legitimately exceeded the frozen genesis paths). On-device QA checklist
in openspec/changes/alarm-system-overhaul/tasks.md pending before
archive.
2026-07-22 23:52:36 +02:00

213 lines
13 KiB
Markdown

# Design: Alarm System Overhaul — Fail-Safe Stop/Dismiss
## Technical Approach
Evolve the existing native-owns-audio architecture; no rewrite. The single key enabler is that
the service, activity and receivers share ONE process (confirmed: no `android:process`), so
`MainActivity` can read a `@Volatile` companion field on `PluriWaveAlarmService` synchronously to
build verifiable stop results without a service round-trip. Four surgical additions: (1) an
id-agnostic fail-safe stop that can never no-op a live ring; (2) an atomic `stopEverything()` all
stop paths funnel through; (3) an AlarmManager-armed `FIRED→MISSED` auto-silence bound; (4) a
durable firing record in the existing device-protected prefs for process-death recovery. All
decision logic lives in Dart (mutation-while-ringing guard, stop-result handling) behind new fake
switches; Kotlin stays trivially static-grep-verifiable. Maps to proposal P0 (stop-safety) + P1
(fallbacks); P2 deferred.
## Architecture Decisions
### Decision 1 — Native stop semantics (verify-and-report)
| Option | Tradeoff | Decision |
|---|---|---|
| Service writes result to static field, channel polls | Race between async `onStartCommand` and channel read | Rejected |
| Channel reads `@Volatile` companion snapshot (same process), then dispatches id-agnostic stop | Snapshot is authoritative pre-stop; stop cannot no-op | **Chosen** |
`PluriWaveAlarmService` gains companion `@Volatile var activeRingingId: String?` (set in
`startAlarm`, cleared in `stopEverything`). New channel methods:
- `getActiveRingingAlarmId(): String?` — synchronous companion read; this is HOW Dart knows what
is ringing.
- `stopActiveAlarm(): { stopped: bool, wasRinging: bool, activeAlarmId: String? }` — reads the
snapshot, dispatches `ACTION_STOP_ACTIVE` (id-agnostic full teardown), returns the pre-stop
snapshot. `stopped` = a non-no-op teardown was dispatched (always true on success path);
`wasRinging` = `activeAlarmId != null`. **Error contract**: internal exception →
`result.error("STOP_FAILED", msg, null)`; Dart treats a thrown channel error OR `stopped==false`
as failure → retryable force-stop.
**Concurrent case**: only one alarm rings at a time (`startAlarm` early-returns while
`activeAlarmId != null`). `stopActiveAlarm` silences the ONE audible ring and returns its id so
Dart reconciles; the id-agnostic form is used ONLY by the ringing UI Stop and the notification Stop
action (explicit "silence what I hear" intents). Mutation guards (Decision 5) gate on the returned
`activeAlarmId` so a background toggle of a non-ringing alarm never stops a live one. Ambiguity
fails toward silence per the acceptance bar.
### Decision 2 — Atomic `stopEverything()`
Extract the current teardown block (`PluriWaveAlarmService.kt` L410-433) into one private
`stopEverything()`: cancel fallback+fade runnables, `player.stop()/release()`, `activeAlarmId=null`,
clear companion, `releaseWakeLock()`, `abandonAlarmAudioFocus()`, cancel notification,
`AlarmScheduler(this).clearFiringRecord(id)` + `cancelAutoSilence(id)`,
`stopForeground(REMOVE)`, `stopSelf()`. Every path routes through it: `ACTION_STOP`
(id match/null), `ACTION_STOP_ACTIVE`, `ACTION_SNOOZE`, `ACTION_MISSED`, `onDestroy`. `stopAlarm(id)`
stays as the id-scoped wrapper (foreign-id mismatch → cancel that id's notification only, then
return; else `stopEverything()`).
### Decision 3 — Auto-silence (AlarmManager-armed MISSED)
| Option | Tradeoff | Decision |
|---|---|---|
| In-service `Handler.postDelayed(10min)` | Dies with process; no missed-notification/rearm if service later gone | Rejected |
| AlarmManager `setExactAndAllowWhileIdle` → receiver `ACTION_MISSED` | Survives process death; posts missed notification + rearm even if FGS was killed; reuses existing scheduling infra | **Chosen** |
Fixed 10 min (matches the existing wakelock cap; configurability = P2). Armed in `onAlarmFired`
(already runs at fire time and already rearms the next occurrence, so `onAlarmMissed` must NOT
re-rearm — only silence + notify + clear record). Cancelled in `snooze`, `skipNext`, `cancelAlarm`
and `stopEverything`. `onAlarmMissed(id)`: stop the service if this id still rings, post a missed
notification (reuse the pre-notice non-FSI channel + `AlarmNotificationStrings`, new keys
`missedTitle`/`missedText`), clear the firing record.
### Decision 4 — Durable firing record
Store in the existing device-protected prefs `pluriwave_alarm_scheduler` (`AlarmScheduler.prefs()`,
direct-boot safe). Schema: `KEY_FIRING_IDS` (string set) + `firing_<id>``firedAtMillis` (Long).
**Write** in `onAlarmFired` BEFORE the service/audio starts (receiver runs `onAlarmFired` first).
**Clear** in `stopEverything`/`onAlarmMissed`. **`onStartCommand` re-validation**: `startAlarm`
checks `firingRecordAgeMillis(id)`; if `> AUTO_SILENCE_MILLIS` → abort start, run `onAlarmMissed`
cleanup (defends against redelivery/resurrection; `START_NOT_STICKY` already prevents blind
restart). **Boot cleanup**: `reschedulePersistedAlarms` calls `cleanupStaleFiringRecords()` first —
any record older than the window is cleared as missed, so a reboot mid-ring never resurrects audio.
### Decision 5 — Dart orchestration
`PuertoAlarmasAndroid` gains `Future<String?> alarmaSonandoId()` and
`Future<ResultadoDetencion> detenerSonidoActivo()` (new value type
`ResultadoDetencion{ bool detenido; bool estabaSonando; String? alarmaId; }`); `detenerSonidoNativo`
stays for compatibility. `EstadoAlarmas` gains a shared guard
`_detenerSiEstaSonando(String id)`: query `alarmaSonandoId()`; if it equals `id`
`detenerSonidoActivo()`. Wired into `guardarAlarma`/`cambiarActiva(false)`/`eliminarAlarma`
(upgrade its existing `detenerSonidoNativo` call) so any mutation of the ringing alarm silences it
first. `finalizarEjecucion` (the Stop path) calls `detenerSonidoActivo()` directly; on failure it
sets `_error` (same channel the snooze SnackBar already reads). New `forzarDetencion()` re-invokes
`detenerSonidoActivo()` for the retry action.
**UX** (`pantalla_alarma_sonando.dart`): keep dismiss-by-design, EXCEPT on a verified `_detener()`
failure — a timed SnackBar would auto-dismiss while the alarm is still audibly ringing, hiding the
only retry affordance. **Amended by review round 2**: instead the screen stays up and renders a
persistent in-screen banner (`alarmStopFailedMessage` + a `alarmForceStopAction` button calling
`forzarDetencion()`) that clears only on a confirmed stop, never on a timer.
**l10n** (new keys): `alarmStopFailedMessage`, `alarmForceStopAction`,
`alarmMissedNotificationTitle`, `alarmMissedNotificationText({name})`. Provide `en` (template, with
`@`-metadata + placeholders) and `es`. **Policy for the other 12 locales**: given the ARB
placeholder-corruption CI guard, define placeholder metadata ONLY in the template and mirror it
byte-exactly in `es`; omit the keys from the remaining locales so gen-l10n falls back to `en` at
runtime (untranslated-message warning is acceptable) — this avoids introducing placeholder metadata
into 12 files and the corruption risk the guard protects against. Flag full translation as a
follow-up.
### Decision 6 — P1 permission/FSI fallbacks
FSI fallback is largely automatic: `buildNotification` keeps `IMPORTANCE_HIGH` +
`setFullScreenIntent`, which the platform degrades to heads-up when `canUseFullScreenIntent()` is
false. The P1 work is the in-app WARNING: `diagnostics` already exposes
`canUseFullScreenIntent`/`notificationsEnabled`/`canScheduleExactAlarms`; surface warning banners on
the existing diagnostics/settings surface bound to `EstadoAlarmas.diagnostico` (new warning strings).
Lightly specified; sliced after P0.
## Data Flow
Fire: Receiver(ACTION_FIRE) ─→ AlarmScheduler.onAlarmFired
│ writes firing record (before audio) + arms MISSED + rearms next
└─→ Service.startAlarm ─→ companion.activeRingingId=id ─→ audio
Stop: RingingUI/NotifStop ─→ (channel) stopActiveAlarm
│ reads companion snapshot ─→ result{stopped,wasRinging,activeAlarmId}
└─→ ACTION_STOP_ACTIVE ─→ stopEverything() ─→ clear record + cancel MISSED
Mutate: EstadoAlarmas.guardar/cambiar/eliminar ─→ alarmaSonandoId()
└─ if == target ─→ detenerSonidoActivo() ─→ stopEverything()
Timeout: AlarmManager(MISSED) ─→ Receiver(ACTION_MISSED) ─→ onAlarmMissed
└─→ stop service + missed notification + clear record (no re-rearm)
## File Changes
| File | Action | Change |
|---|---|---|
| `PluriWaveAlarmService.kt` | Modify | companion `activeRingingId`/`ACTION_STOP_ACTIVE`/`AUTO_SILENCE_MILLIS`/`stopActive()`; `onStartCommand` +ACTION_STOP_ACTIVE; `startAlarm` sets companion + stale-record re-validation; extract `stopEverything()` |
| `AlarmScheduler.kt` | Modify | `recordFiring`/`clearFiringRecord`/`firingRecordAgeMillis`/`cleanupStaleFiringRecords`; `armAutoSilence`/`cancelAutoSilence`/`onAlarmMissed`; `onAlarmFired`+arm+record; `reschedulePersistedAlarms`+cleanup; cancel in snooze/skipNext/cancelAlarm |
| `PluriWaveAlarmReceiver.kt` | Modify | `ACTION_MISSED` constant + branch → `onAlarmMissed`; `pendingMissedIntent` helper |
| `MainActivity.kt` | Modify | channel `stopActiveAlarm` (returns map) + `getActiveRingingAlarmId`; pass missed strings in `setNotificationStrings` |
| `AlarmNotificationStrings.kt` | Modify | `missedTitle`/`missedText` getters+setters |
| `servicio_alarmas_android.dart` | Modify | `ResultadoDetencion`; `alarmaSonandoId()`; `detenerSonidoActivo()`; interface additions |
| `estado/estado_alarmas.dart` | Modify | `_detenerSiEstaSonando` guard; wire into guardar/cambiar/eliminar/finalizar; `forzarDetencion()`; missed-event handling |
| `pantallas/pantalla_alarma_sonando.dart` | Modify | force-stop SnackBar action on stop failure |
| `l10n/arb/app_en.arb`, `app_es.arb` | Modify | 4 new keys (en template + es) |
| `test/helpers/fakes_alarmas.dart` | Modify | `fallaDetener`, `alarmaSonandoIdValor`, `detencionesActivas`, new interface impls |
| `test/**` (Dart) | New | stop/mutation/force-stop/missed tests |
## Interfaces / Contracts
```dart
class ResultadoDetencion {
final bool detenido; // stop dispatched, cannot no-op
final bool estabaSonando; // audio was live
final String? alarmaId; // what was actually ringing
}
abstract class PuertoAlarmasAndroid {
Future<String?> alarmaSonandoId(); // getActiveRingingAlarmId
Future<ResultadoDetencion> detenerSonidoActivo(); // stopActiveAlarm
// ...existing members unchanged
}
```
Channel (`pluriwave/alarm_scheduler`): `getActiveRingingAlarmId → String?`,
`stopActiveAlarm → {stopped,wasRinging,activeAlarmId}`.
## Testing Strategy
| Layer | What | How |
|---|---|---|
| Unit (Dart) | Stop success/failure surfaces result | `fallaDetener``EstadoAlarmas.error` set; assert `detenerSonidoActivo` called |
| Unit (Dart) | Mutation-while-ringing stops audio | `alarmaSonandoIdValor=target``cambiarActiva(false)`/`guardarAlarma(inactive)`/`eliminarAlarma` assert force-stop invoked |
| Unit (Dart) | No false stop on non-ringing target | mismatched `alarmaSonandoIdValor` → assert force-stop NOT invoked |
| Widget | Force-stop retry affordance | stop failure → SnackBar with action → action calls `forzarDetencion` |
| Unit (Dart) | Missed event bookkeeping | native `missed` event → `completarEjecucion` recorded |
| On-device QA | Native-only proofs | see checklist |
**On-device QA checklist** (only a device proves): Stop from ringing UI (id match AND mismatch);
Stop from lock-screen notification; disable/edit/delete while ringing; 10-min untouched →
auto-silence + missed notification + repeating rearm; kill app mid-ring (audio stops); reboot
mid-ring (boot cleanup, no resurrection); concurrent second alarm; FSI-denied heads-up fallback.
## Threat Matrix
N/A — no routing, shell, subprocess, VCS/PR automation, executable-file classification, or
shell/process-integration boundary. (Android service/IPC is not in scope of the shell/subprocess
threat matrix.)
## Migration / Rollout
No migration. New pref keys (`firing_<id>`, `KEY_FIRING_IDS`) are additive and self-cleaning by age;
orphaned entries are ignored if the reader is reverted. Single feature branch to `main`; P0 first,
P1 as a follow-up slice within budget.
## Consequences / Rollback
Ships to live alarm users. Regression manifestations & mitigations:
- **Wrong concurrent alarm silenced** — bounded by the one-ring-at-a-time guard; `stopActiveAlarm`
returns the stopped id for reconciliation.
- **Auto-silence fires early** — bounded to 10 min = wakelock cap; user re-arm unaffected.
- **Firing-record bug → false "missed" or blocked start** — age-gated; stale-only cleanup; worst
case a legitimate ring is cut at the 10-min bound (still better than the unbounded incident).
**Fastest rollback**: `git revert` the merge/PR — native and Dart changes are additive to existing
stop paths, so revert restores current behavior; orphaned firing-record keys go unread (no migration
rollback needed).
## Open Questions
- [ ] Missed notification channel: reuse the pre-notice channel vs. a dedicated low-importance
`missed` channel? (Leaning reuse for minimal diff; confirm in tasks.)
- [ ] Should `finalizarEjecucion` return `Future<bool>` for a cleaner contract, or keep the
`_error`-field convention the snooze path uses? (Leaning `_error` for consistency.)