-
Notifications
You must be signed in to change notification settings - Fork 1
Multi-Matrix-server support (Phase 1): registry, namespaced KV, v3 migration #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| { | ||
| "default_server_config": { | ||
| "m.homeserver": { | ||
| "base_url": "http://localhost:8888", | ||
| "server_name": "localhost" | ||
| } | ||
| }, | ||
| "disable_custom_urls": false, | ||
| "disable_guests": false, | ||
| "brand": "Element (dev)", | ||
| "default_country_code": "US", | ||
| "show_labs_settings": true | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,6 @@ import ( | |
| type Configuration interface { | ||
| GetMatrixServerURL() string | ||
| GetMatrixServerName() string | ||
| GetMatrixUsernamePrefixForServer(serverURL string) string | ||
| } | ||
|
|
||
| // MigrationResult holds the results of a migration operation | ||
|
|
@@ -54,6 +53,9 @@ type PluginAccessor interface { | |
| // Shared channel access | ||
| GetRemoteID() string | ||
|
|
||
| // Server registry access | ||
| GetServerID() string | ||
|
|
||
| // Migration access | ||
| RunKVStoreMigrations() error | ||
| RunKVStoreMigrationsWithResults() (*MigrationResult, error) | ||
|
|
@@ -388,8 +390,12 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, roomIdentifier stri | |
| } | ||
|
|
||
| // Save both directions of the mapping | ||
| serverID := c.plugin.GetServerID() | ||
| mappingKey := kvstore.BuildChannelMappingKey(args.ChannelId) | ||
| err := c.kvstore.Set(mappingKey, []byte(roomIdentifier)) | ||
| mappingValue, err := kvstore.BuildSingleChannelMapping(serverID, roomIdentifier) | ||
| if err == nil { | ||
| err = c.kvstore.Set(mappingKey, mappingValue) | ||
| } | ||
| if err != nil { | ||
| c.client.Log.Error("Failed to save channel mapping", "error", err, "channel_id", args.ChannelId, "room_identifier", roomIdentifier) | ||
| return &model.CommandResponse{ | ||
|
|
@@ -398,8 +404,8 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, roomIdentifier stri | |
| } | ||
| } | ||
|
|
||
| // Store reverse mapping: room_mapping_<roomIdentifier> -> channelID | ||
| roomMappingKey := kvstore.BuildRoomMappingKey(roomIdentifier) | ||
| // Store reverse mapping: room_mapping_<serverID>_<roomIdentifier> -> channelID | ||
| roomMappingKey := kvstore.BuildRoomMappingKey(serverID, roomIdentifier) | ||
| err = c.kvstore.Set(roomMappingKey, []byte(args.ChannelId)) | ||
| if err != nil { | ||
| c.client.Log.Error("Failed to save room mapping", "error", err, "room_identifier", roomIdentifier, "channel_id", args.ChannelId) | ||
|
|
@@ -409,7 +415,7 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, roomIdentifier stri | |
| // If roomIdentifier is an alias, also resolve to room ID and store that mapping | ||
| if strings.HasPrefix(roomIdentifier, "#") { | ||
| if resolvedRoomID, err := matrixClient.ResolveRoomAlias(roomIdentifier); err == nil { | ||
| roomIDMappingKey := kvstore.BuildRoomMappingKey(resolvedRoomID) | ||
| roomIDMappingKey := kvstore.BuildRoomMappingKey(serverID, resolvedRoomID) | ||
| if err := c.kvstore.Set(roomIDMappingKey, []byte(args.ChannelId)); err != nil { | ||
| c.client.Log.Error("Failed to save room ID mapping", "error", err, "room_id", resolvedRoomID, "channel_id", args.ChannelId) | ||
| } | ||
|
|
@@ -503,18 +509,36 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes | |
| } | ||
|
|
||
| // Check if this channel has a Matrix room mapping | ||
| serverID := c.plugin.GetServerID() | ||
| channelMappingKey := kvstore.BuildChannelMappingKey(args.ChannelId) | ||
| roomIDBytes, err := c.kvstore.Get(channelMappingKey) | ||
| if err != nil { | ||
|
|
||
| // A corrupt (unparseable) value is distinct from an unmapped channel. Clear | ||
| // the bad record so the admin can recover with /matrix map, rather than being | ||
| // told the channel is not mapped with no way to fix it. | ||
| if err == nil && len(roomIDBytes) > 0 { | ||
| if _, parseErr := kvstore.ParseChannelServerMappings(roomIDBytes); parseErr != nil { | ||
| c.client.Log.Error("Corrupt channel mapping value; clearing it", "error", parseErr, "channel_id", args.ChannelId) | ||
| if delErr := c.kvstore.Delete(channelMappingKey); delErr != nil { | ||
| c.client.Log.Error("Failed to clear corrupt channel mapping", "error", delErr, "channel_id", args.ChannelId) | ||
| } | ||
| return &model.CommandResponse{ | ||
| ResponseType: model.CommandResponseTypeEphemeral, | ||
| Text: fmt.Sprintf("⚠️ **Corrupt Mapping Cleared**\n\nChannel `%s` had an unreadable Matrix room mapping, which has been removed. Use `/matrix map` to remap it if needed.", channelName), | ||
| } | ||
|
Comment on lines
+522
to
+528
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Report failure when the corrupt record cannot be deleted. Line 522 logs a deletion failure, but Lines 525-528 still report that the mapping was cleared. Return an error response instead; the corrupt value remains and subsequent 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
| mappings, _ := kvstore.ParseChannelServerMappings(roomIDBytes) | ||
| matrixRoomIdentifier := kvstore.RoomIDForServer(mappings, serverID) | ||
| if err != nil || matrixRoomIdentifier == "" { | ||
| // Key not found is expected for unmapped channels | ||
| return &model.CommandResponse{ | ||
| ResponseType: model.CommandResponseTypeEphemeral, | ||
| Text: fmt.Sprintf("❌ **No Mapping Found**\n\nChannel `%s` is not currently mapped to any Matrix room.", channelName), | ||
| } | ||
| } | ||
|
|
||
| matrixRoomIdentifier := string(roomIDBytes) | ||
|
|
||
| // Clear the Matrix room state to prevent fallback lookups - this is critical | ||
| matrixClient := c.plugin.GetMatrixClient() | ||
| if matrixClient == nil { | ||
|
|
@@ -545,7 +569,7 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes | |
| } | ||
|
|
||
| // Remove the room->channel mapping | ||
| roomMappingKey := kvstore.BuildRoomMappingKey(matrixRoomIdentifier) | ||
| roomMappingKey := kvstore.BuildRoomMappingKey(serverID, matrixRoomIdentifier) | ||
| if err := c.kvstore.Delete(roomMappingKey); err != nil { | ||
| c.client.Log.Warn("Failed to remove room mapping", "error", err, "room_identifier", matrixRoomIdentifier, "channel_id", args.ChannelId) | ||
| // Continue - the main mapping was removed | ||
|
|
@@ -634,17 +658,22 @@ func (c *Handler) executeCreateRoomCommand(args *model.CommandArgs, roomName str | |
| } | ||
|
|
||
| // Automatically map the created room to this channel (both directions) | ||
| serverID := c.plugin.GetServerID() | ||
| mappingKey := kvstore.BuildChannelMappingKey(args.ChannelId) | ||
| if err := c.kvstore.Set(mappingKey, []byte(roomID)); err != nil { | ||
| c.client.Log.Error("Failed to save channel mapping", "error", err, "channel_id", args.ChannelId, "room_id", roomID) | ||
| mappingValue, mErr := kvstore.BuildSingleChannelMapping(serverID, roomID) | ||
| if mErr == nil { | ||
| mErr = c.kvstore.Set(mappingKey, mappingValue) | ||
| } | ||
| if mErr != nil { | ||
| c.client.Log.Error("Failed to save channel mapping", "error", mErr, "channel_id", args.ChannelId, "room_id", roomID) | ||
| return &model.CommandResponse{ | ||
| ResponseType: model.CommandResponseTypeEphemeral, | ||
| Text: fmt.Sprintf("✅ **Matrix Room Created:** `%s`\n\n❌ Failed to save channel mapping. Use `/matrix map %s` to map manually.", roomID, roomID), | ||
| } | ||
| } | ||
|
|
||
| // Store reverse mapping: room_mapping_<roomID> -> channelID | ||
| roomMappingKey := kvstore.BuildRoomMappingKey(roomID) | ||
| // Store reverse mapping: room_mapping_<serverID>_<roomID> -> channelID | ||
| roomMappingKey := kvstore.BuildRoomMappingKey(serverID, roomID) | ||
| if err := c.kvstore.Set(roomMappingKey, []byte(args.ChannelId)); err != nil { | ||
| c.client.Log.Error("Failed to save room mapping", "error", err, "room_id", roomID, "channel_id", args.ChannelId) | ||
| // Continue anyway - the forward mapping was saved successfully | ||
|
|
@@ -672,6 +701,7 @@ func (c *Handler) executeListMappingsCommand(args *model.CommandArgs) *model.Com | |
| responseText.WriteString("**Channel-to-Room Mappings:**\n\n") | ||
|
|
||
| // Get channel mapping keys using efficient prefix filtering | ||
| serverID := c.plugin.GetServerID() | ||
| mappings := make(map[string]string) | ||
| channelMappingPrefix := kvstore.KeyPrefixChannelMapping | ||
| page := 0 | ||
|
|
@@ -696,8 +726,16 @@ func (c *Handler) executeListMappingsCommand(args *model.CommandArgs) *model.Com | |
| for _, key := range keys { | ||
| channelID := strings.TrimPrefix(key, channelMappingPrefix) | ||
| roomIDBytes, err := c.kvstore.Get(key) | ||
| if err == nil && len(roomIDBytes) > 0 { | ||
| mappings[channelID] = string(roomIDBytes) | ||
| if err != nil || len(roomIDBytes) == 0 { | ||
| continue | ||
| } | ||
| channelMappings, parseErr := kvstore.ParseChannelServerMappings(roomIDBytes) | ||
| if parseErr != nil { | ||
| c.client.Log.Warn("Failed to parse channel mapping value", "channel_id", channelID, "error", parseErr) | ||
| continue | ||
| } | ||
| if roomID := kvstore.RoomIDForServer(channelMappings, serverID); roomID != "" { | ||
| mappings[channelID] = roomID | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Transient registry-read failure silently falls back to the exact "wrong prefix" scenario the comment warns against.
The doc comment says a registry-read failure "must not silently change the prefix" and should "surface it loudly, mirroring
GetMatrixRoomID" — but unlikeGetMatrixRoomID, this method returns only astring, so on KV read/parse failure it can only log and fall back toDefaultMatrixUsernamePrefix, which is a silent prefix change if the registry's actual entry differs from the default. Since this prefix drives ghost-user creation and Matrix-user reconstruction (line 501), a transient KV blip could momentarily split identity resolution.Consider caching the last successfully-resolved prefix (per
serverID) and falling back to that cached value instead of the static default on transient failure, reserving the static default only for the case where no successful read has ever occurred.🔧 Suggested approach
type BridgeUtils struct { logger Logger API plugin.API kvstore kvstore.KVStore matrixClient *matrix.Client serverID string remoteID string maxProfileImageSize int64 maxFileSize int64 configGetter ConfigurationGetter + // cache of the last successfully-resolved username prefix, used as a + // safer fallback than the static default on transient registry errors + usernamePrefixMu sync.RWMutex + lastKnownUsernamePrefix string } func (s *BridgeUtils) matrixUsernamePrefix() string { data, err := s.kvstore.Get(kvstore.KeyServersConfig) if err != nil { s.logger.LogError("Failed to read server registry for username prefix; using last known value", "server_id", s.serverID, "error", err) - return DefaultMatrixUsernamePrefix + return s.fallbackUsernamePrefix() } servers, err := kvstore.ParseServersConfig(data) if err != nil { s.logger.LogError("Corrupt server registry; using last known username prefix", "server_id", s.serverID, "error", err) - return DefaultMatrixUsernamePrefix + return s.fallbackUsernamePrefix() } if server, ok := kvstore.ServerConfigForID(servers, s.serverID); ok && server.UsernamePrefix != "" { + s.usernamePrefixMu.Lock() + s.lastKnownUsernamePrefix = server.UsernamePrefix + s.usernamePrefixMu.Unlock() return server.UsernamePrefix } - return DefaultMatrixUsernamePrefix + return s.fallbackUsernamePrefix() +} + +func (s *BridgeUtils) fallbackUsernamePrefix() string { + s.usernamePrefixMu.RLock() + defer s.usernamePrefixMu.RUnlock() + if s.lastKnownUsernamePrefix != "" { + return s.lastKnownUsernamePrefix + } + return DefaultMatrixUsernamePrefix }🤖 Prompt for AI Agents