mixed conversation bug

integration/integration.cabal

@@ -182,6 +182,7 @@ library
     Test.Migration.DomainRegistration
     Test.Migration.TeamFeatures
     Test.Migration.Util
+    Test.MixedConversationReproducer
     Test.MLS
     Test.MLS.Clients
     Test.MLS.History

integration/test/Test/MixedConversationReproducer.hs

@@ -0,0 +1,164 @@
+{-# OPTIONS_GHC -Wno-ambiguous-fields #-}
+
+-- This file is part of the Wire Server implementation.
+--
+-- Copyright (C) 2026 Wire Swiss GmbH <opensource@wire.com>
+--
+-- This program is free software: you can redistribute it and/or modify it under
+-- the terms of the GNU Affero General Public License as published by the Free
+-- Software Foundation, either version 3 of the License, or (at your option) any
+-- later version.
+--
+-- This program is distributed in the hope that it will be useful, but WITHOUT
+-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+-- details.
+--
+-- You should have received a copy of the GNU Affero General Public License along
+-- with this program. If not, see <https://www.gnu.org/licenses/>.
+
+module Test.MixedConversationReproducer where
+
+import API.Brig (addClient)
+import API.Galley
+import Control.Lens ((.~), (^?!))
+import qualified Data.Aeson as A
+import qualified Data.ProtoLens as Proto
+import Data.ProtoLens.Labels ()
+import MLS.Util (createMLSClient)
+import Notifications
+import Numeric.Lens (hex)
+import qualified Proto.Otr as Proto
+import qualified Proto.Otr_Fields as Proto
+import SetupHelpers
+import Testlib.Prelude
+
+-- copied from Test.FeatureFlags.MlsMigration to keep this reproducer self-contained
+mlsEnableConfig :: Value
+mlsEnableConfig =
+  object
+    [ "protocolToggleUsers" .= ([] :: [String]),
+      "defaultProtocol" .= "mls",
+      "supportedProtocols" .= ["mls"],
+      "allowedCipherSuites" .= ([1] :: [Int]),
+      "defaultCipherSuite" .= A.Number 1
+    ]
+
+mlsEnable :: Value
+mlsEnable =
+  object
+    [ "status" .= "enabled",
+      "config" .= mlsEnableConfig
+    ]
+
+-- | Reproducer for bug-report.md: "Not receiving messages in mixed and
+-- migrated groups with MLS protocol if hosted on another backend".
+--
+-- Steps, following bug-report.md:
+--   1. Team A on backend A: user A (admin), user C (no MLS device)
+--   2. Team B on backend B: user B (admin)
+--   3. Connect A<->B, C<->B
+--   4. User B creates a *proteus* group with A and C
+--   5. Enable MLS for team A and team B
+--   6. Refresh/create clients for A and B (C intentionally stays without an
+--      MLS client, so the group cannot fully migrate to MLS)
+--   7. Start migration for team B (protocol -> mixed), don't finalise
+--   8. Start migration for team A too, don't finalise
+--   9. Send a message as B, assert A receives it
+--  10. Send a message as A, assert B receives it
+--
+-- Per bug-report.md's "Additional Notes", step 10 is where the asymmetry
+-- is expected to show: B stops receiving messages sent by A in this
+-- conversation, even though A keeps receiving messages sent by B.
+testFederatedMixedProtocolMessageDelivery :: (HasCallStack) => Domain -> App ()
+testFederatedMixedProtocolMessageDelivery secondDomain = do
+  -- Step 1: Team A on backend A with user A (admin) and user C
+  (userA, tidA, _) <- createTeam OwnDomain 1
+  userC <- randomUser OwnDomain def
+
+  -- Step 2: Team B on backend B with user B (admin)
+  (userB, tidB, _) <- createTeam secondDomain 1
+
+  -- Step 3: connect everyone
+  connectUsers [userA, userB, userC]
+
+  -- legacy proteus clients, used to send/receive proteus messages
+  clientA <- addClient userA def >>= getJSON 201 >>= objId
+  clientB <- addClient userB def >>= getJSON 201 >>= objId
+  clientC <- addClient userC def >>= getJSON 201 >>= objId
+
+  -- Step 4: User B creates a proteus group with A and C
+  convId <-
+    postConversation
+      userB
+      defProteus {qualifiedUsers = [userA, userC], team = Just tidB}
+      >>= getJSON 201
+      >>= objConvId
+
+  bindResponse (getConversation userB (convIdToQidObject convId)) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "protocol" `shouldMatch` "proteus"
+
+  -- Step 5: enable the MLS team feature for both teams
+  void $ setTeamFeatureConfig userA tidA "mls" mlsEnable >>= getJSON 200
+  void $ setTeamFeatureConfig userB tidB "mls" mlsEnable >>= getJSON 200
+
+  -- Step 6: refresh/create MLS clients for A and B; C stays without one so
+  -- the conversation cannot fully migrate to MLS and remains mixed.
+  void $ createMLSClient def userA
+  void $ createMLSClient def userB
+
+  -- Step 7: start migration for team B (creator/owner of the conversation)
+  bindResponse (putConversationProtocol userB convId "mixed") $ \resp ->
+    resp.status `shouldMatchInt` 200
+
+  -- Step 8: start migration for team A too, without finalising
+  bindResponse (putConversationProtocol userA convId "mixed") $ \resp ->
+    resp.status `shouldMatchOneOf` [Number 200, Number 204]
+
+  -- The conversation should now be mixed protocol from both sides
+  bindResponse (getConversation userA (convIdToQidObject convId)) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "protocol" `shouldMatch` "mixed"
+  bindResponse (getConversation userB (convIdToQidObject convId)) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "protocol" `shouldMatch` "mixed"
+
+  -- Step 9: B sends a message; A should receive it
+  -- userA and userC are both on OwnDomain, so they share one recipient entry.
+  withWebSocket userA $ \wsA -> do
+    msgFromB <-
+      mkProteusRecipients
+        userA
+        [(userA, [clientA]), (userC, [clientC])]
+        "message from B"
+    let protoMsgFromB =
+          Proto.defMessage @Proto.QualifiedNewOtrMessage
+            & #sender . Proto.client .~ (clientB ^?! hex)
+            & #recipients .~ [msgFromB]
+            & #ignoreAll .~ Proto.defMessage
+    bindResponse (postProteusMessage userB (convIdToQidObject convId) protoMsgFromB) $ \resp ->
+      resp.status `shouldMatchInt` 201
+
+    n <- awaitMatch isNewMessageNotif wsA
+    n %. "payload.0.qualified_conversation" `shouldMatch` convIdToQidObject convId
+
+  -- Step 10: A sends a message; B should receive it -- expected failure
+  -- point for the bug being reproduced.
+  -- userB is on secondDomain, userC is on OwnDomain -- different domains,
+  -- so each needs its own qualified recipient entry.
+  withWebSocket userB $ \wsB -> do
+    msgFromAToB <- mkProteusRecipients userB [(userB, [clientB])] "message from A"
+    msgFromAToC <- mkProteusRecipients userC [(userC, [clientC])] "message from A"
+    let protoMsgFromA =
+          Proto.defMessage @Proto.QualifiedNewOtrMessage
+            & #sender . Proto.client .~ (clientA ^?! hex)
+            & #recipients .~ [msgFromAToB, msgFromAToC]
+            & #ignoreAll .~ Proto.defMessage
+    bindResponse (postProteusMessage userA (convIdToQidObject convId) protoMsgFromA) $ \resp ->
+      resp.status `shouldMatchInt` 201
+
+    -- If the bug is present, this awaitMatch times out, proving the
+    -- asymmetric delivery failure described in bug-report.md.
+    n <- awaitMatch isNewMessageNotif wsB
+    n %. "payload.0.qualified_conversation" `shouldMatch` convIdToQidObject convId

libs/wire-api/src/Wire/API/Message.hs

@@ -56,6 +56,10 @@ module Wire.API.Message
     UserClients (..),
     ReportMissing (..),
     IgnoreMissing (..),
+
+    -- * Only exported for tests
+    parseMap,
+    parseMapMerge,
   )
 where
 
