Fix ghost notification on alarm delete and implement ringing snooze#192
Open
ScottMorris wants to merge 5 commits intomainfrom
Open
Fix ghost notification on alarm delete and implement ringing snooze#192ScottMorris wants to merge 5 commits intomainfrom
ScottMorris wants to merge 5 commits intomainfrom
Conversation
Adds `AlarmUtils.isAlarmLive(context, id)` as a focused prefs-backed helper and calls it at the top of `AlarmReceiver.onReceive()` before starting `AlarmRingingService`. If the alarm's prefs entry is absent (removed atomically by `cancelAlarm()`), the broadcast is silently dropped. This closes the race window in #191 where a `BroadcastReceiver` dispatch that was already in-flight when the user deleted an alarm would still fire the ringing service and show a ghost notification. The prefs entry is the canonical source of truth used by `BootReceiver` for the same reason. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tch loop Registers an `alarm:cancelled` listener in `AlarmManagerService.init()` that immediately calls `cancelNativeAlarm()` and `cancelUpcomingNotification()` when Rust emits the event. This fires before `alarms:batch:updated` arrives and eliminates the async gap where a deleted alarm's native schedule could still fire before `syncNativeAlarms()` ran. `deleteAlarm()` is simplified to only call `AlarmService.delete()` — all cancellation is now driven by the event. The batch listener still runs and harmlessly double-cancels via `syncNativeAlarms()`. This is the event-driven pattern aligned with the app's architecture direction; existing callers that emit `alarm:cancelled` (toggle off, dismiss one-shot) now automatically trigger native cancellation too. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolves the snooze anchor ambiguity from #165. The `snooze_alarm` Rust command now accepts `snoozed_until: i64` (an explicit epoch-millisecond timestamp) instead of a duration in minutes. The TS layer is responsible for computing the right anchor: - **Ringing path** (`snoozeRinging`): `now + N minutes` — the user is awake and pressing snooze; they want N more minutes from right now. - **Upcoming path** (`snoozeUpcoming`): `nextTrigger + N minutes`, floored to `now + 60s` — the alarm hasn't rung yet; snoozing from the lock-screen notification should push it to just after when it was originally set. The old `snoozeAlarm(id, minutes, stopCurrentRinging)` method is replaced by `snoozeRinging` and `snoozeUpcoming` — each path has different prerequisites (upcoming needs the alarm record) and different post-conditions (ringing stops audio; upcoming does not). The watch-initiated snooze in `lib.rs` continues to use now-anchoring since it always fires from a ringing state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wires the full snooze flow when the user presses Snooze on the Android
lock-screen notification while an alarm is ringing.
**Native (Kotlin):**
- `AlarmRingingService` gains an `ACTION_SNOOZE` intent and a Snooze button
on the foreground notification. Both the Dismiss and Snooze action intents
now carry `ALARM_ID` as an extra.
- The snooze action calls `AlarmManagerPlugin.notifySnoozeRequested(alarmId)`
and then stops the service (tears down audio, vibration, notification).
- `AlarmManagerPlugin` adds a `set_snooze_event_handler` command and a
dedicated `snoozeEventChannel` that forwards snooze requests to the Rust layer.
**Rust (plugin bridge):**
- `mobile.rs` registers the snooze channel in `init()` and re-emits
`alarm-manager:snooze-requested` as a Tauri event.
- `NativeSnoozeRequestedPayload { id }` added to `models.rs`.
**TypeScript:**
- `AlarmNotificationService` listens for `alarm-manager:snooze-requested` and
routes it to `handlers.onSnoozeRinging(alarmId)` — now typed as `(alarmId: number) => Promise<void>`.
- `AlarmManagerService.onSnoozeRinging` calls `snoozeRinging(alarmId, snoozeLength)`
and emits a confirmation toast.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2604aa184
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`notifySnoozeRequested` now mirrors the existing `notifyAlarmFired` queue-and-replay pattern: if the plugin instance is missing or `alarmPipelineReady` is false (cold-start from lock screen), the event is persisted to `KEY_PENDING_SNOOZE_EVENTS` in SharedPrefs and replayed when `load()` runs or `mark_alarm_pipeline_ready` fires. **Why:** A user pressing **Snooze** on the ringing notification while the app/plugin has not been initialised would previously hit the `instance ?: return` guard. The ringing service still stopped, but the snooze never reached the TS/Rust pipeline so no new trigger was scheduled. This brings snooze to parity with alarm-fired's cold-start handling. `dispatchSnoozeRequestedEvent` now returns `Boolean` like its alarm-fired sibling so the caller can decide between dispatch and queue. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ScottMorris
added
bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two notification lifecycle issues (#191, #165) and completes the ringing snooze flow that was previously stubbed out.
AlarmReceiver.onReceive()that checks SharedPrefs before startingAlarmRingingService. If the alarm was deleted while its broadcast was in-flight, the service is never started. Also added analarm:cancelledevent listener inAlarmManagerServiceso native cancellation fires immediately when Rust emits the event, before the sloweralarms:batch:updatedbatch loop arrives.snooze_alarmnow accepts an explicitsnoozed_untilepoch-millisecond timestamp. The TS layer computes the anchor: ringing snooze usesnow + N minutes; upcoming notification snooze usesnextTrigger + N minutes(floored tonow + 60s). This gives the right UX in both paths — pressing snooze from a ringing alarm gets N more minutes from right now; pressing snooze from the lock-screen upcoming notification keeps the alarm close to its originally scheduled time.AlarmRingingServicenow has a Snooze button on the foreground notification. The action sendsACTION_SNOOZEwithALARM_IDthrough the plugin channel bridge (alarm-manager:snooze-requestedTauri event) toAlarmManagerService, which callssnoozeRinging()and emits a confirmation toast.Test plan
AlarmReceiverlog shows "skipping fire"alarm:cancelledlistener fires immediately, native alarm cancelled before batch resync🤖 Generated with Claude Code
Closes #165 and closes #191.