chore: remove old federation #37059
Merged
chore: remove old federation #37059
Conversation
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
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
WalkthroughLarge-scale removal of legacy federation and DNS functionality across server and client: endpoints, hooks, handlers, helpers, normalizers, configuration, cron, statistics, admin UI, and REST typings. Startup now explicitly calls generateFederationKeys. Directory page hard-disables federation UI logic. DNS v1 API and typings are removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Startup as Server Startup
participant Migrate as performMigrationProcedure()
participant Keys as generateFederationKeys()
participant Cron as startCronJobs()
Startup->>Migrate: Run migrations
Migrate-->>Startup: Done
Startup->>Keys: Generate federation keys (if missing)
Keys-->>Startup: Done
Note over Startup,Cron: Federation cron/configuration removed
Startup->>Cron: Start remaining cron jobs
Cron-->>Startup: Scheduled
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37059 +/- ##
===========================================
- Coverage 67.35% 67.35% -0.01%
===========================================
Files 3325 3326 +1
Lines 113211 113195 -16
Branches 20541 20535 -6
===========================================
- Hits 76256 76242 -14
- Misses 34348 34350 +2
+ Partials 2607 2603 -4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/directory/DirectoryPage.tsx (1)
17-64: Keep the Directory usable when the default tab is still set to “external”.Many workspaces have
Accounts_Directory_DefaultViewpersisted as'external'. With federation now hard-disabled,tabwill remain'external', no tab highlights, and the page renders empty content. We need to normalize the default and current tab when federation is off so the UI falls back to a supported tab instead of getting stuck.- const defaultTab = useSetting<TabName>('Accounts_Directory_DefaultView', 'users'); - const federationEnabled = false; // TODO old federation removed - const tab = useRouteParameter('tab') as TabName | undefined; + const rawDefaultTab = useSetting<TabName>('Accounts_Directory_DefaultView', 'users'); + const federationEnabled = false; // TODO old federation removed + const defaultTab = rawDefaultTab === 'external' ? 'users' : rawDefaultTab; + const tabParam = useRouteParameter('tab') as TabName | undefined; + const tab = tabParam === 'external' && !federationEnabled ? defaultTab : tabParam;
📜 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 selected for processing (69)
apps/meteor/app/api/server/index.ts(0 hunks)apps/meteor/app/api/server/v1/dns.ts(0 hunks)apps/meteor/app/federation/README.md(0 hunks)apps/meteor/app/federation/server/constants.ts(0 hunks)apps/meteor/app/federation/server/endpoints/dispatch.js(0 hunks)apps/meteor/app/federation/server/endpoints/index.ts(0 hunks)apps/meteor/app/federation/server/endpoints/requestFromLatest.js(0 hunks)apps/meteor/app/federation/server/endpoints/uploads.js(0 hunks)apps/meteor/app/federation/server/endpoints/users.js(0 hunks)apps/meteor/app/federation/server/functions/addUser.js(0 hunks)apps/meteor/app/federation/server/functions/dashboard.js(0 hunks)apps/meteor/app/federation/server/functions/errors.js(0 hunks)apps/meteor/app/federation/server/functions/helpers.ts(0 hunks)apps/meteor/app/federation/server/functions/resolveDNS.ts(0 hunks)apps/meteor/app/federation/server/handler/index.ts(0 hunks)apps/meteor/app/federation/server/hooks/afterAddedToRoom.js(0 hunks)apps/meteor/app/federation/server/hooks/afterCreateDirectRoom.js(0 hunks)apps/meteor/app/federation/server/hooks/afterCreateRoom.js(0 hunks)apps/meteor/app/federation/server/hooks/afterDeleteMessage.js(0 hunks)apps/meteor/app/federation/server/hooks/afterLeaveRoom.js(0 hunks)apps/meteor/app/federation/server/hooks/afterMuteUser.js(0 hunks)apps/meteor/app/federation/server/hooks/afterRemoveFromRoom.js(0 hunks)apps/meteor/app/federation/server/hooks/afterSaveMessage.js(0 hunks)apps/meteor/app/federation/server/hooks/afterSetReaction.js(0 hunks)apps/meteor/app/federation/server/hooks/afterUnmuteUser.js(0 hunks)apps/meteor/app/federation/server/hooks/afterUnsetReaction.js(0 hunks)apps/meteor/app/federation/server/hooks/beforeDeleteRoom.js(0 hunks)apps/meteor/app/federation/server/index.ts(0 hunks)apps/meteor/app/federation/server/lib/callbacks.ts(0 hunks)apps/meteor/app/federation/server/lib/context.ts(0 hunks)apps/meteor/app/federation/server/lib/crypt.js(0 hunks)apps/meteor/app/federation/server/lib/dns.js(0 hunks)apps/meteor/app/federation/server/lib/getFederationDiscoveryMethod.ts(0 hunks)apps/meteor/app/federation/server/lib/getFederationDomain.ts(0 hunks)apps/meteor/app/federation/server/lib/http.js(0 hunks)apps/meteor/app/federation/server/lib/isFederationEnabled.ts(0 hunks)apps/meteor/app/federation/server/lib/logger.ts(0 hunks)apps/meteor/app/federation/server/methods/dashboard.ts(0 hunks)apps/meteor/app/federation/server/methods/index.ts(0 hunks)apps/meteor/app/federation/server/methods/loadContextEvents.ts(0 hunks)apps/meteor/app/federation/server/methods/testSetup.ts(0 hunks)apps/meteor/app/federation/server/normalizers/index.ts(0 hunks)apps/meteor/app/federation/server/normalizers/message.js(0 hunks)apps/meteor/app/federation/server/normalizers/room.js(0 hunks)apps/meteor/app/federation/server/normalizers/subscription.js(0 hunks)apps/meteor/app/federation/server/normalizers/user.js(0 hunks)apps/meteor/app/federation/server/startup/index.ts(0 hunks)apps/meteor/app/federation/server/startup/registerCallbacks.js(0 hunks)apps/meteor/app/statistics/server/lib/statistics.ts(0 hunks)apps/meteor/client/views/admin/federationDashboard/FederationDashboardPage.stories.tsx(0 hunks)apps/meteor/client/views/admin/federationDashboard/FederationDashboardPage.tsx(0 hunks)apps/meteor/client/views/admin/federationDashboard/FederationDashboardRoute.tsx(0 hunks)apps/meteor/client/views/admin/federationDashboard/OverviewSection.stories.tsx(0 hunks)apps/meteor/client/views/admin/federationDashboard/OverviewSection.tsx(0 hunks)apps/meteor/client/views/admin/federationDashboard/ServersSection.stories.tsx(0 hunks)apps/meteor/client/views/admin/federationDashboard/ServersSection.tsx(0 hunks)apps/meteor/client/views/admin/routes.tsx(0 hunks)apps/meteor/client/views/directory/DirectoryPage.tsx(1 hunks)apps/meteor/server/configuration/federation.ts(0 hunks)apps/meteor/server/configuration/index.ts(0 hunks)apps/meteor/server/cron/federation.ts(0 hunks)apps/meteor/server/importPackages.ts(0 hunks)apps/meteor/server/methods/browseChannels.ts(1 hunks)apps/meteor/server/methods/createDirectMessage.ts(1 hunks)apps/meteor/server/startup/cron.ts(0 hunks)apps/meteor/server/startup/generateKeys.ts(1 hunks)apps/meteor/server/startup/index.ts(2 hunks)packages/rest-typings/src/index.ts(0 hunks)packages/rest-typings/src/v1/dns.ts(0 hunks)
💤 Files with no reviewable changes (64)
- apps/meteor/app/federation/server/lib/context.ts
- apps/meteor/client/views/admin/routes.tsx
- apps/meteor/client/views/admin/federationDashboard/ServersSection.stories.tsx
- apps/meteor/app/federation/server/methods/testSetup.ts
- apps/meteor/app/federation/server/lib/getFederationDiscoveryMethod.ts
- apps/meteor/app/federation/README.md
- apps/meteor/app/federation/server/methods/loadContextEvents.ts
- apps/meteor/app/federation/server/hooks/afterAddedToRoom.js
- apps/meteor/client/views/admin/federationDashboard/FederationDashboardPage.tsx
- apps/meteor/app/federation/server/normalizers/room.js
- apps/meteor/app/federation/server/functions/errors.js
- apps/meteor/client/views/admin/federationDashboard/FederationDashboardRoute.tsx
- apps/meteor/app/federation/server/functions/resolveDNS.ts
- apps/meteor/app/federation/server/hooks/afterSaveMessage.js
- apps/meteor/app/federation/server/hooks/afterUnmuteUser.js
- apps/meteor/app/federation/server/normalizers/index.ts
- apps/meteor/server/importPackages.ts
- apps/meteor/server/startup/cron.ts
- apps/meteor/app/api/server/v1/dns.ts
- apps/meteor/app/federation/server/lib/dns.js
- apps/meteor/app/federation/server/hooks/afterCreateRoom.js
- apps/meteor/app/federation/server/methods/dashboard.ts
- apps/meteor/app/federation/server/hooks/afterLeaveRoom.js
- apps/meteor/app/federation/server/index.ts
- apps/meteor/app/federation/server/lib/logger.ts
- apps/meteor/client/views/admin/federationDashboard/OverviewSection.tsx
- apps/meteor/server/configuration/federation.ts
- apps/meteor/app/federation/server/hooks/afterCreateDirectRoom.js
- apps/meteor/app/federation/server/lib/crypt.js
- apps/meteor/app/api/server/index.ts
- apps/meteor/app/federation/server/endpoints/index.ts
- apps/meteor/app/federation/server/normalizers/user.js
- apps/meteor/app/federation/server/lib/getFederationDomain.ts
- apps/meteor/app/federation/server/lib/http.js
- apps/meteor/app/federation/server/functions/dashboard.js
- packages/rest-typings/src/v1/dns.ts
- packages/rest-typings/src/index.ts
- apps/meteor/app/federation/server/lib/isFederationEnabled.ts
- apps/meteor/app/federation/server/endpoints/users.js
- apps/meteor/app/federation/server/normalizers/message.js
- apps/meteor/app/federation/server/normalizers/subscription.js
- apps/meteor/client/views/admin/federationDashboard/OverviewSection.stories.tsx
- apps/meteor/app/federation/server/startup/index.ts
- apps/meteor/server/configuration/index.ts
- apps/meteor/app/federation/server/endpoints/requestFromLatest.js
- apps/meteor/app/federation/server/functions/helpers.ts
- apps/meteor/app/statistics/server/lib/statistics.ts
- apps/meteor/app/federation/server/hooks/afterDeleteMessage.js
- apps/meteor/app/federation/server/hooks/beforeDeleteRoom.js
- apps/meteor/app/federation/server/endpoints/dispatch.js
- apps/meteor/app/federation/server/methods/index.ts
- apps/meteor/app/federation/server/hooks/afterRemoveFromRoom.js
- apps/meteor/app/federation/server/functions/addUser.js
- apps/meteor/client/views/admin/federationDashboard/FederationDashboardPage.stories.tsx
- apps/meteor/app/federation/server/hooks/afterUnsetReaction.js
- apps/meteor/app/federation/server/constants.ts
- apps/meteor/server/cron/federation.ts
- apps/meteor/app/federation/server/hooks/afterMuteUser.js
- apps/meteor/app/federation/server/startup/registerCallbacks.js
- apps/meteor/app/federation/server/hooks/afterSetReaction.js
- apps/meteor/app/federation/server/lib/callbacks.ts
- apps/meteor/app/federation/server/endpoints/uploads.js
- apps/meteor/app/federation/server/handler/index.ts
- apps/meteor/client/views/admin/federationDashboard/ServersSection.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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.
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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:
apps/meteor/client/views/directory/DirectoryPage.tsxapps/meteor/server/methods/browseChannels.ts
🧬 Code graph analysis (2)
apps/meteor/server/startup/generateKeys.ts (1)
packages/models/src/index.ts (1)
FederationKeys(159-159)
apps/meteor/server/startup/index.ts (1)
apps/meteor/server/startup/generateKeys.ts (1)
generateFederationKeys(4-8)
⏰ 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). (17)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (1/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (4/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (2/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (5/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (1/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (5/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (4/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (1/5) - Alpine (Official)
- GitHub Check: 🔨 Test API (EE) / MongoDB 5.0 (1/1) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (2/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (2/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (2/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (1/4) - Alpine (Official)
🔇 Additional comments (2)
apps/meteor/server/startup/generateKeys.ts (1)
4-8: Explicit federation key generation helper looks goodThe extracted async helper keeps the existing logic intact while making invocation explicit for callers.
apps/meteor/server/startup/index.ts (1)
7-20: Startup now awaits key generation at the right timeImporting and awaiting
generateFederationKeys()immediately after migrations ensures the keys are ready before cron jobs and other services start up.
Comment on lines
+233
to
238
| const { cursor, totalCount } = Users.findPaginatedByActiveExternalUsersExcept<FederatedUser>(text, [], options, searchFields); | ||
| const [results, total] = await Promise.all([cursor.toArray(), totalCount]); | ||
| return { | ||
| total, | ||
| results, | ||
| }; |
Contributor
There was a problem hiding this comment.
Keep federation fallback until external search is re-scoped.
We still expose the “external” workspace selector to clients, but without the remote-search fallback the branch now returns only local records marked as external. On production that set is usually empty—the “external” tab was populated exclusively by the federation search results we just removed—so the UI turns into a blank list. Either retire the workspace === 'external' path end-to-end or keep the previous federation-backed search until the client is adjusted; otherwise we’re shipping a regression for anyone clicking the External tab.
🤖 Prompt for AI Agents
In apps/meteor/server/methods/browseChannels.ts around lines 233 to 238, the
current change replaced the federation-backed "external" workspace search with a
local-only query, causing the External tab to show empty results in production;
restore the federation fallback for workspace === 'external' (or fully
remove/retire that client-visible path) so behavior is unchanged: detect when
workspace === 'external' and call the previous federation/remote-search flow
(await remote cursor/totalCount or Promise.all as before) instead of only
querying local external-marked records, keeping existing pagination/results
shape; alternatively, if you choose to retire the external workspace, remove all
client exposure and ensure clients are updated before removing the fallback.
Comment on lines
+42
to
45
| const users = await Promise.all(usernames.filter((username) => username !== me.username)); | ||
| const roomUsers = excludeSelf ? users : [me, ...users]; | ||
|
|
||
| // allow self-DMs |
Contributor
There was a problem hiding this comment.
Blocking: roomUsers now contains raw strings and crashes on _id access
By dropping the per-username lookup, users becomes a plain string[]. When a caller lacks create-d but has view-d-room, we hit roomUsers.map(({ _id }) => _id) and immediately throw TypeError: Cannot destructure property '_id' of 'alice' as it is a string. Even when that path is skipped, createRoom still expects IUser objects (with _id, username, etc.), so the DM creation breaks.
Restore the user-document resolution (minus the old federation-specific branches) so roomUsers remains a list of IUser objects before proceeding.
- const users = await Promise.all(usernames.filter((username) => username !== me.username));
+ const users = await Promise.all(
+ usernames
+ .filter((username) => username !== me.username)
+ .map(async (username) => {
+ const user = await Users.findOneByUsername(username, { projection: { username: 1, name: 1 } });
+ if (!user) {
+ throw new Meteor.Error('error-invalid-user', 'Invalid user', {
+ method: 'createDirectMessage',
+ });
+ }
+ return user;
+ }),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const users = await Promise.all(usernames.filter((username) => username !== me.username)); | |
| const roomUsers = excludeSelf ? users : [me, ...users]; | |
| // allow self-DMs | |
| const users = await Promise.all( | |
| usernames | |
| .filter((username) => username !== me.username) | |
| .map(async (username) => { | |
| const user = await Users.findOneByUsername(username, { projection: { username: 1, name: 1 } }); | |
| if (!user) { | |
| throw new Meteor.Error('error-invalid-user', 'Invalid user', { | |
| method: 'createDirectMessage', | |
| }); | |
| } | |
| return user; | |
| }), | |
| ); | |
| const roomUsers = excludeSelf ? users : [me, ...users]; | |
| // allow self-DMs |
🤖 Prompt for AI Agents
In apps/meteor/server/methods/createDirectMessage.ts around lines 42 to 45,
roomUsers currently contains raw username strings because the per-username
lookup was removed; this causes TypeError when code later destructures _id and
when createRoom expects IUser objects. Reintroduce the username→user-document
resolution: map the filtered username list to async lookups (e.g. find user by
username), await Promise.all to produce IUser[], filter out any not-found users,
then build roomUsers as excludeSelf ? users : [me, ...users] so downstream code
always receives IUser objects. Ensure you remove federation-specific branches
but keep error handling for missing users and preserve allow self-DMs behavior.
ggazzo
approved these changes
Sep 25, 2025
This was referenced Oct 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
https://rocketchat.atlassian.net/browse/FDR-143
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Chores
Documentation