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,50 @@ 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;
+      // 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;
+      }
+      final chat = ref.read(chatRoomsProvider(orderId));
+      if (chat.messages.isEmpty) 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/key_manager/key_manager_cache_test.dart

@@ -71,4 +71,24 @@ void main() {
     expect(derivator.privateToPublicKey(cached),
         derivator.privateToPublicKey(direct));
   });
+
+  test('getNextKeyIndex reserves an index that deriveTradeKey cannot reuse',
+      () async {
+    // Arrange: the counter points at the next index to hand out.
+    await manager.setCurrentKeyIndex(5);
+
+    // Act: reserve an index for a range order's child session, then derive
+    // the trade key for the next order the user creates or takes.
+    final reserved = await manager.getNextKeyIndex();
+    final reservedKey = await manager.deriveTradeKeyFromIndex(reserved);
+    final nextKey = await manager.deriveTradeKey();
+
+    // Assert: reusing the reserved index would give two live sessions the
+    // same trade key, and with a shared counterparty the same ECDH shared
+    // key — i.e. one conversation surfacing as two chats.
+    expect(reserved, 5, reason: 'reserves the index deriveTradeKey was on');
+    expect(reservedKey.public, isNot(nextKey.public));
+    expect(await manager.getCurrentKeyIndex(), 7,
+        reason: 'both handed-out indices are consumed, leaving no gap');
+  });
 }

test/notifiers/session_notifier_test.dart

@@ -4,6 +4,7 @@ import 'package:mockito/mockito.dart';
 import 'package:mostro_mobile/data/models/enums/role.dart';
 import 'package:mostro_mobile/features/key_manager/key_manager.dart';
 import 'package:mostro_mobile/features/key_manager/key_manager_provider.dart';
+import 'package:mostro_mobile/data/models/session.dart';
 import 'package:mostro_mobile/features/settings/settings.dart';
 import 'package:mostro_mobile/shared/notifiers/session_notifier.dart';
 
@@ -25,6 +26,10 @@ void main() {
     private:
         'abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890',
   );
+  final derivedTradeKey = NostrKeyPairs(
+    private:
+        '0fedcba9876543210fedcba9876543210fedcba9876543210fedcba987654321',
+  );
 
   setUpAll(() {
     provideDummy<KeyManager>(MockKeyManager());
@@ -40,6 +45,9 @@ void main() {
     when(mockKeyManager.masterKeyPair).thenReturn(masterKey);
     when(mockPushService.registerToken(any)).thenAnswer((_) async => true);
     when(mockStorage.putSession(any)).thenAnswer((_) async {});
+    when(mockKeyManager.getCurrentKeyIndex()).thenAnswer((_) async => 1);
+    when(mockKeyManager.deriveTradeKey())
+        .thenAnswer((_) async => derivedTradeKey);
 
     notifier = SessionNotifier(
       mockRef,
@@ -131,4 +139,78 @@ void main() {
       verifyNever(mockPushService.registerToken(any));
     });
   });
+
+  group('duplicate order sessions', () {
+    Session buildSession(String orderId, NostrKeyPairs tradeKey) => Session(
+          masterKey: masterKey,
+          tradeKey: tradeKey,
+          keyIndex: 7,
+          fullPrivacy: false,
+          startTime: DateTime.now(),
+          orderId: orderId,
+          role: Role.seller,
+        );
+
+    test(
+        'saveSession drops a stale request session that already carries the '
+        'same orderId', () async {
+      // Arrange: a pending create-order session (keyed by requestId) that has
+      // already been assigned its orderId by Mostro's newOrder response.
+      final pending = await notifier.newSession(requestId: 42, role: Role.seller);
+      pending.orderId = 'order-1';
+
+      // Act: a *different* Session object for the same order is persisted,
+      // as the restore flow does (it rebuilds sessions from scratch).
+      await notifier.saveSession(buildSession('order-1', childTradeKey));
+
+      // Assert: the order appears exactly once in the emitted state.
+      expect(
+        notifier.state.where((s) => s.orderId == 'order-1').length,
+        1,
+      );
+    });
+
+    test(
+        'linkChildSessionToOrderId drops a stale session that already carries '
+        'the same orderId', () async {
+      // Arrange: an order already known by requestId that resolved to
+      // 'child-order-id', plus a pending child session for the same order.
+      final pending = await notifier.newSession(requestId: 7, role: Role.seller);
+      pending.orderId = 'child-order-id';
+      await notifier.createChildOrderSession(
+        tradeKey: childTradeKey,
+        keyIndex: 5,
+        parentOrderId: 'parent-order-id',
+        role: Role.seller,
+      );
+
+      // Act
+      await notifier.linkChildSessionToOrderId(
+        'child-order-id',
+        childTradeKey.public,
+      );
+
+      // Assert
+      expect(
+        notifier.state.where((s) => s.orderId == 'child-order-id').length,
+        1,
+      );
+    });
+
+    test('registerSessionInMemory never emits the same orderId twice',
+        () async {
+      // Arrange
+      final pending = await notifier.newSession(requestId: 9, role: Role.seller);
+      pending.orderId = 'order-2';
+
+      // Act
+      notifier.registerSessionInMemory(buildSession('order-2', childTradeKey));
+
+      // Assert
+      expect(
+        notifier.state.where((s) => s.orderId == 'order-2').length,
+        1,
+      );
+    });
+  });
 }