Fix pre release bugs by RankoR · Pull Request #281 · GrapheneOS/Messaging

RankoR · 2026-08-29T19:44:46Z

No description provided.

m4pl · 2026-08-30T17:30:47Z

-                        builder.setContentText(line2);
-                    }

                    if (builder != null) {


builder is created a few lines up and cannot be null. The block is rewritten anyway, drop the check?

m4pl · 2026-08-30T17:37:36Z


+    private fun onPhoneNumberConfirmed(phoneNumber: String) {
+        val isValid = phoneNumber.isEmpty() ||
+            PhoneUtils.get(subId.value).getValidSelfE164Number(phoneNumber) != null


Could this be an injected use case?

m4pl · 2026-08-30T17:45:22Z

    modifier: Modifier = Modifier,
 ) {
    val scrollBehavior = TopAppBarDefaults.exitUntilCollapsedScrollBehavior()
    var showGroupMmsDialog by remember { mutableStateOf(false) }


showGroupMmsDialog is still local, so one dialog survives rotation and the other does not. Pick one?

m4pl · 2026-08-30T17:49:01Z

+            return@LaunchedEffect
+        }
+
+        BuglePrefs.getApplicationPrefs().putBoolean(


BuglePrefs and ParticipantRefresh straight from the composable, elsewhere this goes through a use case. Not blocking.

m4pl · 2026-08-30T17:56:59Z

+                _phoneNumberDialogState.update { it.copy(isInvalid = false) }
+            }
+
            is Action.PhoneNumberChanged -> {


PhoneNumberConfirmed would match what this now does?

m4pl · 2026-08-30T18:15:57Z

+                    if (isInvalid) {
+                        onNumberEdited()
+                    }


This only fires while the error is showing, so the name promises more than it does. onErrorDismissed?

m4pl · 2026-08-30T18:24:33Z

+        final PhoneNumberUtil phoneNumberUtil = PhoneNumberUtil.getInstance();
+        try {
+            final PhoneNumber phoneNumber = phoneNumberUtil.parse(phoneText, country);
+            if (phoneNumberUtil.isPossibleNumber(phoneNumber)) {


isPossibleNumber accepts a number lifted out of text: Call me at 5551234567 becomes +3725551234567 on an EE SIM and +15551234567 on a US one, and that value goes out as the MMS sender. Reject input with letters? Worth a test either way.

sdsantos

Just left one comment.

sdsantos · 2026-08-31T16:01:21Z

+        val durationsByContentUri = coroutineScope {
+            audioContentUris
+                .map { contentUri ->
+                    async { contentUri to resolveAudioDurationMillis(contentUri) }
+                }
+                .awaitAll()
+                .toMap()
+        }


Could it be a good idea to batch these resolves? Is there some parallelism cap somewhere I missed? Or is this a less expensive operation than it seems?

inthewaves · 2026-09-01T11:57:52Z

-                message.setData(mAttachmentType,
-                        SharedMemoryImageProvider.Companion.buildUri(mAttachmentUri, mAttachmentType));
+                message.setData(mAttachmentType, mAttachmentUri);


The deleted provider decoded the image and recompressed it as PNG before exposing it. That stripped the original container metadata. The replacement puts the original MMS part URI into MessagingStyle. NotificationRecord collects that URI, and NotificationManagerService grants it to each visible NotificationListenerService before invoking notifyPosted().

An enabled notification listener can now read the original attachment, including EXIF/location or other container data that is not needed to render the notification.

Fixed. BugleNotifications.getNotificationImageUri() decodes the attachment, bakes the EXIF orientation into it and writes a JPEG into the media scratch space.

inthewaves · 2026-09-01T12:20:42Z

+        // Send per-conversation notifications (if there are multiple conversations). The list
+        // holds every conversation with unseen messages, so it has to be iterated: picking one
+        // entry notifies whichever conversation holds the newest unseen message and silently
+        // drops the rest, and there is no summary notification to surface them.
+        state.mConversationsList.mConversations.stream()
                .filter(conv -> !isConversationBlocked(conv.mConversationId))
-                .findFirst();
-        conversation.ifPresent(conv -> processAndSend(state, conv));
+                .filter(conv -> !ConversationSnoozeQuery.isConversationSnoozed(
+                        conv.mConversationId))
+                .forEach(conv -> processAndSend(state, conv));


Note that NotificationManagerService enforces maximum of 50 notifications (https://github.com/GrapheneOS/platform_frameworks_base/blob/17/services/core/java/com/android/server/notification/NotificationManagerService.java#L493), maybe the remainder can be in an overflow/summary notification when at >= 51

Good catch, done.

inthewaves · 2026-09-01T12:24:10Z

+    /**
+     * Purges the rows orphaned while the declared foreign keys went unenforced, parent before
+     * child, so foreign_key_check comes back empty before onConfigure() turns enforcement on. The
+     * view change needs no statement, the views are rebuilt unconditionally above - but the handler
+     * must still exist, or the version check throws and every table is wiped.


foreign_key_check comes back empty before onConfigure() turns enforcement on

SQLiteOpenHelper calls onConfigure() before it begins the upgrade transaction and invokes onUpgrade(), so enforcement is already enabled here

https://github.com/GrapheneOS/platform_frameworks_base/blob/17/core/java/android/database/sqlite/SQLiteOpenHelper.java#L411

The cleanup is needed because enabling foreign keys does not retroactively repair existing violations. This comment could be rewritten to preserve that actual invariant; the current lifecycle description is reversed

The only snooze check guarded the conversation id passed by the caller, so it was bypassed entirely whenever that id was null -- which is what the boot, app-update and redownload paths pass. Every reboot re-notified conversations snoozed "Always". A snoozed conversation could also consume the single notification slot and silence the conversation that had actually received a message. Filter snooze per conversation in the selection stream, where the blocked check already lives, instead of relying on the caller's argument. The guard in update() stays for its cancel() side effect.

setIdentifier(currentTimeMillis()) made every clear intent unique, so the platform minted a fresh PendingIntent on each notification update instead of reusing one per conversation. Uniqueness has to come from the intent itself: PendingIntent identity is (requestCode, filterEquals), the request code is constant across conversations, and filterEquals ignores extras -- so the conversation id set, which lives only in an extra, cannot distinguish them. Set the data URI to the conversation's metadata URI instead. Dismissing one conversation's notification then marks only that conversation as seen.

The conversations list holds every conversation with unseen incoming messages, ordered newest first. Taking findFirst() from it notified whichever conversation happened to hold the globally-newest unseen message and silently dropped the rest, with no summary notification to surface them -- so a reboot with three unread threads announced exactly one. It also ignored the conversation id the update was triggered for, so an arriving message could be passed over in favour of an unrelated newer thread. Iterate the list instead. The early returns in processAndSend already diff against the active notification, so untouched conversations do not re-alert.

inthewaves · 2026-09-01T21:56:04Z

+        final Uri scratchUri = MediaScratchFileProvider.buildMediaScratchSpaceUri(
+                partId,
+                NOTIFICATION_IMAGE_EXTENSION
+        );


This creates a successful scratch space file without a cleanup path. Most of the existing scratch users clean up when ownership ends:

GIF transcoding deletes its output immediately after reading it

Failed/discarded camera, video, and audio captures are deleted

Rejected or removed draft attachments use deleteTemporaryAttachment()

Scratch attachments used for MMS retries are destroyed once permanent telephony parts replace them

Notification images don't have an equivalent terminal lifecycle and will accumulate until Android evicts the cache. Could these use a dedicated notification image cache with e.g. bounded age/active-notification-based pruning (e.g. via NotificationManagerCompat.from(context).getActiveNotifications())? Cleanup should probably be rate-limited like run when creating a notification image only if the last sweep is old enough or the cache exceeds a size threshold

Notification images keyed by part ID should also be invalidated when DatabaseHelper.rebuildTables() runs, since old part IDs would no longer be valid. rebuildTables is after an upgrade handler fails or when opening the database with an older schema version (so rare fallback paths and not a normal startup or a successful upgrade)

Also, while the MMS remains in the telephony database, the cached JPEG is mostly a duplicate, although it has had EXIF removed. But after the message or conversation is deleted, the derived JPEG can remain, retaining the visible image beyond the expected message lifetime.

Internal cache files are app-private and in credential-encrypted storage in Android 17, so unrelated apps cannot directly read them.

Android may evict them, but its documentation explicitly says applications should not rely on automatic cache cleanup. Android app-specific storage guidance (https://developer.android.com/training/data-storage/app-specific#internal-remove-cache)

Even though Android sometimes deletes cache files on its own, you shouldn't rely on the system to clean up these files for you. You should always maintain your app's cache files within internal storage.

General issue on cache management for scratch space URIs: #284

Done. Notification images now live in their own provider and cache directory, swept after every notification pass against getActiveNotifications().

RankoR requested a review from m4pl August 29, 2026 19:44

Fall back to full name for the snippet sender without first name

e6c6b70

RankoR force-pushed the fix-pre-release-bugs branch from 8ff691f to ecf9026 Compare August 29, 2026 19:49

RankoR requested a review from inthewaves August 29, 2026 19:49

RankoR force-pushed the fix-pre-release-bugs branch 9 times, most recently from b175fa4 to 954bd42 Compare August 30, 2026 09:24

m4pl reviewed Aug 30, 2026

View reviewed changes

RankoR force-pushed the fix-pre-release-bugs branch from 954bd42 to 44390ab Compare August 31, 2026 12:40

RankoR requested a review from sdsantos August 31, 2026 12:57

sdsantos approved these changes Aug 31, 2026

View reviewed changes

m4pl approved these changes Aug 31, 2026

View reviewed changes

inthewaves suggested changes Sep 1, 2026

View reviewed changes

RankoR added 10 commits September 1, 2026 22:15

Enforce the foreign keys the schema declares

3f6c578

Show the formatted destination wherever a number is user-visible

5206080

Show the audio attachment length before it is played

296e53c

Move Jacoco setup to a buildSrc plugin to unbreak lint

d762485

Never cluster a failed message with its neighbours

96855e8

Label outgoing messages with the timestamp the thread is sorted by

13036a5

Show a no-results state when a recipient search matches nothing

64da8ba

RankoR force-pushed the fix-pre-release-bugs branch from 44390ab to 85347e8 Compare September 1, 2026 19:53

RankoR requested a review from inthewaves September 1, 2026 20:48

inthewaves suggested changes Sep 1, 2026

View reviewed changes

RankoR force-pushed the fix-pre-release-bugs branch from 85347e8 to 68c7ec8 Compare September 2, 2026 17:46

RankoR added 15 commits September 2, 2026 22:19

Show the MMS image in its notification

7eb5166

Give each notification button its own pending intent

fc2d3ec

Say which message failed in the failure notification

30b9e06

Resolve the self phone number from the carrier when the SIM lacks it

af7c72c

Only accept a phone number as the self phone number

93deebf

Announce pinned conversations in the order they are shown

d1666b5

Decide selection toggles from the whole selection, not the first row

8b2ff75

Let the attachment menu take back without taking the keyboard down

c5aa30d

Fix the subscription settings test for state-driven dialog visibility

a0c2c23

Warn that deleting a conversation cannot be undone

0d4885b

Store the delivery report sent timestamp in milliseconds

12800ed

Run instrumented tests from a known device state

af471eb

Run instrumented tests on an Android 17 emulator image

c05d7f5

Drop the dead null check in the message failure notification

939a2c4

Remember the group MMS dialog across configuration changes

f2e5935

RankoR force-pushed the fix-pre-release-bugs branch from 68c7ec8 to f2e5935 Compare September 2, 2026 19:22

RankoR requested a review from inthewaves September 2, 2026 19:48

inthewaves approved these changes Sep 2, 2026

View reviewed changes

thestinger merged commit 22cd55e into GrapheneOS:main Sep 3, 2026
7 checks passed

Uh oh!

Conversation

RankoR commented Aug 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sdsantos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inthewaves Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inthewaves Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inthewaves Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

inthewaves Sep 1, 2026 •

edited

Loading

inthewaves Sep 1, 2026 •

edited

Loading

inthewaves Sep 1, 2026 •

edited

Loading