WPB-21964: move conversation creation to wire-subsystems by blackheaven · Pull Request #4977 · wireapp/wire-server

blackheaven · 2026-01-22T17:01:02Z

https://wearezeta.atlassian.net/browse/WPB-21964

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

Copilot

Pull request overview

This PR refactors conversation creation logic by moving it from Galley to the wire-subsystems library, aligning with the architectural goal of separating concerns into reusable subsystems.

Changes:

Moved conversation creation logic from Galley.API.Create to Wire.ConversationSubsystem.Interpreter
Relocated utility modules: Galley.API.Error → Galley.Types.Error, Galley.API.One2One → Wire.ConversationSubsystem.One2One, Galley.API.Util → Wire.ConversationSubsystem.Util, Galley.Effects.ClientStore → Wire.Effects.ClientStore
Removed Galley.Validation module (functionality moved to interpreter)
Updated background-worker to include new dependencies and error handlers
Updated imports across numerous files to reflect module movements

Reviewed changes

Copilot reviewed 63 out of 64 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
services/galley/src/Galley/API/Create.hs	Simplified to delegate conversation creation to subsystem
libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs	New implementation containing conversation creation logic
libs/wire-subsystems/src/Wire/ConversationSubsystem/Util.hs	Utility functions moved from Galley
libs/wire-subsystems/src/Wire/ConversationSubsystem/Federation.hs	New module for federation-related conversation logic
libs/wire-subsystems/src/Wire/ConversationSubsystem/Types.hs	Configuration types for conversation subsystem
services/background-worker/src/Wire/BackgroundWorker/Jobs/Registry.hs	Updated to support new conversation subsystem dependencies
libs/galley-types/src/Galley/Types/Error.hs	Error types moved to galley-types library
services/galley/src/Galley/Effects.hs	Reordered effect stack and added new error types
Multiple import updates	Updated imports across ~30 files to reflect module movements

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs

services/background-worker/src/Wire/BackgroundWorker/Jobs/Registry.hs

libs/wire-api/src/Wire/API/Routes/Public/Brig.hs

services/brig/src/Brig/IO/Intra.hs

services/background-worker/src/Wire/BackgroundWorker/Jobs/Registry.hs

services/galley/src/Galley/API/Mapping.hs

services/background-worker/src/Wire/BackgroundWorker/Jobs/Registry.hs

battermann

Great effort! Thanks!. I feel there is more to do and to be said to get this really clean, so here are a few comments already.

charts/background-worker/templates/configmap.yaml

libs/wire-subsystems/src/Wire/ConversationSubsystem/Federation.hs

libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs

libs/wire-subsystems/src/Wire/ConversationSubsystem/Types.hs

battermann · 2026-01-29T07:50:21Z

services/background-worker/src/Wire/BackgroundWorker/Jobs/Registry.hs

+        . mapError (const ("Invalid input" :: Text) :: InvalidInput -> Text)
        . mapError @MigrationError (T.pack . show)
