WPB-20806 Require history client when history sharing is enabled#5217
Conversation
bf9edb9 to
030a0a5
Compare
030a0a5 to
ac6f8e3
Compare
d7ffc22 to
df9b0b6
Compare
185a808 to
d3363a2
Compare
12d8e74 to
0e80c06
Compare
There was a problem hiding this comment.
Pull request overview
Enforces invariants for MLS conversation history clients: when history sharing is enabled a group must contain exactly one history client, and when disabled it must contain none. The PR introduces a new GroupMember sum type (RegularClient / HistoryClient) used throughout MLS credential parsing, validation, index maps, and proposal processing; persists history clients in a new mls_history_client table (Cassandra V103 migration and Postgres migration); adds new errors MLSHistoryClientConflict / MLSHistoryClientDuplication; and adds integration test coverage for the new invariants and the conversation migration flow.
Changes:
- Introduce
GroupMember(regular vs history client) and propagate it through key-package validation, index map, proposal actions, and group-info checks. - Add
mls_history_clientstorage (Cassandra + Postgres) with newConversationStoreeffects (AddHistoryClient,RemoveHistoryClient,RemoveAllHistoryClients,LookupHistoryClients) and migration support. - Enforce history-client invariants on MLS commits and messages, returning new
MLSHistoryClientConflict/MLSHistoryClientDuplicationerrors, with corresponding integration tests.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| services/galley/src/Galley/Schema/V103_HistoryClient.hs | New Cassandra V103 migration creating the mls_history_client table. |
| services/galley/src/Galley/Schema/Run.hs | Register V103 migration. |
| services/galley/galley.cabal | Expose new schema module. |
| services/brig/src/Brig/API/MLS/KeyPackages/Validation.hs | Wrap identity in RegularClient for key-package validation. |
| postgres-schema.sql | Schema dump for mls_history_client (plus unrelated conversation_parent_conv_idx). |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Proposal.hs | Extend ProposalAction with history-client add/remove; relax checkExternalProposalUser for history clients. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs | Enforce history-client invariants on commits and messages. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/GroupInfoCheck.hs | Use GroupMember in group-info mismatch diagnostics. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Commit/InternalCommit.hs | Apply history client add/remove during commit processing. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Commit/ExternalCommit.hs | Wrap sender in RegularClient when adding to index map. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Commit/Core.hs | Adapt creator action and leaf-node validation to GroupMember. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/Errors.hs | Add new conversation subsystem errors and mappers. |
| libs/wire-subsystems/src/Wire/ConversationSubsystem/Action.hs | Also remove history clients on conversation deletion. |
| libs/wire-subsystems/src/Wire/ConversationStore.hs | New effect constructors for history-client storage. |
| libs/wire-subsystems/src/Wire/ConversationStore/Postgres.hs | Postgres implementations of history-client ops; combined index map from regular+history. |
| libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs | IndexMap now holds GroupMember; helper mkGroupMember / mkIndexMapFromParts. |
| libs/wire-subsystems/src/Wire/ConversationStore/Migration/Types.hs | Carry history clients in migration data. |
| libs/wire-subsystems/src/Wire/ConversationStore/Migration/Cleanup.hs | Fetch history clients when collecting migration data. |
| libs/wire-subsystems/src/Wire/ConversationStore/Migration.hs | Persist history clients to Postgres during migration; add history_depth to conversation insert. |
| libs/wire-subsystems/src/Wire/ConversationStore/Cassandra/Queries.hs | Cassandra CQL for history client table. |
| libs/wire-subsystems/src/Wire/ConversationStore/Cassandra.hs | Cassandra implementations and hybrid interpreter for new ops. |
| libs/wire-subsystems/postgres-migrations/20260507162106-history_client.sql | Postgres migration for mls_history_client. |
| libs/wire-api/src/Wire/API/MLS/Credential.hs | Define GroupMember, parse history-client: prefix, prisms, schema. |
| libs/wire-api/src/Wire/API/MLS/Validation.hs | Switch validation signatures to GroupMember. |
| libs/wire-api/src/Wire/API/MLS/KeyPackage.hs | credentialIdentityAndKey/keyPackageIdentity return GroupMember. |
| libs/wire-api/src/Wire/API/Error/Galley.hs | New MLSHistoryClient* errors and updated group-info diagnostics. |
| libs/wire-api/test/unit/Test/Wire/API/MLS.hs | Adjust test expectation to RegularClient. |
| libs/types-common/src/Data/Id.hs | Add HistoryClientId id tag. |
| integration/test/Testlib/Types.hs | Test GroupMember ADT and history-client state map. |
| integration/test/Testlib/Env.hs | Initialize historyClientState. |
| integration/test/MLS/Util.hs | Support history clients in mlscli, add commit/remove helpers, parse history-client leaves. |
| integration/test/Test/MLS/History.hs | New tests for history-client invariants. |
| integration/test/Test/MLS.hs | Pass Nothing for the new history-client kp parameter. |
| integration/test/Test/Migration/Conversation.hs | Cover migration with a shared-history channel. |
| cassandra-schema.cql | Cassandra schema dump for new table. |
| changelog.d/2-features/WPB-20806 | Changelog entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| @@ -0,0 +1 @@ | |||
| Enforced history client invariants for conversation history sharing: enabled requires exactly one history client, disabled requires none. | |||
There was a problem hiding this comment.
since i don't have any context yet, this makes me wonder how you transition from enabled to disabled or back without transitioning through an invalid state with the wrong number of history clients?
There was a problem hiding this comment.
When history sharing state changes, commit messages and application messages will be rejected. Clients then add/remove the history client and once in sync the conversation is usable again.
| import Galley.Schema.V100_OutOfSync qualified as V100_OutOfSync | ||
| import Galley.Schema.V101_ConversationLowerGCGracePeriod qualified as V101_ConversationLowerGCGracePeriod | ||
| import Galley.Schema.V102_ConversationHistory qualified as V102_ConversationHistory | ||
| import Galley.Schema.V103_HistoryClient qualified as V103_HistoryClient |
There was a problem hiding this comment.
Why both cassandra and postgres?
Checklist
changelog.d