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.
213 lines
13 KiB
Markdown
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.)
|