-
Notifications
You must be signed in to change notification settings - Fork 13k
chore: allow federation setting changes without requiring a restart #37357
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughReplaces per-request HomeserverServices with a centralized Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Meteor as Meteor startup
participant Startup as federation startup file
participant Settings as Settings store
participant License as License service
participant SDK as federationSDK
participant Emitter as Federation Emitter
participant Events as Event handlers
Meteor->>Startup: server start
Startup->>License: check License permits federation
Startup->>Settings: read Federation_* settings
Startup->>SDK: configureFederationMatrixSettings(settings)
SDK-->>Startup: success / throws
alt configured
Startup->>SDK: setupFederationMatrix()
SDK->>Emitter: init emitter
SDK->>Events: registerEvents(emitter)
Events->>Emitter: subscribe handlers (message, member, edus, room)
end
Note over Settings,Startup: settings.watchMultiple -> on change
Settings->>Startup: setting changed
Startup->>SDK: configureFederationMatrixSettings(updated)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
c580e77 to
9ac1999
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37357 +/- ##
===========================================
- Coverage 67.08% 65.62% -1.46%
===========================================
Files 3419 3140 -279
Lines 117938 113712 -4226
Branches 21579 20604 -975
===========================================
- Hits 79114 74620 -4494
- Misses 36135 36705 +570
+ Partials 2689 2387 -302
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dea3b8b to
f8e430a
Compare
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/src/setup.ts (1)
86-86: Inverted logic for enableThumbnails.The condition
process.env.MEDIA_ENABLE_THUMBNAILS !== 'true'will setenableThumbnailstofalsewhen the environment variable is'true', andtrueotherwise. This appears backwards.Apply this diff to fix the logic:
- enableThumbnails: process.env.MEDIA_ENABLE_THUMBNAILS !== 'true', + enableThumbnails: process.env.MEDIA_ENABLE_THUMBNAILS === 'true',
🧹 Nitpick comments (1)
ee/packages/federation-matrix/src/events/member.ts (1)
23-26: Consider consistent error handling approach.Line 24 logs an error and returns when a bridged user is not found, while line 70 throws an error when serverName is missing. Consider whether both cases should throw or both should log+return for consistency.
Also applies to: 69-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
apps/meteor/ee/server/startup/federation.ts(1 hunks)apps/meteor/package.json(1 hunks)ee/packages/federation-matrix/package.json(2 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(21 hunks)ee/packages/federation-matrix/src/api/.well-known/server.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(7 hunks)ee/packages/federation-matrix/src/api/_matrix/key/server.ts(3 hunks)ee/packages/federation-matrix/src/api/_matrix/media.ts(4 hunks)ee/packages/federation-matrix/src/api/_matrix/profiles.ts(7 hunks)ee/packages/federation-matrix/src/api/_matrix/rooms.ts(4 hunks)ee/packages/federation-matrix/src/api/_matrix/send-join.ts(3 hunks)ee/packages/federation-matrix/src/api/_matrix/transactions.ts(10 hunks)ee/packages/federation-matrix/src/api/_matrix/versions.ts(3 hunks)ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts(4 hunks)ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts(2 hunks)ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.ts(1 hunks)ee/packages/federation-matrix/src/api/routes.ts(1 hunks)ee/packages/federation-matrix/src/events/edu.ts(2 hunks)ee/packages/federation-matrix/src/events/index.ts(2 hunks)ee/packages/federation-matrix/src/events/member.ts(4 hunks)ee/packages/federation-matrix/src/events/message.ts(3 hunks)ee/packages/federation-matrix/src/events/room.ts(3 hunks)ee/packages/federation-matrix/src/index.ts(1 hunks)ee/packages/federation-matrix/src/services/MatrixMediaService.ts(2 hunks)ee/packages/federation-matrix/src/setup.ts(3 hunks)packages/core-services/package.json(1 hunks)packages/core-services/src/types/IFederationMatrixService.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.tsapps/meteor/package.jsonee/packages/federation-matrix/src/api/_matrix/rooms.tsee/packages/federation-matrix/src/services/MatrixMediaService.tspackages/core-services/package.jsonee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/package.jsonee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/.well-known/server.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/index.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/edu.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/api/_matrix/media.tspackages/core-services/src/types/IFederationMatrixService.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/package.jsonee/packages/federation-matrix/src/api/_matrix/rooms.tspackages/core-services/package.jsonee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/package.jsonee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/edu.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tspackages/core-services/src/types/IFederationMatrixService.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/package.jsonee/packages/federation-matrix/src/api/middlewares/isAuthenticated.tsee/packages/federation-matrix/src/api/_matrix/rooms.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/index.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/edu.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tspackages/core-services/src/types/IFederationMatrixService.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/api/_matrix/rooms.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tspackages/core-services/src/types/IFederationMatrixService.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/api/_matrix/rooms.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tspackages/core-services/src/types/IFederationMatrixService.ts
📚 Learning: 2025-09-16T22:09:18.041Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-09-16T22:09:18.041Z
Learning: Use Rocket.Chat primary documentation and provided reference files for guidance
Applied to files:
ee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-09-15T13:10:30.049Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 36868
File: packages/http-router/src/Router.ts:416-425
Timestamp: 2025-09-15T13:10:30.049Z
Learning: In packages/http-router/src/Router.ts, the dispatch() method's use of replaceAll('//', '/') on the full path is acceptable because URL normalization and query string handling is performed by the caller function before dispatch() is invoked.
Applied to files:
ee/packages/federation-matrix/src/api/.well-known/server.ts
📚 Learning: 2025-10-16T21:09:51.816Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/keychain.ts:148-156
Timestamp: 2025-10-16T21:09:51.816Z
Learning: In the RocketChat/Rocket.Chat repository, only platforms with native crypto.randomUUID() support are targeted, so fallback implementations for crypto.randomUUID() are not required in E2EE or cryptographic code.
Applied to files:
ee/packages/federation-matrix/src/api/_matrix/media.ts
🧬 Code graph analysis (14)
ee/packages/federation-matrix/src/api/_matrix/rooms.ts (1)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
ee/packages/federation-matrix/src/events/room.ts (1)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
getUsernameServername(73-85)
ee/packages/federation-matrix/src/api/_matrix/send-join.ts (1)
ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
apps/meteor/ee/server/startup/federation.ts (5)
packages/core-services/src/index.ts (1)
License(162-162)ee/packages/federation-matrix/src/setup.ts (2)
configureFederationMatrixSettings(37-101)setupFederationMatrix(103-120)packages/models/src/index.ts (1)
InstanceStatus(164-164)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(137-901)apps/meteor/ee/server/api/federation.ts (1)
registerFederationRoutes(43-52)
ee/packages/federation-matrix/src/api/routes.ts (9)
ee/packages/federation-matrix/src/api/_matrix/key/server.ts (1)
getKeyServerRoutes(35-54)ee/packages/federation-matrix/src/api/_matrix/versions.ts (1)
getFederationVersionsRoutes(28-55)ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
getMatrixInviteRoutes(304-398)ee/packages/federation-matrix/src/api/_matrix/profiles.ts (1)
getMatrixProfilesRoutes(355-512)ee/packages/federation-matrix/src/api/_matrix/rooms.ts (1)
getMatrixRoomsRoutes(126-208)ee/packages/federation-matrix/src/api/_matrix/send-join.ts (1)
getMatrixSendJoinRoutes(227-252)ee/packages/federation-matrix/src/api/_matrix/transactions.ts (1)
getMatrixTransactionsRoutes(318-509)ee/packages/federation-matrix/src/api/_matrix/media.ts (1)
getMatrixMediaRoutes(76-149)ee/packages/federation-matrix/src/api/.well-known/server.ts (1)
getWellKnownRoutes(22-46)
ee/packages/federation-matrix/src/FederationMatrix.ts (2)
ee/packages/federation-matrix/src/events/room.ts (1)
room(8-80)ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
acceptInvite(266-302)
ee/packages/federation-matrix/src/events/index.ts (5)
ee/packages/federation-matrix/src/events/message.ts (1)
message(121-419)ee/packages/federation-matrix/src/events/reaction.ts (1)
reaction(10-93)ee/packages/federation-matrix/src/events/member.ts (1)
member(88-104)ee/packages/federation-matrix/src/events/edu.ts (1)
edus(10-85)ee/packages/federation-matrix/src/events/room.ts (1)
room(8-80)
ee/packages/federation-matrix/src/setup.ts (2)
ee/packages/federation-matrix/src/index.ts (2)
configureFederationMatrixSettings(9-9)setupFederationMatrix(9-9)ee/packages/federation-matrix/src/events/index.ts (1)
registerEvents(11-18)
ee/packages/federation-matrix/src/api/_matrix/transactions.ts (2)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
ee/packages/federation-matrix/src/api/_matrix/profiles.ts (2)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (2)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
getUsernameServername(73-85)ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
ee/packages/federation-matrix/src/events/member.ts (2)
ee/packages/federation-matrix/src/events/room.ts (1)
room(8-80)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
getUsernameServername(73-85)
ee/packages/federation-matrix/src/api/_matrix/media.ts (1)
ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
🔇 Additional comments (37)
ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.ts (1)
5-5: TODO appropriately tracks future migration work.The TODO comment aligns with the PR's broader migration to federationSDK. Since this is a comment-only addition with no logic changes, deferring the actual migration work here is reasonable.
packages/core-services/src/types/IFederationMatrixService.ts (2)
2-2: LGTM: Clean import reduction.Removing unused EventID and PduForType type imports improves code hygiene, especially after the emitJoin method removal.
4-31: Verification complete: emitJoin removal has no orphaned calls.Confirmed that no references to
emitJoinremain in the codebase. The single implementation ofIFederationMatrixService(classFederationMatrixinee/packages/federation-matrix/src/FederationMatrix.ts) has been properly updated. The migration to the federationSDK pattern is complete with no orphaned call sites.ee/packages/federation-matrix/src/services/MatrixMediaService.ts (2)
3-3: LGTM! Clean migration to federationSDK.The import change aligns with the PR's architectural shift from service-based dependency injection to the centralized federationSDK singleton pattern.
82-136: This review comment targets deprecated federation code.Matrix federation is being deprecated, with Rocket.Chat's native built-in federation launching in September 2025. As noted in the learnings, the Federation_Matrix settings are part of the old federation system being removed. The
MatrixMediaServicemigration to usefederationSDKis part of this deprecated codebase; initialization order concerns do not apply to code scheduled for removal.packages/core-services/package.json (1)
38-38: LGTM! Dependency upgrade aligns with PR objectives.The federation-sdk version bump from 0.2.0 to 0.3.0 is consistent with the broader migration to the federationSDK singleton pattern.
ee/packages/federation-matrix/src/api/_matrix/transactions.ts (1)
1-2: LGTM! Clean migration to federationSDK.The refactoring successfully removes HomeserverServices dependency and adopts the federationSDK singleton pattern. Error handling is properly maintained, and the API contract remains consistent.
Also applies to: 318-366
ee/packages/federation-matrix/src/events/member.ts (1)
3-3: LGTM! Clean migration to federationSDK.The refactoring successfully centralizes serverName resolution through federationSDK.getConfig('serverName'), removing the services parameter dependency throughout the membership event handlers.
Also applies to: 18-18, 55-55, 88-96
ee/packages/federation-matrix/package.json (2)
41-41: LGTM! Dependency upgrade aligns with PR objectives.The federation-sdk version bump to 0.3.0 is consistent with the migration strategy.
50-50: Pino downgrade does not appear necessary, but verify rationale.The codebase specifies Node.js 22.16.0, which is fully compatible with pino ^9.11.0. The only breaking change between pino 8.21.0 and 9.11.0 is that v9.0.0 dropped support for Node.js 14, 16, and 19. The downgrade appears coordinated across all 13 packages with no detected version conflicts, but the rationale is unclear. Confirm whether this downgrade to ^8.21.0 was intentional and document the specific reason.
ee/packages/federation-matrix/src/index.ts (1)
9-9: LGTM! New public API surface added.The addition of configureFederationMatrixSettings export aligns with the PR objective to allow federation setting changes without requiring a restart.
apps/meteor/package.json (1)
255-255: LGTM! Dependency upgrade aligns with PR objectives.The federation-sdk version bump to 0.3.0 in the meteor package is consistent with the broader migration strategy.
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
1-1: LGTM! Clean migration to federationSDK singleton.The middleware successfully removes the EventAuthorizationService dependency and adopts federationSDK.verifyRequestSignature. Error handling and authentication flow remain properly intact.
Also applies to: 5-39
ee/packages/federation-matrix/src/events/message.ts (1)
4-11: LGTM! Clean migration to federationSDK configuration.The refactoring successfully removes the serverName parameter dependency and centralizes configuration access through federationSDK.getConfig('serverName'). Message processing logic and error handling remain properly intact.
Also applies to: 121-121, 144-145
ee/packages/federation-matrix/src/api/_matrix/rooms.ts (3)
126-128: LGTM! Clean migration to federationSDK pattern.The function signature and middleware usage have been correctly updated to use the federationSDK singleton instead of injected HomeserverServices. This aligns with the PR objective of enabling dynamic configuration changes.
147-147: LGTM! Consistent federationSDK usage.The migration from
services.state.getAllPublicRoomIdsAndNames()tofederationSDK.getAllPublicRoomIdsAndNames()is correct and maintains the same behavior.
182-182: LGTM! Consistent with GET endpoint.The POST endpoint correctly uses the same federationSDK pattern as the GET endpoint at line 147.
ee/packages/federation-matrix/src/FederationMatrix.ts (5)
11-12: LGTM! Import updates correctly reflect the migration.The imports now use
federationSDKsingleton and relevant types from@rocket.chat/federation-sdk, removing dependencies on HomeserverServices.
220-220: LGTM! Room and message operations correctly migrated.All room creation, direct message creation, and message sending operations now use federationSDK with proper schema validation (userIdSchema, roomIdSchema). The migration maintains type safety and error handling patterns.
Also applies to: 288-296, 308-308, 370-370, 402-408
534-534: LGTM! Deletion, reactions, and room management migrated correctly.Message deletion (redactMessage), reaction operations (sendReaction/unsetReaction), and room management (inviteUserToRoom, leaveRoom, kickUser) all correctly use federationSDK with appropriate schema validation.
Also applies to: 550-550, 564-564, 600-605, 650-655, 689-689, 708-713
671-671: LGTM! Update operations correctly implemented.Event retrieval, message updates, room metadata updates (name/topic), power level changes, and typing notifications all use federationSDK correctly. The
voidusage for sendTypingNotification (line 852) is appropriate for fire-and-forget behavior.Also applies to: 742-748, 770-770, 785-785, 822-827, 852-852
876-885: LGTM! Profile query correctly migrated.The
verifyMatrixIdsmethod now usesfederationSDK.queryProfileRemotewith proper type safety through generics and maintains the same error handling logic.ee/packages/federation-matrix/src/api/_matrix/profiles.ts (3)
355-357: LGTM! Consistent with migration pattern.Function signature and middleware usage correctly updated to use parameterless federationSDK pattern.
371-371: LGTM! All profile and federation operations correctly use federationSDK.Query profile, query keys, make join, get missing events, and event auth all correctly migrated with proper schema validation.
Also applies to: 401-401, 446-450, 477-482, 504-504
440-440: LGTM! Resource access middleware correctly updated.All
canAccessResourceMiddlewarecalls now use the simplified signature with only the entityType parameter, consistent with the migration to federationSDK.Also applies to: 472-472, 500-500
ee/packages/federation-matrix/src/events/edu.ts (2)
10-15: LGTM! Dynamic configuration pattern enables runtime changes.The function now retrieves configuration dynamically via
federationSDK.getConfig('edu')on each event instead of accepting static parameters. This design choice directly supports the PR objective of allowing federation setting changes without requiring a restart.
34-38: LGTM! Consistent dynamic configuration pattern.Presence event handling uses the same dynamic configuration pattern as typing (lines 12-13), ensuring consistent behavior across both EDU types.
ee/packages/federation-matrix/src/api/routes.ts (1)
23-33: LGTM! Route composition correctly simplified.All route builder invocations have been updated to use the parameterless pattern, consistent with the migration from HomeserverServices to federationSDK. The middleware chain and routing logic remain intact.
ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (2)
26-38: LGTM! Middleware correctly uses federationSDK and context.The
canAccessResourcemiddleware now usesfederationSDK.canAccessResourceand retrieves the authenticated server from the context (set byisAuthenticatedMiddleware), eliminating the need for explicit parameter passing.
55-56: LGTM! Simplified middleware composition.The exported
canAccessResourceMiddlewarenow requires only the entityType parameter, composingisAuthenticatedMiddleware()andcanAccessResource(entityType)cleanly with theeverycombinator.ee/packages/federation-matrix/src/setup.ts (3)
10-35: LGTM!The domain validation logic is correct and handles edge cases appropriately (empty values, uppercase letters, invalid URLs).
96-99: LGTM!The EDU configuration section correctly maps the typing and presence settings from the input parameters.
104-106: Clarify database configuration requirements.The TODO comment suggests uncertainty about whether
MONGO_URLandDATABASE_NAMEenvironment variables are required. Verify whether these are mandatory for the federation SDK initialization or if there are alternative configuration paths.ee/packages/federation-matrix/src/api/_matrix/invite.ts (4)
163-163: LGTM!The migration to
federationSDK.joinUseris correct and consistent with the new SDK-based architecture.
166-169: LGTM!The migration to
federationSDK.getLatestRoomState2is correct and properly handles the case when the room is not found.
266-302: LGTM!The refactoring correctly removes the
servicesparameter dependency and usesfederationSDK.getConfig('serverName')to obtain the server name. The logic remains intact while successfully migrating to the new SDK-based architecture.
304-398: LGTM!The route builder has been successfully migrated to the parameterless pattern, using
isAuthenticatedMiddleware()without arguments andfederationSDK.processInvitefor invite processing. The refactoring is consistent with the broader migration to the federationSDK-centric architecture.
f8e430a to
dfe98bc
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ee/packages/federation-matrix/src/setup.ts (1)
72-72: Consider tracking the TODO for signingKeyPath removal.The TODO comment indicates that
signingKeyPathshould be removed, but it's currently hardcoded to an empty string. If the SDK still requires this parameter, consider opening an issue to track its removal in a future version.Would you like me to open an issue to track this technical debt?
ee/packages/federation-matrix/src/events/member.ts (1)
82-82: Use logger.warn for consistency.Line 82 uses
console.warnwhile the rest of the file uses theloggerinstance. Update tologger.warnfor consistent logging.Apply this diff:
- console.warn(`User with ID ${insertedId} not found after insertion`); + logger.warn(`User with ID ${insertedId} not found after insertion`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
apps/meteor/ee/server/startup/federation.ts(1 hunks)apps/meteor/package.json(1 hunks)ee/packages/federation-matrix/package.json(2 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(21 hunks)ee/packages/federation-matrix/src/api/.well-known/server.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(7 hunks)ee/packages/federation-matrix/src/api/_matrix/key/server.ts(3 hunks)ee/packages/federation-matrix/src/api/_matrix/media.ts(4 hunks)ee/packages/federation-matrix/src/api/_matrix/profiles.ts(7 hunks)ee/packages/federation-matrix/src/api/_matrix/rooms.ts(4 hunks)ee/packages/federation-matrix/src/api/_matrix/send-join.ts(3 hunks)ee/packages/federation-matrix/src/api/_matrix/transactions.ts(10 hunks)ee/packages/federation-matrix/src/api/_matrix/versions.ts(3 hunks)ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts(4 hunks)ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts(2 hunks)ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.ts(1 hunks)ee/packages/federation-matrix/src/api/routes.ts(1 hunks)ee/packages/federation-matrix/src/events/edu.ts(2 hunks)ee/packages/federation-matrix/src/events/index.ts(2 hunks)ee/packages/federation-matrix/src/events/member.ts(4 hunks)ee/packages/federation-matrix/src/events/message.ts(3 hunks)ee/packages/federation-matrix/src/events/room.ts(3 hunks)ee/packages/federation-matrix/src/index.ts(1 hunks)ee/packages/federation-matrix/src/services/MatrixMediaService.ts(2 hunks)ee/packages/federation-matrix/src/setup.ts(3 hunks)packages/core-services/package.json(1 hunks)packages/core-services/src/types/IFederationMatrixService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/meteor/package.json
- ee/packages/federation-matrix/src/events/edu.ts
- ee/packages/federation-matrix/src/api/.well-known/server.ts
- ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts
- ee/packages/federation-matrix/src/events/message.ts
- packages/core-services/package.json
- ee/packages/federation-matrix/src/services/MatrixMediaService.ts
- ee/packages/federation-matrix/src/api/_matrix/media.ts
- ee/packages/federation-matrix/package.json
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.767Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.tsee/packages/federation-matrix/src/events/index.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/room.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.ts
📚 Learning: 2025-11-05T21:04:35.767Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.767Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/events/index.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsee/packages/federation-matrix/src/index.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/room.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
ee/packages/federation-matrix/src/events/index.tsapps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/room.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/middlewares/canAccessResource.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/setup.tsee/packages/federation-matrix/src/events/room.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-11-05T20:53:57.749Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: apps/meteor/ee/server/startup/federation.ts:39-74
Timestamp: 2025-11-05T20:53:57.749Z
Learning: In Rocket.Chat (apps/meteor/app/settings/server/CachedSettings.ts), the settings.watchMultiple() method immediately invokes its callback with current values if all requested settings exist in the store, then continues watching for subsequent changes. It does not wait for a setting to change before the first invocation.
Applied to files:
apps/meteor/ee/server/startup/federation.tsee/packages/federation-matrix/src/setup.ts
📚 Learning: 2025-09-16T22:09:18.041Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-09-16T22:09:18.041Z
Learning: Use Rocket.Chat primary documentation and provided reference files for guidance
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/room.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/room.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
🧬 Code graph analysis (13)
ee/packages/federation-matrix/src/events/index.ts (5)
ee/packages/federation-matrix/src/events/message.ts (1)
message(121-419)ee/packages/federation-matrix/src/events/reaction.ts (1)
reaction(10-93)ee/packages/federation-matrix/src/events/member.ts (1)
member(88-104)ee/packages/federation-matrix/src/events/edu.ts (1)
edus(10-85)ee/packages/federation-matrix/src/events/room.ts (1)
room(8-80)
apps/meteor/ee/server/startup/federation.ts (5)
packages/core-services/src/index.ts (1)
License(162-162)ee/packages/federation-matrix/src/setup.ts (2)
configureFederationMatrixSettings(37-101)setupFederationMatrix(103-120)packages/models/src/index.ts (1)
InstanceStatus(164-164)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(137-901)apps/meteor/ee/server/api/federation.ts (1)
registerFederationRoutes(43-52)
ee/packages/federation-matrix/src/api/_matrix/send-join.ts (1)
ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
ee/packages/federation-matrix/src/FederationMatrix.ts (3)
ee/packages/federation-matrix/src/events/room.ts (1)
room(8-80)ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
acceptInvite(266-302)packages/core-typings/src/IUser.ts (2)
IUser(186-255)isUserNativeFederated(276-277)
ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
ee/packages/federation-matrix/src/api/_matrix/profiles.ts (2)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (2)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
getUsernameServername(73-85)ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
ee/packages/federation-matrix/src/events/member.ts (2)
ee/packages/federation-matrix/src/events/room.ts (1)
room(8-80)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
getUsernameServername(73-85)
ee/packages/federation-matrix/src/setup.ts (2)
ee/packages/federation-matrix/src/index.ts (2)
configureFederationMatrixSettings(9-9)setupFederationMatrix(9-9)ee/packages/federation-matrix/src/events/index.ts (1)
registerEvents(11-18)
ee/packages/federation-matrix/src/events/room.ts (1)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
getUsernameServername(73-85)
ee/packages/federation-matrix/src/api/_matrix/transactions.ts (2)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (1)
canAccessResourceMiddleware(55-56)
ee/packages/federation-matrix/src/api/routes.ts (4)
ee/packages/federation-matrix/src/api/_matrix/key/server.ts (1)
getKeyServerRoutes(35-54)ee/packages/federation-matrix/src/api/_matrix/versions.ts (1)
getFederationVersionsRoutes(28-55)ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
getMatrixInviteRoutes(304-398)ee/packages/federation-matrix/src/api/.well-known/server.ts (1)
getWellKnownRoutes(22-46)
ee/packages/federation-matrix/src/api/_matrix/rooms.ts (1)
ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🚀 Notify external services - draft
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (23)
packages/core-services/src/types/IFederationMatrixService.ts (2)
2-2: LGTM! Import cleanup aligns with the API surface reduction.The removal of
EventIDandPduForTypeimports is consistent with the removal of theemitJoinmethod from the interface, whileEventStoreis correctly retained for thegetEventByIdmethod signature on line 12.
4-31: All usages of the removedemitJoinmethod have been properly migrated.Verification confirms:
- No references to
emitJoinexist in the codebase- The
FederationMatrixclass correctly implements all 17 methods ofIFederationMatrixService- No broken call sites or incomplete migrations remain
The breaking change has been cleanly executed with no orphaned references.
ee/packages/federation-matrix/src/api/_matrix/profiles.ts (1)
1-512: LGTM! Clean refactor to federationSDK.The migration from HomeserverServices to the centralized federationSDK is well-executed. All route handlers now use federationSDK methods consistently, and middleware calls have been properly updated to remove the federationAuth parameter.
ee/packages/federation-matrix/src/api/middlewares/isFederationEnabled.ts (1)
5-5: TODO noted for future enhancement.The comment suggests a future refactor to use federationSDK for the enabled check, which would complete the migration. Current implementation via Settings service is fine for now.
ee/packages/federation-matrix/src/index.ts (1)
9-9: LGTM! Public API expanded appropriately.Adding
configureFederationMatrixSettingsto the exports enables dynamic configuration changes, which aligns with the PR's objective of allowing federation setting changes without requiring a restart.ee/packages/federation-matrix/src/api/_matrix/send-join.ts (1)
1-252: LGTM! Consistent refactor pattern.The send-join route has been successfully migrated to use federationSDK, following the same clean pattern applied throughout the codebase.
ee/packages/federation-matrix/src/api/_matrix/key/server.ts (1)
1-54: LGTM! Clean migration.Key server route successfully migrated to federationSDK with the same consistent pattern.
ee/packages/federation-matrix/src/events/index.ts (1)
11-17: LGTM! Simplified event registration.The registerEvents function has been streamlined to only accept the emitter, with event handlers now obtaining configuration from federationSDK directly. This simplification aligns with the broader architectural change.
ee/packages/federation-matrix/src/api/_matrix/rooms.ts (1)
1-208: LGTM! Rooms API migrated successfully.Both public rooms endpoints (GET and POST) have been cleanly migrated to use federationSDK, maintaining consistent behavior while removing the HomeserverServices dependency.
ee/packages/federation-matrix/src/api/_matrix/transactions.ts (1)
1-509: LGTM! Comprehensive transaction routes migration.All transaction-related endpoints have been successfully migrated to federationSDK. The refactor covers multiple complex routes (send, state_ids, state, event, backfill) while preserving all error handling logic.
apps/meteor/ee/server/startup/federation.ts (1)
17-36: ResetserviceEnabledwhen configuration failsIf
configureFederationMatrixSettings()throws (e.g., invalid domain, missing signing key), we log the error but keepserviceEnabledflagged as true. The typing listener and other guarded paths will then execute while the SDK remains unconfigured, leading tofederationSDK.getConfig(...)returningundefinedand breaking downstream handlers/routes. Please revert the flag when setup fails so the service stays disabled until a valid configuration is applied.Apply this diff to ensure the flag reflects the actual state:
- serviceEnabled = (await License.hasModule('federation')) && settings.get('Federation_Service_Enabled'); - if (!serviceEnabled) { + const hasLicense = await License.hasModule('federation'); + const isSettingEnabled = settings.get('Federation_Service_Enabled'); + + serviceEnabled = hasLicense && isSettingEnabled; + if (!serviceEnabled) { return; } try { configureFederationMatrixSettings({ @@ processEDUPresence: settings.get('Federation_Service_EDU_Process_Presence'), }); } catch (error) { + serviceEnabled = false; logger.error('Failed to start federation-matrix service:', error); }⛔ Skipped due to learnings
Learnt from: sampaiodiego Repo: RocketChat/Rocket.Chat PR: 37357 File: ee/packages/federation-matrix/src/setup.ts:103-120 Timestamp: 2025-11-05T21:04:35.767Z Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.Learnt from: rodrigok Repo: RocketChat/Rocket.Chat PR: 36991 File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221 Timestamp: 2025-09-19T15:15:04.642Z Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.Learnt from: ricardogarim Repo: RocketChat/Rocket.Chat PR: 37377 File: apps/meteor/ee/server/hooks/federation/index.ts:86-88 Timestamp: 2025-11-04T16:49:19.107Z Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.ee/packages/federation-matrix/src/setup.ts (2)
3-3: LGTM!The import changes align with the migration to federationSDK-based initialization and configuration flow used throughout the file.
104-106: Remove or make dbConfig optional in init() call.Based on the learning context from your previous work, the SDK's
init()method establishes infrastructure (including database setup) automatically. ThedbConfigparameter withmongoUrianddbNameappears unnecessary here—configuration is applied later via settings watchers only when federation events occur. The TODO comment is justified; consider removing thedbConfigparameter entirely or making it conditional if the SDK requires it for backward compatibility.ee/packages/federation-matrix/src/events/member.ts (3)
3-3: LGTM!The addition of
federationSDKimport supports the migration away from passingservicesas a parameter, allowing direct configuration access viafederationSDK.getConfig('serverName').
11-46: LGTM!The refactoring properly handles both voluntary leaves and kicks, retrieving
serverNamedirectly fromfederationSDK.getConfig()instead of requiring it as a parameter. The kick handling correctly identifies the kicker and passes appropriate context toremoveUserFromRoom.
88-104: LGTM!The simplified function signature and error handling are appropriate. The event handler properly delegates to the refactored action functions without passing the
servicesparameter.ee/packages/federation-matrix/src/api/_matrix/invite.ts (4)
1-4: LGTM!The import changes properly support the migration to
federationSDK-based operations, adding necessary types and removing the old service-based imports.
150-259: LGTM!The
joinRoomfunction properly migrates tofederationSDKmethods for user joining and room state retrieval. The room creation logic appropriately handles different room types (DM, public, private).
266-302: LGTM!The
acceptInviterefactoring properly retrieves configuration fromfederationSDKand maintains the validation logic for preventing native federated users from participating in internal invites.
304-398: LGTM!The route factory function properly migrates to a parameterless signature, using
federationSDK.processInviteand the updatedisAuthenticatedMiddleware(). Error handling appropriately distinguishes betweenNotAllowedError(403) and generic errors (500).ee/packages/federation-matrix/src/api/middlewares/canAccessResource.ts (3)
1-1: LGTM!The import changes properly support the migration to
federationSDK-based resource access checks, removing the old service-based imports.
26-53: LGTM!The refactored function properly delegates resource access checks to
federationSDK.canAccessResource, retrieving the authenticated server from the request context instead of requiring a separatefederationAuthparameter.
55-56: LGTM!The simplified signature properly composes
isAuthenticatedMiddleware()withcanAccessResource(), eliminating the need to passfederationAuthas a parameter.
depends on RocketChat/homeserver#292
replaces #37289
Proposed changes (including videos or screenshots)
Issue(s)
FDR-156
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
Chores
Bug Fixes