Fix/notification deduplication#3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds in-service notification deduplication to reduce repeated forwarding, and introduces a GitHub Actions workflow to build (and on tags, release) APK artifacts.
Changes:
- Add a small in-memory LRU-based dedup cache in
AppNotificationListenerServiceto skip group summaries and suppress rapid duplicates. - Expand notification content extraction to include
EXTRA_BIG_TEXTfor better dedup matching. - Add a CI workflow to build a signed release APK, upload it as an artifact, and create a GitHub Release on version tags.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| app/src/main/java/com/itsazni/notificationforwarder/service/AppNotificationListenerService.kt | Adds dedup logic keyed by notification identity + content signature to skip duplicates/group summaries. |
| .github/workflows/build.yml | Adds CI build-and-release pipeline for APK artifacts and tag-based GitHub Releases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Decode Keystore | ||
| run: | | ||
| echo "${{ secrets.KEYSTORE_BASE64 }}" | base64 --decode > ${{ github.workspace }}/notif-forwarder-release.jks | ||
|
|
||
| - name: Build Release APK | ||
| run: | | ||
| ./gradlew assembleRelease \ | ||
| -Pandroid.injected.signing.store.file=${{ github.workspace }}/notif-forwarder-release.jks \ | ||
| -Pandroid.injected.signing.store.password=${{ secrets.KEYSTORE_PASSWORD }} \ | ||
| -Pandroid.injected.signing.key.alias=${{ secrets.KEY_ALIAS }} \ | ||
| -Pandroid.injected.signing.key.password=${{ secrets.KEY_PASSWORD }} |
There was a problem hiding this comment.
The keystore decode step runs for every push/PR, but secrets are not available for forked PRs (and may be intentionally unset in some environments), which will cause this workflow to fail before any build happens. Consider gating keystore decode/signing to tag builds (or when all required secrets are present), and running an unsigned/debug build for PRs/branch pushes instead.
| run: | | ||
| ./gradlew assembleRelease \ | ||
| -Pandroid.injected.signing.store.file=${{ github.workspace }}/notif-forwarder-release.jks \ | ||
| -Pandroid.injected.signing.store.password=${{ secrets.KEYSTORE_PASSWORD }} \ | ||
| -Pandroid.injected.signing.key.alias=${{ secrets.KEY_ALIAS }} \ | ||
| -Pandroid.injected.signing.key.password=${{ secrets.KEY_PASSWORD }} |
There was a problem hiding this comment.
This workflow uses the "android.injected.signing.*" properties to sign the release. That approach is intended for IDE/injected builds and can be brittle in CI (and may break with future AGP updates). Prefer configuring signingConfigs in Gradle and selecting the signing config via environment variables/Gradle properties, or use a dedicated Gradle task for CI signing.
| run: | | |
| ./gradlew assembleRelease \ | |
| -Pandroid.injected.signing.store.file=${{ github.workspace }}/notif-forwarder-release.jks \ | |
| -Pandroid.injected.signing.store.password=${{ secrets.KEYSTORE_PASSWORD }} \ | |
| -Pandroid.injected.signing.key.alias=${{ secrets.KEY_ALIAS }} \ | |
| -Pandroid.injected.signing.key.password=${{ secrets.KEY_PASSWORD }} | |
| env: | |
| ORG_GRADLE_PROJECT_signingStoreFile: ${{ github.workspace }}/notif-forwarder-release.jks | |
| ORG_GRADLE_PROJECT_signingStorePassword: ${{ secrets.KEYSTORE_PASSWORD }} | |
| ORG_GRADLE_PROJECT_signingKeyAlias: ${{ secrets.KEY_ALIAS }} | |
| ORG_GRADLE_PROJECT_signingKeyPassword: ${{ secrets.KEY_PASSWORD }} | |
| run: | | |
| ./gradlew assembleRelease |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) { | ||
| append(sbn.user.hashCode()) | ||
| } else { | ||
| append("legacy-user") | ||
| } |
There was a problem hiding this comment.
Build.VERSION.SDK_INT >= JELLY_BEAN_MR1 is always true in this app (minSdk is 26), so the else branch is dead code. Consider simplifying this to always include sbn.user (or removing the version check entirely) to reduce noise in the key calculation.
| val now = System.currentTimeMillis() | ||
|
|
||
| synchronized(dedupLock) { | ||
| val previous = recentEvents[stableKey] | ||
| if (previous != null) { | ||
| val sameContent = previous.contentHash == contentHash | ||
| val samePostTime = previous.postedAt == sbn.postTime | ||
| val burstUpdate = now - previous.seenAt <= DUPLICATE_WINDOW_MS | ||
| if (sameContent && (samePostTime || burstUpdate)) { |
There was a problem hiding this comment.
The dedup window uses System.currentTimeMillis() for seenAt and then compares deltas. Wall-clock time can jump backwards/forwards (manual change, NTP), which can break the burstUpdate calculation and lead to missed or extra forwarding. Consider using a monotonic clock (e.g., SystemClock.elapsedRealtime()) for seenAt/window comparisons while keeping sbn.postTime as-is.
| val stableKey = buildStableKey(sbn) | ||
| val contentHash = listOf(title, text, bigText).joinToString("\u001f").hashCode() | ||
| val now = System.currentTimeMillis() |
There was a problem hiding this comment.
contentHash is derived from String.hashCode() over a concatenated string, which can collide and incorrectly suppress distinct notifications. If the goal is correctness over micro-optimizations, consider using a stronger/fewer-collisions content signature (e.g., a 64-bit hash or MessageDigest) and avoid the intermediate List/joined String allocation.
![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: f0b75ef121
ℹ️ 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".
| - name: Build Release APK | ||
| run: | | ||
| ./gradlew assembleRelease \ | ||
| -Pandroid.injected.signing.store.file=${{ github.workspace }}/notif-forwarder-release.jks \ | ||
| -Pandroid.injected.signing.store.password=${{ secrets.KEYSTORE_PASSWORD }} \ |
There was a problem hiding this comment.
This workflow runs on pull_request, but the release build always injects signing credentials from secrets.*. On pull requests from forks, GitHub does not provide repository secrets, so these values are empty and assembleRelease tries to sign with an invalid/empty keystore, causing the CI job to fail for external contributors. Add a condition to skip signed-release steps (or build unsigned) when secrets are not available.
Useful? React with 👍 / 👎.
No description provided.