fix: stop one chat conversation from showing as two chat rooms

lib/features/chat/notifiers/chat_rooms_notifier.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 
 import 'package:flutter_riverpod/flutter_riverpod.dart';
 import 'package:mostro_mobile/data/models/chat_room.dart';
+import 'package:mostro_mobile/data/models/session.dart';
 
 import 'package:mostro_mobile/features/chat/providers/chat_room_providers.dart';
 import 'package:mostro_mobile/services/logger_service.dart';
@@ -52,19 +53,7 @@ class ChatRoomsNotifier extends StateNotifier<List<ChatRoom>> {
     final now = DateTime.now();
 
     try {
-      final chats = sessions
-          .where(
-        (s) =>
-            s.orderId != null &&
-            (s.peer != null ||
-                s.startTime.isAfter(now.subtract(const Duration(hours: 1)))),
-      )
-          .map((s) {
-        final chat = ref.read(chatRoomsProvider(s.orderId!));
-        return chat;
-      })
-          .where((chat) => chat.messages.isNotEmpty)
-          .toList();
+      final chats = _chatsForSessions(sessions, now);
 
       state = chats;
       logger.i("Loaded ${chats.length} chats with messages");
@@ -83,20 +72,7 @@ class ChatRoomsNotifier extends StateNotifier<List<ChatRoom>> {
     final now = DateTime.now();
 
     try {
-      final chats = sessions
-          .where(
-        (s) =>
-            s.orderId != null &&
-            (s.peer != null ||
-                s.startTime.isAfter(now.subtract(const Duration(hours: 1)))),
-      )
-          .map((s) {
-        // Force a fresh read of the chat state
-        final chat = ref.read(chatRoomsProvider(s.orderId!));
-        return chat;
-      })
-          .where((chat) => chat.messages.isNotEmpty)
-          .toList();
+      final chats = _chatsForSessions(sessions, now);
 
       // Skip the emission when nothing visible changed: this runs after
       // every incoming chat event and a fresh list rebuilds the whole
@@ -122,6 +98,55 @@ class ChatRoomsNotifier extends StateNotifier<List<ChatRoom>> {
     }
   }
 
+  /// Builds the visible chat rooms for [sessions], at most one row per
+  /// conversation.
+  ///
+  /// Two rows can otherwise describe a single conversation:
+  ///
+  /// - two sessions sharing an orderId resolve to the very same
+  ///   [chatRoomsProvider], rendering the identical room twice;
+  /// - two sessions sharing a trade key *and* a peer derive the identical
+  ///   ECDH shared key, so both accept the very same chat envelopes and each
+  ///   stores them under its own orderId. `KeyManager.getNextKeyIndex` used
+  ///   to hand out an already-reserved index, which produced exactly this.
+  ///
+  /// The key collision is fixed at the source, but sessions created before
+  /// the fix are still on disk, so collapse them here too and keep the newest.
+  List<ChatRoom> _chatsForSessions(List<Session> sessions, DateTime now) {
+    final cutoff = now.subtract(const Duration(hours: 1));
+    final ordered = [...sessions]
+      ..sort((a, b) => b.startTime.compareTo(a.startTime));
+    final seenOrderIds = <String>{};
+    final seenConversations = <String>{};
+    final chats = <ChatRoom>[];
+    for (final session in ordered) {
+      final orderId = session.orderId;
+      if (orderId == null) continue;
+      if (session.peer == null && !session.startTime.isAfter(cutoff)) continue;
+      if (!seenOrderIds.add(orderId)) continue;
+      // Resolve the room before claiming anything. Envelopes are stored once
+      // globally, under whichever orderId handled them first, and history is
+      // reloaded by that orderId — so after a restart only one of two
+      // colliding rooms holds the conversation. Claiming for an empty room
+      // would drop the one that has the messages and hide the chat entirely.
+      final chat = ref.read(chatRoomsProvider(orderId));
+      if (chat.messages.isEmpty) continue;
+      // Identifies the conversation itself: the chat envelope keys are derived
+      // from this shared secret, so an equal value means literally the same
+      // messages on both rows.
+      final conversationId = session.sharedKey?.public;
+      if (conversationId != null && !seenConversations.add(conversationId)) {
+        logger.w(
+          'Collapsing chat for order $orderId: it shares a conversation key '
+          'with another session (colliding trade keys).',
+        );
+        continue;
+      }
+      chats.add(chat);
+    }
+    return chats;
+  }
+
   void _refreshAllSubscriptions() {
     // No need to manually refresh subscriptions
     // SubscriptionManager now handles this automatically based on SessionNotifier changes

lib/features/key_manager/key_manager.dart

@@ -132,11 +132,20 @@ class KeyManager {
     return _storage.hasPersistedTradeKeyIndex();
   }
 
+  /// Reserve and return the next free trade key index, advancing the counter
+  /// past it.
+  ///
+  /// The stored counter is the index [deriveTradeKey] will hand out next, so
+  /// the reserved index must be that value — returning `currentIndex + 1`
+  /// while storing `currentIndex + 1` handed the very same index to the next
+  /// [deriveTradeKey] call. Two live sessions then shared a trade key, and
+  /// with a common counterparty also the ECDH shared key the chat envelope is
+  /// derived from, so one conversation surfaced as two chat rooms.
   Future<int> getNextKeyIndex() async {
     final currentIndex = await getCurrentKeyIndex();
     await setCurrentKeyIndex(currentIndex + 1);
 
-    return currentIndex + 1;
+    return currentIndex;
   }
 
   Future<void> setCurrentKeyIndex(int index) async {

lib/shared/notifiers/session_notifier.dart

@@ -146,13 +146,61 @@ class SessionNotifier extends StateNotifier<List<Session>> {
   }
 
   void _emitState() {
+    // A session is reachable through three maps and only `_sessions` is keyed
+    // by orderId, so the same order can otherwise surface twice (e.g. a
+    // request-id session whose orderId was assigned by Mostro alongside the
+    // persisted session the restore flow rebuilt for that same order). Every
+    // consumer derived from this state — most visibly the chat list, which
+    // renders one row per session — would then show the order twice.
+    // `_sessions` holds the persisted session, so it wins.
     final combined = <Session>[];
-    combined.addAll(_sessions.values);
-    combined.addAll(_requestIdToSession.values);
-    combined.addAll(_pendingChildSessions.values);
+    final claimedOrderIds = <String>{};
+    for (final session in [
+      ..._sessions.values,
+      ..._requestIdToSession.values,
+      ..._pendingChildSessions.values,
+    ]) {
+      final orderId = session.orderId;
+      if (orderId != null && !claimedOrderIds.add(orderId)) continue;
+      combined.add(session);
+    }
     state = combined;
   }
 
+  /// Drops every *other* in-memory session that carries [orderId] now that
+  /// [owner] is the session of record for it. Identity is not enough: the
+  /// restore flow rebuilds a brand new [Session] for an order that a pending
+  /// request-id or child session may already point at.
+  void _claimOrderId(String orderId, Session owner) {
+    bool isStale(Session session) =>
+        !identical(session, owner) && session.orderId == orderId;
+
+    void logEviction(Session session) {
+      // A differing trade key means the evicted session held key material no
+      // other map can resolve any more, so surface it rather than dropping it
+      // silently.
+      if (session.tradeKey.public != owner.tradeKey.public) {
+        logger.w(
+          'Evicting session for order $orderId with a different trade key '
+          '(${session.tradeKey.public}); it is no longer resolvable.',
+        );
+      } else {
+        logger.d('Evicted a duplicate in-memory session for order $orderId');
+      }
+    }
+
+    _requestIdToSession.removeWhere((_, session) {
+      if (!isStale(session)) return false;
+      logEviction(session);
+      return true;
+    });
+    _pendingChildSessions.removeWhere((_, session) {
+      if (!isStale(session)) return false;
+      logEviction(session);
+      return true;
+    });
+  }
+
   void _scheduleCleanup() {
     _cleanupTimer?.cancel();
     _cleanupTimer = Timer.periodic(
@@ -221,6 +269,7 @@ class SessionNotifier extends StateNotifier<List<Session>> {
 
     if (orderId != null) {
       _sessions[orderId] = session;
+      _claimOrderId(orderId, session);
     } else if (requestId != null) {
       _requestIdToSession[requestId] = session;
     }
@@ -233,6 +282,7 @@ class SessionNotifier extends StateNotifier<List<Session>> {
     _sessions[session.orderId!] = session;
     _requestIdToSession.removeWhere((_, value) => identical(value, session));
     _pendingChildSessions.remove(session.tradeKey.public);
+    _claimOrderId(session.orderId!, session);
     await _storage.putSession(session);
     _emitState();
 
@@ -250,6 +300,7 @@ class SessionNotifier extends StateNotifier<List<Session>> {
     if (orderId == null) return;
     _sessions[orderId] = session;
     _requestIdToSession.removeWhere((_, value) => identical(value, session));
+    _claimOrderId(orderId, session);
     _emitState();
   }
 
@@ -410,6 +461,7 @@ class SessionNotifier extends StateNotifier<List<Session>> {
 
     session.orderId = childOrderId;
     _sessions[childOrderId] = session;
+    _claimOrderId(childOrderId, session);
     await _storage.putSession(session);
     _emitState();
 

test/features/chat/chat_rooms_notifier_dedup_test.dart

@@ -0,0 +1,170 @@
+import 'package:dart_nostr/dart_nostr.dart';
+import 'package:flutter_riverpod/flutter_riverpod.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:mostro_mobile/data/models/chat_room.dart';
+import 'package:mostro_mobile/data/models/peer.dart';
+import 'package:mostro_mobile/data/models/session.dart';
+import 'package:mostro_mobile/features/chat/notifiers/chat_room_notifier.dart';
+import 'package:mostro_mobile/features/chat/providers/chat_room_providers.dart';
+import 'package:mostro_mobile/shared/notifiers/session_notifier.dart';
+import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart';
+import 'package:shared_preferences/shared_preferences.dart';
+
+import '../../mocks.mocks.dart';
+
+/// `KeyManager.getNextKeyIndex` used to hand out an already-reserved trade
+/// key index, so two sessions could share a trade key. With a common
+/// counterparty they also share the ECDH shared key the chat envelope keys
+/// are derived from, and both chat rooms then accept the very same messages.
+/// Sessions created before that fix are still on disk, so the list collapses
+/// them into a single row.
+void main() {
+  const olderOrderId = 'order-older';
+  const newerOrderId = 'order-newer';
+  // A real curve point: the shared key is a genuine ECDH computation.
+  final peerPubkey = NostrKeyPairs(
+    private:
+        '5566778899aabbccddeeff00112233445566778899aabbccddeeff0011223344',
+  ).public;
+  // Both sessions carry this trade key, which is exactly the collision.
+  const sharedTradeKey =
+      'abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890';
+
+  late ProviderContainer container;
+  late Map<String, List<NostrEvent>> messagesByOrderId;
+
+  NostrEvent message(String id) => NostrEvent(
+        id: id,
+        kind: 14,
+        content: 'hola',
+        sig: '',
+        pubkey: 'peer-pubkey',
+        createdAt: DateTime.fromMillisecondsSinceEpoch(1000),
+        tags: const [],
+      );
+
+  Session session({
+    required String orderId,
+    required DateTime startTime,
+    String tradeKeyPrivate = sharedTradeKey,
+  }) {
+    final s = Session(
+      masterKey: NostrKeyPairs(
+          private:
+              '1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'),
+      tradeKey: NostrKeyPairs(private: tradeKeyPrivate),
+      keyIndex: 0,
+      fullPrivacy: false,
+      startTime: startTime,
+      peer: Peer(publicKey: peerPubkey),
+    );
+    s.orderId = orderId;
+    return s;
+  }
+
+  void arrange(List<Session> sessions) {
+    container = ProviderContainer(overrides: [
+      sessionNotifierProvider.overrideWith((ref) {
+        final notifier = _FakeSessionNotifier(ref);
+        notifier.emit(sessions);
+        return notifier;
+      }),
+      chatRoomsProvider.overrideWith((ref, id) => ChatRoomNotifier(
+            ChatRoom(orderId: id, messages: messagesByOrderId[id] ?? <NostrEvent>[]),
+            id,
+            ref,
+          )),
+    ]);
+    addTearDown(container.dispose);
+  }
+
+  setUp(() {
+    SharedPreferences.setMockInitialValues({});
+    messagesByOrderId = {};
+  });
+
+  test('two sessions sharing a conversation key render a single row', () {
+    // Arrange: both rooms hold the conversation (as they do while the app is
+    // running and both notifiers accept the same live envelopes).
+    messagesByOrderId = {
+      olderOrderId: [message('m1')],
+      newerOrderId: [message('m1')],
+    };
+    arrange([
+      session(
+        orderId: olderOrderId,
+        startTime: DateTime.now().subtract(const Duration(minutes: 30)),
+      ),
+      session(orderId: newerOrderId, startTime: DateTime.now()),
+    ]);
+
+    // Act
+    final chats = container.read(chatRoomsNotifierProvider);
+
+    // Assert: the newest session wins, and the conversation is shown once.
+    expect(chats.map((c) => c.orderId), [newerOrderId]);
+  });
+
+  test(
+      'the conversation survives when only the older room holds the history',
+      () {
+    // Arrange: after a restart only one room reloads the history — envelopes
+    // are stored globally once, under whichever orderId first handled them
+    // (ChatRoomNotifier._onChatEvent), and _loadHistoricalMessages filters on
+    // that orderId. Here the newer session's room comes up empty.
+    messagesByOrderId = {
+      olderOrderId: [message('m1')],
+      newerOrderId: <NostrEvent>[],
+    };
+    arrange([
+      session(
+        orderId: olderOrderId,
+        startTime: DateTime.now().subtract(const Duration(minutes: 30)),
+      ),
+      session(orderId: newerOrderId, startTime: DateTime.now()),
+    ]);
+
+    // Act
+    final chats = container.read(chatRoomsNotifierProvider);
+
+    // Assert: claiming the conversation for the empty newer room would drop
+    // the older one too and make the chat vanish entirely.
+    expect(chats.map((c) => c.orderId), [olderOrderId]);
+  });
+
+  test('distinct conversations are both kept', () {
+    // Arrange: different trade keys, so different ECDH shared keys.
+    messagesByOrderId = {
+      olderOrderId: [message('m1')],
+      newerOrderId: [message('m2')],
+    };
+    arrange([
+      session(
+        orderId: olderOrderId,
+        startTime: DateTime.now().subtract(const Duration(minutes: 30)),
+      ),
+      session(
+        orderId: newerOrderId,
+        startTime: DateTime.now(),
+        tradeKeyPrivate:
+            '0fedcba9876543210fedcba9876543210fedcba9876543210fedcba987654321',
+      ),
+    ]);
+
+    // Act
+    final chats = container.read(chatRoomsNotifierProvider);
+
+    // Assert
+    expect(chats.map((c) => c.orderId), [newerOrderId, olderOrderId]);
+  });
+}
+
+/// Session list the provider can read without touching storage.
+class _FakeSessionNotifier extends SessionNotifier {
+  _FakeSessionNotifier(Ref ref)
+      : super(ref, MockSessionStorage(), MockSettings()) {
+    state = const [];
+  }
+
+  void emit(List<Session> sessions) => state = sessions;
+}