@@ -366,10 +370,10 @@ protolensOtrRecipientsToOtrRecipients entries =
   QualifiedOtrRecipients . QualifiedUserClientMap <$> protolensToQualifiedUCMap entries
   where
     protolensToQualifiedUCMap :: [Proto.Otr.QualifiedUserEntry] -> Either String (Map Domain (Map UserId (Map ClientId ByteString)))
-    protolensToQualifiedUCMap qualifiedEntries = parseMap (mkDomain . view Proto.Otr.domain) (protolensToUCMap . view Proto.Otr.entries) qualifiedEntries
+    protolensToQualifiedUCMap qualifiedEntries = parseMapMerge (mkDomain . view Proto.Otr.domain) (protolensToUCMap . view Proto.Otr.entries) qualifiedEntries
 
     protolensToUCMap :: [Proto.Otr.UserEntry] -> Either String (Map UserId (Map ClientId ByteString))
-    protolensToUCMap es = parseMap parseUserId parseClientMap es
+    protolensToUCMap es = parseMapMerge parseUserId parseClientMap es
 
     parseUserId :: Proto.Otr.UserEntry -> Either String UserId
     parseUserId =
@@ -413,6 +417,15 @@ qualifiedOtrRecipientsToProtolens (QualifiedOtrRecipients (QualifiedUserClientMa
 parseMap :: (Applicative f, Ord k) => (a -> f k) -> (a -> f v) -> [a] -> f (Map k v)
 parseMap keyParser valueParser xs = Map.fromList <$> traverse (\x -> (,) <$> keyParser x <*> valueParser x) xs
 
+-- | Like 'parseMap', but for values that are themselves maps: entries sharing
+-- a key are merged instead of the last one silently clobbering the rest. A
+-- well-formed request can legitimately contain multiple protobuf entries for
+-- the same key (e.g. two 'QualifiedUserEntry's for the same domain), and
+-- 'Map.fromList' (used by 'parseMap') would otherwise silently drop every
+-- recipient from all but the last entry for that key.
+parseMapMerge :: (Applicative f, Ord k, Ord k2) => (a -> f k) -> (a -> f (Map k2 v)) -> [a] -> f (Map k (Map k2 v))
+parseMapMerge keyParser valueParser xs = Map.fromListWith Map.union <$> traverse (\x -> (,) <$> keyParser x <*> valueParser x) xs
+
 --------------------------------------------------------------------------------
 -- Filter
 

libs/wire-api/test/unit/Test/Wire/API/Message.hs

@@ -0,0 +1,82 @@
+-- This file is part of the Wire Server implementation.
+--
+-- Copyright (C) 2026 Wire Swiss GmbH <opensource@wire.com>
+--
+-- This program is free software: you can redistribute it and/or modify it under
+-- the terms of the GNU Affero General Public License as published by the Free
+-- Software Foundation, either version 3 of the License, or (at your option) any
+-- later version.
+--
+-- This program is distributed in the hope that it will be useful, but WITHOUT
+-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+-- details.
+--
+-- You should have received a copy of the GNU Affero General Public License along
+-- with this program. If not, see <https://www.gnu.org/licenses/>.
+
+module Test.Wire.API.Message (tests) where
+
+import Data.Map qualified as Map
+import Data.Set qualified as Set
+import Imports
+import Test.Tasty
+import Test.Tasty.HUnit
+import Wire.API.Message (parseMap, parseMapMerge)
+
+tests :: TestTree
+tests =
+  testGroup
+    "Message"
+    [ testParseMapDropsRecipientsOnDuplicateDomain,
+      testParseMapMergeKeepsRecipientsOnDuplicateDomain
+    ]
+
+-- | Simulates the shape of a real OTR request: a list of qualified entries,
+-- where each entry maps a "domain" to a set of "users" it targets. Nothing in
+-- the wire protocol forbids two entries for the same domain -- e.g. a client
+-- assembling a mixed-protocol proteus fallback message may independently emit
+-- an entry for "recipients not yet migrated to MLS" and another for
+-- "recipients needing legacy delivery for some other reason", both of which
+-- can legitimately target the same domain.
+sameDomainEntries :: [(String, Map String (Set Int))]
+sameDomainEntries =
+  [ ("a.example.com", Map.fromList [("alice", Set.fromList [1])]),
+    ("a.example.com", Map.fromList [("bob", Set.fromList [2])]),
+    ("b.example.com", Map.fromList [("carl", Set.fromList [3])])
+  ]
+
+-- | This is the bug: 'parseMap' is built on 'Map.fromList', which is
+-- last-write-wins on duplicate keys. The second "a.example.com" entry
+-- (bob) silently replaces the first (alice) instead of being combined with
+-- it -- alice's recipients vanish with no error, no missing-clients report,
+-- nothing. This is exactly how messages went missing for real users in
+-- mixed-protocol conversations (see bug-report.md).
+testParseMapDropsRecipientsOnDuplicateDomain :: TestTree
+testParseMapDropsRecipientsOnDuplicateDomain =
+  testCase "parseMap silently drops earlier entries for a repeated domain" $ do
+    let result :: Either String (Map String (Map String (Set Int)))
+        result = parseMap (Right . fst) (Right . snd) sameDomainEntries
+    result
+      @?= Right
+        ( Map.fromList
+            [ ("a.example.com", Map.fromList [("bob", Set.fromList [2])]), -- alice is gone!
+              ("b.example.com", Map.fromList [("carl", Set.fromList [3])])
+            ]
+        )
+
+-- | 'parseMapMerge' is the fix: it uses 'Map.fromListWith Map.union', so
+-- entries for a repeated domain are merged instead of one clobbering the
+-- other. Both alice and bob end up reachable under "a.example.com".
+testParseMapMergeKeepsRecipientsOnDuplicateDomain :: TestTree
+testParseMapMergeKeepsRecipientsOnDuplicateDomain =
+  testCase "parseMapMerge merges entries for a repeated domain" $ do
+    let result :: Either String (Map String (Map String (Set Int)))
+        result = parseMapMerge (Right . fst) (Right . snd) sameDomainEntries
+    result
+      @?= Right
+        ( Map.fromList
+            [ ("a.example.com", Map.fromList [("alice", Set.fromList [1]), ("bob", Set.fromList [2])]),
+              ("b.example.com", Map.fromList [("carl", Set.fromList [3])])
+            ]
+        )

libs/wire-api/test/unit/Test/Wire/API/Run.hs

@@ -24,6 +24,7 @@ import Test.Wire.API.Call.Config qualified as Call.Config
 import Test.Wire.API.Conversation qualified as Conversation
 import Test.Wire.API.MLS qualified as MLS
 import Test.Wire.API.MLS.Group qualified as Group
+import Test.Wire.API.Message qualified as Message
 import Test.Wire.API.OAuth qualified as OAuth
 import Test.Wire.API.RawJson qualified as RawJson
 import Test.Wire.API.Roundtrip.Aeson qualified as Roundtrip.Aeson
@@ -63,6 +64,7 @@ main =
         Roundtrip.CSV.tests,
         Routes.tests,
         Conversation.tests,
+        Message.tests,
         MLS.tests,
         Group.tests,
         Routes.Version.tests,

libs/wire-api/wire-api.cabal

@@ -713,6 +713,7 @@ test-suite wire-api-tests
     Paths_wire_api
     Test.Wire.API.Call.Config
     Test.Wire.API.Conversation
+    Test.Wire.API.Message
     Test.Wire.API.MLS
     Test.Wire.API.MLS.Group
     Test.Wire.API.OAuth