fix: [SDK-5011] detect and correctly suppress restored notifications - #2723
Open
nan-li wants to merge 11 commits into
Open
fix: [SDK-5011] detect and correctly suppress restored notifications#2723nan-li wants to merge 11 commits into
nan-li wants to merge 11 commits into
Conversation
…efault markNotificationAsDismissed returned early whenever isNotificationToDisplay was false, which is always the case on the suppress path, so a restored notification the app rejected was never marked dismissed and came back on every later restore. Co-authored-by: Cursor <cursoragent@cursor.com>
Apps had no way to tell a restored notification apart from a new one inside their notification service extension, so work meant to run once ran again on every restore. v3 exposed this flag and it was dropped in the v4 refactor. Co-authored-by: Cursor <cursoragent@cursor.com>
…nnel An extender calling setChannelId moved a restored notification off the silent Restored channel, and since the channel governs alerting on Android O+, removeNotifyOptions could not stop it from showing as a heads-up banner. Co-authored-by: Cursor <cursoragent@cursor.com>
…around orgFlags was written after every extender ran and never read. Its sibling orgSound is still needed by the pre-Android 7.0 grouped-sound workaround, whose minSdk 21 version guard and stale comment are cleaned up here instead. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)14/44 touched executable lines covered (31.8% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 31.8% (minimum 80%). |
nan-li
marked this pull request as draft
August 21, 2026 17:58
… restore isRestoring meant two different things. NotificationSummaryManager.restoreSummary re-enters generation with isRestoring = true when a group drops to one member, which happens on an ordinary swipe or open. Dismissing on the suppress path therefore took down the surviving sibling the user never touched, for any app whose extension calls preventDefault(true) on a received event. Thread a NotificationRestoreReason through enqueue so the two are distinguishable. Only SHADE_RESTORE records the dismissal; GROUP_REGROUP keeps its previous behavior of leaving the notification and its summary in place. Record that dismissal without cancelling the shade. The restore pass includes notifications that are still showing on API 21 and 22, and whenever getActiveNotifications swallows a throwable and reports an empty shade. Skip the dependent summary rebuild as well. The restore pass already walks every outstanding notification, so rebuilding from here enqueues generation a second time for a sibling that is about to be restored anyway. Co-authored-by: Cursor <cursoragent@cursor.com>
The previous wording said restoring only meant Android cleared the shade, which is not what the SDK reports. It is also true when a group collapses to a single notification that has to be rebuilt. Point restore suppression at preventDefault(true). The no-argument overload parks on the display waiter for the full 30 second timeout, which is the wrong tool for dropping a restore and would hold a worker per notification. Warn that setExtender still has to run when restoring is true, since the rebuilt notification is what the app's customizations get applied to. Give the property a default so implementations outside this SDK keep compiling. Co-authored-by: Cursor <cursoragent@cursor.com>
An app that calls preventDefault() and never calls display() reaches the same suppress path, just after the callback timeout rather than immediately. Pin that it dismisses the record without cancelling the shade. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the source-compat note and the API-level walkthroughs. Keep when restoring is true, which preventDefault to call, and why regroup must not cancel the shade. Co-authored-by: Cursor <cursoragent@cursor.com>
A group-collapse rebuild is left in place. Saying "drop it" for every restoring event would surprise an app that still sees the leftover child and summary. Co-authored-by: Cursor <cursoragent@cursor.com>
Processor tests mock the repository, so they never showed that markAsDismissedWithoutCancel leaves the shade. readRestoreReason was untested for the enum, the legacy boolean, and an unknown name. Co-authored-by: Cursor <cursoragent@cursor.com>
nan-li
marked this pull request as ready for review
August 21, 2026 21:56
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.
Description
One Line Summary
(API Addition)
Let apps detect restored notifications, and stop restored ones from re-alerting or reappearing after they are suppressed.
Details
Motivation
When Android clears the shade (reboot, app update, force-stop) the SDK re-posts saved notifications and re-fires
onNotificationReceived. Three problems fell out of that:restoringflag; it was dropped in the v4 refactor.preventDefault()during a restore did not stick.markNotificationAsDismissedbailed out unlessisNotificationToDisplaywas set, which never happens when display is suppressed, so the row was never marked dismissed and the notification returned on every later restore.setChannelIdmoved a restored notification off the silentrestored_OS_notificationschannel. The channel governs alerting on Android O+, soremoveNotifyOptions()could not keep it quiet and it arrived as a heads-up banner.Scope
INotificationReceivedEventgains a read-onlyrestoring: Boolean, defaulted so existing implementations still compile. True for both a shade restore and a group collapsing to one member.preventDefault(true)marks the row dismissed and leaves the shade. Group collapse +preventDefaultis a no-op; the remaining child stays, including its summary.NotificationGenerationJob.orgFlags, written after every extender and read nowhere. Its siblingorgSoundis kept, still needed by the pre-Android 7.0 grouped-sound workaround.Testing
Unit testing
NotificationDisplayerTestscovers the restore channel, an extender trying to override it, an extender keeping its channel on a normal notification, and the payload-vs-extender sound split thatorgSoundexists for.NotificationGenerationProcessorTestscoversrestoringfor both reasons, shadepreventDefault(true), no-arg shadepreventDefault(), and regroup not dismissing.NotificationRepositoryTestscoversmarkAsDismissedWithoutCancelleaving the shade.NotificationGenerationWorkManagerTestscoversreadRestoreReasonfor the enum, the legacy boolean, and an unknown name.JaCoCo does not attribute Robolectric-executed lines, so this PR uses Skip Coverage Check. The processor and work-manager cases are plain JVM and do count.
Manual testing
Confirmed behavior before and after with force kill
Affected code checklist
Checklist
Overview
Testing
Final pass