Skip to content

regression(federation): fix mixed DM invite not working #37924

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 4 commits into from
Dec 23, 2025
+70 −56
Merged

regression(federation): fix mixed DM invite not working #37924

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a15071
regression(federation): fix not able to accept mixed DM invite
sampaiodiego Dec 22, 2025
e7d880e
add retry
sampaiodiego Dec 22, 2025
83e98ed
regression(federation): fix duplicated DMs priority
sampaiodiego Dec 22, 2025
0702489
fix(federation): add missing await before retry() in room.spec.ts (#3…
cubic-dev-ai[bot] Dec 22, 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
4 changes: 2 additions & 2 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 @@ -36,7 +36,7 @@ import { removeRoomOwner } from '../../methods/removeRoomOwner';
export class RoomService extends ServiceClassInternal implements IRoomService {
protected name = 'room';

async updateDirectMessageRoomName(room: IRoom): Promise<boolean> {
async updateDirectMessageRoomName(room: IRoom, ignoreStatusFromSubs?: string[]): Promise<boolean> {
const subs = await Subscriptions.findByRoomId(room._id, { projection: { u: 1, status: 1 } }).toArray();

const uids = subs.map((sub) => sub.u._id);
Expand All @@ -47,7 +47,7 @@ export class RoomService extends ServiceClassInternal implements IRoomService {

for await (const sub of subs) {
// don't update the name if the user is invited but hasn't accepted yet
if (sub.status === 'INVITED') {
if (!ignoreStatusFromSubs?.includes(sub._id) && sub.status === 'INVITED') {
continue;
}
await Subscriptions.updateOne({ _id: sub._id }, { $set: roomNames[sub.u._id] });
Expand Down
8 changes: 1 addition & 7 deletions ee/packages/federation-matrix/src/FederationMatrix.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,9 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
continue;
}

if (!isUserNativeFederated(member)) {
continue;
}

try {
await federationSDK.inviteUserToRoom(
userIdSchema.parse(member.username),
isUserNativeFederated(member) ? userIdSchema.parse(member.username) : `@${member.username}:${this.serverName}`,
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

P2: Inconsistent user ID validation: constructed user ID for non-native federated users should be validated with userIdSchema.parse() for consistency with inviteUsersToRoom (line 553) and to ensure proper type safety.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At ee/packages/federation-matrix/src/FederationMatrix.ts, line 296:

<comment>Inconsistent user ID validation: constructed user ID for non-native federated users should be validated with `userIdSchema.parse()` for consistency with `inviteUsersToRoom` (line 553) and to ensure proper type safety.</comment>

<file context>
@@ -291,13 +291,9 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
 					try {
 						await federationSDK.inviteUserToRoom(
-							userIdSchema.parse(member.username),
+							isUserNativeFederated(member) ? userIdSchema.parse(member.username) : `@${member.username}:${this.serverName}`,
 							roomIdSchema.parse(matrixRoomResult.room_id),
 							userIdSchema.parse(actualMatrixUserId),
</file context>
Suggested change
isUserNativeFederated(member) ? userIdSchema.parse(member.username) : `@${member.username}:${this.serverName}`,
userIdSchema.parse(isUserNativeFederated(member) ? member.username : `@${member.username}:${this.serverName}`),
Fix with Cubic

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I've fixed the inconsistent user ID validation at line 296 in ee/packages/federation-matrix/src/FederationMatrix.ts.

The issue: When inviting non-native federated users to a group DM room, the constructed user ID (@${member.username}:${this.serverName}) was passed directly to federationSDK.inviteUserToRoom() without validation through userIdSchema.parse().

The fix: Now both branches of the conditional are validated through userIdSchema.parse():

userIdSchema.parse(isUserNativeFederated(member) ? member.username : `@${member.username}:${this.serverName}`)

This makes the code consistent with the pattern used in inviteUsersToRoom (line 553) and ensures proper type safety for the UserID branded type.

PR: #37937

roomIdSchema.parse(matrixRoomResult.room_id),
userIdSchema.parse(actualMatrixUserId),
);
Expand Down Expand Up @@ -919,8 +915,6 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS

if (action === 'accept') {
await federationSDK.acceptInvite(room.federation.mrid, matrixUserId);

await Room.performAcceptRoomInvite(room, subscription, user);
}

if (action === 'reject') {
Expand Down
2 changes: 1 addition & 1 deletion ee/packages/federation-matrix/src/events/member.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ async function handleJoin({

// update room name for DMs
if (room.t === 'd') {
await Room.updateDirectMessageRoomName(room);
await Room.updateDirectMessageRoomName(room, [subscription._id]);
}

if (!subscription.status) {
Expand Down
5 changes: 2 additions & 3 deletions ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,7 @@ const waitForRoomEvent = async (
});
});

it.failing('should accept the invitation by the Rocket.Chat user', async () => {
it('should accept the invitation by the Rocket.Chat user', async () => {
const response = await acceptRoomInvite(rcRoom._id, rcUserConfig2);
expect(response.success).toBe(true);
});
Expand Down Expand Up @@ -1435,7 +1435,7 @@ const waitForRoomEvent = async (
});
});

it.failing('should create a federated DM between rcUser1 and rcUser2 and the Synapse user', async () => {
it('should create a federated DM between rcUser1 and rcUser2 and the Synapse user', async () => {
const fedDmResponse = await rcUser1.config.request
.post(api('dm.create'))
.set(rcUser1.config.credentials)
Expand Down Expand Up @@ -1486,7 +1486,6 @@ const waitForRoomEvent = async (
{ delayMs: 100 },
);

// TODO accept not working
const response = await acceptRoomInvite(rcRoom._id, rcUser2.config);
expect(response.success).toBe(true);
});
Expand Down
22 changes: 15 additions & 7 deletions ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../../../../../apps/meteor/tests/data/rooms.helper';
import { type IRequestConfig, getRequestConfig, createUser, deleteUser } from '../../../../../apps/meteor/tests/data/users.helper';
import { IS_EE } from '../../../../../apps/meteor/tests/e2e/config/constants';
import { retry } from '../../../../../apps/meteor/tests/end-to-end/api/helpers/retry';
import { federationConfig } from '../helper/config';
import { createDDPListener } from '../helper/ddp-listener';
import { SynapseClient } from '../helper/synapse-client';
Expand Down Expand Up @@ -1612,13 +1613,20 @@ import { SynapseClient } from '../helper/synapse-client';

describe('It should reflect all the members and messagens on the rocket.chat side', () => {
it('It should show all the three users in the members list', async () => {
const members = await getRoomMembers(rid, rc1AdminRequestConfig);
expect(members.members.length).toBe(3);
expect(members.members.find((member: IUser) => member.username === federationConfig.rc1.adminUser)).not.toBeNull();
expect(
members.members.find((member: IUser) => member.username === federationConfig.rc1.additionalUser1.username),
).not.toBeNull();
expect(members.members.find((member: IUser) => member.username === federationConfig.hs1.adminMatrixUserId)).not.toBeNull();
retry(
'Getting room members until all are present',
async () => {
const members = await getRoomMembers(rid, rc1AdminRequestConfig);

expect(members.members.length).toBe(3);
expect(members.members.find((member: IUser) => member.username === federationConfig.rc1.adminUser)).not.toBeNull();
expect(
members.members.find((member: IUser) => member.username === federationConfig.rc1.additionalUser1.username),
).not.toBeNull();
expect(members.members.find((member: IUser) => member.username === federationConfig.hs1.adminMatrixUserId)).not.toBeNull();
},
{ delayMs: 200 },
);
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/core-services/src/types/IRoomService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,5 @@ export interface IRoomService {
status?: 'INVITED';
roles?: ISubscription['roles'];
}): Promise<string | undefined>;
updateDirectMessageRoomName(room: IRoom): Promise<boolean>;
updateDirectMessageRoomName(room: IRoom, ignoreStatusFromSubs?: string[]): Promise<boolean>;
}
Loading