feat(federation): Add XMPP bridge support#40758
Merged
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds XMPP bridging support to Rocket.Chat's federation system: username availability now checks a callback and rejects handles reserved by exclusive bridge namespaces; federation startup gains XMPP settings and a slash command; a large new Matrix client API (accounts, profile, presence, directory, rooms, media, messaging) is added; message ingestion and file uploads gain federation-aware handling. ChangesFederation XMPP Bridge
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant xmpp-join slash command
participant FederationMatrixService
participant federationSDK
User->>xmpp-join slash command: /xmpp-join `#channel`
xmpp-join slash command->>FederationMatrixService: joinXMPPChatRoom(roomAlias, user)
FederationMatrixService->>federationSDK: joinXMPPChatRoom(...)
federationSDK-->>FederationMatrixService: success/failure
FederationMatrixService-->>xmpp-join slash command: boolean result
xmpp-join slash command-->>User: ephemeral success/failure
sequenceDiagram
participant MatrixHomeserver
participant homeserver.matrix.message
participant FederationMatrix
participant Message
MatrixHomeserver->>homeserver.matrix.message: m.room.message event
homeserver.matrix.message->>FederationMatrix: saveFederationMessage(event)
FederationMatrix->>FederationMatrix: classify thread/reply/edit/media
FederationMatrix->>Message: saveMessageFromFederation(...)
Message-->>FederationMatrix: saved
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40758 +/- ##
===========================================
- Coverage 68.49% 68.43% -0.06%
===========================================
Files 4092 4096 +4
Lines 158285 158357 +72
Branches 28687 28553 -134
===========================================
- Hits 108414 108374 -40
- Misses 44838 44950 +112
Partials 5033 5033
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
sampaiodiego
force-pushed
the
add-federation-bridges-support
branch
3 times, most recently
from
June 17, 2026 02:13
6a0ad82 to
b6fe68b
Compare
sampaiodiego
force-pushed
the
add-federation-bridges-support
branch
4 times, most recently
from
July 2, 2026 14:50
c4f0dcc to
e6c7d29
Compare
sampaiodiego
marked this pull request as ready for review
July 2, 2026 19:19
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/ee/server/startup/federation.ts (1)
66-96: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSetup failure is swallowed but the watcher registration proceeds anyway, violating the ordering invariant documented right above.
The comment states
setupFederationMatrix()"must complete before the settings watcher," yet on failure the catch only logs and falls through tosettings.watchMultiple(...). Its initial fire will callconfigureFederation()against DB collections/repositories that were never registered, leaving federation silently non-functional with only a log line as evidence.🛠️ Suggested fix: stop initialization on setup failure
try { await setupFederationMatrix(); } catch (err) { logger.error({ msg: 'Failed to setup federation-matrix:', err }); + return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/ee/server/startup/federation.ts` around lines 66 - 96, The setup failure in setupFederationMatrix is being swallowed, but settings.watchMultiple still registers and can trigger configureFederation before the federation DB collections are initialized. Update the startup flow in federation.ts so that a failure from setupFederationMatrix aborts initialization instead of continuing, and only call settings.watchMultiple after the setup has succeeded. Keep the error logging in place, but make sure the catch path prevents the watcher from running when setupFederationMatrix fails.
🧹 Nitpick comments (10)
apps/meteor/app/lib/server/functions/checkUsernameAvailability.ts (1)
65-71: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog the caught callback error for observability.
Any throw from a
checkUsernameAvailabilityCallbackhandler (intentional rejection or an unexpected bug/transient failure) is silently converted tofalsehere with no trace. If a handler misbehaves, legitimate usernames could be rejected with nothing in the logs to explain why.🪵 Suggested logging addition
try { await checkUsernameAvailabilityCallback.run(username); return true; - } catch (error) { + } catch (error) { + logger.debug({ msg: 'Username rejected by checkUsernameAvailability callback', username, error }); return false; } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/app/lib/server/functions/checkUsernameAvailability.ts` around lines 65 - 71, The checkUsernameAvailability flow is swallowing errors from checkUsernameAvailabilityCallback.run and only returning false, so add logging in the catch block to record the caught error before falling back to false. Update the error handling in checkUsernameAvailability so the callback failure is visible in logs while preserving the current boolean result for callers.apps/meteor/server/settings/federation-service.ts (1)
120-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplicit
publicflag missing for new XMPP settings.Every other setting in this file explicitly declares
public(e.g.Federation_Service_Domain,Federation_Service_EDU_Process_Typing), but the four newFederation_XMPP_*settings omit it. Defaulting is likely fine here (secrets/URL shouldn't be public), but explicit declaration keeps the file's convention consistent and avoids ambiguity about intent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/server/settings/federation-service.ts` around lines 120 - 153, The new XMPP settings in the federation settings section are missing the explicit public flag used elsewhere in this file. Update the Federation_XMPP_Enabled, Federation_XMPP_Bridge_URL, Federation_XMPP_Bridge_HS_Token, and Federation_XMPP_Bridge_AS_Token entries in the FederationServiceSettings class to declare public explicitly, matching the existing pattern used by other settings so the intended visibility is unambiguous.ee/packages/federation-matrix/src/api/_matrix/client/versions.ts (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResponse schema doesn't describe the actual payload.
VersionsResponseSchemahas an emptypropertiesobject, so it doesn't validate/document theversionsarray actually returned by the handler.♻️ Proposed fix
const VersionsResponseSchema = { type: 'object', - properties: {}, + properties: { + versions: { type: 'array', items: { type: 'string' } }, + }, + required: ['versions'], };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/_matrix/client/versions.ts` around lines 6 - 9, VersionsResponseSchema currently declares an empty object, so it does not match the payload returned by the versions handler. Update VersionsResponseSchema in versions.ts to describe the actual response shape, especially the versions array and any other fields the endpoint returns, and make sure the schema is used consistently by the versions handler or route definition.ee/packages/federation-matrix/src/api/_matrix/client/presence.ts (1)
34-42: 📐 Maintainability & Code Quality | 🔵 TrivialStub always returns
offline, regardless of requesteduserId.Endpoint ignores
c.req.param('userId')entirely and hardcodespresence: 'offline'. This is already flagged via theTODO(federation-sdk)comment, so likely tracked, but flagging per the TODO-detection policy.Want me to draft the
federationSDK.getPresence(userId)wiring once that API is available, or should this be tracked as a follow-up issue?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/_matrix/client/presence.ts` around lines 34 - 42, The presence endpoint stub in the anonymous handler always returns a hardcoded offline response and never uses the requested userId from c.req.param('userId'). Update this handler to pass the userId through to the federationSDK.getPresence(userId) path when available, and use that result to build the response; if the SDK API is not ready, keep the TODO clearly tied to this stub so the behavior is tracked without pretending to serve real presence data.ee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.ts (1)
79-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winErrors are silently swallowed with no logging.
The catch-all here returns
500without logging the underlyingerror, making auth failures undebuggable. This layer already introduces a sharedlogger(ee/packages/federation-matrix/src/api/logger.ts) that sibling modules (make-leave.ts,send-leave.ts) adopt for their error paths in this same PR — this middleware should do the same.♻️ Suggested fix
+import { logger } from '../logger'; ... } catch (error) { + logger.error({ msg: 'Error authenticating application service request', err: error }); return c.json(errCodes.M_UNKNOWN, 500); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.ts` around lines 79 - 81, The catch block in isAppServiceAuthenticated is swallowing auth failures without any logging, so update this middleware to log the caught error before returning the 500 response. Use the shared logger from the federation-matrix API logger module, following the pattern already used in make-leave and send-leave, and include the underlying error details in the log entry so failures can be diagnosed.ee/packages/federation-matrix/src/api/_matrix/client/directory.ts (1)
80-83: 📐 Maintainability & Code Quality | 🔵 TrivialDirectory alias resolution/creation are stubs.
Both routes return
notImplemented(...)with clearTODO(federation-sdk)markers; acceptable for this incremental PR, flagging for tracking.Also applies to: 103-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/_matrix/client/directory.ts` around lines 80 - 83, Directory alias resolution and creation are intentionally stubbed, so keep the existing notImplemented(...) behavior in the directory handlers and preserve the TODO(federation-sdk) markers in both routes. No functional change is needed here; just ensure the placeholder logic remains clearly tracked in the alias resolution and creation callbacks in directory.ts until the real implementation is added.ee/packages/federation-matrix/src/api/_matrix/client/media.ts (1)
216-221: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
m.upload.sizeis hardcoded, ignoring the server's actual configured max file size.Clients/bridges rely on this endpoint to learn the real upload limit. Reporting a fixed 50MB regardless of the admin-configured
FileUpload_MaxFileSizesetting can mislead clients into attempting uploads that will be rejected, or under-report a higher configured limit.♻️ Proposed fix
- async () => { - return { - statusCode: 200, - body: { - 'm.upload.size': 50 * 1024 * 1024, - }, - }; - }, + async () => { + const maxFileSize = (await Settings.get<number>('FileUpload_MaxFileSize')) || 50 * 1024 * 1024; + return { + statusCode: 200, + body: { + 'm.upload.size': maxFileSize, + }, + }; + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/_matrix/client/media.ts` around lines 216 - 221, The media config response is hardcoding m.upload.size, so clients never see the real server upload limit. Update the response in the media client config handler to read the configured max file size from the server’s upload settings (the value backing FileUpload_MaxFileSize) instead of returning a fixed 50MB, and keep the existing response shape in the same method.ee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.ts (1)
1-19: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test case for server names with ports.
None of the cases cover a room ID whose server name includes a port (e.g.
!abcdef:matrix.org:8448), which would catch the multi-colon bug flagged ingetFederatedRoomName.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.ts` around lines 1 - 19, Add a new test in getFederatedRoomName.spec.ts for a room ID whose server name includes a port, such as !abcdef:matrix.org:8448, to cover the multi-colon case. Verify getFederatedRoomName handles the full room ID deterministically and produces a slug-valid result without breaking on the extra colon, so the bug in getFederatedRoomName is caught by the spec.ee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.ts (1)
271-282: 📐 Maintainability & Code Quality | 🔵 TrivialFragile error detection via string matching.
Detecting a "not found" condition via
error?.message?.toLowerCase?.().includes('not found')is brittle — any wording change in the SDK's error message silently breaks this branch, causing 404s to fall through to generic 500s.Prefer checking a typed/error-code property from
federationSDK.leaveRoom(similar toisUnknownRoomErrorused inrooms-state.ts) instead of message substring matching.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.ts` around lines 271 - 282, The leave-room handler in rooms-lifecycle.ts is using brittle message substring matching to detect a not-found case. Update the catch block in the federation leave flow to use a typed/error-code check from federationSDK.leaveRoom, similar to the existing isUnknownRoomError pattern in rooms-state.ts, and return the 404 only for that explicit error condition. Keep the generic internalError path for all other failures.ee/packages/federation-matrix/src/api/_matrix/client/rooms-state.ts (1)
83-115: 🚀 Performance & Scalability | 🔵 TrivialManual iteration for a single-key lookup.
If
statereturned bygetLatestRoomStateis Map-like (as thefor...of [k, v]destructuring suggests),state.get(key)would be simpler and avoid an O(n) scan per lookup.♻️ Proposed simplification (if `state` is a Map)
- let pe: PersistentEventBase | undefined; - for (const [k, v] of state) { - if (k === key) { - pe = v; - break; - } - } + const pe = state.get(key);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/src/api/_matrix/client/rooms-state.ts` around lines 83 - 115, The getRoomStateEvent lookup in rooms-state.ts is manually iterating over a Map-like state to find a single key, which adds unnecessary O(n) work. Update the getRoomStateEvent flow to use the direct lookup API on the state returned by federationSDK.getLatestRoomState(roomId) if it is Map-like, and keep the existing 404/internal error behavior unchanged. Use the existing getRoomStateEvent identifier to locate the logic and preserve the key format `${eventType}:${stateKey}`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ee/packages/federation-matrix/src/api/_matrix/client/account.ts`:
- Around line 91-113: In the non-full-XMPP branch of the registration flow, the
`/v3/register` response is returning the raw username instead of a canonical
Matrix ID. Update the `account.ts` logic around `createOrUpdateFederatedUser` so
the `user_id` field is built as a fully-qualified MXID in the same format used
by the full-XMPP path (for example, via the same MXID construction helper or
equivalent `@localpart:server` composition). Ensure the response always returns
the canonical Matrix ID regardless of which branch is taken.
In `@ee/packages/federation-matrix/src/api/_matrix/client/directory.ts`:
- Around line 122-141: The publicRooms handler in directory.ts is returning
misleading room metadata and ignores pagination. Update the async public room
listing logic to use real values for num_joined_members, world_readable, and
guest_can_join where possible from federationSDK.getAllPublicRoomIdsAndNames()
or the surrounding Federation SDK, or clearly mark the fields as unsupported if
they cannot be derived. Also add handling for the Matrix directory params like
limit and since so the returned chunk is paginated instead of always returning
every room at once.
In `@ee/packages/federation-matrix/src/api/_matrix/client/media.ts`:
- Around line 151-182: The thumbnail resize handler in the Matrix media API
accepts only finite positive `width` and `height`, so add a hard upper limit
before calling `Upload.streamUploadedFile` and materializing the stream. Update
the validation in the `media.ts` thumbnail path to reject oversized dimensions
with a 400 response, using the existing `width`/`height` query parsing and the
`getLocalFileForMatrixNode` / `streamUploadedFile` flow as the location to apply
the cap.
- Around line 99-108: The media lookup currently allows an unscoped fallback via
MatrixMediaService.getLocalFileForMatrixNode, which can return an upload for the
wrong bridge/server when only mediaId matches. Update the lookup path in the
media handler and MatrixMediaService.getLocalFileForMatrixNode so the result is
always bound to the requested serverName/federation metadata before returning a
file, and avoid falling back to a global id-only lookup when the scoped match
fails.
In `@ee/packages/federation-matrix/src/api/_matrix/client/profile.ts`:
- Around line 158-180: The PUT displayname handler in profile.ts is using
impersonatedUserId without verifying it matches the userId path param, so it can
update the wrong account. In the profile route handler, read
c.req.param('userId') and compare it against c.get('impersonatedUserId') before
calling Users.findOneByUsername or Users.setName. If they differ, return an
appropriate error response instead of applying the update, and keep the existing
success path only for matching identities.
- Around line 109-138: The profile GET handler in the anonymous async callback
should not return arbitrary properties from federationSDK.queryProfile based on
the URL-supplied field. Restrict field to an explicit allowlist of public
profile keys (for example the known Matrix profile fields) before building the
response, and return a bad request or not found-style error when field is not
one of the supported values. Keep the existing queryProfile and internalError
flow, but update the property lookup in the response body to use the validated
field only.
In `@ee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.ts`:
- Around line 174-213: The create room flow in rooms-lifecycle.ts is masking
partial success: if federationSDK.inviteUserToRoom fails after
Rooms.findOne/Room.create has already created the Matrix/RC room, the outer
try/catch returns internalError('Failed to create room', error) and hides the
valid room_id. Update the invite loop in the createRoom path to handle invite
failures separately from room creation, using the existing Room.create,
federationSDK.inviteUserToRoom, and internalError flow so a failed invite does
not make the caller think room creation failed; either continue after logging
invite errors or return room_id with a partial-failure indicator.
- Around line 233-240: The join handler currently returns an empty body even
though the route contract expects a room_id. Update the async handler in
rooms-lifecycle.ts, where federationSDK.joinUser is called, to capture the
joined room ID from the join operation and return it in the response body. Make
sure the returned object matches the declared schema so both room ID and alias
joins resolve consistently.
In `@ee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.ts`:
- Around line 338-344: The `user.activity` broadcast in `notifyUserTyping` is
sending `user.name || user.username`, which can emit a display name instead of
the plain username expected by consumers like
`FederationMatrix.notifyUserTyping`. Update the payload to use the username
field only for the `user` property, keeping the rest of the broadcast unchanged,
so downstream lookups via `Users.findOneByUsername` continue to work correctly.
- Around line 154-163: The org.matrix.bridge.ping branch in roomsMessaging still
calls federationSDK.sendCustomEvent without error handling, unlike the other
outbound paths in this handler. Wrap this send in the same try/catch pattern
used by the other eventType branches in roomsMessaging, and on failure return
the declared internalError response shape so the 500: isMatrixErrorProps
contract stays consistent. Use the existing federationSDK.sendCustomEvent and
roomsMessaging branch structure to place the fix without changing the success
response.
- Line 265: The `roomsMessaging` backfill call is using `never` for the event-id
array, which bypasses type checking. Update the `getBackfillEvents` invocation
in `rooms-messaging.ts` to cast `[fromParam]` to the SDK’s `EventID[]` type,
matching the existing federation-matrix usage so the argument shape remains
compiler-checked.
In `@ee/packages/federation-matrix/src/api/_matrix/media-bridge.ts`:
- Around line 94-112: The media upload flow in MatrixMediaBridge currently
buffers the entire request body without enforcing the advertised 50 MiB limit,
leaving the 413 response path unused. Update the upload handler around the
arrayBuffer/Buffer.from sequence to validate the request size before buffering
(using content-length when present and/or the resulting byte length) and return
the existing 413 response when the payload exceeds the limit; keep the check in
the same upload path that calls MatrixMediaService.uploadFromAppService.
In
`@ee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.ts`:
- Around line 63-74: Malformed user_id parsing in isAppServiceAuthenticated is
escaping the local validation path and being turned into a generic 500 by the
outer catch. Update the decodeXmppUserId/parseXmppUserId handling in this
middleware so malformed client input is caught locally and returns the same 400
M_INVALID_USER_ID response used for a missing resource, instead of throwing up
to the catch block. Keep the fix anchored around the decodeXmppUserId,
parseXmppUserId, and the existing c.json error response in
isAppServiceAuthenticated.
- Around line 63-76: The impersonation path in isAppServiceAuthenticated is
building the wrong Matrix user ID shape because impersonatedUserId is missing
the leading @. Update the assignment that uses decodeXmppUserId/parseXmppUserId
so it matches the other branches and stores a full Matrix ID with @ before the
username/resource, ensuring Users.findOneByUsername receives a valid
Matrix-style identifier.
In `@ee/packages/federation-matrix/src/FederationMatrix.ts`:
- Around line 1065-1071: The message parsing in FederationMatrix’s event
handling should not call body.toString() unconditionally, since malformed
external events can omit body and throw before the empty-message guard runs.
Update the logic around the msgtype/body destructuring to safely handle a
missing body in the same path that currently logs "Received message event with
empty body and no msgtype, skipping processing", so the event is skipped at
debug level instead of bubbling an exception to events/message.ts.
- Around line 1157-1197: The quote-reply path in FederationMatrix.ts returns
before media handling, so replies that are also attachments never reach
handleMediaMessage and lose the file content. Update the logic around the
quoteMessageEventId branch and the isMediaMessage check so media messages are
processed first, or merge the two flows so a quoted reply can still invoke
handleMediaMessage and then save the result with
Message.saveMessageFromFederation; keep the quote formatting via
toInternalQuoteMessageFormat without dropping attachment data.
In `@ee/packages/federation-matrix/src/helpers/getFederatedRoomName.ts`:
- Line 6: getFederatedRoomName currently only replaces the first colon, so
Matrix room IDs with port-qualified server names can still produce invalid
slug-like names. Update getFederatedRoomName to sanitize all colon characters in
the room ID, not just the first one, while preserving the existing removal of
the leading bang so callers like events/member.ts and rooms-lifecycle.ts
continue to receive Room.create-compatible names.
In `@ee/packages/federation-matrix/src/services/MatrixMediaService.ts`:
- Around line 93-109: The app-service upload path in
MatrixMediaService.uploadFromAppService is saving empty room identifiers in both
details.rid and federation.mrid, leaving these uploads detached from room-based
lookup and cleanup. Update this flow to populate the room ID once it is
available, using the existing Upload.uploadFile call and the surrounding
uploadFromAppService logic to ensure the persisted details and federation
metadata carry the correct room association instead of empty strings.
---
Outside diff comments:
In `@apps/meteor/ee/server/startup/federation.ts`:
- Around line 66-96: The setup failure in setupFederationMatrix is being
swallowed, but settings.watchMultiple still registers and can trigger
configureFederation before the federation DB collections are initialized. Update
the startup flow in federation.ts so that a failure from setupFederationMatrix
aborts initialization instead of continuing, and only call
settings.watchMultiple after the setup has succeeded. Keep the error logging in
place, but make sure the catch path prevents the watcher from running when
setupFederationMatrix fails.
---
Nitpick comments:
In `@apps/meteor/app/lib/server/functions/checkUsernameAvailability.ts`:
- Around line 65-71: The checkUsernameAvailability flow is swallowing errors
from checkUsernameAvailabilityCallback.run and only returning false, so add
logging in the catch block to record the caught error before falling back to
false. Update the error handling in checkUsernameAvailability so the callback
failure is visible in logs while preserving the current boolean result for
callers.
In `@apps/meteor/server/settings/federation-service.ts`:
- Around line 120-153: The new XMPP settings in the federation settings section
are missing the explicit public flag used elsewhere in this file. Update the
Federation_XMPP_Enabled, Federation_XMPP_Bridge_URL,
Federation_XMPP_Bridge_HS_Token, and Federation_XMPP_Bridge_AS_Token entries in
the FederationServiceSettings class to declare public explicitly, matching the
existing pattern used by other settings so the intended visibility is
unambiguous.
In `@ee/packages/federation-matrix/src/api/_matrix/client/directory.ts`:
- Around line 80-83: Directory alias resolution and creation are intentionally
stubbed, so keep the existing notImplemented(...) behavior in the directory
handlers and preserve the TODO(federation-sdk) markers in both routes. No
functional change is needed here; just ensure the placeholder logic remains
clearly tracked in the alias resolution and creation callbacks in directory.ts
until the real implementation is added.
In `@ee/packages/federation-matrix/src/api/_matrix/client/media.ts`:
- Around line 216-221: The media config response is hardcoding m.upload.size, so
clients never see the real server upload limit. Update the response in the media
client config handler to read the configured max file size from the server’s
upload settings (the value backing FileUpload_MaxFileSize) instead of returning
a fixed 50MB, and keep the existing response shape in the same method.
In `@ee/packages/federation-matrix/src/api/_matrix/client/presence.ts`:
- Around line 34-42: The presence endpoint stub in the anonymous handler always
returns a hardcoded offline response and never uses the requested userId from
c.req.param('userId'). Update this handler to pass the userId through to the
federationSDK.getPresence(userId) path when available, and use that result to
build the response; if the SDK API is not ready, keep the TODO clearly tied to
this stub so the behavior is tracked without pretending to serve real presence
data.
In `@ee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.ts`:
- Around line 271-282: The leave-room handler in rooms-lifecycle.ts is using
brittle message substring matching to detect a not-found case. Update the catch
block in the federation leave flow to use a typed/error-code check from
federationSDK.leaveRoom, similar to the existing isUnknownRoomError pattern in
rooms-state.ts, and return the 404 only for that explicit error condition. Keep
the generic internalError path for all other failures.
In `@ee/packages/federation-matrix/src/api/_matrix/client/rooms-state.ts`:
- Around line 83-115: The getRoomStateEvent lookup in rooms-state.ts is manually
iterating over a Map-like state to find a single key, which adds unnecessary
O(n) work. Update the getRoomStateEvent flow to use the direct lookup API on the
state returned by federationSDK.getLatestRoomState(roomId) if it is Map-like,
and keep the existing 404/internal error behavior unchanged. Use the existing
getRoomStateEvent identifier to locate the logic and preserve the key format
`${eventType}:${stateKey}`.
In `@ee/packages/federation-matrix/src/api/_matrix/client/versions.ts`:
- Around line 6-9: VersionsResponseSchema currently declares an empty object, so
it does not match the payload returned by the versions handler. Update
VersionsResponseSchema in versions.ts to describe the actual response shape,
especially the versions array and any other fields the endpoint returns, and
make sure the schema is used consistently by the versions handler or route
definition.
In
`@ee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.ts`:
- Around line 79-81: The catch block in isAppServiceAuthenticated is swallowing
auth failures without any logging, so update this middleware to log the caught
error before returning the 500 response. Use the shared logger from the
federation-matrix API logger module, following the pattern already used in
make-leave and send-leave, and include the underlying error details in the log
entry so failures can be diagnosed.
In `@ee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.ts`:
- Around line 1-19: Add a new test in getFederatedRoomName.spec.ts for a room ID
whose server name includes a port, such as !abcdef:matrix.org:8448, to cover the
multi-colon case. Verify getFederatedRoomName handles the full room ID
deterministically and produces a slug-valid result without breaking on the extra
colon, so the bug in getFederatedRoomName is caught by the spec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3312b5b0-95f6-4468-ae5a-db7e4a70f65c
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (47)
apps/meteor/app/file-upload/server/lib/FileUpload.tsapps/meteor/app/lib/server/functions/checkUsernameAvailability.tsapps/meteor/client/startup/slashCommands/federation.tsapps/meteor/ee/server/hooks/federation/index.tsapps/meteor/ee/server/startup/federation.tsapps/meteor/package.jsonapps/meteor/server/lib/callbacks/checkUsernameAvailabilityCallback.tsapps/meteor/server/services/upload/service.tsapps/meteor/server/settings/federation-service.tsee/packages/federation-matrix/package.jsonee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/_matrix/client/_shared.tsee/packages/federation-matrix/src/api/_matrix/client/account.tsee/packages/federation-matrix/src/api/_matrix/client/directory.tsee/packages/federation-matrix/src/api/_matrix/client/index.tsee/packages/federation-matrix/src/api/_matrix/client/media.tsee/packages/federation-matrix/src/api/_matrix/client/presence.tsee/packages/federation-matrix/src/api/_matrix/client/profile.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-state.tsee/packages/federation-matrix/src/api/_matrix/client/user.tsee/packages/federation-matrix/src/api/_matrix/client/versions.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/src/api/_matrix/media-bridge.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/logger.tsee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.tsee/packages/federation-matrix/src/helpers/getThreadMessageId.tsee/packages/federation-matrix/src/helpers/handleMediaMessage.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsee/packages/federation-matrix/src/setup.tspackages/core-services/package.jsonpackages/core-services/src/types/IFederationMatrixService.tspackages/core-services/src/types/IUploadService.tspackages/i18n/src/locales/en.i18n.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/api/_matrix/client/index.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.tsapps/meteor/server/settings/federation-service.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/helpers/getThreadMessageId.tsee/packages/federation-matrix/src/api/logger.tsapps/meteor/server/lib/callbacks/checkUsernameAvailabilityCallback.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/api/_matrix/client/versions.tsee/packages/federation-matrix/src/api/_matrix/client/presence.tsee/packages/federation-matrix/src/api/_matrix/client/profile.tsee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.tsapps/meteor/client/startup/slashCommands/federation.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.tsee/packages/federation-matrix/src/api/_matrix/media-bridge.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.tspackages/core-services/src/types/IUploadService.tsapps/meteor/app/lib/server/functions/checkUsernameAvailability.tsee/packages/federation-matrix/src/helpers/handleMediaMessage.tsapps/meteor/server/services/upload/service.tsee/packages/federation-matrix/src/api/_matrix/client/directory.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/events/message.tsapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/api/_matrix/client/user.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/file-upload/server/lib/FileUpload.tsee/packages/federation-matrix/src/api/_matrix/client/_shared.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.tsee/packages/federation-matrix/src/api/_matrix/client/account.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/client/media.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-state.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.tsee/packages/federation-matrix/src/FederationMatrix.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.ts
🧠 Learnings (12)
📚 Learning: 2026-06-16T14:13:34.463Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:31-31
Timestamp: 2026-06-16T14:13:34.463Z
Learning: In Rocket.Chat’s monorepo, when reviewing a dependency entry and flagging that a specific version “does not exist” (e.g., in package.json), first verify the exact package/version directly against the npm registry (use URLs like https://registry.npmjs.org/<package>/<version> or https://www.npmjs.com/package/<package>/v/<version>). Do not rely on web search results for this check, since they may be stale or cached and may not reflect the latest published versions.
Applied to files:
packages/core-services/package.json
📚 Learning: 2026-06-16T14:13:49.795Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:26-26
Timestamp: 2026-06-16T14:13:49.795Z
Learning: During code reviews that check whether a dependency version exists in package.json (especially for Rocket.Chat’s rocket.chat/fuselage and related rocket.chat/fuselage-* packages), don’t rely on web search results. Instead, verify the version directly against the npm registry (e.g., via the npm registry API or the canonical package URL https://www.npmjs.com/package/<package>/v/<version>) before deciding that a version bump is invalid. If the version is present in the npm registry, do not flag it as invalid.
Applied to files:
packages/core-services/package.json
📚 Learning: 2026-06-16T14:13:59.986Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/ui-video-conf/package.json:25-25
Timestamp: 2026-06-16T14:13:59.986Z
Learning: In the Rocket.Chat monorepo, when reviewing a dependency version bump for rocket.chat/fuselage in a package.json, do not flag the new version constraint as “non-existent” or invalid unless you verify the published versions directly from the npm registry (https://www.npmjs.com/package/rocket.chat/fuselage). Don’t rely on search/web results for available versions since they can be stale.
Applied to files:
packages/core-services/package.json
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/api/_matrix/client/index.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/helpers/getThreadMessageId.tsee/packages/federation-matrix/src/api/logger.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/api/_matrix/client/versions.tsee/packages/federation-matrix/src/api/_matrix/client/presence.tsee/packages/federation-matrix/src/api/_matrix/client/profile.tsee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.tsee/packages/federation-matrix/src/api/_matrix/media-bridge.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.tsee/packages/federation-matrix/src/helpers/handleMediaMessage.tsee/packages/federation-matrix/src/api/_matrix/client/directory.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/api/_matrix/client/user.tsee/packages/federation-matrix/src/api/_matrix/client/_shared.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.tsee/packages/federation-matrix/src/api/_matrix/client/account.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsee/packages/federation-matrix/src/api/_matrix/client/media.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-state.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/api/_matrix/client/index.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.tsapps/meteor/server/settings/federation-service.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/helpers/getThreadMessageId.tsee/packages/federation-matrix/src/api/logger.tsapps/meteor/server/lib/callbacks/checkUsernameAvailabilityCallback.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/api/_matrix/client/versions.tsee/packages/federation-matrix/src/api/_matrix/client/presence.tsee/packages/federation-matrix/src/api/_matrix/client/profile.tsee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.tsapps/meteor/client/startup/slashCommands/federation.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.tsee/packages/federation-matrix/src/api/_matrix/media-bridge.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.tspackages/core-services/src/types/IUploadService.tsapps/meteor/app/lib/server/functions/checkUsernameAvailability.tsee/packages/federation-matrix/src/helpers/handleMediaMessage.tsapps/meteor/server/services/upload/service.tsee/packages/federation-matrix/src/api/_matrix/client/directory.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/events/message.tsapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/api/_matrix/client/user.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/file-upload/server/lib/FileUpload.tsee/packages/federation-matrix/src/api/_matrix/client/_shared.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.tsee/packages/federation-matrix/src/api/_matrix/client/account.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/client/media.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-state.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/api/_matrix/client/index.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.tsapps/meteor/server/settings/federation-service.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/helpers/getThreadMessageId.tsee/packages/federation-matrix/src/api/logger.tsapps/meteor/server/lib/callbacks/checkUsernameAvailabilityCallback.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/api/_matrix/client/versions.tsee/packages/federation-matrix/src/api/_matrix/client/presence.tsee/packages/federation-matrix/src/api/_matrix/client/profile.tsee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.tsapps/meteor/client/startup/slashCommands/federation.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.tsee/packages/federation-matrix/src/api/_matrix/media-bridge.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.tspackages/core-services/src/types/IUploadService.tsapps/meteor/app/lib/server/functions/checkUsernameAvailability.tsee/packages/federation-matrix/src/helpers/handleMediaMessage.tsapps/meteor/server/services/upload/service.tsee/packages/federation-matrix/src/api/_matrix/client/directory.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/events/message.tsapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/api/_matrix/client/user.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/file-upload/server/lib/FileUpload.tsee/packages/federation-matrix/src/api/_matrix/client/_shared.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.tsee/packages/federation-matrix/src/api/_matrix/client/account.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/client/media.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-state.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
ee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.spec.tsee/packages/federation-matrix/src/api/_matrix/client/index.tsee/packages/federation-matrix/src/helpers/isUsernameReservedByExclusiveBridge.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.tsapps/meteor/server/settings/federation-service.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/helpers/getThreadMessageId.tsee/packages/federation-matrix/src/api/logger.tsapps/meteor/server/lib/callbacks/checkUsernameAvailabilityCallback.tsee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.tsee/packages/federation-matrix/src/api/_matrix/client/versions.tsee/packages/federation-matrix/src/api/_matrix/client/presence.tsee/packages/federation-matrix/src/api/_matrix/client/profile.tsee/packages/federation-matrix/src/api/middlewares/isAppServiceAuthenticated.tsapps/meteor/client/startup/slashCommands/federation.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.spec.tsee/packages/federation-matrix/src/api/_matrix/media-bridge.tsee/packages/federation-matrix/src/helpers/parseXmppUserId.tspackages/core-services/src/types/IUploadService.tsapps/meteor/app/lib/server/functions/checkUsernameAvailability.tsee/packages/federation-matrix/src/helpers/handleMediaMessage.tsapps/meteor/server/services/upload/service.tsee/packages/federation-matrix/src/api/_matrix/client/directory.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/events/message.tsapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/api/_matrix/client/user.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/file-upload/server/lib/FileUpload.tsee/packages/federation-matrix/src/api/_matrix/client/_shared.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-messaging.tsee/packages/federation-matrix/src/api/_matrix/client/account.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/client/media.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-state.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/_matrix/client/rooms-lifecycle.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/startup/slashCommands/federation.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/startup/slashCommands/federation.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 48 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/lib/server/functions/checkUsernameAvailability.ts">
<violation number="1" location="apps/meteor/app/lib/server/functions/checkUsernameAvailability.ts:65">
P2: Username checks now collapse all callback exceptions into a generic unavailable result, so intentional federation errors and unexpected callback failures become indistinguishable. Consider letting callback errors propagate (or only mapping known/expected errors) so reserved-namespace rejections keep their specific failure semantics.</violation>
</file>
<file name="ee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.ts">
<violation number="1" location="ee/packages/federation-matrix/src/helpers/getFederatedRoomName.spec.ts:8">
P2: Tests cover only the canonical happy path (`!opaqueId:server.domain`). Missing edge cases that would validate real robustness: empty input, missing `!` sigil, unicode/special characters in room ID, multiple `:` separators (e.g., server port), and server names already containing `_` (which creates ambiguity with the `:`→`_` replacement). Per the project's emphasis on meaningful test coverage, consider adding targeted edge-case assertions — especially for inputs that would produce non-slug-safe output, since test 2 asserts slug-validity but never challenges it with non-slug inputs.</violation>
</file>
<file name="apps/meteor/ee/server/hooks/federation/index.ts">
<violation number="1" location="apps/meteor/ee/server/hooks/federation/index.ts:388">
P2: This hook now applies bridge namespace restrictions to every `checkUsernameAvailability` caller, including team/room naming flows, so valid non-user names can be rejected. Consider scoping this reservation check to user-username assignment paths instead of the shared availability helper.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
sampaiodiego
force-pushed
the
add-federation-bridges-support
branch
from
July 3, 2026 19:56
97be2dd to
e3e0c1d
Compare
![hacktron-app[bot]](https://avatars.githubusercontent.com/in/1250513?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
| Severity | Count |
|---|---|
 |
1 |
Findings outside your changes (1)
1 additional security finding was found outside your changes. Details are hidden on public repositories — review it in Hacktron: View finding
Contributor
There was a problem hiding this comment.
12 issues found across 50 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
|
should this close #404? |
sampaiodiego
force-pushed
the
add-federation-bridges-support
branch
from
July 7, 2026 00:03
1aeacea to
1ca1719
Compare
sampaiodiego
force-pushed
the
add-federation-bridges-support
branch
from
July 7, 2026 20:12
1ca1719 to
65a4973
Compare
jessicaschelly
added
the
stat: QA assured
sampaiodiego
force-pushed
the
add-federation-bridges-support
branch
from
July 16, 2026 17:39
96c2c2b to
fdf0233
Compare
ggazzo
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
closes #404
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
FEDCORE-47