pneumo

M4: Push notifications with Copy and Open actions (ENG-263) #26

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-263-m4-push-notifications-with-copy-and-open-actions

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-263-m4-push-notifications-with-copy-and-open-actions

Linear: https://linear.app/engineereddev/issue/ENG-263

What's in this PR (22 files, all under apps/android/):

notify/PushNotifier implements PushSink: channel pushes (IMPORTANCE_HIGH), eligibility (source ≠ self AND target null or self), just-in-time decryption, §14.6 layouts (note BigText + Copy; link title-or-host + Open + Copy; file name/size + Download → DownloadRequested, "Coming soon" for now), brand icon/accent, setWhen = createdAt, stable id + tag = push id, group summary at 3+.
CopyReceiver (non-exported, immutable broadcast PendingIntent): plain-text clip, toast only below API 33, cancels the notification. Open goes through a non-exported OpenActivity that cancels the notification, then ACTION_VIEW for http(s) only.
Body tap → MainActivity with pushId, passed through PneumoNavHost (Home consumes it in ENG-264). push_deleted cancels the notification (SyncEngine hook).
POST_NOTIFICATIONS denied: no crash; Settings shows "Notifications are off" with a button to system settings (new preview + 2 Roborazzi baselines).
Tests: PushNotifierTest, NotificationActionsTest (Robolectric, ShadowNotificationManager, SDK 32 vs 34 toast), SyncEngineTest delete cancel, ShellRestorationTest.

Verify (lead, on eca40d7):

mise run check:android → exit 0 (Spotless, detekt, Lint, Roborazzi verify)
./gradlew testDebugUnitTest --rerun → exit 0 (165 tests, 0 failures)
./gradlew pixel6Api34DebugAndroidTest --rerun → exit 0 (11 tests, 0 failures)
Acceptance (implementer, API 34 emulator, local random-port server): dev-client send note "hello" → notification → Copy → paste shows hello (screenshot checked by the lead, not committed). Open, three-push grouping and permission recovery also exercised.

Deviation: Open uses a non-exported OpenActivity dispatcher because action-button taps don't honour auto-cancel.

🤖 Generated with Claude Code

Commits 2

M4: Preserve grouped notifications and isolate Open task (ENG-263)

+209 -20

M4: Add push notification actions and record banner baselines (ENG-263)

+1163 -74

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, REQUEST_CHANGES #

Layouts, eligibility, the Copy/Open/Download wiring, PendingIntent identity, push_deleted cancel, the pushId nav pass-through and the notifications-off banner match the issue; JVM, Roborazzi and managed-device suites are green. Two problems only show on a real API 34 system.

FIX (score ≥ 5)

often=5 bad=6 score=30 — notify/PushNotifier.kt:212-221 (PushNotificationGroup.reconcile): cancelling the group summary when the count drops below 3 makes Android cancel every child too. With exactly 3 showing, copying, opening, tapping or swiping one wipes the other two (reproduced 2/2 on an emulator via the dismiss path, 1/2 via cancelChild); a fast burst can also leave no summary because activeNotifications lags. The Robolectric test passes only because the shadow has no cascade. Fix: never cancel the summary while a child remains; count from the known change; add a managed-device test (3 posted, one acted on, two remain).
often=5 bad=2 score=10 — AndroidManifest.xml:44 (OpenActivity): the trampoline shares the app's task affinity, so Open brings Pneumo's task forward and Back from the browser lands in Pneumo. Fix: android:taskAffinity="" and Theme.NoDisplay, keeping excludeFromRecents/noHistory (verified in a throwaway clone).

FILE (score < 5, code left alone, recorded on ENG-275)

score=4 — a decrypt exception in PushNotifier.accept (tampered/malformed envelope) escapes the connection scope and crashes the process once per bad push.
score=1 — the exported MainActivity honours a pushId extra from any app (cancels that notification).

Spec §14.6/§14.7/§8 OK apart from findings 1–2; plaintext stays in the notification and PendingIntent extras only. DoD present (managed-device run: 8 tests). Next: omp round 2 for findings 1–2.

v1 1w ago

demi.dev 1w ago

pneumo-reviewer: round 2 (head 349a190), APPROVE #

Both round-1 FIX findings are resolved; only the 6 listed files changed (the rebase left apps/android byte-identical).

FIX 1 resolved (group cascade, burst race): the summary is cancelled only at 0 children; the count comes from a synchronized set updated on each known post and removal; every removal path goes through cancelChild. On the managed device the reviewer's probe passes 5/5 per case (burst shows the summary; dismiss or copy one of 3 keeps the other two; drain to empty). The new PushNotificationGroupTest fails on the round-1 code with actual=[] and passes now. The Robolectric assertions now match the device.
FIX 2 resolved (OpenActivity): taskAffinity="" + Theme.NoDisplay, keeping excludeFromRecents, noHistory, exported=false; the implementer's real-SystemUI check returns launcher → Chrome → launcher.

New FILE (score < 5, recorded on ENG-275): score=2 — the tracked child set can go stale if the system removes children without a delete intent (e.g. the Pushes channel is blocked mid-session); cosmetic.

Verify (reviewer and lead): check:android exit 0; testDebugUnitTest --rerun 165/0 failures; pixel6Api34DebugAndroidTest --rerun 10/0. CI 3/3 on 349a190.

Versions

pneumo-reviewer: round 1, REQUEST_CHANGES #

pneumo-reviewer: round 2 (head 349a190), APPROVE #