From 7b9551aba830019a875b6fc24b5a25d74eceeb65 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Wed, 14 Oct 2026 12:35:58 -0700 Subject: [PATCH] fix(ios): open push replies in their containing thread (#8294) Tapping a push notification for a thread reply currently opens the channel at its parent. Preserve the verified event's direct-parent thread ID through the iOS notification payload and Flutter bridge so the tap opens the containing thread and highlights that exact reply, even when newer replies have arrived. This matches Inbox's destination selection for nested replies; broadcast replies still open in the channel. Closest related work: https://github.com/block/buzz/pull/6159 (original push-tap routing). ### Validation - Follow-up [5b70e1a](https://github.com/block/buzz/pull/8294/changes/5b70e1a5cb58104b5e17b48493bbde2bb6a6f207) selects the last valid 64-character ASCII-hex reply marker, matching relay ingest. Regression tests failed before the fix; all 80 XCTest and 47 Swift Testing tests passed afterward with Xcode 26.5. [Current-head CI](https://github.com/block/buzz/actions/runs/38607128406) passed, including Flutter tests, Android build, native package tests, complete iOS simulator and unsigned release builds, and native accessibility tests. The [current-head security review](https://github.com/block/buzz/pull/8294#issuecomment-6174205374) reports no findings. Physical evidence below predates this validation-only follow-up. - [Baseline CI on f465f91](https://github.com/block/buzz/actions/runs/38436936153) passed, including Flutter and native package tests, Android build, complete iOS simulator and unsigned release builds, and native accessibility tests. - Full local validation passed on [f465f91](https://github.com/block/buzz/commit/f465f9196fdb6054e302c7ad31ea9de91381c32b), including 3,215 Flutter tests (four skipped) and three unconfigured-push tests. The 49 focused navigation tests also passed after the baseline merge. - Added coverage for cold/warm bridge delivery, exact reply highlighting with newer arrivals, direct-parent and broadcast destinations, and malformed optional thread IDs. The Codex finding for an empty reply marker is fixed and resolved. - Physical acceptance passed on the managed iPhone 12 mini using signed Buzz Dev 10140, built from f465f91 with SDK 26.5. Real push taps and Inbox opened the same containing thread and highlighted the exact reply. An older alert retained its target after a newer reply arrived; a retained alert also worked after terminating the app. Top-level push navigation reached the channel. - Installed in place with the existing identity, app data and pairing retained. No key reset or rotation was performed. Notifications were restored to off and device control released. Xcode 27 hits an unchanged dependency compiler error; the matching Xcode 26.5 toolchain built and signed this tested candidate successfully. ### Physical acceptance | Warm push: older reply A | Inbox: reply B | Cold push: reply B | | --- | --- | --- | | ![Older reply A highlighted with newer B visible](https://raw.githubusercontent.com/block/buzz/87eb00d209cf0b0dd3f4146e3219b15884133b9d/pr-8294--warm-older-reply.png) | ![Inbox highlights reply B](https://raw.githubusercontent.com/block/buzz/87eb00d209cf0b0dd3f4146e3219b15884133b9d/pr-8294--inbox-exact-reply.png) | ![Cold push highlights reply B](https://raw.githubusercontent.com/block/buzz/87eb00d209cf0b0dd3f4146e3219b15884133b9d/pr-8294--cold-push-exact-reply.png) | [Physical test details](https://github.com/block/buzz/pull/8294#issuecomment-6151991840). ### Before and after Synthetic Flutter widget captures with a newer reply already present. | Before: channel at outer parent | After: exact reply in containing thread | | --- | --- | | ![Before](https://raw.githubusercontent.com/block/buzz/d9a0ff6c7a5ea481b04cb9c61d301553b4c23c10/pr-8294--before.png) | ![After](https://raw.githubusercontent.com/block/buzz/d9a0ff6c7a5ea481b04cb9c61d301553b4c23c10/pr-8294--after.png) | After merge, repeat warm/cold push versus Inbox in the integrated mobile candidate, including older alerts, top-level and broadcast notifications. --------- Signed-off-by: Tom Brow --- .../BuzzPushNavigationTarget.swift | 37 +++++++- .../BuzzPushNotificationResolver.swift | 3 +- .../BuzzPushNavigationTargetTests.swift | 62 +++++++++++++ .../BuzzPushNotificationResolverTests.swift | 31 +++++++ mobile/ios/Runner/AppDelegate.swift | 4 +- mobile/lib/shared/push/push_bridge.dart | 6 +- .../channels/channel_detail_page_test.dart | 86 +++++++++++++++++++ mobile/test/shared/push/push_bridge_test.dart | 53 ++++++++++++ 8 files changed, 275 insertions(+), 7 deletions(-) diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNavigationTarget.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNavigationTarget.swift index 9218655021..c6cb813648 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNavigationTarget.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNavigationTarget.swift @@ -9,18 +9,43 @@ public struct BuzzPushNavigationTarget: Codable, Equatable, Sendable { public let communityID: String public let channelID: String - public init(eventID: String, communityID: String, channelID: String) { + /// Direct parent whose thread contains the target reply, matching Inbox. + public let threadRootID: String? + + public init( + eventID: String, communityID: String, channelID: String, threadRootID: String? = nil + ) { self.eventID = eventID self.communityID = communityID self.channelID = channelID + self.threadRootID = threadRootID } public var userInfoValue: [String: String] { - [ + var value = [ "event_id": eventID, "community_id": communityID, "channel_id": channelID, ] + value["thread_root_id"] = threadRootID + return value + } + + static func threadRootID(in tags: [[String]]) -> String? { + // Inbox opens broadcast replies in the channel and other replies in their + // direct parent's thread, even when the NIP-10 root names an outer thread. + if tags.contains(where: { $0.count >= 2 && $0[0] == "broadcast" && $0[1] == "1" }) { + return nil + } + // Match relay ingest: the last valid 64-character ASCII-hex reply ID wins. + // A later malformed marker must not hide an earlier valid parent. + return tags.last(where: { tag in + guard tag.count >= 4, tag[0] == "e", tag[3] == "reply" else { return false } + let bytes = tag[1].utf8 + return bytes.count == 64 && bytes.allSatisfy { + (48...57).contains($0) || (65...70).contains($0) || (97...102).contains($0) + } + })?[1] } /// Decodes a target without trusting other fields from the APNs payload. @@ -28,7 +53,7 @@ public struct BuzzPushNavigationTarget: Codable, Equatable, Sendable { from userInfo: [AnyHashable: Any] ) -> BuzzPushNavigationTarget? { guard let raw = userInfo[userInfoKey] as? [String: Any], - raw.count == 3, + Set(raw.keys).isSubset(of: ["event_id", "community_id", "channel_id", "thread_root_id"]), let eventID = raw["event_id"] as? String, let communityID = raw["community_id"] as? String, let channelID = raw["channel_id"] as? String, @@ -38,10 +63,14 @@ public struct BuzzPushNavigationTarget: Codable, Equatable, Sendable { else { return nil } + if let rawThread = raw["thread_root_id"] { + guard let thread = rawThread as? String, !thread.isEmpty else { return nil } + } return BuzzPushNavigationTarget( eventID: eventID, communityID: communityID, - channelID: channelID + channelID: channelID, + threadRootID: raw["thread_root_id"] as? String ) } diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift index 7836385a77..d195748021 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzPushNotificationResolver.swift @@ -462,7 +462,8 @@ public final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { BuzzPushNavigationTarget( eventID: event.id, communityID: community.id, - channelID: $0 + channelID: $0, + threadRootID: BuzzPushNavigationTarget.threadRootID(in: event.tags) ) }, senderPubkey: event.pubkey.lowercased(), diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNavigationTargetTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNavigationTargetTests.swift index 414e9446a2..c2bba98b7d 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNavigationTargetTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNavigationTargetTests.swift @@ -75,3 +75,65 @@ import Testing #expect(buffer.take() == first) #expect(buffer.take() == nil) } + +@Test func `Reply target preserves containing thread through user info and cold-start buffer`() { + let target = BuzzPushNavigationTarget( + eventID: "reply", communityID: "community", channelID: "channel", threadRootID: "parent" + ) + let decoded = BuzzPushNavigationTarget.decodeIfPresent( + from: [BuzzPushNavigationTarget.userInfoKey: target.userInfoValue] + ) + #expect(decoded == target) + let buffer = BuzzPushNavigationBuffer() + buffer.record(target) + #expect(buffer.take() == target) +} + +@Test func `Reject malformed optional thread and unknown navigation fields`() { + let cases: [[String: Any]] = [ + ["thread_root_id": ""], ["thread_root_id": 42], ["unknown": "value"], + ] + for extra in cases { + var raw: [String: Any] = [ + "event_id": "reply", "community_id": "community", "channel_id": "channel", + ] + raw.merge(extra) { _, new in new } + #expect(BuzzPushNavigationTarget.decodeIfPresent( + from: [BuzzPushNavigationTarget.userInfoKey: raw] + ) == nil) + } +} + +@Test func `Empty reply marker preserves channel navigation`() { + let thread = BuzzPushNavigationTarget.threadRootID(in: [["e", "", "", "reply"]]) + #expect(thread == nil) + let target = BuzzPushNavigationTarget( + eventID: "message", communityID: "community", channelID: "channel", threadRootID: thread + ) + #expect(BuzzPushNavigationTarget.decodeIfPresent( + from: [BuzzPushNavigationTarget.userInfoKey: target.userInfoValue] + ) == target) +} + +@Test func `Thread destination keeps direct parent and broadcast semantics`() { + let parent = String(repeating: "a", count: 64) + let tags = [["e", "outer-root", "", "root"], ["e", parent, "", "reply"]] + #expect(BuzzPushNavigationTarget.threadRootID(in: tags) == parent) + #expect(BuzzPushNavigationTarget.threadRootID(in: tags + [["broadcast", "1"]]) == nil) + #expect(BuzzPushNavigationTarget.threadRootID(in: [["e", "outer-root", "", "root"]]) == nil) +} + +@Test func `Last valid reply marker wins despite trailing malformed markers`() { + let first = String(repeating: "a", count: 64) + let last = String(repeating: "B", count: 64) + let invalidIDs = ["", "bad", String(repeating: "a", count: 63), + String(repeating: "a", count: 65), String(repeating: "g", count: 64), + String(repeating: "é", count: 64)] + for invalid in invalidIDs { + let invalidTag = ["e", invalid, "", "reply"] + #expect(BuzzPushNavigationTarget.threadRootID(in: [invalidTag]) == nil) + #expect(BuzzPushNavigationTarget.threadRootID(in: [ + ["e", first, "", "reply"], ["e", last, "", "reply"], invalidTag, + ]) == last) + } +} diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift index 6620f19369..8d8de632be 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift @@ -162,6 +162,37 @@ final class BuzzPushNotificationResolverTests: XCTestCase { } } + func testReplyNavigationUsesDirectParentAndKeepsExactEvent() throws { + let parent = String(repeating: "a", count: 64) + let reply = event(id: "reply-id", content: "Reply preview", tags: [ + ["h", Self.channelID], + ["e", "outer-root", "", "root"], + ["e", parent, "", "reply"], + ["e", "bad", "", "reply"], + ["e", "", "", "reply"], + ]) + let resolution = try XCTUnwrap(BuzzPushNotificationResolver.decodeResolution( + events: [reply], community: community() + )?.0) + let target = try XCTUnwrap(resolution.navigationTarget) + XCTAssertEqual(target.eventID, "reply-id") + XCTAssertEqual(target.userInfoValue["thread_root_id"], parent) + } + + func testNavigationKeepsTopLevelAndBroadcastMessagesInChannel() throws { + for tags in [ + [["h", Self.channelID]], + [["h", Self.channelID], ["e", "mention", "", "mention"]], + [["h", Self.channelID], ["e", "root", "", "root"]], + [["h", Self.channelID], ["e", "parent", "", "reply"], ["broadcast", "1"]], + ] { + let resolution = try XCTUnwrap(BuzzPushNotificationResolver.decodeResolution( + events: [event(content: "Preview", tags: tags)], community: community() + )?.0) + XCTAssertNil(resolution.navigationTarget?.threadRootID) + } + } + func testResolveSucceedsAndMutatesGatewayContent() throws { let event = try JSONDecoder().decode( VerifiedNostrEvent.self, diff --git a/mobile/ios/Runner/AppDelegate.swift b/mobile/ios/Runner/AppDelegate.swift index 4d1b95c386..ce7dc6e543 100644 --- a/mobile/ios/Runner/AppDelegate.swift +++ b/mobile/ios/Runner/AppDelegate.swift @@ -1074,10 +1074,12 @@ import os.log extension BuzzPushNavigationTarget { fileprivate var flutterArguments: [String: String] { - [ + var arguments = [ "eventId": eventID, "communityId": communityID, "channelId": channelID, ] + arguments["threadRootId"] = threadRootID + return arguments } } diff --git a/mobile/lib/shared/push/push_bridge.dart b/mobile/lib/shared/push/push_bridge.dart index 49d2c96cde..dc11f0e5f7 100644 --- a/mobile/lib/shared/push/push_bridge.dart +++ b/mobile/lib/shared/push/push_bridge.dart @@ -106,18 +106,22 @@ MessageDeepLink? _pushNotificationLink(Object? arguments) { final eventId = arguments['eventId']; final communityId = arguments['communityId']; final channelId = arguments['channelId']; + final threadRootId = arguments['threadRootId']; if (eventId is! String || eventId.isEmpty || communityId is! String || communityId.isEmpty || channelId is! String || - channelId.isEmpty) { + channelId.isEmpty || + (threadRootId != null && + (threadRootId is! String || threadRootId.isEmpty))) { return null; } return MessageDeepLink( communityId: communityId, channelId: channelId, messageId: eventId, + threadRootId: threadRootId as String?, ); } diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index f768564391..cf746acde5 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'package:buzz/shared/push/push_bridge.dart'; import 'package:buzz/features/channels/channel_actions_sheet.dart'; import '../profile/presence_snapshot_test.dart' @@ -11415,6 +11416,91 @@ void main() { }); }); + testWidgets('push tap highlights its exact reply after newer arrivals', ( + tester, + ) async { + pendingPushNotificationLink.value = null; + installBuzzPushMethodHandler(); + addTearDown(() { + pendingPushNotificationLink.value = null; + }); + const channel = MethodChannel('buzz/push'); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage( + channel.name, + channel.codec.encodeMethodCall( + const MethodCall('notificationOpened', { + 'eventId': 'target', + 'communityId': 'community', + 'channelId': _channelId, + 'threadRootId': 'parent', + }), + ), + (_) {}, + ); + final link = pendingPushNotificationLink.value!; + final root = _textMsg( + id: 'root', + pubkey: 'alice', + content: 'Outer root', + createdAt: 1000, + ); + final parent = _textMsg( + id: 'parent', + pubkey: 'alice', + content: 'Containing thread', + createdAt: 1100, + extraTags: const [ + ['e', 'root', '', 'reply'], + ], + ); + final target = _textMsg( + id: 'target', + pubkey: 'alice', + content: 'Notification reply', + createdAt: 1200, + extraTags: const [ + ['e', 'root', '', 'root'], + ['e', 'parent', '', 'reply'], + ], + ); + final newer = _textMsg( + id: 'newer', + pubkey: 'alice', + content: 'Newer reply', + createdAt: 1300, + extraTags: const [ + ['e', 'root', '', 'root'], + ['e', 'parent', '', 'reply'], + ], + ); + await tester.pumpWidget( + _buildTestable( + messages: [root, parent, target, newer], + initialMessageId: link.messageId, + initialThreadRootId: link.threadRootId, + threadReplies: { + 'root': [parent, target, newer], + }, + ), + ); + await tester.pumpAndSettle(); + final page = tester.widget( + find.byType(ThreadDetailPage), + ); + expect(page.threadHead.id, 'parent'); + expect(page.initialMessageId, 'target'); + final decoration = + tester + .widget( + find.byKey(const ValueKey('thread-message-target')), + ) + .decoration + as BoxDecoration; + expect(decoration.color!.a, closeTo(0.12, 0.001)); + expect(find.text('Newer reply'), findsOneWidget); + }); + testWidgets('opens a nested reply in its direct-parent thread', ( tester, ) async { diff --git a/mobile/test/shared/push/push_bridge_test.dart b/mobile/test/shared/push/push_bridge_test.dart index 1c76b9a69b..926cb48b5d 100644 --- a/mobile/test/shared/push/push_bridge_test.dart +++ b/mobile/test/shared/push/push_bridge_test.dart @@ -340,6 +340,57 @@ void main() { ); }); + test( + 'preserves the exact reply and containing thread on a warm tap', + () async { + final response = Completer(); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage( + _channel.name, + _channel.codec.encodeMethodCall( + const MethodCall('notificationOpened', { + 'eventId': 'older-reply', + 'communityId': 'community-id', + 'channelId': 'channel-id', + 'threadRootId': 'direct-parent', + }), + ), + response.complete, + ); + expect( + _channel.codec.decodeEnvelope((await response.future)!), + 'handled', + ); + expect( + pendingPushNotificationLink.value, + const MessageDeepLink( + communityId: 'community-id', + channelId: 'channel-id', + messageId: 'older-reply', + threadRootId: 'direct-parent', + ), + ); + }, + ); + + for (final invalidThread in ['', 42]) { + test('rejects malformed thread ID $invalidThread', () async { + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler( + _channel, + (_) async => { + 'eventId': 'reply', + 'communityId': 'community', + 'channelId': 'channel', + 'threadRootId': invalidThread, + }, + ); + await syncPendingBuzzPushNotificationResponse(); + expect(pendingPushNotificationLink.value, isNull); + }); + } + test('rejects a notification response with an empty channel ID', () async { final response = Completer(); await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger @@ -412,6 +463,7 @@ void main() { expect(call.method, 'takePendingNotificationResponse'); return { 'eventId': 'b' * 64, + 'threadRootId': 'a' * 64, 'communityId': 'community-id', 'channelId': '123e4567-e89b-42d3-a456-426614174000', }; @@ -425,6 +477,7 @@ void main() { communityId: 'community-id', channelId: '123e4567-e89b-42d3-a456-426614174000', messageId: 'b' * 64, + threadRootId: 'a' * 64, ), ); });