Skip to content

refactor(federation): Branded types #37131

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.

Sign up for GitHub

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

Merged
ggazzo merged 8 commits into from
Oct 8, 2025
Merged

refactor(federation): Branded types #37131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fff5c36
refactor(federation): enhance validation logic by integrating schemas…
ggazzo Oct 2, 2025
b38e46d
feat(callbacks): add beforeAddUsersToRoom callback for batch user add…
ggazzo Oct 3, 2025
fad30a0
fix upsert using ObjectId
sampaiodiego Oct 3, 2025
58c9b07
fix: process joins from full state (#37156)
debdutdeb Oct 7, 2025
697d294
chore: update dependencies in yarn.lock, including multiple Babel pac…
ggazzo Oct 7, 2025
2dbc26d
fix: kick users
ggazzo Oct 7, 2025
683a6dd
fix: topic not reflecting from matrix
rodrigok Oct 7, 2025
94445d8
fix(federation-matrix): add subscription check before user leaves roo…
ggazzo Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions apps/meteor/app/channel-settings/server/functions/saveRoomTopic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Message, Room } from '@rocket.chat/core-services';
import type { IUser } from '@rocket.chat/core-typings';
import { Rooms } from '@rocket.chat/models';
import { Match } from 'meteor/check';
import { Meteor } from 'meteor/meteor';
Expand All @@ -8,10 +9,7 @@ import { callbacks } from '../../../../lib/callbacks';
export const saveRoomTopic = async (
rid: string,
roomTopic: string | undefined,
user: {
username: string;
_id: string;
},
user: Pick<IUser, 'username' | '_id' | 'federation' | 'federated'>,
sendMessage = true,
) => {
if (!Match.test(rid, String)) {
Expand Down
3 changes: 3 additions & 0 deletions apps/meteor/app/lib/server/methods/addUsersToRoom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Subscriptions, Users, Rooms } from '@rocket.chat/models';
import { Match } from 'meteor/check';
import { Meteor } from 'meteor/meteor';

import { beforeAddUsersToRoom } from '../../../../lib/callbacks/beforeAddUserToRoom';
import { i18n } from '../../../../server/lib/i18n';
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { addUserToRoom } from '../functions/addUserToRoom';
Expand Down Expand Up @@ -77,6 +78,8 @@ export const addUsersToRoomMethod = async (userId: string, data: { rid: string;
});
}

await beforeAddUsersToRoom.run({ usernames: data.users, inviter: user }, room);