+        . mapError @InternalError (TL.toStrict . internalErrorDescription)
+        . mapError @UnreachableBackends (T.pack . show)
+        . mapError @NonFederatingBackends (T.pack . show)
+        . mapError @TeamCollaboratorsError (const ("Team collaborators error" :: Text))
+        . mapError @TeamFeatureStoreError (const ("Team feature store error" :: Text))
+        . mapError @(Tagged OperationDenied ()) (const ("Operation denied" :: Text))
+        . mapError @(Tagged 'NotATeamMember ()) (const ("Not a team member" :: Text))
+        . mapError @(Tagged 'ConvAccessDenied ()) (const ("Conversation access denied" :: Text))
+        . mapError @(Tagged 'NotConnected ()) (const ("Not connected" :: Text))
+        . mapError @(Tagged 'MLSNotEnabled ()) (const ("MLS not enabled" :: Text))
+        . mapError @(Tagged 'MLSNonEmptyMemberList ()) (const ("MLS non-empty member list" :: Text))
+        . mapError @(Tagged 'MissingLegalholdConsent ()) (const ("Missing legalhold consent" :: Text))
+        . mapError @(Tagged 'NonBindingTeam ()) (const ("Non-binding team" :: Text))
+        . mapError @(Tagged 'NoBindingTeamMembers ()) (const ("No binding team members" :: Text))
+        . mapError @(Tagged 'TeamNotFound ()) (const ("Team not found" :: Text))
+        . mapError @(Tagged 'InvalidOperation ()) (const ("Invalid operation" :: Text))
+        . mapError @(Tagged 'ConvNotFound ()) (const ("Conversation not found" :: Text))
+        . mapError @(Tagged 'ChannelsNotEnabled ()) (const ("Channels not enabled" :: Text))
+        . mapError @(Tagged 'NotAnMlsConversation ()) (const ("Not an MLS conversation" :: Text))


This seems verbose and like some duplication. Don't we have instances for these somewhere. Can we reuse them? Or can this be extracted into a singel mapping function?

in galley they are converted to http errors, should we somehow group them?

battermann · 2026-01-29T08:00:22Z

services/galley/src/Galley/API/Create.hs

+  let remoteDomains = void <$> snd (partitionQualified lusr $ newConv.newConvQualifiedUsers)
+  enforceFederationProtocol (baseProtocolToProtocol newConv.newConvProtocol) remoteDomains
+  checkFederationStatus (RemoteDomains $ Set.fromList remoteDomains)


I wonder if checking the federation status should be implemented in a Federation subsystem, and if the conversation subsystem interpreter should be using it instead of having this here? Should that be part of this PR, too?

Does the new design answers your questions?

I like the FederationSubsystem, however here we still have code that integrates conversation and federation subsystems, my understanding is that we want to remove this responsibility from the services (galley in this case).

I strongly agree, but there are places we use it directly:

services/galley/src/Galley/API/Internal.hs:188:getFederationStatus

services/galley/src/Galley/API/Action.hs:585:enforceFederationProtocol

services/galley/src/Galley/API/Action.hs:607:checkFederationStatus

What should I do then?

libs/wire-subsystems/src/Wire/ClientStore.hs

services/galley/src/Galley/API/Create.hs

…alley.API.Action`, `Galley.API.One2One`, `Galley.API.Util`, partially `Galley.API.Mappings`, partially `Galley.API.Create` to `wire-subsystems`

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 79 out of 80 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

services/background-worker/test/Test/Wire/Util.hs

services/background-worker/src/Wire/BackgroundWorker/Jobs/Registry.hs

Copilot · 2026-02-04T13:52:53Z

integration/test/Test/FeatureFlags.hs

+testInternalGetConfiguredFeatureFlags :: (HasCallStack) => App ()
+testInternalGetConfiguredFeatureFlags = do
+  response <- Internal.getConfiguredFeatureFlags
+  response.status `shouldMatchInt` 200


Missing test implementation: The test case testInternalGetConfiguredFeatureFlags is added but there's no verification of the response content beyond the status code. Consider adding assertions to verify the structure and content of the returned feature flags to ensure the endpoint returns valid data.

libs/wire-subsystems/src/Wire/ConversationSubsystem/Types.hs

libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs

libs/wire-subsystems/src/Wire/GalleyAPIAccess/Rpc.hs

services/galley/src/Galley/API/Internal.hs

libs/galley-types/src/Galley/Types/Teams.hs

libs/wire-subsystems/src/Wire/FederationSubsystem/Interpreter.hs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs

battermann · 2026-02-04T14:15:34Z

libs/wire-api/src/Wire/API/Conversation/Config.hs

+import Data.OpenApi (NamedSchema (..), ToSchema (..))
+import Imports
+
+newtype ConfiguredConversationSubsystem = ConfiguredConversationSubsystem Value


I am confused about ConfiguredConversationSubsystem vs ConversationSubsystemConfig?

That's one of the design issue I had: ConversationSubsystemConfig (there is also another type), live in wire-subsystems, should I move it in wire-api?

battermann · 2026-02-04T14:38:13Z

services/galley/src/Galley/API/Create.hs

+  let remoteDomains = void <$> snd (partitionQualified lusr $ newConv.newConvQualifiedUsers)
+  enforceFederationProtocol (baseProtocolToProtocol newConv.newConvProtocol) remoteDomains
+  checkFederationStatus (RemoteDomains $ Set.fromList remoteDomains)


I like the FederationSubsystem, however here we still have code that integrates conversation and federation subsystems, my understanding is that we want to remove this responsibility from the services (galley in this case).

charts/background-worker/templates/configmap.yaml

charts/background-worker/values.yaml

Co-authored-by: Sven Tennie <sven.tennie@wire.com>

blackheaven requested review from a team as code owners January 22, 2026 17:01

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 22, 2026

blackheaven force-pushed the gdifolco/WPB-21964-extract-create-conversations-wire-subsystems branch 5 times, most recently from 1e8e214 to e7b9141 Compare January 28, 2026 11:14

battermann requested a review from Copilot January 29, 2026 07:16

Copilot started reviewing on behalf of battermann January 29, 2026 07:17 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

battermann reviewed Jan 29, 2026

View reviewed changes

blackheaven requested a review from battermann January 29, 2026 16:45

blackheaven and others added 14 commits January 29, 2026 19:49

WPB-21964: move conversation creation to wire-subsystems

03e752d

refactor: move Galley.Types.Clients to galley-types, partially `G…

62cbfd3

…alley.API.Action`, `Galley.API.One2One`, `Galley.API.Util`, partially `Galley.API.Mappings`, partially `Galley.API.Create` to `wire-subsystems`

fix: try to fix incorrect notifications (3)

43c4b97

refactor: integrate creation operations in the effect

3d78372

refactor: try to reduce PR size

154998e

fix: notifications

8f9466a

fix: last extra event

622d157

fix: Cells/Events/Conversations regressions

e721f32

fix: 201 vs 200 on creation

73520b7

Apply suggestions from code review

81a828a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: dupplication

9f51182

fix: minor Leif/Copilot feedbacks

5f8e9b9

refactor: Leif feedbacks (FederationSubsystem introduction)

eb6c61c

refactor: Leif feedbacks (mapError legacy)

4614d96

blackheaven force-pushed the gdifolco/WPB-21964-extract-create-conversations-wire-subsystems branch from 9402379 to 4614d96 Compare January 29, 2026 18:52

fix: rebase

2171607

blackheaven added 4 commits January 29, 2026 21:25

fix: background-worker.integration.yaml

fbc3b4a

Hello CI

a08c8cf

refactor: expose all default features in galley

2e3359f

Hello CI

c7b2a42

blackheaven force-pushed the gdifolco/WPB-21964-extract-create-conversations-wire-subsystems branch 2 times, most recently from 7b92819 to c7b2a42 Compare February 3, 2026 21:05

fix: hard coded config in background-worker Registry

62ec8d3

battermann requested a review from Copilot February 4, 2026 13:46

Copilot started reviewing on behalf of battermann February 4, 2026 13:47 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

Update libs/galley-types/src/Galley/Types/Teams.hs

e46740c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

battermann reviewed Feb 4, 2026

View reviewed changes

blackheaven added 2 commits February 4, 2026 18:01

fix: copilot feedbacks

ca807d5

fix: Leif feedbacks

cc72217

blackheaven requested a review from battermann February 4, 2026 17:41

supersven reviewed Feb 5, 2026

View reviewed changes

charts/background-worker/templates/configmap.yaml Show resolved Hide resolved

supersven reviewed Feb 5, 2026

View reviewed changes

charts/background-worker/values.yaml Outdated Show resolved Hide resolved

blackheaven and others added 2 commits February 5, 2026 19:40

Update charts/background-worker/values.yaml

b0c2180

Co-authored-by: Sven Tennie <sven.tennie@wire.com>

Update charts/background-worker/templates/configmap.yaml

bba99aa

Co-authored-by: Sven Tennie <sven.tennie@wire.com>

Conversation

blackheaven commented Jan 22, 2026

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

battermann left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

battermann left a comment •

edited

Loading