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.
11 KiB
11 KiB
Tasks: Alarm System Overhaul — Fail-Safe Stop/Dismiss
Review Workload Forecast
| Field | Value |
|---|---|
| Estimated changed lines (Slice A, code+tests) | ~650–800 |
| Estimated changed lines (Slice B, P1 follow-up) | ~80–120 |
| 400-line budget risk | High (well above the 400-line reviewer-cognition guard) |
| 800-line session budget risk | Medium (near the ceiling; any test-file growth pushes over) |
| Chained PRs recommended | No (session delivery is direct commits to main, no PRs) |
| Suggested split | Slice A = this delivery (Phases 1–7); Slice B = P1 follow-up (Phase 8) |
| Delivery strategy | single-pr-default (direct commits to main, no PRs) |
| Chain strategy | size-exception |
Decision needed before apply: Yes Chained PRs recommended: No Chain strategy: size-exception 400-line budget risk: High
Rationale: no-PR direct-commit strategy forecloses PR chaining, but Slice A crosses the 400-line
guard on its own. Maintainer must explicitly accept a size:exception for Slice A before sdd-apply
proceeds; Slice B (P1) ships as a separate, smaller follow-up commit set once Slice A lands.
Suggested Work Units (sequenced commit batches, not PRs)
| Unit | Goal | Focused test command | Runtime harness | Rollback boundary |
|---|---|---|---|---|
| A1 — Native stop core (Ph.1–3) | id-agnostic stop, atomic teardown, notification route fix, channel bridge | N/A (no Kotlin build env; grep-verifiable) | Device QA checklist below | Revert PluriWaveAlarmService.kt/MainActivity.kt hunks; additive-only |
| A2 — Native auto-silence + durable record (Ph.2) | MISSED transition, firing record, boot cleanup | N/A (grep-verifiable) | Device QA: 10-min + reboot scenarios | Revert AlarmScheduler.kt/PluriWaveAlarmReceiver.kt; orphaned pref keys unread |
| A3 — Dart orchestration (Ph.4–6) | mutation guard, force-stop UI, missed bookkeeping | flutter test test/estado/estado_alarmas_test.dart test/servicios/servicio_alarmas_android_test.dart test/pantallas/pantalla_alarma_sonando_test.dart |
N/A (unit/widget-covered) | Revert Dart hunks; guard is additive to existing stop calls |
| A4 — l10n (Ph.7) | 4 new keys, en+es only | flutter test + CI ARB guard |
N/A | Revert 2 ARB files |
| B1 — P1 fallback banners (Phase 8, follow-up) | FSI/exact-alarm/notification-permission warnings | flutter test test/estado |
Device QA: FSI-denied scenario | Revert diagnostics-banner hunk only |
Phase 1: Native Stop Core (Kotlin, static-edit)
- 1.1
PluriWaveAlarmService.kt: add companion@Volatile var activeRingingId: String?, set instartAlarm, cleared instopEverything. [kotlin-static] → Decision 1. Accept: grep shows@Volatilefield + both set/clear sites. - 1.2 Add
ACTION_STOP_ACTIVEconst; extractstopEverything()from the L410-433 teardown (runnables, player stop/release,activeAlarmId=null, companion clear, wakelock release, audio-focus abandon, notification cancel,clearFiringRecord+cancelAutoSilence,stopForeground(STOP_FOREGROUND_REMOVE),stopSelf()). [kotlin-static] → NA-2, Decision 2. Accept: grep shows all 3 teardown calls instopEverything()and every stop path (ACTION_STOP,ACTION_STOP_ACTIVE,ACTION_SNOOZE,ACTION_MISSED,onDestroy) calls it. - 1.3
onStartCommandACTION_STOP_ACTIVEbranch →stopEverything()id-agnostically;stopAlarm(id)keeps id-scoped wrapper (foreign-id mismatch cancels only that id's notification). [kotlin-static] → NA-1a, NA-1b, NA-1c, Decision 1. Accept: branch present, ignores extras id. - 1.4 (feedback item 1) Fix
stopPendingIntent(alarmId): change dispatchedactionfromACTION_STOPtoACTION_STOP_ACTIVEso the notification Stop button routes to the id-agnostic stop. [kotlin-static] → SS-4a, NA-1a. Accept: grep confirmsaction = ACTION_STOP_ACTIVEinsidestopPendingIntent.
Phase 2: Native Auto-Silence + Durable Firing Record (Kotlin, static-edit)
- 2.1
AlarmScheduler.kt: addAUTO_SILENCE_MILLIS(10 min),recordFiring(id)/clearFiringRecord(id)/firingRecordAgeMillis(id)onKEY_FIRING_IDS+firing_<id>inpluriwave_alarm_schedulerprefs. [kotlin-static] → NA-4a, Decision 4. - 2.2
armAutoSilence(id)/cancelAutoSilence(id)viaAlarmManager.setExactAndAllowWhileIdle→ACTION_MISSED; arm+record inonAlarmFired; cancel insnooze,skipNext,cancelAlarm,stopEverything. [kotlin-static] → NA-3a, Decision 3. Accept: grep shows 1 arm site + 4 cancel sites. - 2.3
onAlarmMissed(id):stopEverything()if id still rings, post missed notification (reuse pre-notice channel +AlarmNotificationStrings.missedTitle/missedText), clear firing record, no rearm; one-shot stays disabled. [kotlin-static] → NA-3a, NA-3b, NA-3c. Accept: grep shows no rearm/programarSiguientecall insideonAlarmMissed. - 2.4
PluriWaveAlarmReceiver.kt:ACTION_MISSEDconst + branch →onAlarmMissed;pendingMissedIntenthelper. [kotlin-static] → NA-3a. - 2.5
startAlarm: iffiringRecordAgeMillis(id) > AUTO_SILENCE_MILLIS→ abort +onAlarmMissedcleanup; elserecordFiring(id)beforeMediaPlayer.start(); keepSTART_NOT_STICKY. [kotlin-static] → NA-4a, NA-4b. Accept: grep showsrecordFiringtextually precedesMediaPlayer.start()and the stale-check branch exists. - 2.6
cleanupStaleFiringRecords(); call fromreschedulePersistedAlarmsbefore rescheduling. [kotlin-static] → NA-5a. Accept: grep shows call precedes the reschedule loop. - 2.7
AlarmNotificationStrings.ktmissedTitle/missedTextgetters+setters;MainActivity.kt setNotificationStringspasses them through. [kotlin-static] → NA-3a.
Phase 3: Channel Bridge (Kotlin, static-edit)
- 3.1
MainActivity.kt: addgetActiveRingingAlarmId(sync companion read) andstopActiveAlarm({stopped, wasRinging, activeAlarmId}; internal exception →result.error("STOP_FAILED", msg, null)). [kotlin-static] → Decision 1, NA-1a, NA-1b. Accept: grep shows both method names in the channel dispatcher + theerror()call.
Phase 4: Dart Orchestration — Stop Result + Mutation Guard (TDD)
- 4.1 RED —
test/servicios/servicio_alarmas_android_test.dart: failing test forResultadoDetencion{detenido,estabaSonando,alarmaId}mapping fromstopActiveAlarm/alarmaSonandoId(). [dart-testable] → Decision 5 interface.flutter testfails. - 4.2 GREEN —
servicio_alarmas_android.dart: addResultadoDetencion,alarmaSonandoId(),detenerSonidoActivo(),PuertoAlarmasAndroidadditions.flutter testgreen. - 4.3 RED —
test/helpers/fakes_alarmas.dart+estado_alarmas_test.dart: addfallaDetener/alarmaSonandoIdValor/detencionesActivas; failing tests for SS-1a/SS-1b/SS-1c/SS-1d (cambiarActiva(false)/guardarAlarma/eliminarAlarmastop only when target id == ringing id). [dart-testable] - 4.4 GREEN —
estado_alarmas.dart: add_detenerSiEstaSonando(id)guard; wire intoguardarAlarma,cambiarActiva(false),eliminarAlarma.flutter testgreen. - 4.5 RED — failing tests for SS-2a/SS-2b:
finalizarEjecucioncallsdetenerSonidoActivo()directly; confirmed → no_error; unconfirmed/failure →_errorset. [dart-testable] - 4.6 GREEN —
estado_alarmas.dart: wirefinalizarEjecucionaccordingly.flutter testgreen.
Phase 5: Dart UX — Force-Stop Affordance (TDD)
- 5.1 RED — failing test for SS-3b:
forzarDetencion()re-invokesdetenerSonidoActivo(); success clears failure state, failure keeps it. [dart-testable] - 5.2 GREEN —
estado_alarmas.dart: addforzarDetencion().flutter testgreen. - 5.3 RED —
test/pantallas/pantalla_alarma_sonando_test.dart: failing widget test for SS-3a/SS-3c (failure → screen stays up with a persistentalarmForceStopActionbanner; confirmed → no banner). [dart-testable] - 5.4 GREEN —
pantalla_alarma_sonando.dart: on_detener()failure, stay on screen and render a persistent in-screen banner (not a timed SnackBar) with a force-stop action →forzarDetencion(); clears only on a confirmed stop.flutter testgreen. - 5.5 (review round 3 correction)
_forzarDetencion()now respects_salidaEnCurso(guard reset on failure) and the ringing screen reconciles an external MISSED transition for its own alarm id (EstadoAlarmas.ultimaAlarmaPerdidaId) by auto-dismissing. [dart-testable]
Phase 6: Dart — Missed-Event Bookkeeping (TDD)
- 6.1 RED — failing test: native
missedevent →completarEjecucionrecords it. [dart-testable] - 6.2 GREEN —
estado_alarmas.dart: wire missed event in_alRecibirEventoNativo.flutter testgreen.
Phase 7: l10n (corrected policy — en+es only)
- 7.1
lib/l10n/app_en.arb: addalarmStopFailedMessage,alarmForceStopAction,alarmMissedNotificationTitle,alarmMissedNotificationText({name})with@-metadata. [dart-testable] Accept:flutter test(Phase 5 widget test) resolves the new keys. - 7.2
lib/l10n/app_es.arb: mirror the 4 keys, placeholder metadata byte-exact withen. [dart-testable] - 7.3 (review round 2 — superseded correction) The original "do NOT touch the other 11 locales" policy above was itself superseded by the round-2 review fix: the 4 keys were deliberately ADDED, with real translations, to all 13
lib/l10n/app_*.arbfiles instead of falling back toen. Accept: CI ARB placeholder-corruption guard passes across all 13 files. - 7.4 Wire missed-alarm strings through setNotificationStrings (gap found in apply verification)
- 7.5 (review round 3 correction) Native KDoc (
AlarmScheduler.AUTO_SILENCE_MILLIS,AlarmNotificationStrings.missedText) updated to reference all 13 ARB files instead of onlyen/es, matching 7.3.
Phase 8 — Slice B (P1 follow-up, not part of this delivery)
- 8.1 Diagnostics banners bound to
EstadoAlarmas.diagnostico.canUseFullScreenIntent/notificationsEnabled/canScheduleExactAlarms; new warning strings (en+es only, same policy as Phase 7). [dart-testable] → NA-6b, NA-6c. - 8.2 RED→GREEN Dart tests for exact-alarm-denied and
POST_NOTIFICATIONS-denied warnings. [dart-testable] → NA-6b, NA-6c. - 8.3 Confirm FSI auto-fallback needs no code change (platform degrades
setFullScreenIntentwhencanUseFullScreenIntent()is false); log as device-QA-only. [kotlin-static]+[device-qa] → NA-6a.
On-Device QA Checklist (collected — run on real hardware)
- Stop from ringing UI, id match AND id mismatch (NA-1a, NA-1c)
- Stop from lock-screen notification (SS-4a, NA-1a — after Phase 1.4 fix)
- Disable/edit/delete the ringing alarm from the list (SS-1a/b/c)
- Leave a fired alarm untouched 10 minutes → audio stops, missed notification posts, repeating rearms, one-shot stays disabled (NA-3a/b/c)
- Kill the app mid-ring → audio stops
- Reboot mid-ring → boot cleanup runs, no audio resurrection (NA-5a)
- Trigger a concurrent second alarm while one rings (NA-1c)
- Force FSI-denied state → heads-up fallback posts instead of full-screen intent (NA-6a)
Open Questions Carried to Apply
- Missed-notification channel: reuse pre-notice channel (default assumed above) vs. dedicated low-importance channel — confirm before Phase 2.3 lands.
finalizarEjecucionreturn contract:Future<bool>vs. existing_error-field convention (default assumed above:_error) — confirm before Phase 4.6 lands.