await Promise.all(
data.users.map(async (username) => {
const newUser = await Users.findOneByUsernameIgnoringCase(username);
Expand Down
8 changes: 7 additions & 1 deletion apps/meteor/ee/server/hooks/federation/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Rooms } from '@rocket.chat/models';
import { callbacks } from '../../../../lib/callbacks';
import { afterLeaveRoomCallback } from '../../../../lib/callbacks/afterLeaveRoomCallback';
import { afterRemoveFromRoomCallback } from '../../../../lib/callbacks/afterRemoveFromRoomCallback';
import { beforeAddUserToRoom } from '../../../../lib/callbacks/beforeAddUserToRoom';
import { beforeAddUsersToRoom, beforeAddUserToRoom } from '../../../../lib/callbacks/beforeAddUserToRoom';
import { beforeChangeRoomRole } from '../../../../lib/callbacks/beforeChangeRoomRole';
import { FederationActions } from '../../../../server/services/room/hooks/BeforeFederationActions';

Expand Down Expand Up @@ -70,6 +70,12 @@ callbacks.add(
'native-federation-after-delete-message',
);

beforeAddUsersToRoom.add(async ({ usernames }, room) => {
if (FederationActions.shouldPerformFederationAction(room)) {
await FederationMatrix.ensureFederatedUsersExistLocally(usernames);
}
});

beforeAddUserToRoom.add(
async ({ user, inviter }, room) => {
if (!user.username || !inviter) {
Expand Down
5 changes: 4 additions & 1 deletion apps/meteor/lib/callbacks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,10 @@ type ChainedCallbackSignatures = {
'roomAvatarChanged': (room: IRoom) => void;
'beforeGetMentions': (mentionIds: string[], teamMentions: MessageMention[]) => Promise<string[]>;
'livechat.manageDepartmentUnit': (params: { userId: string; departmentId: string; unitId?: string }) => void;
'afterRoomTopicChange': (params: undefined, { room, topic, user }: { room: IRoom; topic: string; user: IUser }) => void;
'afterRoomTopicChange': (
params: undefined,
{ room, topic, user }: { room: IRoom; topic: string; user: Pick<IUser, 'username' | '_id' | 'federation' | 'federated'> },
) => void;
};

export type Hook =
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/lib/callbacks/beforeAddUserToRoom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ import type { IUser, IRoom } from '@rocket.chat/core-typings';
import { Callbacks } from './callbacksBase';

export const beforeAddUserToRoom = Callbacks.create<(args: { user: IUser; inviter?: IUser }, room: IRoom) => void>('beforeAddUserToRoom');
export const beforeAddUsersToRoom =
Callbacks.create<(args: { usernames: string[]; inviter?: IUser }, room: IRoom) => void>('beforeAddUsersToRoom');
5 changes: 1 addition & 4 deletions apps/meteor/server/services/room/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,7 @@ export class RoomService extends ServiceClassInternal implements IRoomService {
async saveRoomTopic(
roomId: string,
roomTopic: string | undefined,
user: {
username: string;
_id: string;
},
user: Pick<IUser, 'username' | '_id' | 'federation' | 'federated'>,
sendMessage = true,
): Promise<void> {
await saveRoomTopic(roomId, roomTopic, user, sendMessage);
Expand Down
20 changes: 10 additions & 10 deletions apps/meteor/tests/end-to-end/api/livechat/14-units.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,8 +635,8 @@ import { IS_EE } from '../../../e2e/config/constants';
expect(updatedUnit).to.have.property('numDepartments', 1);

const fullDepartment = await getDepartmentById(department._id);
expect(fullDepartment).to.have.property('parentId').that.is.null;
expect(fullDepartment).to.have.property('ancestors').that.is.null;
expect(fullDepartment).to.not.have.property('parentId');
expect(fullDepartment).to.not.have.property('ancestors');
});

it('should fail adding a department into an existing unit that a monitor does not supervise', async () => {
Expand All @@ -658,8 +658,8 @@ import { IS_EE } from '../../../e2e/config/constants';
expect(updatedUnit).to.have.property('numDepartments', 1);

const fullDepartment = await getDepartmentById(department._id);
expect(fullDepartment).to.have.property('parentId').that.is.null;
expect(fullDepartment).to.have.property('ancestors').that.is.null;
expect(fullDepartment).to.not.have.property('parentId');
expect(fullDepartment).to.not.have.property('ancestors');
});

it('should succesfully add a department into an existing unit that a monitor supervises', async () => {
Expand Down Expand Up @@ -732,8 +732,8 @@ import { IS_EE } from '../../../e2e/config/constants';

const fullDepartment = await getDepartmentById(department._id);
expect(fullDepartment).to.have.property('name', updatedName);
expect(fullDepartment).to.have.property('parentId').that.is.null;
expect(fullDepartment).to.have.property('ancestors').that.is.null;
expect(fullDepartment).to.not.have.property('parentId');
expect(fullDepartment).to.not.have.property('ancestors');
});
});

Expand Down Expand Up @@ -872,8 +872,8 @@ import { IS_EE } from '../../../e2e/config/constants';
expect(updatedUnit).to.have.property('numDepartments', 1);

const fullDepartment = await getDepartmentById(testDepartmentId);
expect(fullDepartment).to.have.property('parentId').that.is.null;
expect(fullDepartment).to.have.property('ancestors').that.is.null;
expect(fullDepartment).to.not.have.property('parentId');
expect(fullDepartment).to.not.have.property('ancestors');
});

it('should succesfully add an existing department to a unit as an admin', async () => {
Expand Down Expand Up @@ -904,8 +904,8 @@ import { IS_EE } from '../../../e2e/config/constants';
expect(updatedUnit).to.have.property('numDepartments', 1);

const fullDepartment = await getDepartmentById(testDepartmentId);
expect(fullDepartment).to.have.property('parentId').that.is.null;
expect(fullDepartment).to.have.property('ancestors').that.is.null;
expect(fullDepartment).to.not.have.property('parentId');
expect(fullDepartment).to.not.have.property('ancestors');
});

it('should succesfully add an existing department to a unit that a monitor supervises', async () => {
Expand Down
5 changes: 4 additions & 1 deletion ee/packages/federation-matrix/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"@babel/preset-env": "~7.26.0",
"@babel/preset-typescript": "~7.26.0",
"@rocket.chat/eslint-config": "workspace:^",
"@rocket.chat/federation-sdk": "0.1.21",
"@types/emojione": "^2.2.9",
"@types/node": "~22.14.0",
"@types/sanitize-html": "^2",
Expand Down Expand Up @@ -38,7 +39,6 @@
"@rocket.chat/core-services": "workspace:^",
"@rocket.chat/core-typings": "workspace:^",
"@rocket.chat/emitter": "^0.31.25",
"@rocket.chat/federation-sdk": "0.1.13",
"@rocket.chat/http-router": "workspace:^",
"@rocket.chat/license": "workspace:^",
"@rocket.chat/models": "workspace:^",
Expand All @@ -53,5 +53,8 @@
"sanitize-html": "^2.17.0",
"tsyringe": "^4.10.0",
"tweetnacl": "^1.0.3"
},
"peerDependencies": {
"@rocket.chat/federation-sdk": "*"
}
Comment on lines +58 to 59
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Pin peer dependency to the minimum compatible SDK version.

Wildcarding @rocket.chat/federation-sdk means consumers can satisfy this peer with older builds (e.g., 0.1.13) that lack the branded types introduced in 0.1.21, breaking this package at runtime. Tighten the range to reflect the true minimum and upper bound expectation, e.g.:

-"@rocket.chat/federation-sdk": "*"
+"@rocket.chat/federation-sdk": ">=0.1.21 <0.2.0"
📝 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
"@rocket.chat/federation-sdk": "*"
}
"@rocket.chat/federation-sdk": ">=0.1.21 <0.2.0"
}
🤖 Prompt for AI Agents 
In ee/packages/federation-matrix/package.json around lines 58 to 59, the
peerDependency for "@rocket.chat/federation-sdk" is a wildcard ("*"); replace it
with a bounded semver that sets the minimum compatible SDK (e.g. "^0.1.21") so
consumers cannot install older builds that lack the branded types—update the
peerDependencies entry accordingly to "^0.1.21" (or another agreed upper bound)
and save the change.

}
Loading
Loading