fix(alarms): no fire for an occurrence already accounted for

A natural fire whose instant equals the skip or the handled-occurrence watermark is ignored, so a dismissal racing a reschedule cannot ring. Dismissing a repeating alarm persists its resolution before arming the skip, so an unspent old skip cannot absorb it. The deferred notice sync runs even if starting the ring throws, the swipe record is one synchronised read-modify-write, and it stays out of backups.

Fixes #16

app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt

@@ -290,6 +290,10 @@ class AlarmEngine @Inject constructor(
      */
     private suspend fun dismissComingLocked(alarm: Alarm): DismissOutcome {
         if (alarm.repeatDays.isRepeating) {
+            // Persist the resolution first: an earlier skip that has gone by but
+            // not yet been spent in storage would otherwise absorb this one,
+            // and the occurrence just dismissed would ring.
+            rescheduleLocked()
             alarms.setSkipNextOccurrence(alarm.id, true)
             rescheduleLocked()
             return DismissOutcome.SkippedNext(alarm.id)
@@ -309,12 +313,16 @@ class AlarmEngine @Inject constructor(
         // delivered late (after a reboot, or by Doze) still rings. Rejecting it
         // here would only re-register the same past instant and fire again.
         val lateBy = if (isSnoozeFire) AlarmRing.AUTO_SILENCE_AFTER else AlarmRing.FIRE_GRACE
-        // A skip armed on this very occurrence after its fire was already on its
-        // way: the user dismissed it in advance (#16), so it must not ring.
-        val skipped = !isSnoozeFire && state.skippedOccurrence == scheduledFor
-        // Deleted, switched off, skipped, or a PendingIntent so stale the moment
-        // has gone: re-resolve the slot and stay quiet.
-        if (alarm == null || !alarm.enabled || skipped || now - scheduledFor > lateBy) {
+        // An occurrence already accounted for: skipped in advance (#16) — armed or,
+        // once a reschedule spent the skip, recorded as handled — or a duplicate
+        // of one that already rang. A fire already on its way when that was
+        // written must not ring it. Equality only: a watermark ahead of a clock
+        // moved backwards must never silence an earlier occurrence.
+        val accountedFor = !isSnoozeFire &&
+            (scheduledFor == state.skippedOccurrence || scheduledFor == state.handledOccurrence)
+        // Deleted, switched off, accounted for, or a PendingIntent so stale the
+        // moment has gone: re-resolve the slot and stay quiet.
+        if (alarm == null || !alarm.enabled || accountedFor || now - scheduledFor > lateBy) {
             rescheduleLocked()
             return FireOutcome.Ignored
         }
@@ -343,10 +351,13 @@ class AlarmEngine @Inject constructor(
         val upcoming = mutableListOf<UpcomingAlarm>()
         rescheduleLocked(deferUpcomingTo = upcoming)
 
-        states.save(states.state(alarmId).copy(ringingSince = now))
-        ring.startRinging(alarmId)
-        scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId)
-        syncUpcomingLocked(upcoming, now)
+        try {
+            states.save(states.state(alarmId).copy(ringingSince = now))
+            ring.startRinging(alarmId)
+            scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId)
+        } finally {
+            syncUpcomingLocked(upcoming, now)
+        }
         return FireOutcome.Ringing(alarmId)
     }
 

app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt

@@ -73,9 +73,10 @@ class AndroidAlarmNotifier @Inject constructor(
 
             // A swipe is the user's word for that occurrence; the record keeps
             // only the occurrences still noticed.
-            val swiped = UpcomingNotifications.swipedKeys(context)
-            val stillSwiped = swiped.intersect(notices.map(UpcomingNotifications::key).toSet())
-            if (stillSwiped != swiped) UpcomingNotifications.retainSwiped(context, stillSwiped)
+            val stillSwiped = UpcomingNotifications.retainSwiped(
+                context,
+                notices.map(UpcomingNotifications::key).toSet(),
+            )
 
             // POST_NOTIFICATIONS is a runtime permission from API 33. Denied, the
             // user loses the notice and keeps the alarm and the row's Dismiss.

app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt

@@ -13,8 +13,9 @@ import kotlin.time.Instant
 import javax.inject.Inject
 
 /**
- * The AlarmManager slots arriving: the fire, the backstop and the upcoming check. Thin by design: every decision the engine
- * could make is the engine's, so the receiver only unpacks and dispatches.
+ * The AlarmManager slots arriving: the fire, the backstop and the upcoming
+ * check. Thin by design: every decision the engine could make is the
+ * engine's, so the receiver only unpacks and dispatches.
  */
 @AndroidEntryPoint
 class AlarmFireReceiver : HiltBroadcastReceiver() {

app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt

@@ -81,19 +81,28 @@ object UpcomingNotifications {
      * a new process, never brings back a notice the user got rid of, and a
      * notice the system took away does come back.
      */
-    fun swipedKeys(context: Context): Set<String> =
-        prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty()
-
+    @Synchronized
     fun recordSwiped(context: Context, alarmId: Long, fireAtMillis: Long) {
-        val keys = swipedKeys(context) + key(alarmId, fireAtMillis)
+        val keys = read(context) + key(alarmId, fireAtMillis)
         prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply()
     }
 
-    /** Drops the keys for occurrences no longer noticed, so the record stays small. */
-    fun retainSwiped(context: Context, keys: Set<String>) {
-        prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply()
+    /**
+     * Drops the keys for occurrences no longer in [noticed], so the record stays
+     * small, and returns what is left. One synchronised read-modify-write, so a
+     * swipe recorded from the receiver meanwhile is never overwritten.
+     */
+    @Synchronized
+    fun retainSwiped(context: Context, noticed: Set<String>): Set<String> {
+        val swiped = read(context)
+        val kept = swiped.intersect(noticed)
+        if (kept != swiped) prefs(context).edit().putStringSet(SWIPED_KEY, kept).apply()
+        return kept
     }
 
+    private fun read(context: Context): Set<String> =
+        prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty().toSet()
+
     fun key(notice: UpcomingNotice): String = key(notice.alarm.id, notice.fireAt.toEpochMilliseconds())
 
     private fun key(alarmId: Long, fireAtMillis: Long): String = "$alarmId@$fireAtMillis"
@@ -138,6 +147,7 @@ object UpcomingNotifications {
             PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,
         )
 
+    /** Excluded from backup and device transfer: it describes this device's shade. */
     private const val RECORD_FILE = "alarm_upcoming_notices"
     private const val SWIPED_KEY = "swiped"
 

app/src/main/res/xml/backup_rules.xml

@@ -5,4 +5,6 @@
          otherwise prompt again after a restore. Keep in step with
          data_extraction_rules.xml. -->
     <exclude domain="file" path="crash/" />
+    <!-- Which upcoming-alarm notices were swiped away: this device's shade only. -->
+    <exclude domain="sharedpref" path="alarm_upcoming_notices.xml" />
 </full-backup-content>

app/src/main/res/xml/data_extraction_rules.xml

@@ -1,10 +1,13 @@
 <?xml version="1.0" encoding="utf-8"?>
 <data-extraction-rules>
-    <!-- As backup_rules.xml: everything but a pending crash report. -->
+    <!-- As backup_rules.xml: everything but a pending crash report and the
+         upcoming-notice swipe record. -->
     <cloud-backup>
         <exclude domain="file" path="crash/" />
+        <exclude domain="sharedpref" path="alarm_upcoming_notices.xml" />
     </cloud-backup>
     <device-transfer>
         <exclude domain="file" path="crash/" />
+        <exclude domain="sharedpref" path="alarm_upcoming_notices.xml" />
     </device-transfer>
 </data-extraction-rules>

app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt

@@ -8,6 +8,7 @@ import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotices
 import de.jeanlucmakiola.clockula.testing.RingEvent
 import de.jeanlucmakiola.clockula.testing.alarmEngineHarness
 import de.jeanlucmakiola.clockula.testing.offsetInstant
+import de.jeanlucmakiola.clockula.testing.ringState
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.io.TempDir
@@ -207,4 +208,41 @@ class AlarmEngineUpcomingTest {
 
         assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound)
     }
+
+    @Test
+    fun `a fire that lands after the dismissed skip was spent still does not ring`(@TempDir tempDir: Path) =
+        runTest {
+            val harness = alarmEngineHarness(tempDir)
+            harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY)
+            harness.engine.reschedule()
+            harness.engine.dismissUpcomingOccurrence(1L, sixThirty)
+
+            // Another entry point reschedules first and spends the skip.
+            harness.wallClock.instant = sixThirty
+            harness.engine.reschedule()
+            val outcome = harness.engine.onFire(1L, sixThirty)
+
+            assertThat(outcome).isEqualTo(FireOutcome.Ignored)
+            assertThat(harness.ring.events).isEmpty()
+        }
+
+    @Test
+    fun `an old skip gone by but not yet spent does not swallow the new dismissal`(@TempDir tempDir: Path) =
+        runTest {
+            val harness = alarmEngineHarness(tempDir)
+            harness.givenAlarm(
+                id = 1L,
+                hour = 6,
+                minute = 30,
+                repeatDays = RepeatDays.EVERY_DAY,
+                skipNextOccurrence = true,
+            )
+            // Yesterday's skip, never spent because nothing rescheduled since.
+            harness.states.save(ringState(alarmId = 1L, skippedOccurrence = offsetInstant("2026-06-09T06:30+02:00")))
+
+            val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty)
+
+            assertThat(outcome).isEqualTo(DismissOutcome.SkippedNext(1L))
+            assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow)
+        }
 }

docs/ARCHITECTURE.md

@@ -1250,8 +1250,12 @@ the alarm's row for whoever opens the app instead.
   round its challenge. When it matches, it takes `dismissUpcoming`'s own
   branches: a repeating alarm skips that occurrence, a one-shot is switched off
   (or deleted, if transient). A fire broadcast already in flight when a skip
-  was armed is `Ignored` by `onFire`, which checks the skip watermark against
-  the instant it carries.
+  was armed is `Ignored` by `onFire`: a natural fire whose instant *equals*
+  the skip or the handled-occurrence watermark is already accounted for.
+  Equality, not order, so a watermark left ahead of a clock moved backwards
+  never silences an earlier occurrence. A repeating alarm's resolution is
+  persisted before the skip is armed, so an old skip that has gone by but was
+  never spent cannot absorb the new one.
 - **No dismiss challenge before the ring**, as in Google Clock. The challenge
   proves you are awake while an alarm sounds; before it, skipping is already one
   unchallenged switch away in the editor. The notification's Dismiss does ask